fix(compose): Stop SentryTraced from reusing stale parent spans - #6057
fix(compose): Stop SentryTraced from reusing stale parent spans#60570xadam-brown wants to merge 3 commits into
Conversation
Fix the stale parent problem that causes spans from all SentryTraced instances for the entire app process to be dropped once the initial active transaction finishes. Prior to this commit, SentryTraced used process-wide composition locals to bind the transaction active whenever the first SentryTraced for an app process entered the composition, and to reuse it for all SentryTraced instances thereafter. That meant all SentryTraced spans for the entire app would be dropped for the lifetime of the app process once the initial transaction finished. Oof. This commit sets things right by having each SentryTraced composable request the current active transaction and update the generation of spans accordingly. Because we're no longer relying on a single transaction + parent span pair, SentryTraced now needs to manage the creation of possibly multiple parent span pairs, as the owning transaction updates. That logic lives in the new ParentSpans class.
302c95b to
e508334
Compare
| getByName("androidUnitTest") { | ||
| dependencies { | ||
| implementation(libs.androidx.compose.foundation) | ||
| implementation(libs.androidx.compose.foundation.layout) |
There was a problem hiding this comment.
Fyi, new deps are test-only.
foundation and foundation-layout are needed to keep the Robolectric Compose test runtime from picking up navigation-compose's obsolete transitive foundation artifacts, which led to a BoxKt.maybeCachedBoxMeasurePolicy(...) linkage failure (given the Box used by SentryTraced).
📲 Install BuildsAndroid
|
![sentry-warden[bot]](https://avatars.githubusercontent.com/in/2755838?s=60&v=4){kind=small}
|
|
||
| private const val OP_TRACE_ORIGIN = "auto.ui.jetpack_compose" | ||
|
|
||
| private val localSentryCompositionParentSpan = compositionLocalOf { |
There was a problem hiding this comment.
These composition locals were the cause of the stale parent bug.
The lambda here is executed when localSentryCompositionParentSpan.current is first read (ie, by the first SentryTraced in the app process to be invoked). CompositionLocal then caches that default value and returns it every time .current is called. Once the original transaction returned by getRootSpan() expired, all remaining spans generated by SentryTraced through the lifetime of the app process were dropped.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Want reviews to match your repository better? Bugbot Learning can learn team-specific rules from PR activity. A team admin can enable Learning in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit e508334. Configure here.
Pass the captured start timestamp through the timestamp-aware span overload when creating shared SentryTraced parent spans. This keeps parent spans from starting after their child composition or render spans. Add regression coverage for parent span start ordering. Co-Authored-By: OpenAI GPT-5.5 <noreply@openai.com>
1 participant
📜 Description
PR fixes the stale parent problem that causes spans to be dropped from all SentryTraced instances for the entire app process after the initial active transaction finishes.
Prior to this PR, SentryTraced used process-wide composition locals to bind the transaction active whenever the first SentryTraced for an app process entered the composition, and to reuse it for all SentryTraced instances thereafter. That meant all SentryTraced spans for the entire app would be dropped for the lifetime of the app process once the initial transaction finished.
This PR sets things right by having each SentryTraced composable request the current active transaction and update the generation of spans accordingly.
Because we're no longer relying on a single transaction + parent span pair, SentryTraced now needs to manage the creation of possibly multiple parent span pairs, as the owning transaction updates. That logic lives in the new ParentSpans class.
💡 Motivation and Context
(See above.)
Measuring performance
Performance of SentryTraced is essentially unchanged, if not slightly better with the introduction of this PR. Expand below to see the details.
Details
I had my clanker run 8 time trials, comparing
mainagainst this PR.Key Timing Results
ui.composechild countComposeActivitychild set in the stock multi-screen flowui.composechild countComposeActivitybuttons_pageui.composeavg msComposeActivitybutton_nav_githubui.composeavg msComposeActivitybutton_dialog#2ui.composeavg msComposeActivitybuttons_pageui.composeavg msComposeActivitybutton_nav_githubui.composeavg msComposeActivitybutton_dialog#2ui.composeavg msComposeActivitybuttons_pageui.renderavg msComposeActivitynavhostui.renderavg msExecutive Summary
ui.composechild countComposeActivitychild spans in the stock flowbuttons_pageui.composeavg msbutton_nav_githubui.composeavg ms💚 How did you test it?
New unit tests + I had my clanker run manual tests for both correctness and performance against the Compose and Nav2 sample apps.
📝 Checklist
sendDefaultPIIis enabled.🔮 Next steps