Skip to content

Conversation

@antonpirker
Copy link
Contributor

@antonpirker antonpirker commented Jul 23, 2025

Make sure all OpenTelemetry semantic conventions for AI clients and also the Sentry Conventions are adhered to.

Also cleans up the code a bit for better readability/maintenance.

@codecov
Copy link

codecov bot commented Jul 23, 2025

Codecov Report

❌ Patch coverage is 78.04878% with 36 lines in your changes missing coverage. Please review.
✅ Project coverage is 80.72%. Comparing base (5cc512f) to head (eb629de).
⚠️ Report is 1 commits behind head on antonpirker/openai-overhaul.
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
sentry_sdk/integrations/openai.py 83.33% 7 Missing and 15 partials ⚠️
sentry_sdk/utils.py 54.83% 12 Missing and 2 partials ⚠️
Additional details and impacted files
@@                       Coverage Diff                       @@
##           antonpirker/openai-overhaul    #4613      +/-   ##
===============================================================
+ Coverage                        80.68%   80.72%   +0.04%     
===============================================================
  Files                              156      156              
  Lines                            16579    16612      +33     
  Branches                          2814     2830      +16     
===============================================================
+ Hits                             13376    13410      +34     
- Misses                            2311     2313       +2     
+ Partials                           892      889       -3     
Files with missing lines Coverage Δ
sentry_sdk/consts.py 94.66% <ø> (ø)
sentry_sdk/integrations/openai_agents/utils.py 80.24% <100.00%> (+7.43%) ⬆️
sentry_sdk/utils.py 87.01% <54.83%> (-1.22%) ⬇️
sentry_sdk/integrations/openai.py 84.25% <83.33%> (+2.74%) ⬆️

... and 6 files with indirect coverage changes

@antonpirker antonpirker changed the title Fixed span names to be otel compatible Make OpenAI span data OTel compatible Jul 25, 2025
Comment on lines +228 to +229
if finish_span:
span.__exit__(None, None, None)
Copy link
Contributor

Choose a reason for hiding this comment

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

Looks like we have this check in each branch (maybe I'm mistaken), can we just move it after the whole if branch?

Copy link
Contributor

Choose a reason for hiding this comment

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

Actually, can we move it out of this function altogether and end the span after the call to this function? I'd not expect _set_output_data to do any actual span lifecycle management. And _set_input_data also doesn't start spans, so it's a bit inconsistent.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

unfortunately we can not move this out. because the span is finished in the generators defined in this function. if we move the finish outside the span will be finished too early and not only after the decorator is finished iterating over.

@antonpirker antonpirker marked this pull request as ready for review July 28, 2025 10:55
@antonpirker antonpirker requested a review from a team as a code owner July 28, 2025 10:55
@antonpirker antonpirker requested a review from sentrivana July 29, 2025 08:07
@antonpirker antonpirker merged commit fbbb5c4 into antonpirker/openai-overhaul Jul 29, 2025
136 of 137 checks passed
@antonpirker antonpirker deleted the antonpirker/openai-otel-2 branch July 29, 2025 09:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants