Let clients of DropwizardExports handle validation errors #1354
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
I like the idea of making this customizable. Instead of subclassing, I'd prefer to use a callback and builder - would that work? |
|
Yes - that is a much cleaner solution. I have updated the PR to allow the callback to decide if an exception is to be suppressed. |
mnssn
force-pushed
the
main
branch
5 times, most recently
from
56cb38c to
292a1bc
Compare
zeitlinger
reviewed
May 22, 2025
| @@ -179,7 +202,7 @@ MetricSnapshot fromTimer(String dropwizardName, Timer timer) { | |||
| } | |||
|
|
|||
| /** Export a Meter as a prometheus COUNTER. */ | |||
| MetricSnapshot fromMeter(String dropwizardName, Meter meter) { | |||
| protected MetricSnapshot fromMeter(String dropwizardName, Meter meter) { | |||
There was a problem hiding this comment.
should not have to be protected
| * @param metricFilter a custom metric filter. | ||
| * @param labelMapper a labelMapper to use to map labels. | ||
| */ | ||
| public DropwizardExports( |
There was a problem hiding this comment.
this ctor should be private - i.e. you have to use the builder for that
In the current form, if e.g. a Counter contains invalid data the entire `collect` calls fails with InvalidArgumentException. In large applications, it may be preferable to gracefully degrade the available metrics and report the failure via e.g. logging. This change allows that by letting the user add a exception handler. This allows the user to inspect the exception, and the metric name it was associated with, and decide if the exception should be suppressed or not. Metrics for which exceptions were suppressed will be absent from the collection. Signed-off-by: Anders Månsson <[email protected]>
mnssn
changed the title
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
A possible solution for #1353.
In the current form, if e.g. a Counter contains invalid data the entire
collectcalls fails with InvalidArgumentException.In large applications, it may be preferable to gracefully degrade the available metrics and report the failure via e.g. logging. This change allows just that - by changing the access modifiers to the
fromXYZmethods subclasses can now handle the errors.Finally, to allow for skipping of invalid metrics, we follow the example in
fromGaugeto interpret the meaning of a returned null to be that the metric should be skipped.