-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Configure histogram flavor/max buckets per meter #5974
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
Configure histogram flavor/max buckets per meter #5974
Conversation
Adds new configuration methods to override the registry-wide configuration at a per Meter (name) level. This gives more fine-grain control of the configuration.
e0b7515 to
5dd7e2c
Compare
...entations/micrometer-registry-otlp/src/main/java/io/micrometer/registry/otlp/OtlpConfig.java
Outdated
Show resolved
Hide resolved
|
Pinging some folks who expressed interest in or provided input on this feature before to get any feedback they may have on this proposal: @lenin-jaganathan @pirgeo @sfc-gh-rscott @sfc-gh-dguy. Of course anyone else is welcome to give feedback too. In #5459 I expressed a desire to expose the configuration at the |
|
We have the last 1.15 milestone release on Monday, so I'd like to merge something for this in time for that. If anyone has feedback on the proposal, please share. |
| void histogramFlavorPerMeter() { | ||
| Map<String, String> properties = new HashMap<>(); | ||
| properties.put("otlp.histogramFlavorPerMeter", | ||
| "a.b.c=explicit_bucket_histogram ,expo =base2_exponential_bucket_histogram"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is spacing intentional?
| "a.b.c=explicit_bucket_histogram ,expo =base2_exponential_bucket_histogram"); | |
| "a.b.c=explicit_bucket_histogram, expo=base2_exponential_bucket_histogram"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, this was intentionally testing that it can handle weird spacing. That could be made more explicit in a separate test perhaps.
| @Test | ||
| void maxBucketsPerMeter() { | ||
| Map<String, String> properties = new HashMap<>(); | ||
| properties.put("otlp.maxBucketsPerMeter", "a.b.c = 10"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is spacing intentional?
| properties.put("otlp.maxBucketsPerMeter", "a.b.c = 10"); | |
| properties.put("otlp.maxBucketsPerMeter", "a.b.c=10"); |
|
|
||
| import static org.assertj.core.api.Assertions.assertThat; | ||
|
|
||
| class PropertyValidatorTest { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would put tests here with tricky spacing (like above):
a=b ,c =d, e = f
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done in 8db4d6e
Adds new configuration methods to override the registry-wide configuration at a per Meter (name) level. This gives more fine-grain control of the configuration.
The registry-wide HistogramFlavor configuration can be overridden at the meter level with the new
histogramFlavorPerMeterconfiguration method, which returns aMap<String, HistogramFlavor>where the key is the exact Meter name to match.Likewise, the registry-wide max bucket count can be overridden at the meter level with the new
maxBucketsPerMeterconfiguration method, which returns aMap<String, Integer>where the key is the exact Meter name to match.Closes #5459