Skip to content

Add drop_histogram_buckets function #40281

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

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

iblancasa
Copy link
Contributor

@iblancasa iblancasa commented May 26, 2025

Description

Add drop_histogram_buckets function.

Link to tracking issue

Fixes #40280

Testing

  • Added new tests.

Documentation

  • Added documentation to README.md

@iblancasa iblancasa requested a review from edmocosta June 3, 2025 09:14
@iblancasa iblancasa changed the title Add drop_bucket function Add drop_histogram_buckets function Jun 3, 2025
@iblancasa
Copy link
Contributor Author

@jsuereth
Copy link
Contributor

This seems to break the Histogram data model, particularly removing sum.

Why not (when removing buckets) place the removed data into some single bucket or 'overflow' bucket?

I'm not really sure I understand the use case at all, but I'd be afraid this would break many metric backends and generally be a large footgun. Can you describe that more either in this PR or in the issue?

@iblancasa
Copy link
Contributor Author

Why not (when removing buckets) place the removed data into some single bucket or 'overflow' bucket?

I'm not sure about that solution, to be honest. One of the benefits is to reduce unused data.

I'm not really sure I understand the use case at all, but I'd be afraid this would break many metric backends and generally be a large footgun.

Just if they use it and don't know what they are doing (as the rest of features we have in the OpenTelemetry Collector). Don't see the issue adding more flexibility.

Can you describe that more either in this PR or in the issue?

It is common for Prometheus users doing this:

metric_relabel_configs:
  - source_labels: [__name__, le]
    regex: 'example_latency_seconds_bucket;0\.0.*'
    action: drop

I want to replicate this feature in OpenTelemetry Collector.

One ref, for instance https://www.robustperception.io/why-are-prometheus-histograms-cumulative/

@jsuereth
Copy link
Contributor

Ah, so if you're removing buckets prometheus style, you have the benefit that buckets are just less than a threshod, I.e. they include counts from previous buckets.

If you do the same in opentelemetry you NEED to add the counts from previous buckets into future buckets or you have literally lost data. I.e. if you use this function as it is defined, you just have a broken histogram, you wouldn't wind up with the same value as prometheus dropped buckets gives you.

At a minimum you should:

  • Shift bucket counts to nearest bucket (either up or down)
  • Expand bucket boundaries or nearest bucket to incorporate the lost bucket.

Then you'd get the prometheus behavior.

@iblancasa
Copy link
Contributor Author

  • Shift bucket counts to nearest bucket (either up or down)

Any idea about how to do this? Because per my understanding
#40281 (comment)

Thanks for your comments @jsuereth

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[processor/transform] Add drop_bucket OTTL function
4 participants