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 values for histogram bucket boundaries are oriented around milliseconds rather than seconds #5821

Open
jakegavin opened this issue Sep 13, 2024 · 1 comment
Labels
enhancement New feature or request

Comments

@jakegavin
Copy link

Problem Statement

Histograms are commonly used for recording latencies. The default values for bucket boundaries are []float64{0, 5, 10, 25, 50, 75, 100, 250, 500, 750, 1000, 2500, 5000, 7500, 10000} (code). This works well when working with milliseconds however the Prometheus documentation recommends using seconds, rather than milliseconds for units. When recording latency metrics in seconds with the default buckets, the vast majority of timings will land in the 0 second to 5 seconds bucket. This results inaccurate histogram quantile calculations.

This is very similar to this issue in the .NET repo: open-telemetry/opentelemetry-dotnet#4797

Proposed Solution

opentelemetry-go could use a different set of default buckets when the histogram units are known to be seconds.

This was implemented in the .NET library here: open-telemetry/opentelemetry-dotnet#4820

Alternatives

The current workaround is to use the WithExplicitBucketBoundaries option on all histograms dealing in seconds.

Prior Art

.NET issue: open-telemetry/opentelemetry-dotnet#4797
.NET solution: open-telemetry/opentelemetry-dotnet#4820

Additional Context

This would likely be a breaking change.

@jakegavin jakegavin added the enhancement New feature or request label Sep 13, 2024
@MrAlias
Copy link
Contributor

MrAlias commented Sep 13, 2024

This would likely be a breaking change.

Agreed. This is the reason we have not made the change.

It's also the reason explicitly called out in the specification:

SDKs SHOULD use the default value when boundaries are not explicitly provided, unless they have good reasons to use something different (e.g. for backward compatibility reasons in a stable SDK release).

This does not look like a proposal we plan to accept.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants