fix: unbind finished span from scope regardless of sampling - #2158
Merged
Merged
Conversation
sentry_span_finish only unbound the span from the scope when the span was recorded, so a bound child of an unsampled transaction stayed on the scope and later events reported the finished span as their trace context. Unbind right after the null check, like transaction finish does. The scope is flushed only if the span was actually bound, so the moved unbind doesn't add a scope flush to every span finish.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #2158 +/- ##
==========================================
- Coverage 75.40% 75.34% -0.07%
==========================================
Files 103 103
Lines 28105 28105
Branches 5133 5132 -1
==========================================
- Hits 21193 21175 -18
- Misses 5577 5597 +20
+ Partials 1335 1333 -2 🚀 New features to boost your workflow:
|
limbonaut
approved these changes
Oct 2, 2026
|
|
||
| sentry_scope_t *scope = sentry__scope_getref(); | ||
| bool removed = sentry__scope_remove_span_value(scope, opaque_span->inner); | ||
| sentry__scope_finish_mut(scope, removed); |
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
sentry_span_finishonly unbinds the span from the scope when the span is recorded, i.e. sampled. A bound child of an unsampled transaction therefore stays on the scope after it finishes, and later events report the already-finished span as their trace context. Transaction finish andsentry_span_discardunbind before any early exit, whether the transaction was sampled or not.Moving the unbind before the early exits would also move its scope flush onto the hot path.
SENTRY_WITH_SCOPE_MUTalways flushes the scope to the backend, so every span finish would pay for it, including unsampled ones. Instead, the scope is now flushed only if the span was actually bound. That also removes the redundant flush every sampled span finish paid before, even when the scope didn't change.Key Changes
sentry_span_finish_tsunbinds the span right after the null check, before any early exit, matching transaction finish.unsampled_span_finish_unbinds_from_scope.