feat(core): Wire TurboModulePerfLogger on iOS and Android#6307
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
Semver Impact of This PR⚪ None (no version bump detected) 📋 Changelog PreviewThis is how your changes will appear in the changelog.
Plus 5 more 🤖 This preview updates automatically when you update the PR. |
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}
|
@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.
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 995956f. Configure here.
| id enableTurboModuleTracking = [options objectForKey:@"enableTurboModuleTracking"]; | ||
| if ([enableTurboModuleTracking isKindOfClass:[NSNumber class]]) { | ||
| Sentry_SetTurboModuleTrackingEnabled([(NSNumber *)enableTurboModuleTracking boolValue] ? 1 : 0); | ||
| } |
There was a problem hiding this comment.
Late tracking enable misses startup
Medium Severity
With enableTurboModuleTracking set to true, the native forwarder stays disabled until initNativeSdk runs. TurboModule perf callbacks during earlier bridge startup only hit the installed logger’s isEnabled() fast path and are dropped, so startup module create/require/sync activity is never forwarded for that init.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 995956f. Configure here.
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
| buildFeatures { | ||
| prefab true | ||
| } |
There was a problem hiding this comment.
Bug: In build.gradle, two separate buildFeatures blocks exist. The second block overwrites the first when both are active, potentially disabling BuildConfig generation on AGP 8+ and causing build failures.
Severity: HIGH
Suggested Fix
Merge the two buildFeatures blocks in build.gradle into a single block to ensure both buildConfig = true (for AGP 8+) and prefab = true (for New Architecture) are applied correctly. This prevents one configuration from overwriting the other.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.
Location: packages/core/android/build.gradle#L55-L57
Potential issue: The `build.gradle` file defines two separate `buildFeatures` blocks
within the same `android` scope. When both an Android Gradle Plugin (AGP) version of 8
or higher is used and the new architecture is enabled, both blocks are active. However,
Gradle's DSL behavior causes the second `buildFeatures { prefab = true }` block to
overwrite the first `buildFeatures { buildConfig = true }` block. This removes the
explicit `buildConfig = true` setting, which is likely required for AGP 8+ to generate
the `BuildConfig` class. Consequently, the `buildConfigField` for
`IS_NEW_ARCHITECTURE_ENABLED` may fail, leading to a build error or a runtime failure in
`RNSentryPackage.java` which depends on this field.
Also affects:
packages/core/android/src/main/java/io/sentry/react/RNSentryPackage.java:22~22
Did we get this right? 👍 / 👎 to inform future reviews.
antonis
left a comment
antonis
left a comment
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.
2 participants
📢 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.