feat(core): Add base64 polyfill carrier hook for envelope encoding#6312
Closed
alwx wants to merge 3 commits into
Closed
feat(core): Add base64 polyfill carrier hook for envelope encoding#6312alwx wants to merge 3 commits into
alwx wants to merge 3 commits into
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
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.
Introduce a carrier-level base64 polyfill (`carrier.base64Polyfill`) that mirrors the existing `encodePolyfill` indirection. `captureEnvelope` in the native wrapper now goes through `encodeToBase64`, which lazy-installs and dispatches via the carrier, instead of calling the vendored JS base64 encoder directly. The default implementation is unchanged — it still delegates to the JS `base64StringFromByteArray` from `vendor/base64-js`, so there is no behavior change in this commit. The point is to put the indirection in place so a future change can swap in a native (JSI/TurboModule) base64 encoder without touching any call sites — addressing the >10x perf opportunity tracked in #4884. The React Native carrier type is extended locally with an optional `base64Polyfill` field; upstream `@sentry/core` is untouched.
Contributor
Semver Impact of This PR⚪ None (no version bump detected) 📋 Changelog PreviewThis is how your changes will appear in the changelog.
Plus 4 more 🤖 This preview updates automatically when you update the PR. |
Contributor
If a user enables AGP
Our docs instruct React Native users to set
Instructions and example for changelogPlease add an entry to Example: ## Unreleased
### Features
- Add base64 polyfill carrier hook for envelope encoding ([#6312](https://github.com/getsentry/sentry-react-native/pull/6312))If none of the above apply, you can opt out of this check by adding |
10 tasks
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
1 participant
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
Adds a carrier-level base64 polyfill hook (
carrier.base64Polyfill) that mirrors the existingencodePolyfillindirection used for UTF-8 encoding.packages/core/src/js/transports/base64Polyfill.ts— exposesuseBase64Polyfill()and the defaultbase64Polyfillimplementation (delegates to the existing JSbase64StringFromByteArrayfromvendor/base64-js).packages/core/src/js/utils/base64.ts—encodeToBase64()helper that lazy-installs the polyfill and dispatches through the carrier.wrapper.ts(captureEnvelope) now callsencodeToBase64(envelopeBytes)instead ofbase64StringFromByteArray(envelopeBytes)directly.sdk.tsxinstalls the polyfill at init alongsideuseEncodePolyfill().base64Polyfill?: (input: Uint8Array | number[]) => stringfield. Upstream@sentry/coreis not touched.Behavior is unchanged — the same JS encoder runs by default. This PR is plumbing only.
💡 Motivation and Context
Stage 1 of #4884. The pure-JS base64 encoder is a measurable bottleneck on the envelope hot path (
RNSentry.captureEnvelope) for larger payloads (profiles, attachments, replays). Putting the carrier hook in place now means a future native (JSI / TurboModule) encoder can be dropped in by swapping a single carrier assignment, without touching any call sites in event-processing code.Refs #4884.
💚 How did you test it?
packages/core/test/transports/base64Polyfill.test.ts— verifies the polyfill is installed on the carrier and that the default encoder produces correct base64 output.yarn test— all 1619 tests pass.yarn build— passes.yarn lint:lerna— 0 warnings, 0 errors.yarn circularDepCheck— no circular dependencies.yarn api-report:check— API report up to date (no public API changes).📝 Checklist
sendDefaultPIIis enabled🔮 Next steps
Stage 2 of #4884: implement the native fast path behind the same carrier hook. Two candidates to evaluate:
ArrayBuffervia TurboModule (new-arch only; fall back to the JS polyfill on old arch).react-native-quick-base64-style) and install it as the polyfill.