Repository navigation
fix(profiling): Drive the native UI profiler from JS - #6916
Conversation
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.
Plus 1 more 🤖 This preview updates automatically when you update the PR. |
![sentry-warden[bot]](https://avatars.githubusercontent.com/in/2755838?s=60&v=4){kind=small}
![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.
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.
|
Q: Should this integration be enabled when user sets |
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.
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.
…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.
be8626e to
ff7b4bb
Compare
|
@lucas-zimerman Good catch. I would keep the |
…s: false With defaultIntegrations: false, the UI profiling integration was not installed and could not be added, because it was not exported. The native SDK still received profilingOptions, so trace mode never ran, Sentry.profiler did nothing, and with startOnAppStart nothing ever stopped the app start profile. Export uiProfilingIntegration. It now reads profilingOptions from the client options, so users add it without arguments. If profilingOptions is set but the integration is not installed, the client logs a warning and does not send profilingOptions to the native SDK.
|
@lucas-zimerman Done in 565a986. |
📲 Install BuildsAndroid
|
|
@antonis Thanks! Rebased on main, and full CI is running.
No. I tested this PR end to end with the stock sentry-java 8.61.0: transactions get the About the dashboard link: I will do one more run with this PR and the stock native SDK, and add a trace link to the description. |
|
The trace link added :) |
3 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.Example with this PR and the stock sentry-java 8.61.0 (Android 16 emulator,
tracelifecycle). The transactionssingle,firstandsecondhave theprofiler_idand the JSthread.id, and the profile has samples on the JS thread during each of them:📝 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.