-
Notifications
You must be signed in to change notification settings - Fork 16
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
Make it configurable to send zero values #8
Comments
@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? |
Perfect! I will test it.
On Fri 15. Dec 2017 at 18:21, Alexander Zagniotov ***@***.***> wrote:
@marcelboettcher <https://github.com/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?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#8 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AFmMPP1Ua-N1mdDtVnA3XTIXmwlIrHwdks5tAqqLgaJpZM4RDrn4>
.
--
Viele Grüße
Marcel
Mobil 0162 / 36 20 374
mail@marcelboettcher.de
|
@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: |
Hi @azagniotov, 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 , Thank you for your help! |
Hey @azagniotov, thank you! 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 |
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
The text was updated successfully, but these errors were encountered: