azagniotov/codahale-aggregated-metrics-cloudwatch-reporter

Make it configurable to send zero values

marcelboettcher opened this issue · 8 comments

Hey Alexander,

first of all: Great work, exactly what I was looking for! Thx!

I would love to see a builder option to send zero values, which isn't done at the moment to save costs. That makes sense, but for our use case we would like to see zeros as well as we would interprete missing values as possible error in the monitoring system.

What do you think? Does this make sense for you?

All the best from Germany
Marcel

@marcelboettcher hello! Thank you for the kind words. I can look into it and add an option of sending zeroes. I will issue a PR and if you could build locally and test it, would be great.. Thoughts?

@marcelboettcher btw, I thought to point your attention to one of the closed issues: #4 @ben-manes has observed an exception happening when submitted values were zero.

Hi @marcelboettcher Please have a look at my PR: #9 and you can try building an artifact from the following branch: zero_values_submission. If it does what you need, I will push a release to Maven Central

Hi @azagniotov,
thanks a lot for the quick adjustment!
I tested withZeroValuesSubmission() successfully for counters and a histograms with snapshot size > 0 (so an active one).

An inactive histogram (snapshot size == 0) does report count and the three percentiles correctly, but fails with the mentioned InvalidParameterValueException for arithmeticMean, stdDev and statisticSet.

So I would propose to move the snapshot.size() > 0 check around those metric types only. Please check my PR #10

Hi @marcelboettcher , v1.0.6 has been released. Please wait a few hours until Maven Central syncs .

Thank you for your help!

Hey @azagniotov, thank you!
But it does not show up on Maven Central. The version in gradle.properties is still 1.0.5 by the way.

Cheers

@marcelboettcher , It is there on Maven: https://search.maven.org/#artifactdetails%7Cio.github.azagniotov%7Cdropwizard-metrics-cloudwatch%7C1.0.6%7Cjar ... Sorry, I released from the branch . instead of the master. Will merge to master right now