Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
42 changes: 34 additions & 8 deletions packages/core/src/js/replay/mobilereplay.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Comment on lines +177 to +186

This comment was marked as outdated.


async function processEvent(event: Event, hint: EventHint): Promise<Event> {
const hasException = event.exception?.values && event.exception.values.length > 0;
if (!hasException) {
Expand All @@ -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}.`,
);
}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@lucas-zimerman I wonder if it's sufficient. In this case, updateCachedReplayId is being called only when processEvent is called, and I think it makes sense — we call NATIVE.captureReplay there, get the ID, and update it in our "cache".
BUT is it possible for replay ID to be updated outside of this scenario?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the replay id would change on the native side since we get the replay id from the current native scope (Android, iOS)
I think there is no reliable way to cache the value on RN without a callback from the native code.


return event;
Expand All @@ -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;
}
Expand All @@ -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
Expand Down
205 changes: 204 additions & 1 deletion packages/core/test/replay/mobilereplay.test.ts
Original file line number Diff line number Diff line change
@@ -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';
Expand Down Expand Up @@ -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);
});
});
});
Loading