-
-
Notifications
You must be signed in to change notification settings - Fork 226
fix: Removed redundant scope sync after transaction finish #4623
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
jamescrosswell
merged 3 commits into
version6
from
fix/do-not-sync-transaction-reset-v6
Oct 12, 2025
Merged
fix: Removed redundant scope sync after transaction finish #4623
jamescrosswell
merged 3 commits into
version6
from
fix/do-not-sync-transaction-reset-v6
Oct 12, 2025
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
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## version6 #4623 +/- ##
===========================================
Coverage ? 73.08%
===========================================
Files ? 479
Lines ? 17381
Branches ? 3430
===========================================
Hits ? 12703
Misses ? 3819
Partials ? 859 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
jamescrosswell
approved these changes
Oct 12, 2025
Collaborator
jamescrosswell
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice! Thanks @bitsandfoxes ❤️
This was referenced Dec 7, 2025
This was referenced Dec 14, 2025
This was referenced Dec 15, 2025
This was referenced Dec 28, 2025
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.
Problem Description
When a transaction gets finished it also resets itself on the scope. This triggers a "manual" scope sync to the native layer. After finishing a transaction the
PropagationContextget renewed to keep new events from having an old trace context.The SDK syncs the trace in immediate succession, overwriting itself every time.
Context
The logs are taken from a Unity game but should apply to anywhere there is a transaction that gets finished
After a transaction gets finished it gets reset on the
Scopeand afterwards thePropagationContextgets regenerated so that new events don't have a trace context older than the just finished transaction. We even point this out in the comments.There are two places the transaction gets reset on the scope:
UnsampledTransactionsentry-dotnet/src/Sentry/Internal/UnsampledTransaction.cs
Lines 65 to 71 in 6eedb71
TransactionTracersentry-dotnet/src/Sentry/TransactionTracer.cs
Lines 386 to 392 in 6eedb71
Proposal
The SDK does not need to manually restore the trace context on the native layer, as it gets overwritten basically in the next line.