8000 Recommend using a time-weighted reservoir sampling algorithm for histograms by dashpole · Pull Request #4678 · open-telemetry/opentelemetry-specification · GitHub
[go: up one dir, main page]
Skip to content

Recommend using a time-weighted reservoir sampling algorithm for histograms #4678

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Recommend using a time-weighted reservoir sampling algorithm for histograms #4678

wants to merge 4 commits into from
+15 −4

Conversation

dashpole
Copy link
Contributor
@dashpole dashpole commented Oct 6, 2025 •
edited
Loading

Fixes #4675

Changes

Change the recommended algorithm for histogram reservoirs to be time-weighted.

I've left the previous algorithm as an option to ensure this change is backwards-compatible.

Go prototype: open-telemetry/opentelemetry-go#7458

  • Links to the prototypes (when adding or changing features)
  • CHANGELOG.md file updated for non-trivial changes

@dashpole dashpole force-pushed the time_based_histogram branch from de93eba to 80f9b71 Compare October 6, 2025 14:00
@dashpole dashpole force-pushed the time_based_histogram branch from 80f9b71 to 106246d Compare October 6, 2025 14:02
@dashpole dashpole mentioned this pull request Oct 6, 2025
Draft
@dashpole dashpole marked this pull request as ready for review October 6, 2025 16:05
@dashpole dashpole requested review from a team as code owners October 6, 2025 16:05
MrAlias
MrAlias approved these changes Oct 6, 2025
View reviewed changes
Copy link
Contributor
@MrAlias MrAlias left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good overall.

dashpole and others added 2 commits October 6, 2025 13:14
@dashpole @MrAlias
Update specification/metrics/sdk.md
393f4c9 
Co-authored-by: Tyler Yahn <MrAlias@users.noreply.github.com>
@dashpole @MrAlias
Update specification/metrics/sdk.md
aa40556 
Co-authored-by: Tyler Yahn <MrAlias@users.noreply.github.com>
@dashpole
Copy link
Contributor Author
dashpole commented Oct 7, 2025

There is a concern from @tigrannajaryan that this is a breaking change. @jsuereth says there is language that allows us to make this change in the spec

@dashpole
Copy link
Contributor Author
dashpole commented Oct 7, 2025 •
edited
Loading

Feedback from @austinlparker: We should be careful about how we communicate this to users. Is there a way to make this opt-in for users?

@dashpole
Copy link
Contributor Author
dashpole commented Oct 7, 2025

@carlosalberto requests that we leave this open for comment for a while.

@dashpole
Copy link
Contributor Author
dashpole commented Oct 7, 2025

From https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/metrics/sdk.md#exemplar-defaults

Exemplar default reservoirs MAY change in a minor version bump. No guarantees are made on the shape or statistical properties of returned exemplars.

jmacd
jmacd approved these changes Oct 7, 2025
View reviewed changes
Copy link
Contributor
@jmacd jmacd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice. This was a goal of mine in the original version, glad to see it improve.
I believe this is Jeffrey Vitter's "Algorithm R", for the record.

@jmacd
Copy link
Contributor
jmacd commented Oct 7, 2025

I might call this "time-unbiased" or "temporal uniform", not time weighted.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jmacd jmacd jmacd approved these changes

@MrAlias MrAlias MrAlias approved these changes

Assignees
No one assigned
Labels
changelog.opentelemetry.io
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

AlignedHistogramBucketExemplarReservoir should use time-weighted sampling
4 participants
@dashpole @jmacd @MrAlias @arminru
0