From 4009e3f7765660deb9816b531f31460da840195d Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Thu, 6 Jun 2024 14:38:40 +0200 Subject: [PATCH] fix(replay): Ignore old events when manually starting replay (#12349) This PR ensures that if a replay is manually started (=no sample rates are defined at all, and a user later calls `start()` or `startBuffering()`, we do not back-port the `initialTimestamp` of the replay based on the event buffer. By default (for the first segment) we'll backport the `initialTimestamp` to the time of the first event in the event buffer, to ensure that e.g. the pageload browser metrics that may be emitted with an earlier timestamp all show up correctly. However, this may be unexpected if manually calling `startBuffering()` and seeing things for stuff that happened before. Now, we keep track of this and adjust the behavior accordingly. Fixes https://github.com/getsentry/sentry-javascript/issues/11984 --------- Co-authored-by: Catherine Lee <55311782+c298lee@users.noreply.github.com> --- packages/replay-internal/src/replay.ts | 14 ++- .../test/integration/earlyEvents.test.ts | 107 ++++++++++++++++++ 2 files changed, 119 insertions(+), 2 deletions(-) create mode 100644 packages/replay-internal/test/integration/earlyEvents.test.ts diff --git a/packages/replay-internal/src/replay.ts b/packages/replay-internal/src/replay.ts index 16f7a6e7d884..1d3b1ef340c4 100644 --- a/packages/replay-internal/src/replay.ts +++ b/packages/replay-internal/src/replay.ts @@ -100,6 +100,9 @@ export class ReplayContainer implements ReplayContainerInterface { */ public readonly timeouts: Timeouts; + /** The replay has to be manually started, because no sample rate (neither session or error) was provided. */ + private _requiresManualStart: boolean; + private _throttledAddEvent: ( event: RecordingEvent, isCheckout?: boolean, @@ -170,6 +173,7 @@ export class ReplayContainer implements ReplayContainerInterface { this._lastActivity = Date.now(); this._isEnabled = false; this._isPaused = false; + this._requiresManualStart = false; this._hasInitializedCoreListeners = false; this._context = { errorIds: new Set(), @@ -246,7 +250,11 @@ export class ReplayContainer implements ReplayContainerInterface { // If neither sample rate is > 0, then do nothing - user will need to call one of // `start()` or `startBuffering` themselves. - if (errorSampleRate <= 0 && sessionSampleRate <= 0) { + const requiresManualStart = errorSampleRate <= 0 && sessionSampleRate <= 0; + + this._requiresManualStart = requiresManualStart; + + if (requiresManualStart) { return; } @@ -1049,7 +1057,9 @@ export class ReplayContainer implements ReplayContainerInterface { /** Update the initial timestamp based on the buffer content. */ private _updateInitialTimestampFromEventBuffer(): void { const { session, eventBuffer } = this; - if (!session || !eventBuffer) { + // If replay was started manually (=no sample rate was given), + // We do not want to back-port the initial timestamp + if (!session || !eventBuffer || this._requiresManualStart) { return; } diff --git a/packages/replay-internal/test/integration/earlyEvents.test.ts b/packages/replay-internal/test/integration/earlyEvents.test.ts new file mode 100644 index 000000000000..b30c28841491 --- /dev/null +++ b/packages/replay-internal/test/integration/earlyEvents.test.ts @@ -0,0 +1,107 @@ +import { BASE_TIMESTAMP } from '..'; +import { resetSdkMock } from '../mocks/resetSdkMock'; +import { useFakeTimers } from '../utils/use-fake-timers'; + +useFakeTimers(); + +describe('Integration | early events', () => { + beforeEach(() => { + vi.clearAllMocks(); + }); + + it('updates initialTimestamp for early performance entries', async () => { + const earlyTimeStampSeconds = BASE_TIMESTAMP / 1000 - 10; + + const { replay } = await resetSdkMock({ + replayOptions: { + stickySession: true, + }, + sentryOptions: { + replaysSessionSampleRate: 0, + replaysOnErrorSampleRate: 1.0, + }, + }); + + expect(replay.session).toBeDefined(); + expect(replay['_requiresManualStart']).toBe(false); + + const initialTimestamp = replay.getContext().initialTimestamp; + + expect(initialTimestamp).not.toEqual(earlyTimeStampSeconds * 1000); + + // A performance entry that happend before should not extend the session when we manually started + replay.replayPerformanceEntries.push({ + type: 'largest-contentful-paint', + name: 'largest-contentful-paint', + start: earlyTimeStampSeconds, + end: earlyTimeStampSeconds, + data: { + value: 100, + size: 100, + nodeId: undefined, + }, + }); + + // _too_ early events are always thrown away + replay.replayPerformanceEntries.push({ + type: 'largest-contentful-paint', + name: 'largest-contentful-paint', + start: earlyTimeStampSeconds - 999999, + end: earlyTimeStampSeconds - 99999, + data: { + value: 100, + size: 100, + nodeId: undefined, + }, + }); + + await replay.flushImmediate(); + vi.runAllTimers(); + + expect(replay.getContext().initialTimestamp).toEqual(earlyTimeStampSeconds * 1000); + }); + + it('does not change initialTimestamp when replay is manually started', async () => { + const earlyTimeStampSeconds = Date.now() / 1000 - 5; + + const { replay } = await resetSdkMock({ + replayOptions: { + stickySession: true, + }, + sentryOptions: { + replaysSessionSampleRate: 0.0, + replaysOnErrorSampleRate: 0.0, + }, + }); + + expect(replay.session).toBe(undefined); + expect(replay['_requiresManualStart']).toBe(true); + + replay.start(); + vi.runAllTimers(); + + const initialTimestamp = replay.getContext().initialTimestamp; + + expect(initialTimestamp).not.toEqual(earlyTimeStampSeconds * 1000); + expect(replay.session).toBeDefined(); + expect(replay['_requiresManualStart']).toBe(true); + + // A performance entry that happened before should not extend the session when we manually started + replay.replayPerformanceEntries.push({ + type: 'largest-contentful-paint', + name: 'largest-contentful-paint', + start: earlyTimeStampSeconds, + end: earlyTimeStampSeconds, + data: { + value: 100, + size: 100, + nodeId: undefined, + }, + }); + + await replay.flushImmediate(); + vi.runAllTimers(); + + expect(replay.getContext().initialTimestamp).toEqual(initialTimestamp); + }); +});