From ad36f86df12f0dcfa2d71130770af98d18cb2ef0 Mon Sep 17 00:00:00 2001 From: Billy Vong Date: Fri, 26 May 2023 15:08:06 -0400 Subject: [PATCH 01/11] feat(replay): Promote `mutatationBreadcrumbLimit` and `mutationLimit` to regular feature Instead of taking a fullsnapshot when `mutationLimit` is reached, lets be aggressive and stop the replay to ensure end-users are not negatively affected performance wise. The default for showing a breadcrumb is at 750 mutations, and the default limit to stop recording is 1500 mutations. --- packages/replay/src/integration.ts | 21 +++++++++++++++++++++ packages/replay/src/replay.ts | 11 ++++++----- packages/replay/src/types.ts | 17 +++++++++++++++++ 3 files changed, 44 insertions(+), 5 deletions(-) diff --git a/packages/replay/src/integration.ts b/packages/replay/src/integration.ts index 114d015f2702..ddc25682ce44 100644 --- a/packages/replay/src/integration.ts +++ b/packages/replay/src/integration.ts @@ -60,6 +60,9 @@ export class Replay implements Integration { maskAllInputs = true, blockAllMedia = true, + mutationBreadcrumbLimit = 750, + mutationLimit = 1500, + networkDetailAllowUrls = [], networkCaptureBodies = true, networkRequestHeaders = [], @@ -127,6 +130,8 @@ export class Replay implements Integration { blockAllMedia, maskAllInputs, maskAllText, + mutationBreadcrumbLimit, + mutationLimit, networkDetailAllowUrls, networkCaptureBodies, networkRequestHeaders: _getMergedNetworkHeaders(networkRequestHeaders), @@ -136,6 +141,22 @@ export class Replay implements Integration { _experiments, }; + if (typeof _experiments.mutationBreadcrumbLimit === 'number') { + // eslint-disable-next-line + console.warn( + `[Replay] \`mutationBreadcrumbLimit\` is no longer an experiment. To configure, pass to the +Replay constructor. e.g.: Sentry.Replay({ mutationBreadcrumbLimit: 750 });`, + ); + } + + if (typeof _experiments.mutationLimit === 'number') { + // eslint-disable-next-line + console.warn( + `[Replay] \`mutationLimit\` is no longer an experiment. To configure, pass to the +Replay constructor. e.g.: Sentry.Replay({ mutationLimit: 1500 });`, + ); + } + if (typeof sessionSampleRate === 'number') { // eslint-disable-next-line console.warn( diff --git a/packages/replay/src/replay.ts b/packages/replay/src/replay.ts index 53f263b5fa3f..e41702781f14 100644 --- a/packages/replay/src/replay.ts +++ b/packages/replay/src/replay.ts @@ -1056,8 +1056,9 @@ 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 || this._options._experiments.mutationLimit || 0; + const mutationBreadcrumbLimit = + this._options.mutationBreadcrumbLimit || this._options._experiments.mutationBreadcrumbLimit || 1000; const overMutationLimit = mutationLimit && count > mutationLimit; // Create a breadcrumb if a lot of mutations happen at the same time @@ -1067,15 +1068,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(); return false; } diff --git a/packages/replay/src/types.ts b/packages/replay/src/types.ts index 3d1d8616bb79..38bc5070fbde 100644 --- a/packages/replay/src/types.ts +++ b/packages/replay/src/types.ts @@ -285,6 +285,23 @@ export interface ReplayPluginOptions extends ReplayNetworkOptions { */ beforeAddRecordingEvent?: BeforeAddRecordingEvent; + /** + * Threshold of number of mutations that will be processed before creating a warning breadcrumb. + */ + /** + * 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. From 7321508be82e7930f8ec73529b0a0ab082b5b518 Mon Sep 17 00:00:00 2001 From: Billy Vong Date: Mon, 29 May 2023 09:50:44 -0400 Subject: [PATCH 02/11] update test --- .../suites/replay/largeMutations/mutationLimit/init.js | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/packages/browser-integration-tests/suites/replay/largeMutations/mutationLimit/init.js b/packages/browser-integration-tests/suites/replay/largeMutations/mutationLimit/init.js index 5c30f352959c..3aa2548f1522 100644 --- a/packages/browser-integration-tests/suites/replay/largeMutations/mutationLimit/init.js +++ b/packages/browser-integration-tests/suites/replay/largeMutations/mutationLimit/init.js @@ -4,9 +4,7 @@ window.Sentry = Sentry; window.Replay = new Sentry.Replay({ flushMinDelay: 500, flushMaxDelay: 500, - _experiments: { - mutationLimit: 250, - }, + mutationLimit: 250, }); Sentry.init({ From 0db9fcae776ffba5854544aab0a11053ecf6095e Mon Sep 17 00:00:00 2001 From: Billy Vong Date: Mon, 29 May 2023 11:38:33 -0400 Subject: [PATCH 03/11] update test --- .../largeMutations/mutationLimit/test.ts | 35 ++++++++----------- 1 file changed, 14 insertions(+), 21 deletions(-) diff --git a/packages/browser-integration-tests/suites/replay/largeMutations/mutationLimit/test.ts b/packages/browser-integration-tests/suites/replay/largeMutations/mutationLimit/test.ts index 5d5dbb9d3f93..66c30e0e6412 100644 --- a/packages/browser-integration-tests/suites/replay/largeMutations/mutationLimit/test.ts +++ b/packages/browser-integration-tests/suites/replay/largeMutations/mutationLimit/test.ts @@ -1,10 +1,10 @@ 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(); @@ -34,36 +34,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); }, ); From 53973e51c57eaa748065cd6ca6848a622bdc6521 Mon Sep 17 00:00:00 2001 From: Billy Vong Date: Mon, 29 May 2023 11:39:02 -0400 Subject: [PATCH 04/11] change default mutationLimit to 10k --- packages/replay/src/integration.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/replay/src/integration.ts b/packages/replay/src/integration.ts index ddc25682ce44..c08b027b744a 100644 --- a/packages/replay/src/integration.ts +++ b/packages/replay/src/integration.ts @@ -61,7 +61,7 @@ export class Replay implements Integration { blockAllMedia = true, mutationBreadcrumbLimit = 750, - mutationLimit = 1500, + mutationLimit = 10_000, networkDetailAllowUrls = [], networkCaptureBodies = true, From 559f7d61b57a67b59e34bc182ce6c5a8b65b7136 Mon Sep 17 00:00:00 2001 From: Billy Vong Date: Mon, 29 May 2023 12:56:22 -0400 Subject: [PATCH 05/11] lint --- .../suites/replay/largeMutations/mutationLimit/test.ts | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/packages/browser-integration-tests/suites/replay/largeMutations/mutationLimit/test.ts b/packages/browser-integration-tests/suites/replay/largeMutations/mutationLimit/test.ts index 66c30e0e6412..84f0113263d7 100644 --- a/packages/browser-integration-tests/suites/replay/largeMutations/mutationLimit/test.ts +++ b/packages/browser-integration-tests/suites/replay/largeMutations/mutationLimit/test.ts @@ -1,7 +1,12 @@ import { expect } from '@playwright/test'; import { sentryTest } from '../../../../utils/fixtures'; -import { getReplayRecordingContent, getReplaySnapshot, shouldSkipReplayTest, waitForReplayRequest } from '../../../../utils/replayHelpers'; +import { + getReplayRecordingContent, + getReplaySnapshot, + shouldSkipReplayTest, + waitForReplayRequest, +} from '../../../../utils/replayHelpers'; sentryTest( 'handles large mutations by stopping replay when `mutationLimit` configured', @@ -53,7 +58,7 @@ sentryTest( 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']); + expect(replayData1.breadcrumbs.map(({ category }) => category).sort()).toEqual(['replay.mutations', 'ui.click']); replay = await getReplaySnapshot(page); expect(replay.session).toBe(undefined); From 640ce447aad8830a3c842c2659977f96fbeb73fb Mon Sep 17 00:00:00 2001 From: Billy Vong Date: Tue, 30 May 2023 19:02:47 -0400 Subject: [PATCH 06/11] remove backwards compat --- packages/replay/src/integration.ts | 16 ---------------- packages/replay/src/replay.ts | 6 +++--- 2 files changed, 3 insertions(+), 19 deletions(-) diff --git a/packages/replay/src/integration.ts b/packages/replay/src/integration.ts index c08b027b744a..0a8813c14d38 100644 --- a/packages/replay/src/integration.ts +++ b/packages/replay/src/integration.ts @@ -141,22 +141,6 @@ export class Replay implements Integration { _experiments, }; - if (typeof _experiments.mutationBreadcrumbLimit === 'number') { - // eslint-disable-next-line - console.warn( - `[Replay] \`mutationBreadcrumbLimit\` is no longer an experiment. To configure, pass to the -Replay constructor. e.g.: Sentry.Replay({ mutationBreadcrumbLimit: 750 });`, - ); - } - - if (typeof _experiments.mutationLimit === 'number') { - // eslint-disable-next-line - console.warn( - `[Replay] \`mutationLimit\` is no longer an experiment. To configure, pass to the -Replay constructor. e.g.: Sentry.Replay({ mutationLimit: 1500 });`, - ); - } - if (typeof sessionSampleRate === 'number') { // eslint-disable-next-line console.warn( diff --git a/packages/replay/src/replay.ts b/packages/replay/src/replay.ts index e41702781f14..9c9aebdd8d61 100644 --- a/packages/replay/src/replay.ts +++ b/packages/replay/src/replay.ts @@ -1056,9 +1056,9 @@ export class ReplayContainer implements ReplayContainerInterface { private _onMutationHandler = (mutations: unknown[]): boolean => { const count = mutations.length; - const mutationLimit = this._options.mutationLimit || this._options._experiments.mutationLimit || 0; + const mutationLimit = this._options.mutationLimit || 0; const mutationBreadcrumbLimit = - this._options.mutationBreadcrumbLimit || this._options._experiments.mutationBreadcrumbLimit || 1000; + this._options.mutationBreadcrumbLimit || 1000; const overMutationLimit = mutationLimit && count > mutationLimit; // Create a breadcrumb if a lot of mutations happen at the same time @@ -1076,7 +1076,7 @@ export class ReplayContainer implements ReplayContainerInterface { // Stop replay if over the mutation limit if (overMutationLimit) { - void this.stop(); + void this.stop('mutationLimit'); return false; } From e14ca09550a23c1b90038d1c11cf2577b8f6263f Mon Sep 17 00:00:00 2001 From: Billy Vong Date: Tue, 30 May 2023 19:03:36 -0400 Subject: [PATCH 07/11] remove old comment --- packages/replay/src/types.ts | 3 --- 1 file changed, 3 deletions(-) diff --git a/packages/replay/src/types.ts b/packages/replay/src/types.ts index 38bc5070fbde..cd3e0afc4104 100644 --- a/packages/replay/src/types.ts +++ b/packages/replay/src/types.ts @@ -285,9 +285,6 @@ export interface ReplayPluginOptions extends ReplayNetworkOptions { */ beforeAddRecordingEvent?: BeforeAddRecordingEvent; - /** - * Threshold of number of mutations that will be processed before creating a warning breadcrumb. - */ /** * A high number of DOM mutations (in a single event loop) can cause * performance regressions in end-users' browsers. This setting will create From ce857d48729904b8ddc0351a8070aa3e902133ff Mon Sep 17 00:00:00 2001 From: Billy Vong Date: Wed, 31 May 2023 08:47:00 -0400 Subject: [PATCH 08/11] lint --- packages/replay/src/replay.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/packages/replay/src/replay.ts b/packages/replay/src/replay.ts index 9c9aebdd8d61..a71a2f689418 100644 --- a/packages/replay/src/replay.ts +++ b/packages/replay/src/replay.ts @@ -1057,8 +1057,7 @@ export class ReplayContainer implements ReplayContainerInterface { const count = mutations.length; const mutationLimit = this._options.mutationLimit || 0; - const mutationBreadcrumbLimit = - this._options.mutationBreadcrumbLimit || 1000; + const mutationBreadcrumbLimit = this._options.mutationBreadcrumbLimit || 1000; const overMutationLimit = mutationLimit && count > mutationLimit; // Create a breadcrumb if a lot of mutations happen at the same time From e58f780e7e45178f4c803a9fb6ef8a24f007ed67 Mon Sep 17 00:00:00 2001 From: Billy Vong Date: Wed, 31 May 2023 12:25:11 -0400 Subject: [PATCH 09/11] remove redundant defaults --- packages/replay/src/replay.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/replay/src/replay.ts b/packages/replay/src/replay.ts index a71a2f689418..5642b66b8b5a 100644 --- a/packages/replay/src/replay.ts +++ b/packages/replay/src/replay.ts @@ -1056,8 +1056,8 @@ export class ReplayContainer implements ReplayContainerInterface { private _onMutationHandler = (mutations: unknown[]): boolean => { const count = mutations.length; - const mutationLimit = this._options.mutationLimit || 0; - const mutationBreadcrumbLimit = this._options.mutationBreadcrumbLimit || 1000; + const mutationLimit = this._options.mutationLimit; + const mutationBreadcrumbLimit = this._options.mutationBreadcrumbLimit; const overMutationLimit = mutationLimit && count > mutationLimit; // Create a breadcrumb if a lot of mutations happen at the same time From 2396b136617616ab4c1b3e8af93109811c714900 Mon Sep 17 00:00:00 2001 From: Billy Vong Date: Wed, 31 May 2023 12:50:39 -0400 Subject: [PATCH 10/11] fix types --- packages/replay/src/types.ts | 31 +++++++++++++++---------------- 1 file changed, 15 insertions(+), 16 deletions(-) diff --git a/packages/replay/src/types.ts b/packages/replay/src/types.ts index cd3e0afc4104..53cf2d22e9d1 100644 --- a/packages/replay/src/types.ts +++ b/packages/replay/src/types.ts @@ -272,32 +272,33 @@ export interface ReplayPluginOptions extends ReplayNetworkOptions { */ maskAllText: boolean; - /** - * Callback before adding a custom recording event - * - * Events added by the underlying DOM recording library can *not* be modified, - * only custom recording events from the Replay integration will trigger the - * callback listeners. This can be used to scrub certain fields in an event (e.g. URLs from navigation events). - * - * Returning a `null` will drop the event completely. Note, dropping a recording - * event is not the same as dropping the replay, the replay will still exist and - * continue to function. - */ - 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; + 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; + mutationLimit: number; + + /** + * Callback before adding a custom recording event + * + * Events added by the underlying DOM recording library can *not* be modified, + * only custom recording events from the Replay integration will trigger the + * callback listeners. This can be used to scrub certain fields in an event (e.g. URLs from navigation events). + * + * Returning a `null` will drop the event completely. Note, dropping a recording + * event is not the same as dropping the replay, the replay will still exist and + * continue to function. + */ + beforeAddRecordingEvent?: BeforeAddRecordingEvent; /** * _experiments allows users to enable experimental or internal features. @@ -309,8 +310,6 @@ export interface ReplayPluginOptions extends ReplayNetworkOptions { _experiments: Partial<{ captureExceptions: boolean; traceInternals: boolean; - mutationLimit: number; - mutationBreadcrumbLimit: number; slowClicks: { threshold: number; timeout: number; From bbd64ad8b7e3f5e43ade36197cf1af07361f72be Mon Sep 17 00:00:00 2001 From: Billy Vong Date: Wed, 31 May 2023 13:28:14 -0400 Subject: [PATCH 11/11] lint + fix defaults in test --- packages/replay/src/types.ts | 1 - packages/replay/test/utils/setupReplayContainer.ts | 4 ++++ 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/packages/replay/src/types.ts b/packages/replay/src/types.ts index 53cf2d22e9d1..393eed5802a8 100644 --- a/packages/replay/src/types.ts +++ b/packages/replay/src/types.ts @@ -272,7 +272,6 @@ export interface ReplayPluginOptions extends ReplayNetworkOptions { */ maskAllText: boolean; - /** * A high number of DOM mutations (in a single event loop) can cause * performance regressions in end-users' browsers. This setting will create diff --git a/packages/replay/test/utils/setupReplayContainer.ts b/packages/replay/test/utils/setupReplayContainer.ts index 83ced117c464..c7c302cce72b 100644 --- a/packages/replay/test/utils/setupReplayContainer.ts +++ b/packages/replay/test/utils/setupReplayContainer.ts @@ -15,6 +15,8 @@ const DEFAULT_OPTIONS = { networkCaptureBodies: true, networkRequestHeaders: [], networkResponseHeaders: [], + mutationLimit: 1500, + mutationBreadcrumbLimit: 500, _experiments: {}, }; @@ -54,6 +56,8 @@ export const DEFAULT_OPTIONS_EVENT_PAYLOAD = { maskAllText: false, maskAllInputs: false, useCompression: DEFAULT_OPTIONS.useCompression, + mutationLimit: DEFAULT_OPTIONS.mutationLimit, + mutationBreadcrumbLimit: DEFAULT_OPTIONS.mutationBreadcrumbLimit, networkDetailHasUrls: DEFAULT_OPTIONS.networkDetailAllowUrls.length > 0, networkCaptureBodies: DEFAULT_OPTIONS.networkCaptureBodies, networkRequestHeaders: DEFAULT_OPTIONS.networkRequestHeaders.length > 0,