diff --git a/CHANGELOG.md b/CHANGELOG.md index f44bdd85bb..b2f2aa9710 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,7 @@ - Improves Expo Router integration to optionally include full paths to components instead of just component names ([#5414](https://github.com/getsentry/sentry-react-native/pull/5414)) - Report slow and frozen frames as TTID/TTFD span data ([#5419](https://github.com/getsentry/sentry-react-native/pull/5419)) - Report slow and frozen frames on spans created through the API ([#5420](https://github.com/getsentry/sentry-react-native/issues/5420)) +- Improve performance by adding caching to `getReplayId` ([#5449](https://github.com/getsentry/sentry-react-native/pull/5449)) ### Fixes diff --git a/packages/core/src/js/replay/mobilereplay.ts b/packages/core/src/js/replay/mobilereplay.ts index ccc9f3b757..d65d794add 100644 --- a/packages/core/src/js/replay/mobilereplay.ts +++ b/packages/core/src/js/replay/mobilereplay.ts @@ -166,6 +166,25 @@ export const mobileReplayIntegration = (initOptions: MobileReplayOptions = defau const options = mergeOptions(initOptions); + // Cache the replay ID in JavaScript to avoid excessive bridge calls + // This will be updated when we know the replay ID changes (e.g., after captureReplay) + let cachedReplayId: string | null = null; + + function updateCachedReplayId(replayId: string | null): void { + cachedReplayId = replayId; + } + + function getCachedReplayId(): string | null { + if (cachedReplayId !== null) { + return cachedReplayId; + } + const nativeReplayId = NATIVE.getCurrentReplayId(); + if (nativeReplayId) { + cachedReplayId = nativeReplayId; + } + return nativeReplayId; + } + async function processEvent(event: Event, hint: EventHint): Promise { const hasException = event.exception?.values && event.exception.values.length > 0; if (!hasException) { @@ -192,19 +211,23 @@ export const mobileReplayIntegration = (initOptions: MobileReplayOptions = defau } const replayId = await NATIVE.captureReplay(isHardCrash(event)); - if (!replayId) { + if (replayId) { + updateCachedReplayId(replayId); + debug.log( + `[Sentry] ${MOBILE_REPLAY_INTEGRATION_NAME} Captured recording replay ${replayId} for event ${event.event_id}.`, + ); + } else { + // Check if there's an ongoing recording and update cache if found const recordingReplayId = NATIVE.getCurrentReplayId(); if (recordingReplayId) { + updateCachedReplayId(recordingReplayId); debug.log( `[Sentry] ${MOBILE_REPLAY_INTEGRATION_NAME} assign already recording replay ${recordingReplayId} for event ${event.event_id}.`, ); } else { + updateCachedReplayId(null); debug.log(`[Sentry] ${MOBILE_REPLAY_INTEGRATION_NAME} not sampled for event ${event.event_id}.`); } - } else { - debug.log( - `[Sentry] ${MOBILE_REPLAY_INTEGRATION_NAME} Captured recording replay ${replayId} for event ${event.event_id}.`, - ); } return event; @@ -215,13 +238,16 @@ export const mobileReplayIntegration = (initOptions: MobileReplayOptions = defau return; } + // Initialize the cached replay ID on setup + cachedReplayId = NATIVE.getCurrentReplayId(); + client.on('createDsc', (dsc: DynamicSamplingContext) => { if (dsc.replay_id) { return; } - // TODO: For better performance, we should emit replayId changes on native, and hold the replayId value in JS - const currentReplayId = NATIVE.getCurrentReplayId(); + // Use cached replay ID to avoid bridge calls + const currentReplayId = getCachedReplayId(); if (currentReplayId) { dsc.replay_id = currentReplayId; } @@ -231,7 +257,7 @@ export const mobileReplayIntegration = (initOptions: MobileReplayOptions = defau } function getReplayId(): string | null { - return NATIVE.getCurrentReplayId(); + return getCachedReplayId(); } // TODO: When adding manual API, ensure overlap with the web replay so users can use the same API interchangeably diff --git a/packages/core/test/replay/mobilereplay.test.ts b/packages/core/test/replay/mobilereplay.test.ts index 5db21ef0e8..aa76c65a36 100644 --- a/packages/core/test/replay/mobilereplay.test.ts +++ b/packages/core/test/replay/mobilereplay.test.ts @@ -1,5 +1,5 @@ import { afterEach, beforeEach, describe, expect, it, jest } from '@jest/globals'; -import type { Event, EventHint } from '@sentry/core'; +import type { Client, DynamicSamplingContext, Event, EventHint } from '@sentry/core'; import { mobileReplayIntegration } from '../../src/js/replay/mobilereplay'; import * as environment from '../../src/js/utils/environment'; import { NATIVE } from '../../src/js/wrapper'; @@ -279,4 +279,207 @@ describe('Mobile Replay Integration', () => { expect(integration.processEvent).toBeUndefined(); }); }); + + describe('replay ID caching', () => { + let mockClient: jest.Mocked; + let mockOn: jest.Mock; + + beforeEach(() => { + mockOn = jest.fn(); + mockClient = { + on: mockOn, + } as unknown as jest.Mocked; + // Reset mocks for each test + jest.clearAllMocks(); + }); + + it('should initialize cache with native replay ID on setup', () => { + const initialReplayId = 'initial-replay-id'; + mockGetCurrentReplayId.mockReturnValue(initialReplayId); + + const integration = mobileReplayIntegration(); + if (integration.setup) { + integration.setup(mockClient); + } + + expect(mockGetCurrentReplayId).toHaveBeenCalledTimes(1); + expect(mockOn).toHaveBeenCalledWith('createDsc', expect.any(Function)); + }); + + it('should use cached replay ID in createDsc handler to avoid bridge calls', () => { + const cachedReplayId = 'cached-replay-id'; + mockGetCurrentReplayId.mockReturnValue(cachedReplayId); + + const integration = mobileReplayIntegration(); + if (integration.setup) { + integration.setup(mockClient); + } + + // Extract the createDsc handler BEFORE clearing mocks + const createDscCall = mockOn.mock.calls.find(call => call[0] === 'createDsc'); + expect(createDscCall).toBeDefined(); + const createDscHandler = createDscCall![1] as (dsc: DynamicSamplingContext) => void; + + // Clear the mock to track subsequent calls + jest.clearAllMocks(); + + // Call the handler multiple times + const dsc1: Partial = {}; + const dsc2: Partial = {}; + const dsc3: Partial = {}; + + createDscHandler(dsc1 as DynamicSamplingContext); + createDscHandler(dsc2 as DynamicSamplingContext); + createDscHandler(dsc3 as DynamicSamplingContext); + + // Should not call native bridge after initial setup + expect(mockGetCurrentReplayId).not.toHaveBeenCalled(); + expect(dsc1.replay_id).toBe(cachedReplayId); + expect(dsc2.replay_id).toBe(cachedReplayId); + expect(dsc3.replay_id).toBe(cachedReplayId); + }); + + it('should not override existing replay_id in createDsc handler', () => { + const cachedReplayId = 'cached-replay-id'; + mockGetCurrentReplayId.mockReturnValue(cachedReplayId); + + const integration = mobileReplayIntegration(); + if (integration.setup) { + integration.setup(mockClient); + } + + const createDscCall = mockOn.mock.calls.find(call => call[0] === 'createDsc'); + const createDscHandler = createDscCall![1] as (dsc: DynamicSamplingContext) => void; + + const dsc: Partial = { + replay_id: 'existing-replay-id', + }; + + createDscHandler(dsc as DynamicSamplingContext); + + expect(dsc.replay_id).toBe('existing-replay-id'); + }); + + it('should update cache when captureReplay returns a new replay ID', async () => { + const initialReplayId = 'initial-replay-id'; + const newReplayId = 'new-replay-id'; + mockGetCurrentReplayId.mockReturnValue(initialReplayId); + mockCaptureReplay.mockResolvedValue(newReplayId); + + const integration = mobileReplayIntegration(); + if (integration.setup) { + integration.setup(mockClient); + } + + const event: Event = { + event_id: 'test-event-id', + exception: { + values: [{ type: 'Error', value: 'Test error' }], + }, + }; + const hint: EventHint = {}; + + if (integration.processEvent) { + await integration.processEvent(event, hint); + } + + // Verify cache was updated by checking getReplayId + expect(integration.getReplayId()).toBe(newReplayId); + + // Extract the createDsc handler BEFORE clearing mocks + const createDscCall = mockOn.mock.calls.find(call => call[0] === 'createDsc'); + expect(createDscCall).toBeDefined(); + const createDscHandler = createDscCall![1] as (dsc: DynamicSamplingContext) => void; + + // Clear the mock to track subsequent calls + jest.clearAllMocks(); + + const dsc: Partial = {}; + createDscHandler(dsc as DynamicSamplingContext); + + expect(dsc.replay_id).toBe(newReplayId); + expect(mockGetCurrentReplayId).not.toHaveBeenCalled(); + }); + + it('should update cache when ongoing recording is detected', async () => { + const initialReplayId = 'initial-replay-id'; + const ongoingReplayId = 'ongoing-replay-id'; + mockGetCurrentReplayId.mockReturnValue(initialReplayId); + mockCaptureReplay.mockResolvedValue(null); + // After captureReplay returns null, getCurrentReplayId should return ongoing recording + mockGetCurrentReplayId.mockReturnValueOnce(initialReplayId).mockReturnValue(ongoingReplayId); + + const integration = mobileReplayIntegration(); + if (integration.setup) { + integration.setup(mockClient); + } + + const event: Event = { + event_id: 'test-event-id', + exception: { + values: [{ type: 'Error', value: 'Test error' }], + }, + }; + const hint: EventHint = {}; + + if (integration.processEvent) { + await integration.processEvent(event, hint); + } + + // Verify cache was updated with ongoing recording ID + expect(integration.getReplayId()).toBe(ongoingReplayId); + }); + + it('should clear cache when no recording is in progress', async () => { + const initialReplayId = 'initial-replay-id'; + mockGetCurrentReplayId.mockReturnValue(initialReplayId); + mockCaptureReplay.mockResolvedValue(null); + // After captureReplay returns null, getCurrentReplayId should return null (no recording) + mockGetCurrentReplayId.mockReturnValueOnce(initialReplayId).mockReturnValue(null); + + const integration = mobileReplayIntegration(); + if (integration.setup) { + integration.setup(mockClient); + } + + const event: Event = { + event_id: 'test-event-id', + exception: { + values: [{ type: 'Error', value: 'Test error' }], + }, + }; + const hint: EventHint = {}; + + if (integration.processEvent) { + await integration.processEvent(event, hint); + } + + // Verify cache was cleared + expect(integration.getReplayId()).toBeNull(); + }); + + it('should use cached value in getReplayId to avoid bridge calls', () => { + const cachedReplayId = 'cached-replay-id'; + mockGetCurrentReplayId.mockReturnValue(cachedReplayId); + + const integration = mobileReplayIntegration(); + if (integration.setup) { + integration.setup(mockClient); + } + + // Clear the mock to track subsequent calls + jest.clearAllMocks(); + + // Call getReplayId multiple times + const id1 = integration.getReplayId(); + const id2 = integration.getReplayId(); + const id3 = integration.getReplayId(); + + // Should not call native bridge after initial setup + expect(mockGetCurrentReplayId).not.toHaveBeenCalled(); + expect(id1).toBe(cachedReplayId); + expect(id2).toBe(cachedReplayId); + expect(id3).toBe(cachedReplayId); + }); + }); });