Skip to content

Cleanup op and description mapping #4560

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

Merged
merged 4 commits into from
Jul 14, 2025
Merged

Conversation

sl0thentr0py
Copy link
Member

@sl0thentr0py sl0thentr0py commented Jul 8, 2025

The main change is that op is now optional and name and description are basically interchangeable and map to each other as best as possible.

Preference is given to explicitly set values (SentrySpanAttribute) and if not (in the case of pure otel spans), we try to derive them. The entire flow is much cleaner now instead of repeated calls to the same attributes.

(For the main mapping changes in opentelemetry/test_utils I'd recommend reading the new code instead of the diff.)

Copy link

codecov bot commented Jul 8, 2025

Codecov Report

Attention: Patch coverage is 85.05747% with 13 lines in your changes missing coverage. Please review.

Project coverage is 84.87%. Comparing base (03659a4) to head (6978006).
Report is 1 commits behind head on potel-base.

✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
sentry_sdk/opentelemetry/utils.py 82.89% 8 Missing and 5 partials ⚠️
Additional details and impacted files
@@              Coverage Diff               @@
##           potel-base    #4560      +/-   ##
==============================================
- Coverage       84.92%   84.87%   -0.06%     
==============================================
  Files             158      158              
  Lines           15361    15354       -7     
  Branches         2422     2424       +2     
==============================================
- Hits            13046    13031      -15     
- Misses           1569     1572       +3     
- Partials          746      751       +5     
Files with missing lines Coverage Δ
sentry_sdk/consts.py 99.67% <100.00%> (+<0.01%) ⬆️
sentry_sdk/opentelemetry/span_processor.py 82.55% <100.00%> (+0.30%) ⬆️
sentry_sdk/tracing.py 81.46% <100.00%> (ø)
sentry_sdk/opentelemetry/utils.py 80.43% <82.89%> (-3.11%) ⬇️

... and 3 files with indirect coverage changes

@sl0thentr0py sl0thentr0py force-pushed the neel/refactor-span-data branch 7 times, most recently from ef85b4a to 666b8ed Compare July 10, 2025 14:16
@sl0thentr0py sl0thentr0py changed the title Cleanup op, description and status mapping Cleanup op and descriiption mapping Jul 10, 2025
@sl0thentr0py sl0thentr0py force-pushed the neel/refactor-span-data branch 3 times, most recently from 3777d75 to 12c5914 Compare July 10, 2025 14:40
@@ -822,7 +822,7 @@ def test_clickhouse_dbapi_spans(sentry_init, capture_events, capture_envelopes)
span.pop("span_id", None)
span.pop("start_timestamp", None)
span.pop("timestamp", None)
span.pop("status")
span.pop("status", None)
Copy link
Member Author

Choose a reason for hiding this comment

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

these were all None before anyway

@sl0thentr0py sl0thentr0py force-pushed the neel/refactor-span-data branch from 12c5914 to e83681a Compare July 10, 2025 14:44
@sl0thentr0py sl0thentr0py changed the title Cleanup op and descriiption mapping Cleanup op and description mapping Jul 10, 2025
@sl0thentr0py sl0thentr0py force-pushed the neel/refactor-span-data branch from e83681a to b230b84 Compare July 10, 2025 14:56
assert len(transaction["spans"]) == 7

assert (
render_span_tree(transaction)
== f"""\
- op="caches": description=null
Copy link
Member Author

Choose a reason for hiding this comment

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

here and in all subsequent tests, I made it clear that the transaction name is correct and that the op of the root span is not the transaction name anymore

@sl0thentr0py sl0thentr0py marked this pull request as ready for review July 10, 2025 15:05
@sl0thentr0py sl0thentr0py requested a review from a team as a code owner July 10, 2025 15:05
Copy link
Contributor

@sentrivana sentrivana left a comment

Choose a reason for hiding this comment

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

Looking much better, ty!

See some nits re: types

@sl0thentr0py sl0thentr0py force-pushed the neel/refactor-span-data branch from 3d8a372 to eaf30e5 Compare July 14, 2025 10:52
@sl0thentr0py sl0thentr0py requested a review from sentrivana July 14, 2025 11:31
@sl0thentr0py sl0thentr0py merged commit b23507c into potel-base Jul 14, 2025
124 checks passed
@sl0thentr0py sl0thentr0py deleted the neel/refactor-span-data branch July 14, 2025 11:31
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.

2 participants