Merge branch 'cocoa-v9' into itay/use_regular_podspec

Author: antonis

.github/workflows/ready-to-merge-workflow.yml

@@ -19,9 +19,11 @@ jobs:
       - name: Check for exact 'ready-to-merge' label
         if: ${{ inputs.is-pr }}
         id: check-label
+        env:
+          LABELS: ${{ inputs.labels }}
         run: |
           # Use jq to check for exact label match (avoids substring matching issues with contains())
-          if echo '${{ inputs.labels }}' | jq -e '.[] | select(.name == "ready-to-merge")' > /dev/null; then
+          if echo "$LABELS" | jq -e '.[] | select(.name == "ready-to-merge")' > /dev/null; then
             echo "label_found=true" >> $GITHUB_OUTPUT
           else
             echo "label_found=false" >> $GITHUB_OUTPUT

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
 

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<Event> {
     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

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<Client>;
+    let mockOn: jest.Mock;
+
+    beforeEach(() => {
+      mockOn = jest.fn();
+      mockClient = {
+        on: mockOn,
+      } as unknown as jest.Mocked<Client>;
+      // 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<DynamicSamplingContext> = {};
+      const dsc2: Partial<DynamicSamplingContext> = {};
+      const dsc3: Partial<DynamicSamplingContext> = {};
+
+      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<DynamicSamplingContext> = {
+        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<DynamicSamplingContext> = {};
+      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);
+    });
+  });
 });