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

When the developer wants to ignore the counter attribute in the defin… #1090

Merged
merged 2 commits into from Feb 28, 2017

Conversation

Se7soz
Copy link
Contributor

@Se7soz Se7soz commented Feb 25, 2017

When the developer wants to ignore the counter attribute in the defined meters metrics .. that doesn't mean he/she is still not interested in the defined counter metrics to be logged .. especially that counter metrics contains only the count attribute - this PR will force sending the counter.count even if its configured to be ignored for meters

…ed meters metrics .. that doesn't mean he/she is still not interested in the defined counter metrics to be logged .. especially that counter metrics contains only the count attribute - this PR will force sending the counter.count even if its configured to be ignored for meters
@arteam
Copy link
Member

arteam commented Feb 27, 2017

I think this makes sense. I fail to see a use case when the user wants to disable reporting of a Counter. It disables the whole metric from being reported and this can be done by MetricFilter. On the other hand, I can see a situation when the users wants to disable the counter attribute from a Meter, but allow Counters to be reported.

@Husseincoder Could you please update the GangliaReporter for consistency as well?

@arteam arteam added the bug label Feb 27, 2017
@arteam arteam added this to the 3.2.1 milestone Feb 27, 2017
@Se7soz
Copy link
Contributor Author

Se7soz commented Feb 27, 2017

@arteam I've changed it too with a small unit test edit .. unless you want me to extract it in a separate unit test?

@arteam arteam merged commit 99302d0 into dropwizard:3.2-development Feb 28, 2017
@arteam
Copy link
Member

arteam commented Feb 28, 2017

Thanks for your contribution!

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

Successfully merging this pull request may close these issues.

None yet

2 participants