feat(core): Wire TurboModulePerfLogger on iOS and Android#6307
Open
alwx wants to merge 12 commits into
Open
Conversation
Install a Sentry-owned `facebook::react::NativeModulePerfLogger` on
both platforms so the SDK observes every TurboModule lifecycle event \u2014
`moduleDataCreate*`, `moduleCreate*`, sync/async method call
`start`/`end`/`fail`, async dispatch and execution `start`/`end`/`fail`
\u2014 for follow-up features (crash attribution, per-module spans,
aggregated stats) to plug into.
The implementation is split into:
- **Shared C++** (`packages/core/cpp/`): a single
`SentryTurboModulePerfController` singleton owns the installed logger
and an atomic `enabled` flag. When disabled, every callback hits one
atomic load and returns. When enabled, callbacks are forwarded to a
swappable `ISentryTurboModulePerfSink` \u2014 follow-up issues ship the
sinks; this PR just exposes the hook.
- **iOS**: the perf logger is installed from a dedicated installer
class's `+load` so it fires before `RCTBridge` / `RCTHost` create
their first TurboModule. (`RNSentry`'s own `+load` is reserved by
`RCT_EXPORT_MODULE()`.) The cpp/ directory is added to the podspec
sources; files are guarded with `RCT_NEW_ARCH_ENABLED` so Old Arch
builds compile to empty TUs.
- **Android**: a new `libsentry-tm-perf-logger.so` shared library is
built via CMake under New Architecture only and exposes `JNI_OnLoad`
+ a tiny `nativeSetEnabled` JNI hook. It links against React
Native's `reactnative` prefab; the missing
`<reactperflogger/NativeModulePerfLogger.h>` header is plugged by
pointing the include path at the source tree (mirroring how
react-native-reanimated resolves react-native via the standard
`REACT_NATIVE_NODE_MODULES_DIR` / `require.resolve` fallback).
`RNSentryPackage`'s static initializer `System.loadLibrary`s the
perf-logger lib \u2014 host apps do NOT need to touch their own
`OnLoad.cpp`. A guarded `try { \u2026 } catch (UnsatisfiedLinkError)`
keeps Old Architecture (and any host that strips the lib) working
as before.
Runtime gate: new `enableTurboModuleTracking` option on `Sentry.init`,
default `false` for this first release so the foundation lands without
behavioral change. The native logger is always installed (we never want
to miss early lifecycle events), the flag only decides whether
forwarded callbacks reach the Sentry sink. The option is plumbed
through `initNativeSdk` on both platforms.
Foundation only \u2014 no sink is installed in this PR. Follow-up issues
ship the actual instrumentation.
Closes #6162
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
If a user enables AGP
Our docs instruct React Native users to set
|
![sentry-warden[bot]](https://avatars.githubusercontent.com/in/2755838?s=60&v=4){kind=small}
Contributor
Author
|
@cursor review |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 3b618c5. Configure here.
Address Warden's medium-severity finding on PR #6307: the new `SentryTurboModulePerfController` and `RNSentryTurboModulePerfTracker` shipped without unit coverage. Add focused tests that exercise the state machines independently of React Native's runtime. - **iOS** (`RNSentryCocoaTester/.../RNSentryTurboModulePerfControllerTests.mm`): default `isEnabled() == false`, `setEnabled` toggle, the C-linkage `Sentry_SetTurboModuleTrackingEnabled` entry point matches the typed setter, `setSink`/`sink` round-trips including `nullptr` detach, and `Sentry_InstallTurboModulePerfLogger` idempotency under repeated calls. End-to-end forwarding through `facebook::react::TurboModulePerfLogger` is intentionally not covered here \u2014 it requires `+load` ordering and process-wide singletons that the follow-up sink PRs will integration-test. - **Android** (`RNSentryAndroidTester/.../RNSentryTurboModulePerfTrackerTest.kt`): the JVM-side latch around the JNI symbol. In the test JVM the underlying `.so` is not loaded, so the first `setEnabled` call must catch `UnsatisfiedLinkError` and flip `nativeUnavailable`; subsequent calls must short-circuit. Uses Robolectric so the `android.util.Log.i` call inside the catch branch resolves instead of throwing the not-mocked stub. A small `@TestOnly` window on the tracker exposes the latch state to assertions. Also fix the changelog entry to reference the PR (#6307) rather than the issue (#6162) so danger stops nagging.
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
antonis
reviewed
Jun 18, 2026
antonis
reviewed
Jun 18, 2026
antonis
left a comment
antonis
left a comment
Contributor
There was a problem hiding this comment.
Thank you for you work on this @alwx 🙇
Did a 1st pass and didn't notice anything off other what has been caught by the agents and the lint check. Let's also add the ready-to-merge since there are many changes on the native side that need to be validated.
…g flips on Address two related medium findings on #6307: - Warden: `enableLogging` runs from `+load` / `JNI_OnLoad` regardless of the runtime flag, unconditionally evicting any pre-existing `NativeModulePerfLogger` (Metro, other SDKs, host-app instrumentation). - Cursor: when `enableTurboModuleTracking: true`, callbacks between load time and `initNativeSdk` are dropped by the `enabled_=false` fast-path anyway, so the eager install was not actually delivering on its 'never miss early events' promise \u2014 just on its side effects. The fix is a single one-way ratchet: `setEnabled(true)` lazily calls `install()` on the first transition, and the typed setter doubles as the public lifecycle hook. The `+load` installer class on iOS and the `JNI_OnLoad` install on Android are gone; the C `Sentry_InstallTurboModulePerfLogger` entry stays for hosts that want to claim the perf-logger slot eagerly via their own native code, but it is no longer wired into our load hooks. Header / JSDoc updated to describe the new contract. Also fix two adjacent issues flagged on the same PR: - Sentry HIGH (build.gradle): two sibling `buildFeatures { ... }` blocks under the same Android scope replace rather than merge, so `prefab = true` was clobbering `buildConfig = true` on AGP 8+. Merge into a single conditional block. - Lint: ran `yarn java:format fix`, `yarn fix:clang`, and switched `RNSentryTurboModulePerfTracker.nativeUnavailable` from `volatile` to `AtomicBoolean` to satisfy the project-wide PMD `AvoidUsingVolatile` rule. Removed a Kotlin `no-consecutive-comments` violation from the Robolectric note above the tracker test. Test updates: - iOS: add `testSetEnabledFalseDoesNotInstall` and `testSetEnabledTrueIsLazyInstallAndSticky` to lock down the lazy install ratchet. Existing `testInstallIsIdempotent` still covers explicit-install callers. - Android: tracker tests unchanged in behaviour; only the test-only `isNativeUnavailableForTests` / `resetNativeUnavailableForTests` helpers were updated to go through the new `AtomicBoolean`.
Address Cursor's low-severity finding on #6307: `setEnabled(true)` was storing `enabled_` *after* calling `install()`, so any callback React Native fired synchronously from inside `enableLogging()` would hit the `isEnabled() == false` fast-path and be dropped \u2014 a tiny window of lost events for the very first opted-in invocation. Swap the order: publish `enabled_ = true` (release ordering) before the install, so by the time `enableLogging()` could re-enter us via a synchronous callback, the flag is already visible to other threads. On disable the order does not matter since we never uninstall.
Contributor
Android (legacy) Performance metrics 🚀
|
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| 3ce5254+dirty | 410.57 ms | 448.48 ms | 37.91 ms |
| 1122a96+dirty | 422.22 ms | 464.33 ms | 42.10 ms |
| df5d108+dirty | 527.06 ms | 603.58 ms | 76.52 ms |
| c823bb5+dirty | 409.87 ms | 478.57 ms | 68.70 ms |
| 5b7e8a7+dirty | 418.55 ms | 488.46 ms | 69.91 ms |
| 6177334+dirty | 408.16 ms | 441.14 ms | 32.98 ms |
| 267d3ed+dirty | 413.06 ms | 440.96 ms | 27.90 ms |
| f170ec3+dirty | 428.71 ms | 452.18 ms | 23.47 ms |
| 37a2091+dirty | 407.82 ms | 441.22 ms | 33.40 ms |
| 5fe1c6c+dirty | 401.62 ms | 445.28 ms | 43.66 ms |
App size
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| 3ce5254+dirty | 43.75 MiB | 48.12 MiB | 4.37 MiB |
| 1122a96+dirty | 48.30 MiB | 53.54 MiB | 5.24 MiB |
| df5d108+dirty | 43.75 MiB | 48.08 MiB | 4.33 MiB |
| c823bb5+dirty | 48.30 MiB | 53.58 MiB | 5.28 MiB |
| 5b7e8a7+dirty | 48.30 MiB | 53.58 MiB | 5.28 MiB |
| 6177334+dirty | 48.30 MiB | 53.54 MiB | 5.23 MiB |
| 267d3ed+dirty | 48.30 MiB | 53.58 MiB | 5.28 MiB |
| f170ec3+dirty | 48.30 MiB | 53.57 MiB | 5.26 MiB |
| 37a2091+dirty | 48.30 MiB | 53.58 MiB | 5.28 MiB |
| 5fe1c6c+dirty | 43.75 MiB | 48.14 MiB | 4.39 MiB |
📲 Install BuildsAndroid
|
Contributor
iOS (legacy) Performance metrics 🚀
|
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| 1e5d96d+dirty | 3851.45 ms | 1212.05 ms | -2639.41 ms |
| f170ec3+dirty | 3822.26 ms | 1218.33 ms | -2603.93 ms |
| 68672fc+dirty | 3841.58 ms | 1228.89 ms | -2612.69 ms |
| 0b5a379+dirty | 3828.91 ms | 1214.12 ms | -2614.79 ms |
| f3215d3+dirty | 3842.73 ms | 1219.33 ms | -2623.40 ms |
| 100ce80+dirty | 3842.93 ms | 1229.52 ms | -2613.41 ms |
| 09a902f+dirty | 3835.67 ms | 1217.11 ms | -2618.57 ms |
| 5a010b7+dirty | 3838.85 ms | 1214.98 ms | -2623.87 ms |
| 6176a94+dirty | 3836.50 ms | 1217.64 ms | -2618.86 ms |
| 37a2091+dirty | 3821.77 ms | 1212.34 ms | -2609.43 ms |
App size
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| 1e5d96d+dirty | 4.98 MiB | 6.46 MiB | 1.49 MiB |
| f170ec3+dirty | 5.15 MiB | 6.69 MiB | 1.53 MiB |
| 68672fc+dirty | 5.15 MiB | 6.71 MiB | 1.55 MiB |
| 0b5a379+dirty | 5.15 MiB | 6.70 MiB | 1.54 MiB |
| f3215d3+dirty | 5.15 MiB | 6.67 MiB | 1.52 MiB |
| 100ce80+dirty | 5.15 MiB | 6.67 MiB | 1.51 MiB |
| 09a902f+dirty | 4.98 MiB | 6.46 MiB | 1.49 MiB |
| 5a010b7+dirty | 5.15 MiB | 6.69 MiB | 1.54 MiB |
| 6176a94+dirty | 5.15 MiB | 6.68 MiB | 1.53 MiB |
| 37a2091+dirty | 5.15 MiB | 6.70 MiB | 1.54 MiB |
…debug info Address Warden's medium-severity finding on #6307: passing `-Wl,--strip-all` at CMake link time strips DWARF (and `.symtab`) from `libsentry-tm-perf-logger.so` *before* AGP's `StripDebugSymbolsTask` gets a chance to copy the unstripped artefact for symbolication upload. Any crash inside the library in production would be unsymbolicated even with the Sentry Gradle plugin installed. Drop the manual link option entirely. AGP already strips the .so for the packaged APK while preserving the unstripped copy under `intermediates/merged_native_libs/.../obj`, which is the one Sentry Gradle plugin uploads. Verified locally with `llvm-readelf -S` on the release intermediate: `.debug_*` and `.symtab` sections are now present.
Contributor
iOS (new) Performance metrics 🚀
|
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| 1e5d96d+dirty | 3845.93 ms | 1222.51 ms | -2623.42 ms |
| f170ec3+dirty | 3844.74 ms | 1222.67 ms | -2622.07 ms |
| 68672fc+dirty | 3832.22 ms | 1228.29 ms | -2603.93 ms |
| 0b5a379+dirty | 3857.69 ms | 1230.34 ms | -2627.35 ms |
| f3215d3+dirty | 3846.08 ms | 1231.85 ms | -2614.23 ms |
| 100ce80+dirty | 3843.57 ms | 1226.46 ms | -2617.12 ms |
| 09a902f+dirty | 3847.65 ms | 1221.31 ms | -2626.34 ms |
| 5a010b7+dirty | 3856.76 ms | 1232.65 ms | -2624.11 ms |
| 6176a94+dirty | 3854.15 ms | 1221.77 ms | -2632.38 ms |
| 37a2091+dirty | 3865.27 ms | 1223.92 ms | -2641.35 ms |
App size
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| 1e5d96d+dirty | 4.98 MiB | 6.46 MiB | 1.49 MiB |
| f170ec3+dirty | 5.15 MiB | 6.69 MiB | 1.53 MiB |
| 68672fc+dirty | 5.15 MiB | 6.71 MiB | 1.55 MiB |
| 0b5a379+dirty | 5.15 MiB | 6.70 MiB | 1.54 MiB |
| f3215d3+dirty | 5.15 MiB | 6.67 MiB | 1.52 MiB |
| 100ce80+dirty | 5.15 MiB | 6.67 MiB | 1.51 MiB |
| 09a902f+dirty | 4.98 MiB | 6.46 MiB | 1.49 MiB |
| 5a010b7+dirty | 5.15 MiB | 6.69 MiB | 1.54 MiB |
| 6176a94+dirty | 5.15 MiB | 6.68 MiB | 1.53 MiB |
| 37a2091+dirty | 5.15 MiB | 6.70 MiB | 1.54 MiB |
Contributor
Android (new) Performance metrics 🚀
|
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| 1122a96+dirty | 510.16 ms | 542.00 ms | 31.84 ms |
| c823bb5+dirty | 468.26 ms | 516.16 ms | 47.90 ms |
| 23598c3+dirty | 371.92 ms | 420.65 ms | 48.74 ms |
| 3817909+dirty | 357.52 ms | 391.52 ms | 34.00 ms |
| 5fe1c6c+dirty | 365.84 ms | 408.62 ms | 42.78 ms |
| 5b7e8a7+dirty | 601.58 ms | 634.98 ms | 33.40 ms |
| 6177334+dirty | 404.80 ms | 456.74 ms | 51.94 ms |
| 267d3ed+dirty | 424.69 ms | 483.70 ms | 59.01 ms |
| f170ec3+dirty | 505.96 ms | 551.88 ms | 45.92 ms |
| 37a2091+dirty | 429.71 ms | 477.00 ms | 47.29 ms |
App size
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| 1122a96+dirty | 48.30 MiB | 53.54 MiB | 5.24 MiB |
| c823bb5+dirty | 48.30 MiB | 53.58 MiB | 5.28 MiB |
| 23598c3+dirty | 43.94 MiB | 49.02 MiB | 5.08 MiB |
| 3817909+dirty | 43.94 MiB | 48.94 MiB | 5.00 MiB |
| 5fe1c6c+dirty | 43.94 MiB | 49.00 MiB | 5.06 MiB |
| 5b7e8a7+dirty | 48.30 MiB | 53.58 MiB | 5.28 MiB |
| 6177334+dirty | 48.30 MiB | 53.54 MiB | 5.23 MiB |
| 267d3ed+dirty | 48.30 MiB | 53.58 MiB | 5.28 MiB |
| f170ec3+dirty | 48.30 MiB | 53.57 MiB | 5.26 MiB |
| 37a2091+dirty | 48.30 MiB | 53.58 MiB | 5.28 MiB |
… type Two related findings on #6307: - Cursor MEDIUM (iOS): `Sentry_SetTurboModuleTrackingEnabled` ran *before* `RNSentryStart.startWithOptions`. If init failed and the promise rejected, tracking was already on (and would lazy-install Sentry's perf logger into React Native) while no native SDK was around to consume the data. - Sentry bot LOW (Android): `rnOptions.getBoolean("enableTurboModuleTracking")` was guarded only by `hasKey`. A non-boolean value from JS (number, string, null) would crash with `UnexpectedNativeTypeException`. Move the toggle on both platforms to AFTER `startWithOptions` so nothing is enabled unless the native SDK actually started, and add an explicit `ReadableType.Boolean` check on Android to match the iOS\n`isKindOfClass:[NSNumber class]` guard.
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
|
|
||
| ### Features | ||
|
|
||
| - Wire Sentry's `facebook::react::NativeModulePerfLogger` on both platforms so the SDK observes every TurboModule lifecycle event (`moduleCreate*`, sync/async method call start/end/fail, execution start/end/fail) for crash attribution, per-module spans and aggregated stats in follow-up releases. Install is automatic — no `OnLoad.cpp` changes on Android. Gated by the new `enableTurboModuleTracking` option on `Sentry.init`, default `false` for this first release. New Architecture only ([#6307](https://github.com/getsentry/sentry-react-native/pull/6307)) |
Contributor
There was a problem hiding this comment.
- 🚫 The changelog entry seems to be part of an already released section
## 8.15.1.
Consider moving the entry to the## Unreleasedsection, please.
Contributor
There was a problem hiding this comment.
Let's move this to the unreleased section.
Wdyt of a shorter changelog just for the feature. E.g.
Suggested change
| - Wire Sentry's `facebook::react::NativeModulePerfLogger` on both platforms so the SDK observes every TurboModule lifecycle event (`moduleCreate*`, sync/async method call start/end/fail, execution start/end/fail) for crash attribution, per-module spans and aggregated stats in follow-up releases. Install is automatic — no `OnLoad.cpp` changes on Android. Gated by the new `enableTurboModuleTracking` option on `Sentry.init`, default `false` for this first release. New Architecture only ([#6307](https://github.com/getsentry/sentry-react-native/pull/6307)) | |
| - Add `enableTurboModuleTracking` opt-in experimental option to enable Turbo Module performance tracking in the New Architecture ([#6307](https://github.com/getsentry/sentry-react-native/pull/6307)) |
Contributor
Author
|
@antonis @lucas-zimerman this one is ready to be reviewed |
antonis
reviewed
Jun 22, 2026
Contributor
There was a problem hiding this comment.
I think adding !/cpp/**/* to .npmignore may fix the build issues
antonis
reviewed
Jun 22, 2026
|
|
||
| static { | ||
| // Load `libsentry-tm-perf-logger.so` as early as possible — its | ||
| // `JNI_OnLoad` installs Sentry's `facebook::react::NativeModulePerfLogger` |
Contributor
There was a problem hiding this comment.
I think there is no JNI_OnLoad in OnLoad.cpp
antonis
reviewed
Jun 22, 2026
| // logger to install, so a missing `.so` is expected and we swallow the | ||
| // `UnsatisfiedLinkError` instead of crashing the host. | ||
| try { | ||
| System.loadLibrary("sentry-tm-perf-logger"); |
Contributor
There was a problem hiding this comment.
q: Can we guard this on the new enableTurboModuleTracking option?
antonis
reviewed
Jun 22, 2026
antonis
left a comment
antonis
left a comment
Contributor
There was a problem hiding this comment.
Thank you for your work on this Alex 🙇
Overall looks good. Left a few comments.
…angelog Round of fixes from PR #6307 review: - antonis: `System.loadLibrary("sentry-tm-perf-logger")` is now lazy inside `RNSentryTurboModulePerfTracker` instead of running from `RNSentryPackage`'s static initializer. Hosts that never opt in to `enableTurboModuleTracking` no longer pay the (small but non-zero) cost of mapping a shared library they will never call into. Failed loads still latch `nativeUnavailable` permanently, so we never retry. Updated `@TestOnly` reset to also clear `libraryLoadAttempted`. - antonis: comment in `RNSentryPackage.java` referenced a JNI_OnLoad install path that no longer exists (we made the install lazy in an earlier commit). Replaced with a short note pointing at the tracker. - antonis: add `!/cpp/**/*` to `.npmignore` so the shared C++ sources\n are actually published to npm \u2014 the iOS podspec needs to compile\n them and the previous ignore list shipped an empty `cpp/` directory. - antonis: collapsed the changelog entry to the requested short form\n ("Add `enableTurboModuleTracking` opt-in experimental option to\n enable Turbo Module performance tracking in the New Architecture"). The Sentry bot MEDIUM about Android tracking-on-init-fail was already\nresolved by the previous ordering change \u2014 `startWithOptions` throws on\nfailure and the call to `setEnabled` is now after that point, so a\nfailed init never reaches the toggle.
…read Two findings from the latest PR #6307 review pass: - **Cursor MEDIUM / Sentry bot LOW** (Android): the compareAndSet-based\n lazy `System.loadLibrary` had a race where a second thread could\n return success from `ensureNativeLibraryLoaded` while the first\n thread was still loading the library, then call `nativeSetEnabled`,\n hit `UnsatisfiedLinkError`, and latch `nativeUnavailable` for the\n lifetime of the process. Replace with a synchronized block that\n blocks concurrent first callers on the in-progress load; the\n monitor is bypassed on the fast path once the load completes (the\n early-return on the `AtomicBoolean` flag is preserved). The flags\n are now flipped *after* the load result is established so any reader\n observing `libraryLoadAttempted == true` sees a matching\n `nativeUnavailable` value.\n\n- **Warden MEDIUM** (C++): every forwarded TurboModule callback was\n acquiring `sink_mutex_` to read the sink \u2014 including sync method\n calls on the JS thread, which the sink interface explicitly\n documents must never block. Introduce a two-level storage: keep the\n owning `shared_ptr` (`sink_owner_`) under the mutex for lifetime\n management and for the strong-ownership `sink()` accessor used by\n tests; mirror it to a lock-free `std::atomic<ISink*>` (`sink_cache_`)\n read by a new `sinkRaw()` accessor on the hot path. The forwarder\n macros now read via `sinkRaw()` so every TurboModule callback hits\n the sink with a single acquire-load and no mutex.\n\n The trade-off is a lifetime contract documented on `setSink`: a sink\n installed via `setSink(s)` must remain valid until either the\n controller is destroyed or a subsequent `setSink` call has completed.\n This matches our actual usage \u2014 the SDK installs the sink once at\n init and keeps it alive for the process lifetime \u2014 and avoids\n requiring C++20 atomic-shared_ptr support across every supported RN\n toolchain.
Cursor MEDIUM on #6307: the lock-free `sinkRaw()` pattern introduced to address Warden's earlier hot-path mutex concern is unsafe under a concurrent `setSink` \u2014 a forwarder callback holding the raw pointer can outlive the previous owning `shared_ptr` reference dropped by\nthe swap, causing a use-after-free. The two MEDIUM findings are in tension: Warden wanted no mutex on the hot path, Cursor wants no UAF. The honest cost analysis decides it: the mutex variant pays ~50\u201380 ns per forwarded callback (atomic load + mutex lock/unlock + `shared_ptr` copy) and only when tracking is opted in. At 10K TurboModule calls/sec that is ~0.08% CPU \u2014 the optimization was not worth a real UAF hazard. The default-off path stays at a single atomic load. Revert to mutex-on-hot-path: drop `sinkRaw()` and the `sink_cache_` atomic mirror, restore `sink_` as the single mutex- guarded owning reference, and have the forwarder macros invoke `sink()` again. The header now explains the trade-off explicitly so the next reviewer does not re-litigate it.
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 5d6f39f. Configure here.
| } | ||
| if (!ensureNativeLibraryLoaded()) { | ||
| return; | ||
| } |
There was a problem hiding this comment.
False still loads native library
Medium Severity
setEnabled(false) always runs ensureNativeLibraryLoaded() before calling JNI, so initNativeSdk with enableTurboModuleTracking: false still maps libsentry-tm-perf-logger.so. That breaks the documented lazy-load contract that non-opt-in hosts pay no native library cost.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 5d6f39f. Configure here.
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
Install a Sentry-owned
facebook::react::NativeModulePerfLoggeron both platforms so the SDK observes every TurboModule lifecycle event:moduleDataCreate{Start,End},moduleCreate{Start,CacheHit,Construct*,SetUp*,End,Fail}moduleJSRequireBeginning*,moduleJSRequireEnding*syncMethodCall{Start,ArgConversion*,Execution*,ReturnConversion*,End,Fail}asyncMethodCall{Start,ArgConversion*,Dispatch,End,Fail}asyncMethodCallBatchPreprocess{Start,End}asyncMethodCallExecution{Start,ArgConversion*,End,Fail}This is the foundation that the next three issues in the Turbo Modules instrumentation project build on: JS↔Native crash attribution, per-Turbo-Module spans, and aggregated per-module stats. Each will ship its own
ISentryTurboModulePerfSinkimplementation and plug into the hook this PR exposes.New
enableTurboModuleTrackingoption onSentry.init, defaultfalsefor this first release so the foundation lands without behavioural change. The native logger is always installed (we never want to miss early lifecycle events); the flag only decides whether forwarded callbacks reach the sink. The option is plumbed throughinitNativeSdkon both platforms.💡 Motivation and Context
Closes #6162.
💚 How did you test it?
📝 Checklist
sendDefaultPIIis enabled🔮 Next steps
This is the foundation for the Turbo Modules project. Follow-up issues plug in
ISentryTurboModulePerfSinkimplementations:turbo_module.name/turbo_module.methodof the call that was in flight.duration,status,module.methoddata.