Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Feature Request] Support DogStatsD in Extended Aggregation mode #557

Closed
GrgDev opened this issue May 22, 2024 · 4 comments
Closed

[Feature Request] Support DogStatsD in Extended Aggregation mode #557

GrgDev opened this issue May 22, 2024 · 4 comments

Comments

@GrgDev
Copy link
Contributor

GrgDev commented May 22, 2024

Problem

This tool has worked almost flawlessly for me. However, when using it to try to move one of my tenants from statsd to prometheus, we noticed that the timing metrics were all showing a flat line series when they shouldn't be. Long story short, we found out it was because they were using the extended aggregation config option to reduce network load. See the docs on this mode here.

In case that link gets broken later, here's the TL;DR description. Instead of sending:

my_distribution_metric:21|d|#all,my,tags
my_distribution_metric:43.2|d|#all,my,tags
my_distribution_metric:1657|d|#all,my,tags

it sends

my_distribution_metric:21:43.2:1657|d|#all,my,tags

This only gets applied to distributions, histograms, and timers. Counts and gauges still act as expected. It would be great if the exporter could handle this.

Proposed Possible Solution

I'm a novice in Golang so take my suggestion with a grain of salt.

The LineToEvents function seems to be called on three different paths with all three being packet handling functions.

lines := strings.Split(string(packet), "\n")

at the start. We could replace that with a function that does the following:

  • Split by newlines
  • For each line, check if a line contains |#
    • If it does, check to see if there is more than one : to the left of the |#
      • If it does, copy the string as many times as there are : left of the |#
      • Replace the value section with only one value with each copied string getting one unique value from the list of values in the original line.
      • Replace the original line in the list of lines with the unpacked expanded list of lines without the extended aggregation.

The HandleConn function uses a for loop with Readline instead of a split on newlines, so this approach couldn't be copied there. That being said, that function looks like it is for handling TCP connections which I don't think DogStatsD ever uses. That function might not need to be changed at all.

I originally thought that maybe this code change could be done inside of LineToEvents so the change would only need to be made in one place, but I think the logic there assumes that the one line passed in will stay as one line instead of being expanded.

Anyways, y'all know this code way better than me and I don't know if this change would cause side effects to the existing behavior. Let me know what you think, and thanks again for maintaining this.

@glightfoot
Copy link
Contributor

I think the correct place to handle this would be in LineToEvents. Technically that function does convert a single line into multiple events in the case of sampled metrics. It would need to be refactored to support multiple values, but that's not terrible. I might be able to take a stab at this in a few days if work doesn't get too busy.

@GrgDev
Copy link
Contributor Author

GrgDev commented May 22, 2024

Thank you fellow Greg

@GrgDev
Copy link
Contributor Author

GrgDev commented May 30, 2024

I went ahead and took a stab at implementing this in PR #558 . As noted in the PR description, there's still the outstanding question should statsd exporter should be the enforcer of the expectation that the extended aggregate metrics feature will only ever be applied to distributions, histograms, and timers while counts and gauges remain unchanged?

@matthiasr
Copy link
Contributor

This is released now!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants