Custom metric buckets #844
Conversation
core-api/src/telemetry.rs
Outdated
| /// | ||
| /// See [here](https://docs.rs/opentelemetry_sdk/latest/opentelemetry_sdk/metrics/enum.Aggregation.html#variant.ExplicitBucketHistogram.field.boundaries) | ||
| /// for the exact meaning of boundaries. | ||
| pub overrides: HashMap<&'static str, Vec<f64>>, |
There was a problem hiding this comment.
I am not sure this can be &'static here since it comes from users on lang side. It may have to be String.
There was a problem hiding this comment.
Duh, good point, thanks. I initially meant this to be only overrides of core-defined ones, but, it'll need to apply to user ones too.
| /// Overrides where the key is the metric name and the value is the list of bucket boundaries. | ||
| /// The metric name will apply regardless of name prefixing, if any. IE: the name acts like | ||
| /// `*metric_name`. |
There was a problem hiding this comment.
Does this wildcard thing only apply to OTel or does it apply to Prometheus too?
core-api/src/telemetry.rs
Outdated
| /// | ||
| /// See [here](https://docs.rs/opentelemetry_sdk/latest/opentelemetry_sdk/metrics/enum.Aggregation.html#variant.ExplicitBucketHistogram.field.boundaries) | ||
| /// for the exact meaning of boundaries. | ||
| pub overrides: HashMap<&'static str, Vec<f64>>, |
There was a problem hiding this comment.
I was going to suggest we also provide the configured buckets as part of buffered/custom metric to the user, but that may not be needed.
There was a problem hiding this comment.
Yeah that's what the PR description is about. It's just extremely annoying to do
There was a problem hiding this comment.
To clarify, here I am saying it may be neat to have the MetricEvent::Create buffered metric event include the buckets for those writing generic buffered metric impls, not necessarily concerned about the metric creation side. But I understand that is a bit difficult too.
There was a problem hiding this comment.
Yeah, it's the same thing. The problem is you define these Views at meter creation time, and instrument (metric) creation time is too late. So, not just annoying really but AFAICT may not even be possible.
What was changed
Allow configuring custom buckets for histogram metrics
Why?
Oft-requested feature
Checklist
Closes [Feature Request] Customize metric bucket sizes #459
How was this tested:
Added tests
Any docs updates needed?
I'm looking at passing buckets in with user-created histograms, but my initial inclination is that that may not actually make sense since the otel API separates how aggregation happens from the instrument definition, so we would need to do some odd contortions to take the buckets from the histogram definition and somehow apply that to the overall meter provider definition.
Ultimately, it's pretty easy for the user to just use a constant for their histogram name, and then register the buckets for that in the telemetry options.