Skip to content

Commit

Permalink
fix(replay): Improve capture of errorIds/traceIds (#8678)
Browse files Browse the repository at this point in the history 
We limit to max. 100 IDs being captured per replay, and ensure we do not
capture stuff when replay has been disabled in the meanwhile.
  • Loading branch information
@mydea
mydea authored Aug 1, 2023
1 parent 74dd18e commit f760e6c
Show file tree
Hide file tree
Showing 4 changed files with 145 additions and 32 deletions.
65 changes: 38 additions & 27 deletions packages/replay/src/coreHandlers/handleAfterSendEvent.ts
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { getCurrentHub } from '@sentry/core';
import type { Event, Transport, TransportMakeRequestResponse } from '@sentry/types';
import type { ErrorEvent, Event, TransactionEvent, Transport, TransportMakeRequestResponse } from '@sentry/types';

import type { ReplayContainer } from '../types';
import { isErrorEvent, isTransactionEvent } from '../util/eventUtils';
Expand All @@ -15,7 +15,7 @@ export function handleAfterSendEvent(replay: ReplayContainer): AfterSendEventCal
const enforceStatusCode = isBaseTransportSend();

return (event: Event, sendResponse: TransportMakeRequestResponse | void) => {
if (!isErrorEvent(event) && !isTransactionEvent(event)) {
if (!replay.isEnabled() || (!isErrorEvent(event) && !isTransactionEvent(event))) {
return;
}

Expand All @@ -28,36 +28,47 @@ export function handleAfterSendEvent(replay: ReplayContainer): AfterSendEventCal
return;
}

// Collect traceIds in _context regardless of `recordingMode`
// In error mode, _context gets cleared on every checkout
if (isTransactionEvent(event) && event.contexts && event.contexts.trace && event.contexts.trace.trace_id) {
replay.getContext().traceIds.add(event.contexts.trace.trace_id as string);
if (isTransactionEvent(event)) {
handleTransactionEvent(replay, event);
return;
}

// Everything below is just for error events
if (!isErrorEvent(event)) {
return;
}
handleErrorEvent(replay, event);
};
}

// Add error to list of errorIds of replay. This is ok to do even if not
// sampled because context will get reset at next checkout.
// XXX: There is also a race condition where it's possible to capture an
// error to Sentry before Replay SDK has loaded, but response returns after
// it was loaded, and this gets called.
if (event.event_id) {
replay.getContext().errorIds.add(event.event_id);
}
function handleTransactionEvent(replay: ReplayContainer, event: TransactionEvent): void {
const replayContext = replay.getContext();

// If error event is tagged with replay id it means it was sampled (when in buffer mode)
// Need to be very careful that this does not cause an infinite loop
if (replay.recordingMode === 'buffer' && event.tags && event.tags.replayId) {
setTimeout(() => {
// Capture current event buffer as new replay
void replay.sendBufferedReplayOrFlush();
});
}
};
// Collect traceIds in _context regardless of `recordingMode`
// In error mode, _context gets cleared on every checkout
// We limit to max. 100 transactions linked
if (event.contexts && event.contexts.trace && event.contexts.trace.trace_id && replayContext.traceIds.size < 100) {
replayContext.traceIds.add(event.contexts.trace.trace_id as string);
}
}

function handleErrorEvent(replay: ReplayContainer, event: ErrorEvent): void {
const replayContext = replay.getContext();

// Add error to list of errorIds of replay. This is ok to do even if not
// sampled because context will get reset at next checkout.
// XXX: There is also a race condition where it's possible to capture an
// error to Sentry before Replay SDK has loaded, but response returns after
// it was loaded, and this gets called.
// We limit to max. 100 errors linked
if (event.event_id && replayContext.errorIds.size < 100) {
replayContext.errorIds.add(event.event_id);
}

// If error event is tagged with replay id it means it was sampled (when in buffer mode)
// Need to be very careful that this does not cause an infinite loop
if (replay.recordingMode === 'buffer' && event.tags && event.tags.replayId) {
setTimeout(() => {
// Capture current event buffer as new replay
void replay.sendBufferedReplayOrFlush();
});
}
}

function isBaseTransportSend(): boolean {
Expand Down
5 changes: 5 additions & 0 deletions packages/replay/src/coreHandlers/handleGlobalEvent.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,11 @@ export function handleGlobalEventListener(
const afterSendHandler = includeAfterSendEventHandling ? handleAfterSendEvent(replay) : undefined;

return (event: Event, hint: EventHint) => {
// Do nothing if replay has been disabled
if (!replay.isEnabled()) {
return event;
}

if (isReplayEvent(event)) {
// Replays have separate set of breadcrumbs, do not include breadcrumbs
// from core SDK
Expand Down
89 changes: 84 additions & 5 deletions packages/replay/test/integration/coreHandlers/handleAfterSendEvent.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,58 @@ describe('Integration | coreHandlers | handleAfterSendEvent', () => {
expect(replay.recordingMode).toBe('buffer');
});

it('limits errorIds to max. 100', async () => {
({ replay } = await resetSdkMock({
replayOptions: {
stickySession: false,
},
sentryOptions: {
replaysSessionSampleRate: 1.0,
replaysOnErrorSampleRate: 0.0,
},
}));

const handler = handleAfterSendEvent(replay);

for (let i = 0; i < 150; i++) {
const error = Error({ event_id: `err-${i}` });
handler(error, { statusCode: 200 });
}

expect(Array.from(replay.getContext().errorIds)).toEqual(
Array(100)
.fill(undefined)
.map((_, i) => `err-${i}`),
);
expect(Array.from(replay.getContext().traceIds)).toEqual([]);
});

it('limits traceIds to max. 100', async () => {
({ replay } = await resetSdkMock({
replayOptions: {
stickySession: false,
},
sentryOptions: {
replaysSessionSampleRate: 0.0,
replaysOnErrorSampleRate: 1.0,
},
}));

const handler = handleAfterSendEvent(replay);

for (let i = 0; i < 150; i++) {
const transaction = Transaction(`tr-${i}`);
handler(transaction, { statusCode: 200 });
}

expect(Array.from(replay.getContext().errorIds)).toEqual([]);
expect(Array.from(replay.getContext().traceIds)).toEqual(
Array(100)
.fill(undefined)
.map((_, i) => `tr-${i}`),
);
});

it('allows undefined send response when using custom transport', async () => {
({ replay } = await resetSdkMock({
replayOptions: {
Expand Down Expand Up @@ -123,7 +175,7 @@ describe('Integration | coreHandlers | handleAfterSendEvent', () => {
expect(Array.from(replay.getContext().errorIds)).toEqual(['err1', 'err2', 'err3', 'err4']);
});

it('flushes when in error mode', async () => {
it('flushes when in buffer mode', async () => {
({ replay } = await resetSdkMock({
replayOptions: {
stickySession: false,
Expand Down Expand Up @@ -231,7 +283,7 @@ describe('Integration | coreHandlers | handleAfterSendEvent', () => {
expect(replay.recordingMode).toBe('buffer');
});

it('does not flush in error mode when failing to send the error', async () => {
it('does not flush in buffer mode when failing to send the error', async () => {
({ replay } = await resetSdkMock({
replayOptions: {
stickySession: false,
Expand All @@ -257,7 +309,7 @@ describe('Integration | coreHandlers | handleAfterSendEvent', () => {
jest.runAllTimers();
await new Promise(process.nextTick);

// Remains in error mode & without flushing
// Remains in buffer mode & without flushing
expect(mockSend).toHaveBeenCalledTimes(0);
expect(Array.from(replay.getContext().errorIds)).toEqual([]);
expect(replay.isEnabled()).toBe(true);
Expand Down Expand Up @@ -291,7 +343,7 @@ describe('Integration | coreHandlers | handleAfterSendEvent', () => {
jest.runAllTimers();
await new Promise(process.nextTick);

// Remains in error mode & without flushing
// Remains in buffer mode & without flushing
expect(mockSend).toHaveBeenCalledTimes(0);
expect(Array.from(replay.getContext().errorIds)).toEqual(['err1']);
expect(replay.isEnabled()).toBe(true);
Expand Down Expand Up @@ -325,11 +377,38 @@ describe('Integration | coreHandlers | handleAfterSendEvent', () => {
jest.runAllTimers();
await new Promise(process.nextTick);

// Remains in error mode & without flushing
// Remains in buffer mode & without flushing
expect(mockSend).toHaveBeenCalledTimes(0);
expect(Array.from(replay.getContext().errorIds)).toEqual(['err1']);
expect(replay.isEnabled()).toBe(true);
expect(replay.isPaused()).toBe(false);
expect(replay.recordingMode).toBe('buffer');
});

it('does not flush if replay is not enabled anymore', async () => {
({ replay } = await resetSdkMock({
replayOptions: {
stickySession: false,
},
sentryOptions: {
replaysSessionSampleRate: 0.0,
replaysOnErrorSampleRate: 1.0,
},
}));

const mockSend = getCurrentHub().getClient()!.getTransport()!.send as unknown as jest.SpyInstance<any>;

const error1 = Error({ event_id: 'err1', tags: { replayId: 'replayid1' } });

const handler = handleAfterSendEvent(replay);

handler(error1, { statusCode: 200 });

replay['_isEnabled'] = false;

jest.runAllTimers();
await new Promise(process.nextTick);

expect(mockSend).toHaveBeenCalledTimes(0);
});
});
18 changes: 18 additions & 0 deletions packages/replay/test/integration/coreHandlers/handleGlobalEvent.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,24 @@ describe('Integration | coreHandlers | handleGlobalEvent', () => {
);
});

it('does not add replayId if replay is not enabled', async () => {
const transaction = Transaction();
const error = Error();

replay['_isEnabled'] = false;

expect(handleGlobalEventListener(replay)(transaction, {})).toEqual(
expect.objectContaining({
tags: expect.not.objectContaining({ replayId: expect.anything() }),
}),
);
expect(handleGlobalEventListener(replay)(error, {})).toEqual(
expect.objectContaining({
tags: expect.not.objectContaining({ replayId: expect.anything() }),
}),
);
});

it('tags errors and transactions with replay id for session samples', async () => {
let integration: ReplayIntegration;
({ replay, integration } = await resetSdkMock({}));
Expand Down

0 comments on commit f760e6c

Please sign in to comment.