feat(core): Attach TurboModule breakdown to active spans on spanEnd#6478
Open
alwx wants to merge 11 commits into
Open
feat(core): Attach TurboModule breakdown to active spans on spanEnd#6478alwx wants to merge 11 commits into
alwx wants to merge 11 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}
alwx
added a commit
that referenced
this pull request
Jul 22, 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>
📲 Install BuildsAndroid
|
antonis
reviewed
Jul 22, 2026
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?
Contributor
Android (legacy) Performance metrics 🚀
|
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| a5d243c+dirty | 424.52 ms | 485.18 ms | 60.66 ms |
| e5bb5f6+dirty | 430.09 ms | 459.96 ms | 29.87 ms |
| 7ac3378+dirty | 404.78 ms | 439.84 ms | 35.06 ms |
| 5789645+dirty | 426.82 ms | 495.42 ms | 68.60 ms |
| 853723c+dirty | 405.54 ms | 440.08 ms | 34.54 ms |
| 61cc206+dirty | 522.33 ms | 553.98 ms | 31.65 ms |
| 44c8b3f+dirty | 414.20 ms | 457.28 ms | 43.08 ms |
| 68ae91b+dirty | 416.44 ms | 477.56 ms | 61.12 ms |
| 7a89652+dirty | 537.76 ms | 567.84 ms | 30.08 ms |
| b0d3373+dirty | 557.66 ms | 579.42 ms | 21.76 ms |
App size
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| a5d243c+dirty | 48.30 MiB | 53.54 MiB | 5.23 MiB |
| e5bb5f6+dirty | 49.74 MiB | 55.09 MiB | 5.34 MiB |
| 7ac3378+dirty | 43.75 MiB | 48.13 MiB | 4.37 MiB |
| 5789645+dirty | 49.74 MiB | 54.85 MiB | 5.11 MiB |
| 853723c+dirty | 48.30 MiB | 53.58 MiB | 5.28 MiB |
| 61cc206+dirty | 49.74 MiB | 55.36 MiB | 5.62 MiB |
| 44c8b3f+dirty | 48.30 MiB | 53.46 MiB | 5.15 MiB |
| 68ae91b+dirty | 49.74 MiB | 54.79 MiB | 5.05 MiB |
| 7a89652+dirty | 48.30 MiB | 53.60 MiB | 5.30 MiB |
| b0d3373+dirty | 48.30 MiB | 53.58 MiB | 5.28 MiB |
Contributor
iOS (legacy) Performance metrics 🚀
|
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| 7ff4d0f+dirty | 3838.29 ms | 1225.13 ms | -2613.16 ms |
| 7ac3378+dirty | 1213.37 ms | 1218.15 ms | 4.78 ms |
| ecf47a2+dirty | 3835.20 ms | 1222.43 ms | -2612.77 ms |
| e763471+dirty | 3841.16 ms | 1218.06 ms | -2623.09 ms |
| a0d8cf8+dirty | 3842.33 ms | 1212.40 ms | -2629.93 ms |
| 5748023+dirty | 3840.49 ms | 1227.43 ms | -2613.05 ms |
| c004dae+dirty | 3850.32 ms | 1227.79 ms | -2622.53 ms |
| 3ce5254+dirty | 1219.93 ms | 1221.90 ms | 1.96 ms |
| 37a2091+dirty | 3821.77 ms | 1212.34 ms | -2609.43 ms |
| 5ca03f9+dirty | 3844.55 ms | 1218.36 ms | -2626.19 ms |
App size
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| 7ff4d0f+dirty | 5.15 MiB | 6.70 MiB | 1.55 MiB |
| 7ac3378+dirty | 3.38 MiB | 4.76 MiB | 1.38 MiB |
| ecf47a2+dirty | 4.98 MiB | 6.46 MiB | 1.49 MiB |
| e763471+dirty | 4.98 MiB | 6.51 MiB | 1.53 MiB |
| a0d8cf8+dirty | 5.15 MiB | 6.67 MiB | 1.51 MiB |
| 5748023+dirty | 5.15 MiB | 6.68 MiB | 1.53 MiB |
| c004dae+dirty | 5.15 MiB | 6.67 MiB | 1.51 MiB |
| 3ce5254+dirty | 3.38 MiB | 4.76 MiB | 1.38 MiB |
| 37a2091+dirty | 5.15 MiB | 6.70 MiB | 1.54 MiB |
| 5ca03f9+dirty | 4.98 MiB | 6.53 MiB | 1.55 MiB |
Contributor
iOS (new) Performance metrics 🚀
|
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| ae37560+dirty | 3840.10 ms | 1217.89 ms | -2622.21 ms |
| e763471+dirty | 3826.10 ms | 1221.52 ms | -2604.58 ms |
| 0b1b5e3+dirty | 3820.72 ms | 1207.94 ms | -2612.78 ms |
| 5ca03f9+dirty | 3873.76 ms | 1236.26 ms | -2637.51 ms |
| acd838e+dirty | 3835.94 ms | 1215.87 ms | -2620.07 ms |
| 5789645+dirty | 3841.36 ms | 1214.81 ms | -2626.55 ms |
| 61cc206+dirty | 3822.60 ms | 1206.17 ms | -2616.43 ms |
| a2585ce+dirty | 3837.98 ms | 1214.20 ms | -2623.77 ms |
| bc8f61e+dirty | 3849.87 ms | 1221.19 ms | -2628.68 ms |
| 5b7e8a7+dirty | 3855.46 ms | 1234.47 ms | -2620.99 ms |
App size
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| ae37560+dirty | 5.15 MiB | 6.70 MiB | 1.54 MiB |
| e763471+dirty | 4.98 MiB | 6.51 MiB | 1.53 MiB |
| 0b1b5e3+dirty | 5.15 MiB | 6.70 MiB | 1.54 MiB |
| 5ca03f9+dirty | 4.98 MiB | 6.53 MiB | 1.55 MiB |
| acd838e+dirty | 5.15 MiB | 6.70 MiB | 1.55 MiB |
| 5789645+dirty | 4.98 MiB | 6.50 MiB | 1.52 MiB |
| 61cc206+dirty | 4.98 MiB | 6.55 MiB | 1.57 MiB |
| a2585ce+dirty | 4.98 MiB | 6.55 MiB | 1.57 MiB |
| bc8f61e+dirty | 4.98 MiB | 6.47 MiB | 1.49 MiB |
| 5b7e8a7+dirty | 5.15 MiB | 6.70 MiB | 1.54 MiB |
Contributor
Android (new) Performance metrics 🚀
|
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| 04207c4+dirty | 395.40 ms | 456.55 ms | 61.15 ms |
| e5bb5f6+dirty | 438.85 ms | 466.33 ms | 27.48 ms |
| 3d377b5+dirty | 425.38 ms | 440.67 ms | 15.30 ms |
| 5789645+dirty | 502.66 ms | 594.19 ms | 91.53 ms |
| 4953e94+dirty | 398.80 ms | 431.81 ms | 33.01 ms |
| 7d6fd3a+dirty | 435.06 ms | 458.78 ms | 23.72 ms |
| 7ff4d0f+dirty | 403.38 ms | 427.06 ms | 23.68 ms |
| 61cc206+dirty | 417.44 ms | 466.29 ms | 48.85 ms |
| 853723c+dirty | 415.82 ms | 460.94 ms | 45.12 ms |
| 27d9693+dirty | 438.63 ms | 514.08 ms | 75.46 ms |
App size
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| 04207c4+dirty | 43.94 MiB | 48.98 MiB | 5.04 MiB |
| e5bb5f6+dirty | 49.74 MiB | 55.09 MiB | 5.34 MiB |
| 3d377b5+dirty | 43.94 MiB | 49.00 MiB | 5.06 MiB |
| 5789645+dirty | 49.74 MiB | 54.85 MiB | 5.11 MiB |
| 4953e94+dirty | 43.94 MiB | 48.94 MiB | 5.00 MiB |
| 7d6fd3a+dirty | 43.94 MiB | 49.00 MiB | 5.06 MiB |
| 7ff4d0f+dirty | 48.30 MiB | 53.60 MiB | 5.30 MiB |
| 61cc206+dirty | 49.74 MiB | 55.36 MiB | 5.62 MiB |
| 853723c+dirty | 48.30 MiB | 53.58 MiB | 5.28 MiB |
| 27d9693+dirty | 49.74 MiB | 55.09 MiB | 5.34 MiB |
8 tasks
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>
`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>
- Late-settling async records after `spanEnd` write via
`span.setAttributes` to a frozen span, which the Sentry SDK silently
drops. Also buffer the attribute payload by span_id and merge into
`event.contexts.trace.data` in `processEvent` so the update lands on
the transaction that ships. `MAX_PENDING_SPAN_ATTRIBUTES` caps the
buffer against sampled-out transactions never coming back to claim
their entry.
- Escape `.` in module/method names when composing attribute keys.
`turbo_module.${name}.${method}.*` collides otherwise —
`(name="a.b", method="c")` and `(name="a", method="b.c")` both produce
`turbo_module.a.b.c.*` and overwrite each other. Same fix applied to
the aggregate flush serialisation.
- Fix the pendingCallWindows cap-eviction test: previously the fillers
never settled, so `attachWindowToSpan` returned early on empty
counters and the assertion loop never ran. Settle a filler so the
eviction outcome is actually verified.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Prune verbose block comments that restated the code or referenced issues. Kept one-line WHYs where the reasoning is load-bearing (frozen-span setAttributes, dot-collision escape, merge semantics). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Escape `_` as `__` in safeKeyPart so `(a.b, c)` and `(a_b, c)` no longer collapse to the same attribute key, and apply the same escaping to the `turbo_module.top_module` value. Prompted by Cursor Bugbot review.
antonis
reviewed
Jul 24, 2026
| * of the per-(module, method, kind) counters. | ||
| */ | ||
| ignoreTurboModules?: ReadonlyArray<string>; | ||
| enableSpanAttribution?: boolean; |
Contributor
There was a problem hiding this comment.
q: why we removed the comments? it would be nice to keep the old and new attibutes documented
antonis
reviewed
Jul 24, 2026
antonis
left a comment
antonis
left a comment
Contributor
There was a problem hiding this comment.
Overall LGTM and the CI is 🟢
Left a nit question.
Let's also get a ✅ from the @getsentry/team-mobile since this involves public api changes
Keeps the safeKeyPart collision fix from 643678a but brings back the JSDoc / WHY comments that were trimmed alongside it.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 6b8c23a. Configure here.
| unit: 'millisecond', | ||
| }; | ||
| attributes['turbo_module.top_module'] = `${safeKeyPart(top.name)}.${safeKeyPart(top.method)}`; | ||
| attributes['turbo_module.top_module_duration_ms'] = roundMs(top.totalDurationMs); |
There was a problem hiding this comment.
Escaped top_module display value
Low Severity
turbo_module.top_module stores an escaped name.method string via safeKeyPart, so the summary value no longer matches the real TurboModule identity when names contain _ or .. Escaping is only needed for attribute keys, not for this human-readable summary value.
Reviewed by Cursor Bugbot for commit 6b8c23a. Configure here.
| function serialiseRows(rows: ReadonlyArray<TurboModuleAggregate>): Record<string, number | string> { | ||
| const out: Record<string, number | string> = {}; | ||
| for (const row of rows) { | ||
| const prefix = `turbo_modules.${safeKeyPart(row.name)}.${safeKeyPart(row.method)}.${row.kind}`; |
There was a problem hiding this comment.
Aggregate attribute keys renamed
Medium Severity
Existing aggregate span attributes now run module/method names through safeKeyPart, so any TurboModule name or method containing _ or . gets a different key than before (for example _ → __). That silently changes the already-shipped turbo_modules.* telemetry shape without a changelog or migration note.
Triggered by project rule: PR Review Guidelines for Cursor Bot
Reviewed by Cursor Bugbot for commit 6b8c23a. Configure here.
antonis
approved these changes
Jul 24, 2026
Contributor
There was a problem hiding this comment.
LGTM 🚀
Worths rechecking the latest cursor comments but nothing blocking on my end. Let's wait for the team review and 🚢
Also iterating from a previous previous review
I would advocate towards making the changes opt-in / experimental first to gather some feedback from users (track adoption similar to #6428) and verify stability before releasing for all. Wdyt?
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.