Skip to content

Commit

Permalink
feat(replay): Allow to configure beforeErrorSampling (#9470)
Browse files Browse the repository at this point in the history
This adds a new option to `new Replay()` which allows to ignore certain
errors for error-based sampling:

```js
new Replay({
  beforeErrorSampling: (event) => !event.message.includes('ignore me')
});
```

When returning `false` from this callback, the event will not trigger an
error-based sampling.

Note that returning `true` there does not mean this will 100% be
sampled, but just that it will check the `replaysOnErrorSampleRate`. The
purpose of this callback is to be able to ignore certain groups of
errors from triggering an error-based replay at all.

Closes #9413

Related to #8462
(not 100% but partially)
  • Loading branch information
mydea authored Nov 8, 2023
1 parent 6ddf14d commit 6200b3c
Show file tree
Hide file tree
Showing 6 changed files with 130 additions and 5 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
import * as Sentry from '@sentry/browser';

window.Sentry = Sentry;
window.Replay = new Sentry.Replay({
flushMinDelay: 200,
flushMaxDelay: 200,
minReplayDuration: 0,
beforeErrorSampling: event => !event.message.includes('[drop]'),
});

Sentry.init({
dsn: 'https://public@dsn.ingest.sentry.io/1337',
sampleRate: 1,
replaysSessionSampleRate: 0.0,
replaysOnErrorSampleRate: 1.0,

integrations: [window.Replay],
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
import { expect } from '@playwright/test';

import { sentryTest } from '../../../../utils/fixtures';
import { getReplaySnapshot, shouldSkipReplayTest, waitForReplayRunning } from '../../../../utils/replayHelpers';

sentryTest(
'[error-mode] should not flush if error event is ignored in beforeErrorSampling',
async ({ getLocalTestPath, page, browserName, forceFlushReplay }) => {
// Skipping this in webkit because it is flakey there
if (shouldSkipReplayTest() || browserName === 'webkit') {
sentryTest.skip();
}

await page.route('https://dsn.ingest.sentry.io/**/*', route => {
return route.fulfill({
status: 200,
contentType: 'application/json',
body: JSON.stringify({ id: 'test-id' }),
});
});

const url = await getLocalTestPath({ testDir: __dirname });

await page.goto(url);
await waitForReplayRunning(page);

await page.click('#drop');
await forceFlushReplay();

expect(await getReplaySnapshot(page)).toEqual(
expect.objectContaining({
_isEnabled: true,
_isPaused: false,
recordingMode: 'buffer',
}),
);
},
);
17 changes: 12 additions & 5 deletions packages/replay/src/coreHandlers/handleAfterSendEvent.ts
Original file line number Diff line number Diff line change
Expand Up @@ -63,12 +63,19 @@ function handleErrorEvent(replay: ReplayContainer, event: ErrorEvent): void {

// 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();
});
if (replay.recordingMode !== 'buffer' || !event.tags || !event.tags.replayId) {
return;
}

const { beforeErrorSampling } = replay.getOptions();
if (typeof beforeErrorSampling === 'function' && !beforeErrorSampling(event)) {
return;
}

setTimeout(() => {
// Capture current event buffer as new replay
void replay.sendBufferedReplayOrFlush();
});
}

function isBaseTransportSend(): boolean {
Expand Down
2 changes: 2 additions & 0 deletions packages/replay/src/integration.ts
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,7 @@ export class Replay implements Integration {
maskFn,

beforeAddRecordingEvent,
beforeErrorSampling,

// eslint-disable-next-line deprecation/deprecation
blockClass,
Expand Down Expand Up @@ -178,6 +179,7 @@ export class Replay implements Integration {
networkRequestHeaders: _getMergedNetworkHeaders(networkRequestHeaders),
networkResponseHeaders: _getMergedNetworkHeaders(networkResponseHeaders),
beforeAddRecordingEvent,
beforeErrorSampling,

_experiments,
};
Expand Down
11 changes: 11 additions & 0 deletions packages/replay/src/types/replay.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import type {
Breadcrumb,
ErrorEvent,
FetchBreadcrumbHint,
HandlerDataFetch,
ReplayRecordingData,
Expand Down Expand Up @@ -211,6 +212,16 @@ export interface ReplayPluginOptions extends ReplayNetworkOptions {
*/
beforeAddRecordingEvent?: BeforeAddRecordingEvent;

/**
* An optional callback to be called before we decide to sample based on an error.
* If specified, this callback will receive an error that was captured by Sentry.
* Return `true` to continue sampling for this error, or `false` to ignore this error for replay sampling.
* Note that returning `true` means that the `replaysOnErrorSampleRate` will be checked,
* not that it will definitely be sampled.
* Use this to filter out groups of errors that should def. not be sampled.
*/
beforeErrorSampling?: (event: ErrorEvent) => boolean;

/**
* _experiments allows users to enable experimental or internal features.
* We don't consider such features as part of the public API and hence we don't guarantee semver for them.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -411,4 +411,53 @@ describe('Integration | coreHandlers | handleAfterSendEvent', () => {

expect(mockSend).toHaveBeenCalledTimes(0);
});

it('calls beforeErrorSampling if defined', async () => {
const error1 = Error({ event_id: 'err1', tags: { replayId: 'replayid1' } });
const error2 = Error({ event_id: 'err2', tags: { replayId: 'replayid1' } });

const beforeErrorSampling = jest.fn(event => event === error2);

({ replay } = await resetSdkMock({
replayOptions: {
stickySession: false,
beforeErrorSampling,
},
sentryOptions: {
replaysSessionSampleRate: 0.0,
replaysOnErrorSampleRate: 1.0,
},
}));

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

const handler = handleAfterSendEvent(replay);

expect(replay.recordingMode).toBe('buffer');

handler(error1, { statusCode: 200 });

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

expect(beforeErrorSampling).toHaveBeenCalledTimes(1);

// Not flushed yet
expect(mockSend).toHaveBeenCalledTimes(0);
expect(replay.recordingMode).toBe('buffer');
expect(Array.from(replay.getContext().errorIds)).toEqual(['err1']);

handler(error2, { statusCode: 200 });

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

expect(beforeErrorSampling).toHaveBeenCalledTimes(2);

// Triggers session
expect(mockSend).toHaveBeenCalledTimes(1);
expect(replay.recordingMode).toBe('session');
expect(replay.isEnabled()).toBe(true);
expect(replay.isPaused()).toBe(false);
});
});

0 comments on commit 6200b3c

Please sign in to comment.