Repository navigation
Conversation
UI profiling never ran after app start. React Native turns off native tracing, so the native trace lifecycle never sees a sampled transaction. Also, no API let JS start or stop the native profiler, so the manual lifecycle could not work either. Add the startProfiler and stopProfiler bridge methods. On Android, they call Sentry.startProfiler() and Sentry.stopProfiler(), and startProfiler returns the profiler id. On iOS, they call SentrySDK. sentry-cocoa has no public API for the profiler id, so iOS returns nil. Always set the manual lifecycle on the native side, because both native SDKs refuse manual calls in trace mode. A new default integration, ProfilingIntegration, now chooses when the profiler runs. In trace mode, it runs while a sampled root span is active. In manual mode, it follows Sentry.profiler.startProfiler() and stopProfiler(), which are now exported. The integration waits for native init, stops an app start profile that is no longer needed, and adds the profiler id to transactions so that they link to the profile.
Replace the PR number placeholder in the CHANGELOG entry with #6916.
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. |
![sentry-warden[bot]](https://avatars.githubusercontent.com/in/2755838?s=60&v=4){kind=small}
Warden found two problems with the profiler id. The integration kept one id for all transactions, so transactions that ran after the profile stopped still got the id of the stopped profile. Also, the id arrived asynchronously and was dropped when the root span ended before the native start call resolved. Each start call now creates a session that holds the promise of the native start call. Each sampled root span that is active while a session runs is mapped to that session, and processEvent waits for the promise before it adds the id. A transaction now gets only the id of a profile that ran during its root span. In trace mode, the integration now waits one 60 second chunk after the last root span ends before it calls stop. The native SDKs ignore a start call that comes before a requested stop takes effect at the end of the chunk, so back-to-back root spans lost their profile after the chunk ended.
Sentry opens the profile of a span on the thread from the span's thread.id. React Native spans had no thread.id, so the profile opened on the main thread, and the JS work was in another, unnamed thread. Add the getCurrentThreadId bridge method. It runs synchronously on the JS thread and returns the thread id in the format of the native profiler: the Linux thread id on Android and the Mach thread port on iOS. The UI profiling integration reads it once and sets thread.id on every span that does not have one.
…ampled roots Warden found two more problems in the UI profiling integration. In manual mode, a new session overwrote the session of root spans that started under an earlier session. A transaction could then get the id of a later profile. A root span now keeps the session that was running when it started. In trace mode with startOnAppStart, unsampled root spans were not tracked, so their end never cleared the pending app start profile. With tracesSampleRate: 0, the profiler then ran for the whole session. The end of any root span now ends the app start profile.
![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 4931e63. Configure here.
| if (jsThreadId && spanToJSON(span).data[SPAN_THREAD_ID] === undefined) { | ||
| span.setAttribute(SPAN_THREAD_ID, jsThreadId); |
There was a problem hiding this comment.
onSpanStart crashes when spanToJSON().data is undefined
Use optional chaining (spanToJSON(span).data?.[SPAN_THREAD_ID]) so unsampled/NonRecording spans that omit data do not throw on every spanStart.
Evidence
onSpanStartruns thethread.idcheck for every span before the sampled/root early-return.- It indexes
spanToJSON(span).data[SPAN_THREAD_ID]without?., unlikeaddThreadInfoToSpanwhich usesdata?.. - When
jsThreadIdis set (nativegetCurrentThreadId()), unsampled spans whosespanToJSON()omitsdatathrowTypeErroron start. - Existing unsampled tests keep
getCurrentThreadIdmocked asnull, so this path is untested.
Identified by Warden · code-review · PWQ-W6C
There was a problem hiding this comment.
Not changing this. spanToJSON() always returns data. SpanJSON.data is a required field, and the fallback branch for spans that are not SentrySpan returns data: {}. Unsampled root spans are SentrySpan instances, so getSpanJSON() also sets data. The ?. in addThreadInfoToSpan is not needed either.
| const stopNativeProfiler = (): void => { | ||
| clearStopTimeout(); | ||
| session = undefined; | ||
| NATIVE.stopProfiler().then(undefined, () => {}); | ||
| }; | ||
|
|
||
| const syncNativeProfiler = (): void => { | ||
| if (!isNativeReady) { | ||
| return; | ||
| } | ||
|
|
||
| if (shouldRun()) { | ||
| clearStopTimeout(); | ||
| if (!session) { | ||
| const newSession: ProfileSession = { profilerId: NATIVE.startProfiler() }; | ||
| session = newSession; | ||
| activeRootSpanIds.forEach(spanId => { | ||
| // A root span that started under an earlier session keeps that profile. | ||
| if (!sessionByRootSpanId.has(spanId)) { | ||
| sessionByRootSpanId.set(spanId, newSession); | ||
| } | ||
| }); | ||
| } | ||
| } else if (session && !stopTimeout) { | ||
| if (lifecycle === 'trace') { | ||
| stopTimeout = setTimeout(stopNativeProfiler, TRACE_STOP_DELAY_MS); | ||
| } else { | ||
| stopNativeProfiler(); | ||
| } |
There was a problem hiding this comment.
Immediate start after stop leaves a zombie JS session while native ignores start
stopNativeProfiler() clears session and requests stop, then a later start immediately sets session again; per the file comment native can ignore that start until the current 60s chunk ends, so JS will not call startProfiler again until shouldRun drops and clears session.
Evidence
- Lines 15–18 document that native ignores start while a requested stop is still pending until the current 60s chunk ends.
stopNativeProfiler()(lines 60–64) setssession = undefinedand fire-and-forgetsNATIVE.stopProfiler()without a stopping/cooldown gate.syncNativeProfiler()(lines 71–81) treats any falsysessionas “not running” and immediately assignssession = { profilerId: NATIVE.startProfiler() }whenevershouldRun()is true.- Manual mode calls
stopNativeProfiler()immediately on stop (lines 86–87), sostopProfiler()thenstartProfiler()hits this path with no delay; trace mode hits it on the first root span after the delayed stop fires.
Identified by Warden · find-bugs · 6SZ-X36
There was a problem hiding this comment.
This is the native bug that this PR works around in trace mode, not a JS state problem. The native SDKs ignore a start call that comes before a requested stop takes effect. getsentry/sentry-java#6291 and getsentry/sentry-cocoa#9401 fix that: a start call now cancels the pending stop, and the native profiler keeps running, which matches the JS session. Tracking a JS cooldown here would duplicate the native state and would go wrong as soon as the native fixes ship.
…app start profile With startOnAppStart, the native SDK starts the profile before JS runs. Root spans that started and ended before native init had no session, so their transactions got no profiler id. Also, the afterInit handler then saw nothing to run and stopped the app start profile at once, without the delay that trace mode uses. The integration now creates the app start session in setup. Root spans that start before native init map to it, and its id resolves when native init completes. After that, the normal logic decides when the profiler stops. The session resolves without an id if the client closes before native init, so no transaction waits forever.
antonis
left a comment
antonis
left a comment
There was a problem hiding this comment.
Overall the changes make sense. Let's merge the latest from main and run the full ci checks.
The native fixes are in getsentry/sentry-java#6291 and getsentry/sentry-cocoa#9401.
Do we need those to test end to end?
Let's also add an example link to the Sentry dashboard in the description.
2 participants
📢 Type of change
📜 Description
This PR adds the
startProfilerandstopProfilerbridge methods, and a defaultProfilingIntegrationthat uses them to control the native UI profiler from JS. The native SDKs now always use the manual lifecycle. Intracemode, the integration runs the profiler while a sampled root span is active, and it calls stop one 60 second chunk after the last root span ends. Inmanualmode, it followsSentry.profiler.startProfiler()andstopProfiler(), which are now exported.Each transaction gets the
profiler_idof the profile session that ran during its root span, and every span gets the nativethread.idof the JS thread, so Sentry opens the profile on that thread. On iOS, sentry-cocoa has no public API for the profiler id, so iOS transactions are not linked to the profile yet.💡 Motivation and Context
UI profiling from #5518 ran only for the app start profile. The native
tracelifecycle needs native transactions, but React Native turns off native tracing. Themanuallifecycle had no API that JS could call.💚 How did you test it?
New Jest tests for the integration, the wrapper, and the default integrations. All JS tests,
yarn lint, the build, and the API report pass. The updated AndroidRNSentryProfilingOptionsTestand the iOSRNSentryInitNativeSdkTestspass locally. I checked the change end to end on an Android 16 emulator with a test app and a Sentry project. Transactions get theprofiler_idand the JS thread id, and the profile chunks cover the transactions on that thread.📝 Checklist
sendDefaultPIIis enabled.🔮 Next steps
sentry-cocoa must expose the current profiler id, so that iOS transactions can link to the profile. In sentry-java (both profilers) and in sentry-cocoa, a start call that comes before a requested stop takes effect is ignored. The delayed stop in
tracemode works around this for back-to-back spans, but a start in the window between the stop call and the end of the chunk still loses the rest of the profile. The native fixes are in getsentry/sentry-java#6291 and getsentry/sentry-cocoa#9401.