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

Default boundaries in explicit buckets histograms are not optmized for Latency #2759

Closed
rapphil opened this issue Aug 29, 2022 · 2 comments · Fixed by #2770
Closed

Default boundaries in explicit buckets histograms are not optmized for Latency #2759

rapphil opened this issue Aug 29, 2022 · 2 comments · Fixed by #2770
Assignees
Labels
[label deprecated] triaged-accepted [label deprecated] Issue triaged and accepted by OTel community, can proceed with creating a PR spec:metrics Related to the specification/metrics directory

Comments

@rapphil
Copy link

rapphil commented Aug 29, 2022

What are you trying to achieve?

The defaults for the explicit bucket histogram have the first boundary in the interval (-inf,0]. Since Explicit Bucket histograms only support non-negative values, this first boundary can only record the 0 value, which is not very useful in practice given that the defaults of the Explicit Bucket histogram are supposed to record latency values.

https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/metrics/sdk.md#explicit-bucket-histogram-aggregation

The default values also limits the last boundary to (1000, inf), which limits the calculation of percentiles to 1s, which is not very good for latency.

https://github.com/prometheus/prometheus/blob/main/promql/quantile.go#L75

What did you expect to see?

I expect the defaults to have something more useful for recording latency values.

[5, 10, 25, 50, 75, 100, 250, 500, 1000, 10000]

Additional context.

Add any other context about the problem here. If you followed an existing documentation, please share the link to it.

@rapphil rapphil added the spec:metrics Related to the specification/metrics directory label Aug 29, 2022
@jmacd
Copy link
Contributor

jmacd commented Sep 2, 2022

This looks like a mistake in the spec. I was under the impression that the defaults exactly matched prometheus, meaning the last finite boundary at 10.0 (floating) or 10000 (integer).

@rbailey7210 rbailey7210 added the [label deprecated] triaged-accepted [label deprecated] Issue triaged and accepted by OTel community, can proceed with creating a PR label Sep 2, 2022
@reyang
Copy link
Member

reyang commented Sep 2, 2022

Maybe it should be [ 0, 5, 10, 25, 50, 75, 100, 250, 500, 1000, 2500, 5000, 7500, 10000 ] @jmacd? Is there an official place where the Prometheus or OpenMetrics defaults are documented?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[label deprecated] triaged-accepted [label deprecated] Issue triaged and accepted by OTel community, can proceed with creating a PR spec:metrics Related to the specification/metrics directory
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants