Skip to content

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

Copy link
Contributor

This PR was marked stale due to lack of activity. It will be closed in 14 days.

@github-actions github-actions bot added the Stale label Jul 12, 2025
Copy link
Contributor

Closed as inactive. Feel free to reopen if this PR is still being worked on.

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