fix: Make PropagationContext.from_incoming_data always return a PropagationContext #5186
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
sl0thentr0py
commented
Dec 2, 2025
| self._propagation_context = propagation_context | ||
| self._propagation_context = PropagationContext.from_incoming_data(incoming_data) | ||
|
|
||
| # TODO-neel this below is a BIG code smell but requires a bunch of other refactoring |
Member
Author
There was a problem hiding this comment.
@alexander-alderman-webb since you wanted me to point out more insights like these, this branch and how it interacts with the get_active_propagation_context method logic is still pretty badly implemented.
sl0thentr0py
force-pushed
the
neel/prop-context-incoming-data
branch
from
ad10877 to
3f4dfd4
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #5186 +/- ##
==========================================
+ Coverage 83.94% 83.98% +0.03%
==========================================
Files 181 181
Lines 18419 18417 -2
Branches 3281 3280 -1
==========================================
+ Hits 15462 15467 +5
+ Misses 1944 1938 -6
+ Partials 1013 1012 -1
|
sl0thentr0py
force-pushed
the
neel/prop-context-incoming-data
branch
from
3f4dfd4 to
fe6cb05
Compare
…nContext When there is any sort of incoming data, and since `continue_trace` is always intended to be at a system boundary, we always want to force a new trace if there's no incoming propagation or an incorrect propagation header. What previously happened is in these cases, a single `trace_id` was kept alive in the `propagation_context` even when these were just meant to be new independent traces (see screenshot). I think this will actually solve many complaints about long living traces that were never supposed to be such.
sl0thentr0py
force-pushed
the
neel/prop-context-incoming-data
branch
from
fe6cb05 to
5dae95a
Compare
sl0thentr0py
changed the title
sentrivana
approved these changes
Dec 2, 2025
Contributor
sentrivana
left a comment
sentrivana
left a comment
There was a problem hiding this comment.
So do I get this right:
- Before, if
sentrytrace_datawas wrong or not there we'd simply continue using the existing propagation context, meaning we'd be propagating the trace from before - Now, we instead create a brand new
PropagationContextwith new trace info
If that's the case, sounds like a much needed fix.
Member
Author
|
yep pretty much :/ |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
When there is any sort of incoming data, and since
continue_traceis always intended to be at a system boundary, we always want to force a new trace if there's no incoming propagation or a mismatched propagation headers (forstrict_trace_continuation).What previously happened in these cases: a single
trace_idwas kept alive in thepropagation_contexteven when these were meant to be new independent traces (see screenshot that shows undesired behavior before this fix).I think this will actually solve many complaints about long living traces that were never supposed to be such.