Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(replay): Promote mutationBreadcrumbLimit and mutationLimit to regular feature #8228

Merged
merged 11 commits into from May 31, 2023
Expand Up @@ -4,9 +4,7 @@ window.Sentry = Sentry;
window.Replay = new Sentry.Replay({
flushMinDelay: 500,
flushMaxDelay: 500,
_experiments: {
mutationLimit: 250,
},
mutationLimit: 250,
});

Sentry.init({
Expand Down
@@ -1,10 +1,15 @@
import { expect } from '@playwright/test';

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

sentryTest(
'handles large mutations with _experiments.mutationLimit configured',
'handles large mutations by stopping replay when `mutationLimit` configured',
async ({ getLocalTestPath, page, forceFlushReplay, browserName }) => {
if (shouldSkipReplayTest() || ['webkit', 'firefox'].includes(browserName)) {
sentryTest.skip();
Expand Down Expand Up @@ -34,36 +39,29 @@ sentryTest(
await forceFlushReplay();
const res1 = await reqPromise1;

const reqPromise2 = waitForReplayRequest(page);
// replay should be stopped due to mutation limit
let replay = await getReplaySnapshot(page);
expect(replay.session).toBe(undefined);
expect(replay._isEnabled).toBe(false);

void page.click('#button-modify');
await forceFlushReplay();
const res2 = await reqPromise2;

const reqPromise3 = waitForReplayRequest(page);

void page.click('#button-remove');
await page.click('#button-remove');
await forceFlushReplay();
const res3 = await reqPromise3;

const replayData0 = getReplayRecordingContent(res0);
const replayData1 = getReplayRecordingContent(res1);
const replayData2 = getReplayRecordingContent(res2);
const replayData3 = getReplayRecordingContent(res3);

expect(replayData0.fullSnapshots.length).toBe(1);
expect(replayData0.incrementalSnapshots.length).toBe(0);

// This includes both a full snapshot as well as some incremental snapshots
expect(replayData1.fullSnapshots.length).toBe(1);
// Breadcrumbs (click and mutation);
const replayData1 = getReplayRecordingContent(res1);
expect(replayData1.fullSnapshots.length).toBe(0);
expect(replayData1.incrementalSnapshots.length).toBeGreaterThan(0);
expect(replayData1.breadcrumbs.map(({ category }) => category).sort()).toEqual(['replay.mutations', 'ui.click']);

// This does not trigger mutations, for whatever reason - so no full snapshot either!
expect(replayData2.fullSnapshots.length).toBe(0);
expect(replayData2.incrementalSnapshots.length).toBeGreaterThan(0);

// This includes both a full snapshot as well as some incremental snapshots
expect(replayData3.fullSnapshots.length).toBe(1);
expect(replayData3.incrementalSnapshots.length).toBeGreaterThan(0);
replay = await getReplaySnapshot(page);
expect(replay.session).toBe(undefined);
expect(replay._isEnabled).toBe(false);
},
);
5 changes: 5 additions & 0 deletions packages/replay/src/integration.ts
Expand Up @@ -60,6 +60,9 @@ export class Replay implements Integration {
maskAllInputs = true,
blockAllMedia = true,

mutationBreadcrumbLimit = 750,
mutationLimit = 10_000,

networkDetailAllowUrls = [],
networkCaptureBodies = true,
networkRequestHeaders = [],
Expand Down Expand Up @@ -127,6 +130,8 @@ export class Replay implements Integration {
blockAllMedia,
maskAllInputs,
maskAllText,
mutationBreadcrumbLimit,
mutationLimit,
networkDetailAllowUrls,
networkCaptureBodies,
networkRequestHeaders: _getMergedNetworkHeaders(networkRequestHeaders),
Expand Down
10 changes: 5 additions & 5 deletions packages/replay/src/replay.ts
Expand Up @@ -1056,8 +1056,8 @@ export class ReplayContainer implements ReplayContainerInterface {
private _onMutationHandler = (mutations: unknown[]): boolean => {
const count = mutations.length;

const mutationLimit = this._options._experiments.mutationLimit || 0;
const mutationBreadcrumbLimit = this._options._experiments.mutationBreadcrumbLimit || 1000;
const mutationLimit = this._options.mutationLimit || 0;
const mutationBreadcrumbLimit = this._options.mutationBreadcrumbLimit || 1000;
Copy link
Member Author

@billyvg billyvg May 31, 2023

Choose a reason for hiding this comment

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

Oops these defaults are out of sync, they shouldn't even be needed.

const overMutationLimit = mutationLimit && count > mutationLimit;

// Create a breadcrumb if a lot of mutations happen at the same time
Expand All @@ -1067,15 +1067,15 @@ export class ReplayContainer implements ReplayContainerInterface {
category: 'replay.mutations',
data: {
count,
limit: overMutationLimit,
},
});
this._createCustomBreadcrumb(breadcrumb);
}

// Stop replay if over the mutation limit
if (overMutationLimit) {
// We want to skip doing an incremental snapshot if there are too many mutations
// Instead, we do a full snapshot
this._triggerFullSnapshot(false);
void this.stop('mutationLimit');
return false;
}

Expand Down
14 changes: 14 additions & 0 deletions packages/replay/src/types.ts
Expand Up @@ -285,6 +285,20 @@ export interface ReplayPluginOptions extends ReplayNetworkOptions {
*/
beforeAddRecordingEvent?: BeforeAddRecordingEvent;

/**
* A high number of DOM mutations (in a single event loop) can cause
* performance regressions in end-users' browsers. This setting will create
* a breadcrumb in the recording when the limit has been reached.
*/
mutationBreadcrumbLimit?: number;

/**
* A high number of DOM mutations (in a single event loop) can cause
* performance regressions in end-users' browsers. This setting will cause
* recording to stop when the limit has been reached.
*/
mutationLimit?: number;

/**
* _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