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

Implement the histogram bucket advise API #4140

Open
lsfischer opened this issue Aug 21, 2024 · 5 comments · May be fixed by #4361
Open

Implement the histogram bucket advise API #4140

lsfischer opened this issue Aug 21, 2024 · 5 comments · May be fixed by #4361

Comments

@lsfischer
Copy link

lsfischer commented Aug 21, 2024

Is your feature request related to a problem?

open-telemetry/opentelemetry-js#3619 (from opentelemetry-js)

Describe the solution you'd like

Last year a new specification for the histogram bucket metric advise API or hint API got merged in the opentelemetry-specification repository: open-telemetry/opentelemetry-specification#3216

This seems to have been implemented by a few languages already (see open-telemetry/opentelemetry-java#5217, open-telemetry/opentelemetry-js#3876, open-telemetry/opentelemetry-erlang#628 and go) but not for python AFAIK

Doing so would alleviate the currently more complex way of setting custom bins on a histogram by creating a View and adding it to the a MeterProvider decoupling the bins definition from the histogram creation. This would also allow for different histograms under the same MeterProvider to define different bins seamlessly.

If there's consensus around this feature request, I'd be keen to give it a shot!

Describe alternatives you've considered

No response

Additional Context

Spec PR open-telemetry/opentelemetry-specification#3216

Would you like to implement a fix?

Yes

@lsfischer lsfischer changed the title Implement the histogram bucket metric advise API Implement the histogram bucket advise API Aug 21, 2024
@xrmx
Copy link
Contributor

xrmx commented Dec 12, 2024

@lsfischer are you still interested in this? I think I'll give it a try next week

@lsfischer
Copy link
Author

@xrmx I briefly started a draft PR but haven't found the time to dedicate to it - please feel free to give it a try if you're interested! Let me know if I can assist you in any way

@xrmx
Copy link
Contributor

xrmx commented Dec 12, 2024

I've done the API part some times ago here but need to understand how to hook it later.

@lsfischer
Copy link
Author

lsfischer commented Dec 12, 2024

Then you're in a similar state that I was in my draft :)

how to hook it later

Yeah, my understanding is that we need to update the DefaultAggregation here where we're creating the _ExplicitBucketHistogramAggregation to pass in a boundaries property which takes the boundaries from the advice API

Looking at the JS implementation could also be helpful (open-telemetry/opentelemetry-js#3876)

@xrmx
Copy link
Contributor

xrmx commented Dec 12, 2024

Looking at the JS implementation could also be helpful (open-telemetry/opentelemetry-js#3876)

Yeah, that was the plan. 🚀

@xrmx xrmx linked a pull request Dec 17, 2024 that will close this issue
11 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants