Skip to content

Conversation

@sl0thentr0py
Copy link
Member

Description

The Transaction.continue_from_headers and the Transaction constructor take a Baggage object so this was inconsistent with how I originally designed the baggage handling and created spaghetti loops between the two concepts.

span_id=None, # type: Optional[str]
parent_span_id=None, # type: Optional[str]
parent_sampled=None, # type: Optional[bool]
dynamic_sampling_context=None, # type: Optional[Dict[str, str]]
Copy link
Member Author

@sl0thentr0py sl0thentr0py Nov 26, 2025

Choose a reason for hiding this comment

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

I'm not removing the constructor parameter because that would be technically breaking but it does nothing now.
@sentrivana if we consider this class purely internal, I can also remove, wdyt?
Also, we never call this constructor with any arguments ourselves.

EDIT: Ignore, added backwards compat.

@codecov
Copy link

codecov bot commented Nov 26, 2025

Codecov Report

❌ Patch coverage is 95.23810% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 83.98%. Comparing base (80ba8fb) to head (f0639ce).
⚠️ Report is 1 commits behind head on master.
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
sentry_sdk/tracing_utils.py 93.75% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #5156      +/-   ##
==========================================
- Coverage   84.02%   83.98%   -0.04%     
==========================================
  Files         180      180              
  Lines       18223    18226       +3     
  Branches     3233     3233              
==========================================
- Hits        15311    15307       -4     
- Misses       1922     1929       +7     
  Partials      990      990              
Files with missing lines Coverage Δ
sentry_sdk/scope.py 88.47% <100.00%> (-0.05%) ⬇️
sentry_sdk/tracing_utils.py 87.59% <93.75%> (+0.13%) ⬆️

... and 2 files with indirect coverage changes

@sl0thentr0py sl0thentr0py force-pushed the neel/prop-context-baggage-based branch from 7a0d9d6 to 8aba654 Compare November 26, 2025 14:14
@sl0thentr0py sl0thentr0py marked this pull request as ready for review November 26, 2025 14:24
@sl0thentr0py sl0thentr0py requested a review from a team as a code owner November 26, 2025 14:24
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.

Much better, ty!

@sentrivana
Copy link
Contributor

Maybe check cursor before merging @sl0thentr0py

The tracing incoming headers implementation and the Transaction
constructor take a `Baggage` object so this was incosistent with how I
originally designed the baggage handling and created spaghetti loops
between the two concepts.
@sl0thentr0py sl0thentr0py force-pushed the neel/prop-context-baggage-based branch from 8aba654 to f0639ce Compare November 26, 2025 14:33
@sl0thentr0py sl0thentr0py enabled auto-merge (squash) November 26, 2025 14:33
@sl0thentr0py sl0thentr0py merged commit 027aa6e into master Nov 26, 2025
155 checks passed
@sl0thentr0py sl0thentr0py deleted the neel/prop-context-baggage-based branch November 26, 2025 14:41
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.

3 participants