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

Add support for metric_type gauge_delta #190

Closed
wants to merge 1 commit into from

Conversation

mirkoprescha
Copy link

@mirkoprescha mirkoprescha commented Oct 12, 2018

This PR allows to define "gauge_delta" as new metric_type in jmx.d/conf.yaml. JMXFetch will send the metric as StatsD Count-Type which will be handled as "rate" in DataDog. This allows to do time aggregations like "how many errors we had in last hour".

It might be also possible to use/reuse metric_type counter / count, but I am not sure about breaking semantics. Actually this PR is a POC of how gauge deltas can be implemented, as we really need them.

@hush-hush hush-hush requested review from arbll and removed request for hush-hush December 5, 2018 17:42
@arbll
Copy link
Member

arbll commented Dec 12, 2018

Hi @mirkoprescha!

Thanks a lot for this proof of concept. A way to send counts to datadog is clearly missing in jmxfetch.

I had some testing to do to validate the approach so I took the liberty of extending this PR at https://github.com/DataDog/jmxfetch/tree/arbll/count-support. I mostly added tests and renamed gauge_delta to monotonic_count since what we do here is very similar to the monotonic_count from the agent checks.
I also removed the use of the canonicalRate flag and kept the behavior when it was equal to true as it's something we would like to enable by default for rates but can't since it would break compatibility. As a side note, this means that there won't be any way to report negative deltas.

Apart from these small changes, everything looks good! I'll likely open another PR based on the branch linked above. If nothing comes up it should be merged soon and will be included the version 6.9 of the agent.

Best,
Arthur

@truthbk
Copy link
Member

truthbk commented Jan 22, 2019

As mentioned by @arbll, this we've based #203 off of this great work by @mirkoprescha, so we will close this PR and piggy back off of it in the mentioned PR. Please follow discussion there. Thank you so much for your contribution Mirko! 🙇

@truthbk truthbk closed this Jan 22, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants