-
Notifications
You must be signed in to change notification settings - Fork 647
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
Provide advisory histogram boundaries when creating OpenAI metrics #3225
base: main
Are you sure you want to change the base?
Conversation
LGTM |
instrumentation-genai/opentelemetry-instrumentation-openai-v2/examples/manual/main.py
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, I guess this also applies to the auto-instrumentation, but also might water down how it looks, as we'd need to add code to the zero code ;)
thanks for linking to the issue
views = [ | ||
View( | ||
instrument_name="gen_ai.client.token.usage", | ||
aggregation=ExplicitBucketHistogramAggregation( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IIUC what we are doing is avoiding the default boundaries being used both for token usage and also duration.
From here
_DEFAULT_EXPLICIT_BUCKET_HISTOGRAM_AGGREGATION_BOUNDARIES: Sequence[float] = (
0.0,
5.0,
10.0,
25.0,
50.0,
75.0,
100.0,
250.0,
500.0,
750.0,
1000.0,
2500.0,
5000.0,
7500.0,
10000.0,
)
So, this default might be ok for tokens for some apps, but probably not reasonable for durations which are in seconds (except maybe in reasoning models ;)). Correct me if I'm wrong!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thank you so much
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
Fixes #3235
Update:
I overhauled this PR and it now leverages advisory boundaries - no need for users to configure anything and most users should have good experience with default boundaries. So no OpenAI-specific example is necessary.