diff --git a/CHANGELOG.md b/CHANGELOG.md index 261d6e8b70..cb306a6e59 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,8 @@ - Find `ios/.xcode.env` in the Expo plugin "Upload Debug Symbols to Sentry" phase when the iOS project uses Swift Package Manager instead of CocoaPods ([#6911](https://github.com/getsentry/sentry-react-native/pull/6911)) - Remove the `@sentry/browser` feedback modules and the server-only `@sentry/core` modules from native bundles built on Windows. Before this fix, `includeWebFeedback: false` made these bundles fail at launch ([#6915](https://github.com/getsentry/sentry-react-native/pull/6915)) +- Stop wrapping the SDK's own `RNSentry` module in `turboModuleContextIntegration` by default. Each wrapped call wrote to the native scope several times, which added CPU time to every SDK call on iOS ([#6921](https://github.com/getsentry/sentry-react-native/pull/6921)) + - To wrap `RNSentry` again, add `{ name: 'RNSentry' }` to the `modules` option. ## 8.31.0 diff --git a/packages/core/etc/sentry-react-native.api.md b/packages/core/etc/sentry-react-native.api.md index f666d3121e..9a4c260b57 100644 --- a/packages/core/etc/sentry-react-native.api.md +++ b/packages/core/etc/sentry-react-native.api.md @@ -1259,7 +1259,7 @@ export interface TurboModuleContextOptions { maxTopModulesPerSpan?: number; modules?: Array<{ name: string; - module: object | null | undefined; + module?: object | null; skipMethods?: ReadonlyArray; }>; slowCallThresholdMs?: number; diff --git a/packages/core/src/js/integrations/turboModuleContext.ts b/packages/core/src/js/integrations/turboModuleContext.ts index ad37e56a91..e6fc461dd3 100644 --- a/packages/core/src/js/integrations/turboModuleContext.ts +++ b/packages/core/src/js/integrations/turboModuleContext.ts @@ -49,8 +49,12 @@ export const MAX_PENDING_CALL_WINDOWS = 1024; export const MAX_PENDING_SPAN_ATTRIBUTES = 256; export interface TurboModuleContextOptions { - /** Additional TurboModules to wrap. `RNSentry` is always tracked. */ - modules?: Array<{ name: string; module: object | null | undefined; skipMethods?: ReadonlyArray }>; + /** + * TurboModules to wrap. The SDK's own `RNSentry` module is not wrapped by default; + * add `{ name: 'RNSentry' }` to opt in. The SDK resolves the module itself and always + * skips its scope-sync methods. + */ + modules?: Array<{ name: string; module?: object | null; skipMethods?: ReadonlyArray }>; /** Per-(module, method, kind) counters, flushed on transaction finish and on a periodic timer. Default: `true`. */ enableAggregateStats?: boolean; @@ -147,12 +151,15 @@ export const turboModuleContextIntegration = (options: TurboModuleContextOptions return { name: INTEGRATION_NAME, setupOnce() { - // 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 }); - + // RNSentry is wrapped only on request: each wrapped call costs about six + // native scope writes, which sentry-cocoa persists to disk. for (const entry of options.modules ?? []) { - wrapTurboModule(entry.name, entry.module, { skip: entry.skipMethods, arch: contextArch }); + if (entry.name === 'RNSentry') { + const skip = [...RNSENTRY_SKIP, ...(entry.skipMethods ?? [])]; + wrapTurboModule(entry.name, entry.module ?? getRNSentryModule(), { skip, arch: contextArch }); + } else { + wrapTurboModule(entry.name, entry.module, { skip: entry.skipMethods, arch: contextArch }); + } } if (options.enableLegacyNativeModules === true) { diff --git a/packages/core/test/integrations/turboModuleContext.test.ts b/packages/core/test/integrations/turboModuleContext.test.ts index f5a73b3fe0..ea1b2aee87 100644 --- a/packages/core/test/integrations/turboModuleContext.test.ts +++ b/packages/core/test/integrations/turboModuleContext.test.ts @@ -107,7 +107,44 @@ describe('turboModuleContextIntegration', () => { _resetTurboModuleAggregator(); }); - it('wraps the live RNSentry TurboModule on setup', () => { + it('does not wrap RNSentry by default', () => { + const fakeModule = { fetchNativeFrames: jest.fn() }; + const originalFetchNativeFrames = fakeModule.fetchNativeFrames; + jest.spyOn(wrapper, 'getRNSentryModule').mockReturnValue(fakeModule as never); + const wrapSpy = jest.spyOn(turboModule, 'wrapTurboModule'); + + turboModuleContextIntegration().setupOnce!(); + + expect(wrapSpy).not.toHaveBeenCalledWith('RNSentry', expect.anything(), expect.anything()); + expect(fakeModule.fetchNativeFrames).toBe(originalFetchNativeFrames); + }); + + it('does not write turbo_module scope data for RNSentry calls by default', () => { + const fakeModule = { fetchNativeFrames: jest.fn() }; + jest.spyOn(wrapper, 'getRNSentryModule').mockReturnValue(fakeModule as never); + const isolationScope = new Scope(); + jest.spyOn(SentryCore, 'getIsolationScope').mockReturnValue(isolationScope); + const setContextSpy = jest.spyOn(isolationScope, 'setContext'); + const setTagSpy = jest.spyOn(isolationScope, 'setTag'); + turboModuleContextIntegration().setupOnce!(); + + fakeModule.fetchNativeFrames(); + + expect(setContextSpy).not.toHaveBeenCalled(); + expect(setTagSpy).not.toHaveBeenCalled(); + }); + + it('does not wrap RNSentry when the user passes a custom ignoreTurboModules list', () => { + const fakeModule = { fetchNativeFrames: jest.fn() }; + jest.spyOn(wrapper, 'getRNSentryModule').mockReturnValue(fakeModule as never); + const wrapSpy = jest.spyOn(turboModule, 'wrapTurboModule'); + + turboModuleContextIntegration({ ignoreTurboModules: [] }).setupOnce!(); + + expect(wrapSpy).not.toHaveBeenCalled(); + }); + + it('wraps the live RNSentry TurboModule when it is listed in modules', () => { const fakeModule = { addListener: jest.fn(), removeListeners: jest.fn(), @@ -117,7 +154,7 @@ describe('turboModuleContextIntegration', () => { const wrapSpy = jest.spyOn(turboModule, 'wrapTurboModule'); - turboModuleContextIntegration().setupOnce!(); + turboModuleContextIntegration({ modules: [{ name: 'RNSentry' }] }).setupOnce!(); expect(wrapSpy).toHaveBeenCalledWith( 'RNSentry', @@ -142,6 +179,22 @@ describe('turboModuleContextIntegration', () => { ); }); + it('merges user skipMethods with the RNSentry skip list', () => { + jest.spyOn(wrapper, 'getRNSentryModule').mockReturnValue(undefined); + const fakeModule = { crash: jest.fn(), fetchNativeFrames: jest.fn(), setTag: jest.fn() }; + const wrapSpy = jest.spyOn(turboModule, 'wrapTurboModule'); + + turboModuleContextIntegration({ + modules: [{ name: 'RNSentry', module: fakeModule, skipMethods: ['fetchNativeFrames'] }], + }).setupOnce!(); + + expect(wrapSpy).toHaveBeenCalledWith( + 'RNSentry', + fakeModule, + expect.objectContaining({ skip: expect.arrayContaining(['setTag', 'fetchNativeFrames']) }), + ); + }); + it('does not wrap scope-sync methods on RNSentry (would recurse infinitely)', () => { // Sanity check: every method `scopeSync.ts` forwards to NATIVE.* via // RNSentry must be in the skip list, otherwise scope writes recurse. @@ -161,7 +214,7 @@ describe('turboModuleContextIntegration', () => { jest.spyOn(wrapper, 'getRNSentryModule').mockReturnValue(fakeModule as never); const originalCrash = fakeModule.crash; - turboModuleContextIntegration().setupOnce!(); + turboModuleContextIntegration({ modules: [{ name: 'RNSentry', module: fakeModule }] }).setupOnce!(); // crash is wrapped (replaced with sentryTurboModuleWrapper, which is a plain // function and therefore lacks the `_isMockFunction` marker the jest mocks carry). diff --git a/samples/react-native/src/App.tsx b/samples/react-native/src/App.tsx index 85aae107d2..67077bd7f3 100644 --- a/samples/react-native/src/App.tsx +++ b/samples/react-native/src/App.tsx @@ -39,7 +39,7 @@ const sampleFeatureFlagsIntegration = Sentry.featureFlagsIntegration(); sampleFeatureFlagsIntegration.addFeatureFlag('sample-test-flag', true); // Replaces the default `TurboModuleContext` integration so the sample's own -// TurboModules are instrumented too, not just `RNSentry`. Exercised by the +// TurboModules are instrumented. Exercised by the // "TurboModule Playground" screen on the Errors tab. const sampleTurboModuleContextIntegration = Sentry.turboModuleContextIntegration({