feat(core): Attach TurboModule breakdown to active spans on spanEnd#6478
Open
alwx wants to merge 8 commits into
Open
feat(core): Attach TurboModule breakdown to active spans on spanEnd#6478alwx wants to merge 8 commits into
alwx wants to merge 8 commits into
Conversation
Contributor
Semver Impact of This PR⚪ None (no version bump detected) 📋 Changelog PreviewThis is how your changes will appear in the changelog.
🤖 This preview updates automatically when you update the PR. |
Contributor
|
Contributor
Author
|
@cursor review |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
alwx
added a commit
that referenced
this pull request
Jul 20, 2026
- Nested `name→method` counter map so identifiers with any character can't collide (also removes a stray NUL byte in the compound key). - Fire per-record observers even when `enableAggregateStats: false`, and apply `ignoreTurboModules` whenever any consumer of the record path is active. Fixes span attribution + slow-call breadcrumb going silent when aggregate stats are opted out. - Snapshot the set of open windows at call start via a new `notifyTurboModuleCallStart` hook, so async calls that outlive their originating span still credit that span. Late-settling records re-emit `setAttributes` on the (now closed) span. - CHANGELOG entry references PR #6478 instead of the issue. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
![sentry-warden[bot]](https://avatars.githubusercontent.com/in/2755838?s=60&v=4){kind=small}
Adds per-span attribution to `turboModuleContextIntegration`: on root
span end, writes `turbo_module.<name>.<method>.{call_count,duration_ms,
error_count}` attributes plus summary keys. Async calls above
`slowCallThresholdMs` (default 500ms) also emit a `native.turbo_module`
breadcrumb. New knobs: `enableSpanAttribution`, `slowCallThresholdMs`,
`maxTopModulesPerSpan`.
Closes #6165.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
- Nested `name→method` counter map so identifiers with any character can't collide (also removes a stray NUL byte in the compound key). - Fire per-record observers even when `enableAggregateStats: false`, and apply `ignoreTurboModules` whenever any consumer of the record path is active. Fixes span attribution + slow-call breadcrumb going silent when aggregate stats are opted out. - Snapshot the set of open windows at call start via a new `notifyTurboModuleCallStart` hook, so async calls that outlive their originating span still credit that span. Late-settling records re-emit `setAttributes` on the (now closed) span. - CHANGELOG entry references PR #6478 instead of the issue. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
`startObserver` used to skip writing to `pendingCallWindows` when the snapshot was empty. When the call settled, `recordObserver` couldn't find the recordId and fell back to the currently-open windows — crediting spans that opened *after* the call started. Common case: an async TurboModule call kicked off during app init, before any nav / user span has opened. Always record a snapshot (even an empty one), and reserve the currently-open fallback for records without a recordId (direct `recordTurboModuleCall` callers in tests). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
- Clear per-method attribute keys from a previous emit that no longer fit in the top-N. `setAttributes` merges, so without this a late-settling async call that re-ranks the top-N would leave dropped rows on the span. - Cap `pendingCallWindows` at 1024 so abandoned (never-settling) async promises can't pin `WindowState` indefinitely. Evicted entries are silently dropped on late settle rather than mis-attributed. - Correct the misleading comment about the WeakMap — it enables O(1) `spanEnd` lookup; it does not protect against leaks (the parallel `openWindowList` holds strong refs). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
- Register the record observer whenever either `enableSpanAttribution` or `slowCallThresholdMs > 0` is active, and gate span-attribution and breadcrumb logic separately inside. Previously the breadcrumb code lived inside the `enableSpanAttribution` block, so disabling span attribution silently turned off breadcrumbs — contradicting the documented independent `slowCallThresholdMs` knob. `setIgnoredTurboModules` also now applies in breadcrumbs-only mode. - Sync calls skip `pendingCallWindows`. Sync start + settle happen in the same turn, so they can credit `openWindowList` directly. Skipping them also prevents sync bursts from evicting genuine async entries under the MAX_PENDING_CALL_WINDOWS cap. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
📲 Install BuildsAndroid
|
antonis
reviewed
Jul 22, 2026
|
|
||
| - Attach a per-`(module, method)` TurboModule breakdown to active spans on `spanEnd`, plus `native.turbo_module` breadcrumbs for slow async calls ([#6478](https://github.com/getsentry/sentry-react-native/pull/6478)) | ||
|
|
||
| When a root span ends (idle nav spans from `reactNavigationIntegration` / `expoRouterIntegration`, or a user's own `Sentry.startSpan(...)`), the integration writes `turbo_module.<name>.<method>.{call_count,duration_ms,error_count}` attributes plus summary keys (`turbo_module.total_call_count`, `turbo_module.total_duration_ms`, `turbo_module.top_module`). Async calls above `slowCallThresholdMs` (default 500ms) additionally record a `native.turbo_module` breadcrumb. Both surfaces enabled by default; new knobs `enableSpanAttribution`, `slowCallThresholdMs`, `maxTopModulesPerSpan` on `turboModuleContextIntegration`. |
Contributor
There was a problem hiding this comment.
Nit: Missing line before the Fixes section.
antonis
reviewed
Jul 22, 2026
antonis
left a comment
antonis
left a comment
Contributor
There was a problem hiding this comment.
Overall the PR LGTM once the CI is 🟢 (apart from the known issue related failures).
I would advocate towards making the changes opt-in / experimental first to gather some feedback from users (track adoption similar to this) and verify stability before releasing for all. Wdyt?
`wrapTurboModule` always calls `notifyTurboModuleCallStart` with kind `'sync'` and only relabels to `'async'` once the return value is known to be thenable. Gating the start observer on `kind === 'async'` therefore silently dropped every async attribution in production. Revert to snapshotting on every start — sync entries settle in the same synchronous turn and are removed by their paired record, so they don't accumulate under normal traffic. Also add the missing blank line between the Features and Fixes sections in the CHANGELOG. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
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.
Reviewed by Cursor Bugbot for commit 5c098eb. Configure here.
| // parent transaction is serialised. | ||
| if (window.closed) { | ||
| attachWindowToSpan(window.span, window, maxTopModulesPerSpan); | ||
| } |
There was a problem hiding this comment.
Late async attributes miss transactions
Medium Severity
Late-settling async TurboModule calls re-apply span attributes with setAttributes after spanEnd, but the transaction event is usually already built and captured in that same turn. Those updates therefore never appear on the sent transaction. Similar late data in this codebase (e.g. native frames) is merged in processEvent instead.
Additional Locations (1)
Triggered by project rule: PR Review Guidelines for Cursor Bot
Reviewed by Cursor Bugbot for commit 5c098eb. Configure here.
Contributor
Android (legacy) Performance metrics 🚀
|
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| f3215d3+dirty | 411.11 ms | 454.38 ms | 43.27 ms |
| d2eadf8+dirty | 414.64 ms | 454.56 ms | 39.92 ms |
| b0d3373+dirty | 557.66 ms | 579.42 ms | 21.76 ms |
| 27d9693+dirty | 419.08 ms | 469.12 ms | 50.04 ms |
| 4e0b819+dirty | 420.56 ms | 470.08 ms | 49.52 ms |
| 9474ead+dirty | 411.45 ms | 446.80 ms | 35.35 ms |
| 0bd8916+dirty | 412.77 ms | 451.31 ms | 38.54 ms |
| f9c1ed4+dirty | 431.00 ms | 466.22 ms | 35.22 ms |
| 3d536d1+dirty | 524.34 ms | 547.32 ms | 22.98 ms |
| 04207c4+dirty | 459.19 ms | 518.54 ms | 59.35 ms |
App size
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| f3215d3+dirty | 48.30 MiB | 53.49 MiB | 5.19 MiB |
| d2eadf8+dirty | 48.30 MiB | 53.48 MiB | 5.18 MiB |
| b0d3373+dirty | 48.30 MiB | 53.58 MiB | 5.28 MiB |
| 27d9693+dirty | 49.74 MiB | 55.09 MiB | 5.34 MiB |
| 4e0b819+dirty | 49.74 MiB | 54.81 MiB | 5.07 MiB |
| 9474ead+dirty | 48.30 MiB | 53.61 MiB | 5.30 MiB |
| 0bd8916+dirty | 48.30 MiB | 53.57 MiB | 5.26 MiB |
| f9c1ed4+dirty | 49.74 MiB | 54.86 MiB | 5.12 MiB |
| 3d536d1+dirty | 49.74 MiB | 55.26 MiB | 5.52 MiB |
| 04207c4+dirty | 43.75 MiB | 48.12 MiB | 4.37 MiB |
Contributor
iOS (legacy) Performance metrics 🚀
|
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| b9bebee+dirty | 3850.15 ms | 1227.51 ms | -2622.64 ms |
| 3d377b5+dirty | 1218.48 ms | 1219.51 ms | 1.03 ms |
| 1a2e7e0+dirty | 3842.49 ms | 1220.04 ms | -2622.45 ms |
| e5bb5f6+dirty | 3826.14 ms | 1212.24 ms | -2613.90 ms |
| 6acdf1d+dirty | 3844.33 ms | 1212.96 ms | -2631.38 ms |
| 41d6254+dirty | 3845.71 ms | 1224.51 ms | -2621.20 ms |
| 9210ae6+dirty | 3815.93 ms | 1214.14 ms | -2601.79 ms |
| c004dae+dirty | 3850.32 ms | 1227.79 ms | -2622.53 ms |
| 4e0b819+dirty | 3839.05 ms | 1210.75 ms | -2628.30 ms |
| 0b5120f+dirty | 3838.39 ms | 1232.91 ms | -2605.48 ms |
App size
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| b9bebee+dirty | 5.15 MiB | 6.68 MiB | 1.53 MiB |
| 3d377b5+dirty | 3.38 MiB | 4.76 MiB | 1.38 MiB |
| 1a2e7e0+dirty | 4.98 MiB | 6.46 MiB | 1.49 MiB |
| e5bb5f6+dirty | 4.98 MiB | 6.51 MiB | 1.53 MiB |
| 6acdf1d+dirty | 4.98 MiB | 6.51 MiB | 1.53 MiB |
| 41d6254+dirty | 5.15 MiB | 6.70 MiB | 1.54 MiB |
| 9210ae6+dirty | 5.15 MiB | 6.68 MiB | 1.53 MiB |
| c004dae+dirty | 5.15 MiB | 6.67 MiB | 1.51 MiB |
| 4e0b819+dirty | 4.98 MiB | 6.46 MiB | 1.49 MiB |
| 0b5120f+dirty | 5.15 MiB | 6.68 MiB | 1.53 MiB |
Contributor
iOS (new) Performance metrics 🚀
|
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| b0d3373+dirty | 3842.49 ms | 1218.49 ms | -2624.00 ms |
| ef27341+dirty | 3835.20 ms | 1212.23 ms | -2622.97 ms |
| e5bb5f6+dirty | 3825.74 ms | 1217.30 ms | -2608.43 ms |
| 44c8b3f+dirty | 3849.24 ms | 1209.94 ms | -2639.31 ms |
| 68672fc+dirty | 3832.22 ms | 1228.29 ms | -2603.93 ms |
| 5748023+dirty | 3844.74 ms | 1225.49 ms | -2619.26 ms |
| 6acdf1d+dirty | 3835.35 ms | 1218.30 ms | -2617.06 ms |
| 2c735cc+dirty | 1223.33 ms | 1224.38 ms | 1.04 ms |
| 100ce80+dirty | 3843.57 ms | 1226.46 ms | -2617.12 ms |
| 9210ae6+dirty | 3834.11 ms | 1216.64 ms | -2617.47 ms |
App size
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| b0d3373+dirty | 5.15 MiB | 6.68 MiB | 1.53 MiB |
| ef27341+dirty | 5.15 MiB | 6.68 MiB | 1.53 MiB |
| e5bb5f6+dirty | 4.98 MiB | 6.51 MiB | 1.53 MiB |
| 44c8b3f+dirty | 5.15 MiB | 6.66 MiB | 1.51 MiB |
| 68672fc+dirty | 5.15 MiB | 6.71 MiB | 1.55 MiB |
| 5748023+dirty | 5.15 MiB | 6.68 MiB | 1.53 MiB |
| 6acdf1d+dirty | 4.98 MiB | 6.51 MiB | 1.53 MiB |
| 2c735cc+dirty | 3.38 MiB | 4.74 MiB | 1.35 MiB |
| 100ce80+dirty | 5.15 MiB | 6.67 MiB | 1.51 MiB |
| 9210ae6+dirty | 5.15 MiB | 6.68 MiB | 1.53 MiB |
Contributor
Android (new) Performance metrics 🚀
|
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| a50b33d+dirty | 353.21 ms | 398.48 ms | 45.27 ms |
| d038a14+dirty | 405.08 ms | 444.36 ms | 39.28 ms |
| 68672fc+dirty | 407.55 ms | 442.96 ms | 35.41 ms |
| ad66da3+dirty | 411.49 ms | 449.38 ms | 37.89 ms |
| f170ec3+dirty | 505.96 ms | 551.88 ms | 45.92 ms |
| 27d9693+dirty | 438.63 ms | 514.08 ms | 75.46 ms |
| 41d6254+dirty | 406.20 ms | 445.52 ms | 39.32 ms |
| 68ae91b+dirty | 515.04 ms | 578.08 ms | 63.04 ms |
| 5ee78d6+dirty | 411.18 ms | 437.83 ms | 26.65 ms |
| ae37560+dirty | 428.96 ms | 456.86 ms | 27.90 ms |
App size
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| a50b33d+dirty | 43.94 MiB | 48.94 MiB | 5.00 MiB |
| d038a14+dirty | 48.30 MiB | 53.49 MiB | 5.19 MiB |
| 68672fc+dirty | 48.30 MiB | 53.61 MiB | 5.31 MiB |
| ad66da3+dirty | 48.30 MiB | 53.49 MiB | 5.19 MiB |
| f170ec3+dirty | 48.30 MiB | 53.57 MiB | 5.26 MiB |
| 27d9693+dirty | 49.74 MiB | 55.09 MiB | 5.34 MiB |
| 41d6254+dirty | 48.30 MiB | 53.60 MiB | 5.30 MiB |
| 68ae91b+dirty | 49.74 MiB | 54.79 MiB | 5.05 MiB |
| 5ee78d6+dirty | 48.30 MiB | 53.58 MiB | 5.28 MiB |
| ae37560+dirty | 48.30 MiB | 53.60 MiB | 5.29 MiB |
Comment on lines
+537
to
+540
| for (const [attributes] of span.setAttributes.mock.calls) { | ||
| expect(attributes['turbo_module.First.work.call_count']).toBeUndefined(); | ||
| } | ||
| }); |
Contributor
There was a problem hiding this comment.
Cap eviction test assertion never executes
The evicted call and all fillers are unsettled, so span.setAttributes is never called and this loop assertion never runs. Settle a filler call to validate the eviction behavior.
Evidence
notifyTurboModuleCallStart('First')followed by 1024 filler starts causes the oldest entry (First) to be evicted frompendingCallWindows.recordTurboModuleCallfor the evictedfirstRecordIdfinds no windows, sorecordIntoWindowis never called.- None of the fillers are settled, leaving
window.countersempty;attachWindowToSpanreturns early onspanEnd. - Consequently
span.setAttributesis never invoked,mock.callsis empty, and thefor...ofblock at line 537 never executes.
Identified by Warden code-review · 2PS-4YV
Comment on lines
42
to
+44
| */ | ||
| const MAX_AGGREGATE_ATTRIBUTE_ROWS = 64; | ||
| export const DEFAULT_MAX_TOP_MODULES_PER_SPAN = 16; | ||
|
|
Contributor
There was a problem hiding this comment.
Span attribute key collision when TurboModule names or methods contain dots
Per-span attribute keys like turbo_module.${name}.${method} collide when module or method names contain dots, causing distinct calls to overwrite each other (e.g. ("a.b", "c") and ("a", "b.c") both map to turbo_module.a.b.c.call_count).
Evidence
WindowState.countersuses nested Maps and a comment atturboModuleContext.ts:380claims identifiers with dots "can never collide with the pair separator".- In
attachWindowToSpan(turboModuleContext.ts:452), the prefix is built as`turbo_module.${row.name}.${row.method}`, using.as the only separator with no escaping. - Two distinct
(module, method)pairs such as("a.b", "c")and("a", "b.c")produce the identical prefixturbo_module.a.b.cand overwrite each other in the attributes object passed tospan.setAttributes. - Test coverage in
turboModuleContext.test.tsdoes not exercise names containing dots, so the collision is unguarded.
Identified by Warden find-bugs · STE-MTS
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
2 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.
📢 Type of change
📜 Description
On root
spanEnd, attach per-(module, method)TurboModule call counts / duration / errors asturbo_module.<name>.<method>.*attributes on the span, plus summary keys. Async calls overslowCallThresholdMs(default 500ms) emit anative.turbo_modulebreadcrumb.New options on
turboModuleContextIntegration:enableSpanAttribution(default on),slowCallThresholdMs,maxTopModulesPerSpan.Fed by a new per-record observer API on the JS aggregator so open spans can accumulate independently of the transaction / periodic flush drain path.
💡 Motivation and Context
Closes #6165
💚 How did you test it?
Unit tests for the span-attribution + breadcrumb paths and the observer contract.
yarn test,yarn lint,yarn circularDepCheck,yarn api-report:checkpass locally.📝 Checklist
sendDefaultPIIis enabled.🔮 Next steps
Docs in #6168.