-
Notifications
You must be signed in to change notification settings - Fork 990
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
Support configuring exponential histograms at the meter level #5459
Comments
At this point, to align with the micrometer's design and isolate the implementation-specific things away from common APIs I would like to propose having maxBuckets() as part of the The justification is that EDIT: Even for the concern of this problem of customizing exponential histograms per meter basis, the |
@shakuzen I am willing to work on this before our GA if we can brainstorm on an approach. |
Thanks for the offer to help @lenin-jaganathan. It's appreciated. We're past RC1 now for 1.14, and it's our general policy that we won't make API changes for the GA release after a release candidate is made. So I think this enhancement will have to wait until 1.15.0, but that means we have more time to get the design right and hopefully get more feedback on the feature from users. This lost some priority due to the lack of feedback from any users on trying out exponential histograms, but perhaps that's at least partially because users want per meter customization before they try exponential histograms. It's hard to say since we haven't heard anything from users since exponential histograms was released in a milestone, despite the interest there seemed to be for the feature before it was merged. As for the design, I'm happy to discuss. I think the problem with making max buckets general configuration is that it doesn't make sense with fixed bucket histograms. The way to control the number of buckets there is with expected min/max values, which is also much more intuitive than knowing how many buckets is sufficient for accurate percentile estimation for a given histogram. This is why previously I suggested making it specific to exponential histograms (e.g. maxExponentialBuckets or similar). What do you think? |
It feels like we are kind of in a I am not keen on exposing something like a exponentialHistogramBuckets on meter api's because it seems too specific to an implementation. Why I said maxBuckets is that, we had use-case where we wanted services to export only a limited number of buckets to the back-end. Some backends had hard limit on number of buckets, so we had a custom solution that checks if the buckets generated is within limit if not it keeps only the first n buckets. Also, this provides a full backward compatibility with our current configuration, we can keep the existing behaviour as is. |
This is a follow up to #3861 and requests from users such as #3959 (comment).
Currently, exponential histograms can only be configured at the registry level for the OTLP registry via OtlpConfig. This is fine for users who want to use exponential histograms for everything that has
percentileHistogram
enabled. However, more fine grained control gives more flexibility.As a workaround, configuring any SLO on a meter's DistributionStatisticConfig will cause exponential histograms to not be used (they don't support arbitrary additional buckets).
Another aspect of configuration would be the (max) bucket count, which again is currently controlled by OtlpConfig for all meters in the registry.
The text was updated successfully, but these errors were encountered: