From ffb6280bf2d96df1e4b3f0513eee5c509daa3d9a Mon Sep 17 00:00:00 2001 From: Alexander Pantiukhov Date: Wed, 22 Jul 2026 15:59:07 +0200 Subject: [PATCH 1/2] feat(core): Extend TurboModule instrumentation to legacy NativeModules Wraps `NativeModules.*` on the Old Architecture so bridge calls flow through the same aggregator, span-attribution, and slow-call breadcrumb surface as TurboModules on the New Architecture. - Adds `arch: 'new' | 'legacy'` to TurboModuleRecord / TurboModuleAggregate / TurboModuleCallStart, threaded through `wrapTurboModule`, and folded into the aggregate key so new-arch and legacy calls stay separate on flush. - New `wrapAllNativeModules` enumerates `NativeModules` on the Old Arch and wraps each registered module (skipping RNSentry, which the integration wraps explicitly with a curated recursion-safe skip list). - `turboModuleContextIntegration` auto-invokes it on the Old Arch. Opt out with `enableLegacyNativeModules: false`, or scope with `legacyModulesSkip` / `legacyModulesSkipMethods`. - Aggregate flush emits `turbo_modules....arch`; attributed spans get a top-level `turbo_module.arch` derived from `isTurboModuleEnabled()` at integration construction (an app doesn't switch archs at runtime). Closes getsentry/sentry-react-native#6166. Co-Authored-By: Claude Opus 4.7 --- CHANGELOG.md | 4 + .../src/js/integrations/turboModuleContext.ts | 31 +++++- .../integrations/turboModuleContextFlush.ts | 3 + packages/core/src/js/turbomodule/index.ts | 3 + .../js/turbomodule/turboModuleAggregator.ts | 39 ++++++-- .../src/js/turbomodule/wrapNativeModules.ts | 62 ++++++++++++ .../src/js/turbomodule/wrapTurboModule.ts | 17 ++-- .../integrations/turboModuleContext.test.ts | 36 +++---- .../turbomodule/turboModuleAggregator.test.ts | 36 ++++--- .../turbomodule/wrapNativeModules.test.ts | 96 +++++++++++++++++++ 10 files changed, 280 insertions(+), 47 deletions(-) create mode 100644 packages/core/src/js/turbomodule/wrapNativeModules.ts create mode 100644 packages/core/test/turbomodule/wrapNativeModules.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 8ea29b7f08..d77fb4221e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,10 @@ When a root span ends (idle nav spans from `reactNavigationIntegration` / `expoRouterIntegration`, or a user's own `Sentry.startSpan(...)`), the integration writes `turbo_module...{call_count,duration_ms,error_count}` attributes plus summary keys (`turbo_module.total_call_count`, `turbo_module.total_duration_ms`, `turbo_module.top_module`). Async calls above `slowCallThresholdMs` (default 500ms) additionally record a `native.turbo_module` breadcrumb. Both surfaces enabled by default; new knobs `enableSpanAttribution`, `slowCallThresholdMs`, `maxTopModulesPerSpan` on `turboModuleContextIntegration`. +- Extend TurboModule instrumentation to the Old Architecture `NativeModules` bridge ([#6504](https://github.com/getsentry/sentry-react-native/pull/6504)) + + Apps still on the Old Architecture now flow legacy `NativeModules.*` calls through the same aggregator, span-attribution, and slow-call breadcrumb path as TurboModules on the New Architecture. Records carry `arch: 'new' | 'legacy'` (also surfaced on the flushed aggregate span and each attributed span) so analyses can split the signal by architecture. Auto-wrap is on by default; opt out with `enableLegacyNativeModules: false`, or scope it via `legacyModulesSkip` / `legacyModulesSkipMethods` on `turboModuleContextIntegration`. + ### Changes - Expose `instrumentStateGraph` for manual LangGraph instrumentation ([#6520](https://github.com/getsentry/sentry-react-native/pull/6520)) diff --git a/packages/core/src/js/integrations/turboModuleContext.ts b/packages/core/src/js/integrations/turboModuleContext.ts index f5f75b6a00..708e845dc6 100644 --- a/packages/core/src/js/integrations/turboModuleContext.ts +++ b/packages/core/src/js/integrations/turboModuleContext.ts @@ -14,8 +14,10 @@ import { setOnFirstTurboModuleRecord, type TurboModuleCallStart, type TurboModuleRecord, + wrapAllNativeModules, wrapTurboModule, } from '../turbomodule'; +import { isTurboModuleEnabled } from '../utils/environment'; import { isRootSpan } from '../utils/span'; import { getRNSentryModule } from '../wrapper'; import { @@ -71,6 +73,15 @@ export interface TurboModuleContextOptions { /** Cap on per-`(module, method)` rows attributed to a single span. Default: `16`. */ maxTopModulesPerSpan?: number; + + /** On Old Architecture, auto-wrap every registered `NativeModules.*`. Default: `true`. */ + enableLegacyNativeModules?: boolean; + + /** Modules to skip in the legacy auto-wrap (`RNSentry` is always skipped). */ + legacyModulesSkip?: ReadonlyArray; + + /** Per-module method skips for the legacy auto-wrap. */ + legacyModulesSkipMethods?: Readonly>>; } // Scope-sync methods must NOT be tracked — `enableSyncToNative` calls them on @@ -101,6 +112,7 @@ export const turboModuleContextIntegration = (options: TurboModuleContextOptions const flushIntervalMs = options.aggregateFlushIntervalMs ?? DEFAULT_AGGREGATE_FLUSH_INTERVAL_MS; const slowCallThresholdMs = options.slowCallThresholdMs ?? DEFAULT_SLOW_CALL_THRESHOLD_MS; const maxTopModulesPerSpan = options.maxTopModulesPerSpan ?? DEFAULT_MAX_TOP_MODULES_PER_SPAN; + const contextArch: 'new' | 'legacy' = isTurboModuleEnabled() ? 'new' : 'legacy'; let pendingFlushHandle: ReturnType | undefined; let closed = false; @@ -120,10 +132,19 @@ export const turboModuleContextIntegration = (options: TurboModuleContextOptions return { name: INTEGRATION_NAME, setupOnce() { - wrapTurboModule('RNSentry', getRNSentryModule(), { skip: RNSENTRY_SKIP }); + // Wrap RNSentry first with its curated skip list; `wrapAllNativeModules` + // then skips it implicitly to avoid double-wrapping with a wider surface. + wrapTurboModule('RNSentry', getRNSentryModule(), { skip: RNSENTRY_SKIP, arch: contextArch }); for (const entry of options.modules ?? []) { - wrapTurboModule(entry.name, entry.module, { skip: entry.skipMethods }); + wrapTurboModule(entry.name, entry.module, { skip: entry.skipMethods, arch: contextArch }); + } + + if (options.enableLegacyNativeModules !== false) { + wrapAllNativeModules({ + skipModules: options.legacyModulesSkip, + skipMethodsPerModule: options.legacyModulesSkipMethods, + }); } setAggregateRecordingEnabled(enableAggregate); @@ -175,7 +196,7 @@ export const turboModuleContextIntegration = (options: TurboModuleContextOptions for (const window of windows) { recordIntoWindow(window, record); if (window.closed) { - attachWindowToSpan(window.span, window, maxTopModulesPerSpan, pendingSpanAttributes); + attachWindowToSpan(window.span, window, maxTopModulesPerSpan, contextArch, pendingSpanAttributes); } } } @@ -229,7 +250,7 @@ export const turboModuleContextIntegration = (options: TurboModuleContextOptions openWindowList.splice(idx, 1); } window.closed = true; - attachWindowToSpan(span, window, maxTopModulesPerSpan, pendingSpanAttributes); + attachWindowToSpan(span, window, maxTopModulesPerSpan, contextArch, pendingSpanAttributes); }); } @@ -344,6 +365,7 @@ function attachWindowToSpan( span: Span, window: WindowState, topN: number, + arch: 'new' | 'legacy', pendingSpanAttributes: Map>, ): void { if (window.counters.size === 0) { @@ -369,6 +391,7 @@ function attachWindowToSpan( 'turbo_module.total_error_count': totalErrorCount, 'turbo_module.total_duration_ms': roundMs(totalDurationMs), 'turbo_module.unique_methods': rows.length, + 'turbo_module.arch': arch, }; const top = rows[0]; if (top) { diff --git a/packages/core/src/js/integrations/turboModuleContextFlush.ts b/packages/core/src/js/integrations/turboModuleContextFlush.ts index 8dc70e3a18..056cd85099 100644 --- a/packages/core/src/js/integrations/turboModuleContextFlush.ts +++ b/packages/core/src/js/integrations/turboModuleContextFlush.ts @@ -129,6 +129,7 @@ function serialiseRows(rows: ReadonlyArray): Record void; @@ -89,8 +105,8 @@ let nextRecordId = 0; // don't accumulate into a map that nothing ever drains. let recordingEnabled = true; -function makeKey(name: string, method: string, kind: TurboModuleCallKind): string { - return `${name}|${method}|${kind}`; +function makeKey(name: string, method: string, kind: TurboModuleCallKind, arch: TurboModuleArch): string { + return `${name}|${method}|${kind}|${arch}`; } function bucketIndexForDuration(durationMs: number): number { @@ -137,16 +153,19 @@ export function recordTurboModuleCall(args: { durationMs: number; errored: boolean; recordId?: number; + /** Defaults to `'new'` for backward compatibility with the pre-legacy callers. */ + arch?: TurboModuleArch; }): void { if (ignoredModules.has(args.name)) { return; } + const arch: TurboModuleArch = args.arch ?? 'new'; const duration = args.durationMs > 0 ? args.durationMs : 0; if (recordingEnabled) { const wasEmpty = aggregates.size === 0; - const key = makeKey(args.name, args.method, args.kind); + const key = makeKey(args.name, args.method, args.kind, arch); let entry = aggregates.get(key); if (!entry) { @@ -154,6 +173,7 @@ export function recordTurboModuleCall(args: { name: args.name, method: args.method, kind: args.kind, + arch, callCount: 0, errorCount: 0, totalDurationMs: 0, @@ -191,6 +211,7 @@ export function recordTurboModuleCall(args: { name: args.name, method: args.method, kind: args.kind, + arch, durationMs: duration, errored: args.errored, recordId: args.recordId, @@ -229,12 +250,17 @@ export function removeTurboModuleRecordObserver(observer: TurboModuleRecordObser * Returns the `recordId` even for ignored modules so the wrap layer never has * to branch — the paired record for an ignored module will be filtered out. */ -export function notifyTurboModuleCallStart(name: string, method: string, kind: TurboModuleCallKind): number { +export function notifyTurboModuleCallStart( + name: string, + method: string, + kind: TurboModuleCallKind, + arch: TurboModuleArch = 'new', +): number { const recordId = nextRecordId++; if (ignoredModules.has(name) || startObservers.size === 0) { return recordId; } - const event: TurboModuleCallStart = { recordId, name, method, kind }; + const event: TurboModuleCallStart = { recordId, name, method, kind, arch }; for (const observer of startObservers) { try { observer(event); @@ -297,6 +323,7 @@ export function drainTurboModuleAggregate(): TurboModuleAggregate[] { name: entry.name, method: entry.method, kind: entry.kind, + arch: entry.arch, callCount: entry.callCount, errorCount: entry.errorCount, totalDurationMs: entry.totalDurationMs, diff --git a/packages/core/src/js/turbomodule/wrapNativeModules.ts b/packages/core/src/js/turbomodule/wrapNativeModules.ts new file mode 100644 index 0000000000..998a9a12e6 --- /dev/null +++ b/packages/core/src/js/turbomodule/wrapNativeModules.ts @@ -0,0 +1,62 @@ +import { logger } from '@sentry/core'; +import { NativeModules } from 'react-native'; + +import { isTurboModuleEnabled } from '../utils/environment'; +import { wrapTurboModule } from './wrapTurboModule'; + +export interface WrapNativeModulesOptions { + skipModules?: ReadonlyArray; + skipMethodsPerModule?: Readonly>>; +} + +// `RNSentry` is wrapped explicitly by the integration with a curated skip +// list — re-wrapping here would double-record and recurse via scope-sync. +const IMPLICIT_MODULE_SKIP = new Set(['RNSentry']); + +/** + * Wraps every legacy `NativeModules.*` on Old Architecture so bridge calls + * feed the same aggregator / attribution / breadcrumb surface as TurboModules. + * No-op on New Architecture. + */ +export function wrapAllNativeModules(options: WrapNativeModulesOptions = {}): string[] { + if (isTurboModuleEnabled()) { + return []; + } + + const skipModules = new Set(IMPLICIT_MODULE_SKIP); + for (const name of options.skipModules ?? []) { + skipModules.add(name); + } + const skipMethodsPerModule = options.skipMethodsPerModule ?? {}; + + const wrapped: string[] = []; + let moduleNames: string[]; + try { + moduleNames = Object.keys(NativeModules); + } catch (e) { + logger.warn(`[TurboModuleTracker] Failed to enumerate NativeModules for legacy wrapping: ${String(e)}`); + return wrapped; + } + + for (const name of moduleNames) { + if (skipModules.has(name)) { + continue; + } + let mod: unknown; + try { + mod = (NativeModules as Record)[name]; + } catch { + continue; + } + if (!mod || typeof mod !== 'object') { + continue; + } + wrapTurboModule(name, mod, { + skip: skipMethodsPerModule[name], + arch: 'legacy', + }); + wrapped.push(name); + } + + return wrapped; +} diff --git a/packages/core/src/js/turbomodule/wrapTurboModule.ts b/packages/core/src/js/turbomodule/wrapTurboModule.ts index a2ae9afa2c..fd8251217f 100644 --- a/packages/core/src/js/turbomodule/wrapTurboModule.ts +++ b/packages/core/src/js/turbomodule/wrapTurboModule.ts @@ -2,7 +2,7 @@ import { logger } from '@sentry/core'; import type { TurboModuleCallKind } from './turboModuleTracker'; -import { notifyTurboModuleCallStart, recordTurboModuleCall } from './turboModuleAggregator'; +import { notifyTurboModuleCallStart, recordTurboModuleCall, type TurboModuleArch } from './turboModuleAggregator'; import { popTurboModuleCall, pushTurboModuleCall, relabelTurboModuleCallKind } from './turboModuleTracker'; /** @@ -32,7 +32,7 @@ export function _resetWrappedModules(): void { export function wrapTurboModule( name: string, module: T | null | undefined, - options: { skip?: ReadonlyArray } = {}, + options: { skip?: ReadonlyArray; arch?: TurboModuleArch } = {}, ): T | null | undefined { if (!module) { return module; @@ -43,6 +43,7 @@ export function wrapTurboModule( } const skip = new Set(options.skip ?? []); + const arch: TurboModuleArch = options.arch ?? 'new'; const methodNames = collectMethodNames(module); if (methodNames.length === 0) { @@ -92,7 +93,7 @@ export function wrapTurboModule( logger.warn(`[TurboModuleTracker] push failed for ${name}.${key}: ${String(e)}`); } try { - recordId = notifyTurboModuleCallStart(name, key, 'sync'); + recordId = notifyTurboModuleCallStart(name, key, 'sync', arch); } catch (e) { logger.warn(`[TurboModuleTracker] notifyStart failed for ${name}.${key}: ${String(e)}`); } @@ -102,7 +103,7 @@ export function wrapTurboModule( result = originalFn.apply(this, args); } catch (e) { safePop(callId, name, key); - safeRecord(name, key, 'sync', startedAtMs, true, recordId); + safeRecord(name, key, 'sync', startedAtMs, true, recordId, arch); throw e; } @@ -111,19 +112,19 @@ export function wrapTurboModule( return (result as Promise).then( value => { safePop(callId, name, key); - safeRecord(name, key, 'async', startedAtMs, false, recordId); + safeRecord(name, key, 'async', startedAtMs, false, recordId, arch); return value; }, err => { safePop(callId, name, key); - safeRecord(name, key, 'async', startedAtMs, true, recordId); + safeRecord(name, key, 'async', startedAtMs, true, recordId, arch); throw err; }, ); } safePop(callId, name, key); - safeRecord(name, key, 'sync', startedAtMs, false, recordId); + safeRecord(name, key, 'sync', startedAtMs, false, recordId, arch); return result; }; @@ -206,6 +207,7 @@ function safeRecord( startedAtMs: number, errored: boolean, recordId: number | undefined, + arch: TurboModuleArch, ): void { try { recordTurboModuleCall({ @@ -215,6 +217,7 @@ function safeRecord( durationMs: Date.now() - startedAtMs, errored, recordId, + arch, }); } catch (e) { logger.warn(`[TurboModuleTracker] record failed for ${name}.${method}: ${String(e)}`); diff --git a/packages/core/test/integrations/turboModuleContext.test.ts b/packages/core/test/integrations/turboModuleContext.test.ts index c9e96c67d7..75b1a9e1d6 100644 --- a/packages/core/test/integrations/turboModuleContext.test.ts +++ b/packages/core/test/integrations/turboModuleContext.test.ts @@ -118,21 +118,25 @@ describe('turboModuleContextIntegration', () => { turboModuleContextIntegration().setupOnce!(); - expect(wrapSpy).toHaveBeenCalledWith('RNSentry', fakeModule, { - skip: [ - 'addListener', - 'removeListeners', - 'setContext', - 'setTag', - 'setExtra', - 'setUser', - 'addBreadcrumb', - 'clearBreadcrumbs', - 'setAttribute', - 'setAttributes', - 'removeAttribute', - ], - }); + expect(wrapSpy).toHaveBeenCalledWith( + 'RNSentry', + fakeModule, + expect.objectContaining({ + skip: [ + 'addListener', + 'removeListeners', + 'setContext', + 'setTag', + 'setExtra', + 'setUser', + 'addBreadcrumb', + 'clearBreadcrumbs', + 'setAttribute', + 'setAttributes', + 'removeAttribute', + ], + }), + ); }); it('does not wrap scope-sync methods on RNSentry (would recurse infinitely)', () => { @@ -187,7 +191,7 @@ describe('turboModuleContextIntegration', () => { modules: [{ name: 'Other', module: fakeOther, skipMethods: ['ignored'] }], }).setupOnce!(); - expect(wrapSpy).toHaveBeenCalledWith('Other', fakeOther, { skip: ['ignored'] }); + expect(wrapSpy).toHaveBeenCalledWith('Other', fakeOther, expect.objectContaining({ skip: ['ignored'] })); }); it('tolerates a missing RNSentry module', () => { diff --git a/packages/core/test/turbomodule/turboModuleAggregator.test.ts b/packages/core/test/turbomodule/turboModuleAggregator.test.ts index 28251664cc..0bbb9fc9b8 100644 --- a/packages/core/test/turbomodule/turboModuleAggregator.test.ts +++ b/packages/core/test/turbomodule/turboModuleAggregator.test.ts @@ -89,20 +89,28 @@ describe('turboModuleAggregator', () => { recordTurboModuleCall({ name: 'A', method: 'y', kind: 'async', durationMs: 42, errored: true }); expect(observer).toHaveBeenCalledTimes(2); - expect(observer).toHaveBeenNthCalledWith(1, { - name: 'A', - method: 'x', - kind: 'sync', - durationMs: 3, - errored: false, - }); - expect(observer).toHaveBeenNthCalledWith(2, { - name: 'A', - method: 'y', - kind: 'async', - durationMs: 42, - errored: true, - }); + expect(observer).toHaveBeenNthCalledWith( + 1, + expect.objectContaining({ + name: 'A', + method: 'x', + kind: 'sync', + durationMs: 3, + errored: false, + arch: 'new', + }), + ); + expect(observer).toHaveBeenNthCalledWith( + 2, + expect.objectContaining({ + name: 'A', + method: 'y', + kind: 'async', + durationMs: 42, + errored: true, + arch: 'new', + }), + ); removeTurboModuleRecordObserver(observer); recordTurboModuleCall({ name: 'A', method: 'x', kind: 'sync', durationMs: 1, errored: false }); diff --git a/packages/core/test/turbomodule/wrapNativeModules.test.ts b/packages/core/test/turbomodule/wrapNativeModules.test.ts new file mode 100644 index 0000000000..36c1f74ccc --- /dev/null +++ b/packages/core/test/turbomodule/wrapNativeModules.test.ts @@ -0,0 +1,96 @@ +import { NativeModules } from 'react-native'; + +import { wrapAllNativeModules } from '../../src/js/turbomodule/wrapNativeModules'; +import * as wrapTurboModuleMod from '../../src/js/turbomodule/wrapTurboModule'; +import { _resetWrappedModules } from '../../src/js/turbomodule/wrapTurboModule'; +import * as environment from '../../src/js/utils/environment'; + +describe('wrapAllNativeModules', () => { + const originalKeys = Object.keys(NativeModules); + + beforeEach(() => { + _resetWrappedModules(); + jest.restoreAllMocks(); + }); + + afterEach(() => { + // Clear anything the test added to NativeModules so it doesn't leak. + for (const key of Object.keys(NativeModules)) { + if (!originalKeys.includes(key)) { + // oxlint-disable-next-line typescript-eslint(no-dynamic-delete) + delete (NativeModules as Record)[key]; + } + } + }); + + it('is a no-op on the New Architecture', () => { + jest.spyOn(environment, 'isTurboModuleEnabled').mockReturnValue(true); + const spy = jest.spyOn(wrapTurboModuleMod, 'wrapTurboModule'); + + const wrapped = wrapAllNativeModules(); + + expect(wrapped).toEqual([]); + expect(spy).not.toHaveBeenCalled(); + }); + + it('wraps every legacy module with arch: legacy on the Old Architecture', () => { + jest.spyOn(environment, 'isTurboModuleEnabled').mockReturnValue(false); + const spy = jest.spyOn(wrapTurboModuleMod, 'wrapTurboModule'); + + (NativeModules as Record).LegacyA = { doWork: jest.fn() }; + (NativeModules as Record).LegacyB = { ping: jest.fn() }; + + const wrapped = wrapAllNativeModules(); + + expect(wrapped).toEqual(expect.arrayContaining(['LegacyA', 'LegacyB'])); + expect(spy).toHaveBeenCalledWith('LegacyA', expect.any(Object), expect.objectContaining({ arch: 'legacy' })); + expect(spy).toHaveBeenCalledWith('LegacyB', expect.any(Object), expect.objectContaining({ arch: 'legacy' })); + }); + + it('skips RNSentry implicitly — the integration wraps it with a curated skip list', () => { + jest.spyOn(environment, 'isTurboModuleEnabled').mockReturnValue(false); + const spy = jest.spyOn(wrapTurboModuleMod, 'wrapTurboModule'); + + (NativeModules as Record).RNSentry = { crash: jest.fn() }; + + wrapAllNativeModules(); + + expect(spy).not.toHaveBeenCalledWith('RNSentry', expect.anything(), expect.anything()); + }); + + it('honours caller-supplied skipModules and per-module skipMethodsPerModule', () => { + jest.spyOn(environment, 'isTurboModuleEnabled').mockReturnValue(false); + const spy = jest.spyOn(wrapTurboModuleMod, 'wrapTurboModule'); + + (NativeModules as Record).SkipMe = { foo: jest.fn() }; + (NativeModules as Record).WrapMe = { keep: jest.fn(), drop: jest.fn() }; + + wrapAllNativeModules({ + skipModules: ['SkipMe'], + skipMethodsPerModule: { WrapMe: ['drop'] }, + }); + + expect(spy).not.toHaveBeenCalledWith('SkipMe', expect.anything(), expect.anything()); + expect(spy).toHaveBeenCalledWith( + 'WrapMe', + expect.any(Object), + expect.objectContaining({ arch: 'legacy', skip: ['drop'] }), + ); + }); + + it('tolerates entries whose value is null or not an object', () => { + jest.spyOn(environment, 'isTurboModuleEnabled').mockReturnValue(false); + const spy = jest.spyOn(wrapTurboModuleMod, 'wrapTurboModule'); + + (NativeModules as Record).NullMod = null; + (NativeModules as Record).StringMod = 'not a module'; + (NativeModules as Record).RealMod = { work: jest.fn() }; + + const wrapped = wrapAllNativeModules(); + + expect(wrapped).not.toContain('NullMod'); + expect(wrapped).not.toContain('StringMod'); + expect(wrapped).toContain('RealMod'); + expect(spy).toHaveBeenCalledWith('RealMod', expect.any(Object), expect.objectContaining({ arch: 'legacy' })); + }); +}); From 93761bbc891d0edadcd949ce13dbd76cfb44bd25 Mon Sep 17 00:00:00 2001 From: Alexander Pantiukhov Date: Tue, 28 Jul 2026 12:48:06 +0200 Subject: [PATCH 2/2] fix(core): Keep legacy NativeModules lazy and skip hot RN infra when wrapping Review follow-ups on the legacy NativeModules auto-wrap: - Preserve RN's lazy module initialisation. The previous implementation read every NativeModules entry at Sentry.init, which forces each lazily-exposed native module to initialise during startup (defeating RN's laziness and risking module side effects). Lazy accessors are now replaced with a getter that resolves and wraps on first access. - Skip hot RN infrastructure by default (Timing, UIManager and the animated modules). Timing backs every setTimeout, so wrapping it added tracker work to one of the hottest paths and dominated the aggregate. - Warn instead of silently doing nothing when NativeModules cannot be enumerated (the JSI host-object proxy has no enumerable keys), pointing at the explicit `modules` option. - Export TurboModuleArch, which is part of the public wrapTurboModule signature, and refresh the API report. --- CHANGELOG.md | 2 +- packages/core/etc/sentry-react-native.api.md | 7 + packages/core/src/js/index.ts | 2 +- .../src/js/integrations/turboModuleContext.ts | 13 +- .../src/js/turbomodule/wrapNativeModules.ts | 171 ++++++++++++++++-- packages/core/src/js/utils/worldwide.ts | 2 + .../turbomodule/wrapNativeModules.test.ts | 135 ++++++++++++++ 7 files changed, 311 insertions(+), 21 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d77fb4221e..e2483c4012 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,7 +16,7 @@ - Extend TurboModule instrumentation to the Old Architecture `NativeModules` bridge ([#6504](https://github.com/getsentry/sentry-react-native/pull/6504)) - Apps still on the Old Architecture now flow legacy `NativeModules.*` calls through the same aggregator, span-attribution, and slow-call breadcrumb path as TurboModules on the New Architecture. Records carry `arch: 'new' | 'legacy'` (also surfaced on the flushed aggregate span and each attributed span) so analyses can split the signal by architecture. Auto-wrap is on by default; opt out with `enableLegacyNativeModules: false`, or scope it via `legacyModulesSkip` / `legacyModulesSkipMethods` on `turboModuleContextIntegration`. + Apps still on the Old Architecture now flow legacy `NativeModules.*` calls through the same aggregator, span-attribution, and slow-call breadcrumb path as TurboModules on the New Architecture. Records carry `arch: 'new' | 'legacy'` (also surfaced on the flushed aggregate span and each attributed span) so analyses can split the signal by architecture. Lazily-exposed modules stay lazy — they're wrapped on first access instead of being initialised during `Sentry.init`. `RNSentry` and hot RN infrastructure (`Timing`, `UIManager`, animated modules) are skipped by default. Auto-wrap is on by default; opt out with `enableLegacyNativeModules: false`, or scope it via `legacyModulesSkip` / `legacyModulesSkipMethods` on `turboModuleContextIntegration`. ### Changes diff --git a/packages/core/etc/sentry-react-native.api.md b/packages/core/etc/sentry-react-native.api.md index 3ef7151786..9733b6ffc3 100644 --- a/packages/core/etc/sentry-react-native.api.md +++ b/packages/core/etc/sentry-react-native.api.md @@ -856,6 +856,9 @@ export class TouchEventBoundary extends React_2.Component; + legacyModulesSkip?: ReadonlyArray; + legacyModulesSkipMethods?: Readonly>>; maxTopModulesPerSpan?: number; modules?: Array<{ name: string; @@ -936,6 +942,7 @@ export function wrapExpoRouterErrorBoundary

(name: string, module: T | null | undefined, options?: { skip?: ReadonlyArray; + arch?: TurboModuleArch; }): T | null | undefined; // Warnings were encountered during analysis: diff --git a/packages/core/src/js/index.ts b/packages/core/src/js/index.ts index 62c8585f71..6550c89cb2 100644 --- a/packages/core/src/js/index.ts +++ b/packages/core/src/js/index.ts @@ -175,4 +175,4 @@ export { pushTurboModuleCall, wrapTurboModule, } from './turbomodule'; -export type { TurboModuleCall, TurboModuleCallKind } from './turbomodule'; +export type { TurboModuleArch, TurboModuleCall, TurboModuleCallKind } from './turbomodule'; diff --git a/packages/core/src/js/integrations/turboModuleContext.ts b/packages/core/src/js/integrations/turboModuleContext.ts index 708e845dc6..f601f24ef4 100644 --- a/packages/core/src/js/integrations/turboModuleContext.ts +++ b/packages/core/src/js/integrations/turboModuleContext.ts @@ -74,10 +74,19 @@ export interface TurboModuleContextOptions { /** Cap on per-`(module, method)` rows attributed to a single span. Default: `16`. */ maxTopModulesPerSpan?: number; - /** On Old Architecture, auto-wrap every registered `NativeModules.*`. Default: `true`. */ + /** + * On Old Architecture, auto-wrap registered `NativeModules.*`. Default: `true`. + * + * Lazily-exposed modules stay lazy — they are wrapped on first access rather than + * initialised during `Sentry.init`. + */ enableLegacyNativeModules?: boolean; - /** Modules to skip in the legacy auto-wrap (`RNSentry` is always skipped). */ + /** + * Additional modules to skip in the legacy auto-wrap. `RNSentry` and hot React + * Native infrastructure (`Timing`, `UIManager`, and the animated modules) are + * always skipped; pass them via `modules` to instrument them deliberately. + */ legacyModulesSkip?: ReadonlyArray; /** Per-module method skips for the legacy auto-wrap. */ diff --git a/packages/core/src/js/turbomodule/wrapNativeModules.ts b/packages/core/src/js/turbomodule/wrapNativeModules.ts index 998a9a12e6..9a6401123e 100644 --- a/packages/core/src/js/turbomodule/wrapNativeModules.ts +++ b/packages/core/src/js/turbomodule/wrapNativeModules.ts @@ -1,7 +1,10 @@ import { logger } from '@sentry/core'; import { NativeModules } from 'react-native'; +import type { TurboModuleArch } from './turboModuleAggregator'; + import { isTurboModuleEnabled } from '../utils/environment'; +import { RN_GLOBAL_OBJ } from '../utils/worldwide'; import { wrapTurboModule } from './wrapTurboModule'; export interface WrapNativeModulesOptions { @@ -9,14 +12,42 @@ export interface WrapNativeModulesOptions { skipMethodsPerModule?: Readonly>>; } -// `RNSentry` is wrapped explicitly by the integration with a curated skip -// list — re-wrapping here would double-record and recurse via scope-sync. -const IMPLICIT_MODULE_SKIP = new Set(['RNSentry']); +interface WrapOptions { + skip?: ReadonlyArray; + arch: TurboModuleArch; +} + +type LazyGetter = (this: unknown) => unknown; + +/** + * Never auto-wrapped: + * - `RNSentry` is wrapped explicitly by the integration with a curated skip list — + * re-wrapping here would double-record and recurse via scope-sync. + * - The others sit on the hottest paths in the app: `Timing` backs every + * `setTimeout`/`setInterval`, `UIManager` every view operation, and the animated + * modules run per frame. Instrumenting them would cost more than the signal is + * worth and would dominate the aggregate. Instrument them deliberately with + * `turboModuleContextIntegration({ modules: [...] })` if you need them. + */ +const IMPLICIT_MODULE_SKIP = new Set([ + 'RNSentry', + 'Timing', + 'UIManager', + 'NativeAnimatedModule', + 'NativeAnimatedTurboModule', +]); /** - * Wraps every legacy `NativeModules.*` on Old Architecture so bridge calls - * feed the same aggregator / attribution / breadcrumb surface as TurboModules. - * No-op on New Architecture. + * Wraps legacy `NativeModules.*` on Old Architecture so bridge calls feed the same + * aggregator / attribution / breadcrumb surface as TurboModules. No-op on New + * Architecture. + * + * Modules that React Native exposes lazily stay lazy: rather than reading (and so + * initialising) every native module during `Sentry.init`, the lazy getter is + * replaced with one that wraps the module on first access. + * + * @returns names of instrumented modules — wrapped immediately when already + * materialised, otherwise armed to wrap on first access. */ export function wrapAllNativeModules(options: WrapNativeModulesOptions = {}): string[] { if (isTurboModuleEnabled()) { @@ -29,34 +60,140 @@ export function wrapAllNativeModules(options: WrapNativeModulesOptions = {}): st } const skipMethodsPerModule = options.skipMethodsPerModule ?? {}; - const wrapped: string[] = []; let moduleNames: string[]; try { moduleNames = Object.keys(NativeModules); } catch (e) { logger.warn(`[TurboModuleTracker] Failed to enumerate NativeModules for legacy wrapping: ${String(e)}`); - return wrapped; + return []; + } + + if (moduleNames.length === 0) { + warnNotEnumerable(); + return []; } + const instrumented: string[] = []; for (const name of moduleNames) { if (skipModules.has(name)) { continue; } - let mod: unknown; + if (instrumentModule(name, { skip: skipMethodsPerModule[name], arch: 'legacy' })) { + instrumented.push(name); + } + } + + return instrumented; +} + +/** + * Instruments a single `NativeModules` entry, preserving React Native's lazy + * module initialisation when the entry is still an unresolved accessor. + */ +function instrumentModule(name: string, wrapOptions: WrapOptions): boolean { + let descriptor: PropertyDescriptor | undefined; + try { + descriptor = Object.getOwnPropertyDescriptor(NativeModules, name); + } catch { + return false; + } + + if (descriptor && typeof descriptor.get === 'function') { + return armLazyModule(name, descriptor, wrapOptions); + } + + let mod: unknown = descriptor?.value; + if (!descriptor) { + // Inherited or otherwise exotic property — fall back to a direct read. try { mod = (NativeModules as Record)[name]; } catch { - continue; + return false; } - if (!mod || typeof mod !== 'object') { - continue; + } + return wrapResolvedModule(name, mod, wrapOptions); +} + +/** + * Replaces a lazy `NativeModules` accessor with one that resolves through the + * original getter and wraps the materialised module, so instrumentation costs + * nothing until the app actually uses the module. + */ +function armLazyModule(name: string, descriptor: PropertyDescriptor, wrapOptions: WrapOptions): boolean { + // Invoked below via `.call(NativeModules)`, never as a bare method reference. + // oxlint-disable-next-line typescript-eslint(unbound-method) + const originalGet = descriptor.get as LazyGetter | undefined; + if (!originalGet) { + return false; + } + const enumerable = descriptor.enumerable !== false; + + const wrappingGetter = function sentryLazyNativeModuleGetter(): unknown { + const mod = originalGet.call(NativeModules); + wrapResolvedModule(name, mod, wrapOptions); + // React Native's own lazy getter replaces the property with a plain value on + // first read. If that didn't happen, cache it ourselves so later reads don't + // keep paying for this accessor. + if (getOwnGetter(name) === wrappingGetter) { + defineValue(name, mod, enumerable); } - wrapTurboModule(name, mod, { - skip: skipMethodsPerModule[name], - arch: 'legacy', + return mod; + }; + + try { + Object.defineProperty(NativeModules, name, { + configurable: true, + enumerable, + get: wrappingGetter, + // Mirror RN's lazy-property semantics: assigning replaces the accessor. + set: (next: unknown): void => defineValue(name, next, enumerable), }); - wrapped.push(name); + return true; + } catch { + // Non-configurable or frozen — leave RN's property untouched rather than + // forcing the module to initialise just so we can instrument it. + return false; } +} + +function wrapResolvedModule(name: string, mod: unknown, wrapOptions: WrapOptions): boolean { + if (!mod || typeof mod !== 'object') { + return false; + } + wrapTurboModule(name, mod, wrapOptions); + return true; +} + +function getOwnGetter(name: string): LazyGetter | undefined { + try { + // Compared by identity only, never invoked from this reference. + // oxlint-disable-next-line typescript-eslint(unbound-method) + return Object.getOwnPropertyDescriptor(NativeModules, name)?.get as LazyGetter | undefined; + } catch { + return undefined; + } +} + +function defineValue(name: string, value: unknown, enumerable: boolean): void { + try { + Object.defineProperty(NativeModules, name, { + configurable: true, + enumerable, + writable: true, + value, + }); + } catch { + // Keep the accessor in place — worst case we re-enter the getter, which is + // idempotent because `wrapTurboModule` de-duplicates via a WeakSet. + } +} - return wrapped; +function warnNotEnumerable(): void { + const viaHostObject = RN_GLOBAL_OBJ.nativeModuleProxy != null && RN_GLOBAL_OBJ.nativeModuleProxy === NativeModules; + const reason = viaHostObject + ? 'because `NativeModules` is a JSI host-object proxy, which cannot be enumerated.' + : 'because `NativeModules` exposed no enumerable entries.'; + const hint = + 'Pass the modules you care about to turboModuleContextIntegration({ modules: [{ name, module }] }) instead.'; + logger.warn(`[TurboModuleTracker] Legacy NativeModules auto-wrap found no modules to instrument ${reason} ${hint}`); } diff --git a/packages/core/src/js/utils/worldwide.ts b/packages/core/src/js/utils/worldwide.ts index 326f35aa99..51757c6fe2 100644 --- a/packages/core/src/js/utils/worldwide.ts +++ b/packages/core/src/js/utils/worldwide.ts @@ -24,6 +24,8 @@ export interface ReactNativeInternalGlobal extends InternalGlobal { Promise: unknown; __turboModuleProxy: unknown; RN$Bridgeless: unknown; + /** Old Architecture JSI host-object backing `NativeModules`; not enumerable. */ + nativeModuleProxy?: unknown; nativeFabricUIManager: unknown; ErrorUtils?: ErrorUtils; expo?: ExpoGlobalObject; diff --git a/packages/core/test/turbomodule/wrapNativeModules.test.ts b/packages/core/test/turbomodule/wrapNativeModules.test.ts index 36c1f74ccc..4c598ccc13 100644 --- a/packages/core/test/turbomodule/wrapNativeModules.test.ts +++ b/packages/core/test/turbomodule/wrapNativeModules.test.ts @@ -1,3 +1,4 @@ +import { logger } from '@sentry/core'; import { NativeModules } from 'react-native'; import { wrapAllNativeModules } from '../../src/js/turbomodule/wrapNativeModules'; @@ -93,4 +94,138 @@ describe('wrapAllNativeModules', () => { expect(wrapped).toContain('RealMod'); expect(spy).toHaveBeenCalledWith('RealMod', expect.any(Object), expect.objectContaining({ arch: 'legacy' })); }); + + describe('lazy modules', () => { + it('does not initialise a lazily-exposed module during setup', () => { + jest.spyOn(environment, 'isTurboModuleEnabled').mockReturnValue(false); + const spy = jest.spyOn(wrapTurboModuleMod, 'wrapTurboModule'); + const load = jest.fn(() => ({ doWork: jest.fn() })); + defineLazy('LazyMod', load); + + const wrapped = wrapAllNativeModules(); + + // Arming must not read through RN's lazy getter — doing so would eagerly + // initialise every native module at Sentry.init. + expect(load).not.toHaveBeenCalled(); + expect(spy).not.toHaveBeenCalledWith('LazyMod', expect.anything(), expect.anything()); + expect(wrapped).toContain('LazyMod'); + }); + + it('wraps a lazily-exposed module on first access', () => { + jest.spyOn(environment, 'isTurboModuleEnabled').mockReturnValue(false); + const spy = jest.spyOn(wrapTurboModuleMod, 'wrapTurboModule'); + const instance = { doWork: jest.fn() }; + const load = jest.fn(() => instance); + defineLazy('LazyMod', load); + + wrapAllNativeModules(); + const resolved = (NativeModules as Record).LazyMod; + + expect(load).toHaveBeenCalledTimes(1); + expect(resolved).toBe(instance); + expect(spy).toHaveBeenCalledWith('LazyMod', instance, expect.objectContaining({ arch: 'legacy' })); + }); + + it('caches the resolved module so repeated reads do not re-enter the getter', () => { + jest.spyOn(environment, 'isTurboModuleEnabled').mockReturnValue(false); + const load = jest.fn(() => ({ doWork: jest.fn() })); + defineLazy('LazyMod', load); + + wrapAllNativeModules(); + const first = (NativeModules as Record).LazyMod; + const second = (NativeModules as Record).LazyMod; + + expect(load).toHaveBeenCalledTimes(1); + expect(second).toBe(first); + expect(Object.getOwnPropertyDescriptor(NativeModules, 'LazyMod')?.get).toBeUndefined(); + }); + + it('keeps assignment working through the armed property', () => { + jest.spyOn(environment, 'isTurboModuleEnabled').mockReturnValue(false); + const load = jest.fn(() => ({ doWork: jest.fn() })); + defineLazy('LazyMod', load); + + wrapAllNativeModules(); + const replacement = { other: jest.fn() }; + (NativeModules as Record).LazyMod = replacement; + + expect((NativeModules as Record).LazyMod).toBe(replacement); + expect(load).not.toHaveBeenCalled(); + }); + + it('propagates an error thrown by the underlying lazy getter', () => { + jest.spyOn(environment, 'isTurboModuleEnabled').mockReturnValue(false); + const load = jest.fn(() => { + throw new Error('native module failed to load'); + }); + defineLazy('BrokenMod', load); + + wrapAllNativeModules(); + + expect(() => (NativeModules as Record).BrokenMod).toThrow('native module failed to load'); + }); + }); + + it('skips hot React Native infrastructure modules by default', () => { + jest.spyOn(environment, 'isTurboModuleEnabled').mockReturnValue(false); + const spy = jest.spyOn(wrapTurboModuleMod, 'wrapTurboModule'); + + for (const name of ['Timing', 'UIManager', 'NativeAnimatedModule', 'NativeAnimatedTurboModule']) { + (NativeModules as Record)[name] = { createTimer: jest.fn(), doWork: jest.fn() }; + } + + const wrapped = wrapAllNativeModules(); + + for (const name of ['Timing', 'UIManager', 'NativeAnimatedModule', 'NativeAnimatedTurboModule']) { + expect(wrapped).not.toContain(name); + expect(spy).not.toHaveBeenCalledWith(name, expect.anything(), expect.anything()); + } + }); + + it('warns instead of silently no-oping when NativeModules cannot be enumerated', () => { + jest.spyOn(environment, 'isTurboModuleEnabled').mockReturnValue(false); + // Simulates the JSI host-object proxy, which exposes no enumerable keys. + // Scoped to NativeModules so jest's own use of Object.keys keeps working. + const realKeys = Object.keys; + jest.spyOn(Object, 'keys').mockImplementation((o: object) => (o === NativeModules ? [] : realKeys(o))); + const warn = jest.spyOn(logger, 'warn').mockImplementation(() => undefined); + + const wrapped = wrapAllNativeModules(); + + expect(wrapped).toHaveLength(0); + expect(warn).toHaveBeenCalledWith(expect.stringContaining('found no modules to instrument')); + }); }); + +function defineLazy(name: string, get: () => unknown): void { + let value: unknown; + let resolved = false; + Object.defineProperty(NativeModules, name, { + configurable: true, + enumerable: true, + get: () => { + if (!resolved) { + // Mirror RN's `defineLazyObjectProperty`: self-replace with a plain value. + value = get(); + resolved = true; + Object.defineProperty(NativeModules, name, { + configurable: true, + enumerable: true, + writable: true, + value, + }); + } + return value; + }, + set: (next: unknown) => { + value = next; + resolved = true; + Object.defineProperty(NativeModules, name, { + configurable: true, + enumerable: true, + writable: true, + value: next, + }); + }, + }); +}