From 7d7593e8626177f01b300d22f6ff2ca3276aab88 Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Mon, 24 Jul 2023 17:58:09 +0200 Subject: [PATCH 1/3] fix(replay): Use `session.started` for min/max duration check --- .../suites/replay/maxReplayDuration/init.js | 23 ++++++++ .../replay/maxReplayDuration/template.html | 10 ++++ .../suites/replay/maxReplayDuration/test.ts | 59 +++++++++++++++++++ packages/replay/src/replay.ts | 9 +-- .../replay/src/util/handleRecordingEmit.ts | 5 ++ 5 files changed, 102 insertions(+), 4 deletions(-) create mode 100644 packages/browser-integration-tests/suites/replay/maxReplayDuration/init.js create mode 100644 packages/browser-integration-tests/suites/replay/maxReplayDuration/template.html create mode 100644 packages/browser-integration-tests/suites/replay/maxReplayDuration/test.ts diff --git a/packages/browser-integration-tests/suites/replay/maxReplayDuration/init.js b/packages/browser-integration-tests/suites/replay/maxReplayDuration/init.js new file mode 100644 index 000000000000..e5dfd8115207 --- /dev/null +++ b/packages/browser-integration-tests/suites/replay/maxReplayDuration/init.js @@ -0,0 +1,23 @@ +import * as Sentry from '@sentry/browser'; + +window.Sentry = Sentry; +window.Replay = new Sentry.Replay({ + flushMinDelay: 200, + flushMaxDelay: 200, + minReplayDuration: 0, +}); + +Sentry.init({ + dsn: 'https://public@dsn.ingest.sentry.io/1337', + sampleRate: 0, + replaysSessionSampleRate: 1.0, + replaysOnErrorSampleRate: 0.0, + + integrations: [window.Replay], +}); + +window.Replay._replay.timeouts = { + sessionIdlePause: 1000, // this is usually 5min, but we want to test this with shorter times + sessionIdleExpire: 2000, // this is usually 15min, but we want to test this with shorter times + maxSessionLife: 2000, // default: 60min +}; diff --git a/packages/browser-integration-tests/suites/replay/maxReplayDuration/template.html b/packages/browser-integration-tests/suites/replay/maxReplayDuration/template.html new file mode 100644 index 000000000000..7223a20f82ba --- /dev/null +++ b/packages/browser-integration-tests/suites/replay/maxReplayDuration/template.html @@ -0,0 +1,10 @@ + + + + + + + + + + diff --git a/packages/browser-integration-tests/suites/replay/maxReplayDuration/test.ts b/packages/browser-integration-tests/suites/replay/maxReplayDuration/test.ts new file mode 100644 index 000000000000..b5a940e62ace --- /dev/null +++ b/packages/browser-integration-tests/suites/replay/maxReplayDuration/test.ts @@ -0,0 +1,59 @@ +import { expect } from '@playwright/test'; + +import { sentryTest } from '../../../utils/fixtures'; +import { getExpectedReplayEvent } from '../../../utils/replayEventTemplates'; +import { getReplayEvent, shouldSkipReplayTest, waitForReplayRequest } from '../../../utils/replayHelpers'; + +const SESSION_MAX_AGE = 2000; + +sentryTest('keeps track of max duration accross reloads', async ({ getLocalTestPath, page }) => { + if (shouldSkipReplayTest()) { + sentryTest.skip(); + } + + const reqPromise0 = waitForReplayRequest(page, 0); + const reqPromise1 = waitForReplayRequest(page, 1); + + 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 new Promise(resolve => setTimeout(resolve, SESSION_MAX_AGE / 2)); + + await page.reload(); + + // After the second reload, we should have a new session (because we exceeded max age) + const reqPromise3 = waitForReplayRequest(page, 0); + + await new Promise(resolve => setTimeout(resolve, SESSION_MAX_AGE / 2 + 100)); + + void page.click('#button1'); + await page.evaluate(`Object.defineProperty(document, 'visibilityState', { + configurable: true, + get: function () { + return 'hidden'; + }, + }); + document.dispatchEvent(new Event('visibilitychange'));`); + + const replayEvent0 = getReplayEvent(await reqPromise0); + expect(replayEvent0).toEqual(getExpectedReplayEvent({})); + + const replayEvent1 = getReplayEvent(await reqPromise1); + expect(replayEvent1).toEqual( + getExpectedReplayEvent({ + segment_id: 1, + }), + ); + + const replayEvent3 = getReplayEvent(await reqPromise3); + expect(replayEvent3).toEqual(getExpectedReplayEvent({})); +}); diff --git a/packages/replay/src/replay.ts b/packages/replay/src/replay.ts index 9ca3077e914f..cf0e72ffcc34 100644 --- a/packages/replay/src/replay.ts +++ b/packages/replay/src/replay.ts @@ -426,6 +426,10 @@ export class ReplayContainer implements ReplayContainerInterface { const activityTime = Date.now(); + // eslint-disable-next-line no-console + const log = this.getOptions()._experiments.traceInternals ? console.info : logger.info; + __DEBUG_BUILD__ && log(`[Replay] Converting buffer to session, starting at ${activityTime}`); + // Allow flush to complete before resuming as a session recording, otherwise // the checkout from `startRecording` may be included in the payload. // Prefer to keep the error replay as a separate (and smaller) segment @@ -981,9 +985,6 @@ export class ReplayContainer implements ReplayContainerInterface { const earliestEvent = eventBuffer.getEarliestTimestamp(); if (earliestEvent && earliestEvent < this._context.initialTimestamp) { - // eslint-disable-next-line no-console - const log = this.getOptions()._experiments.traceInternals ? console.info : logger.info; - __DEBUG_BUILD__ && log(`[Replay] Updating initial timestamp to ${earliestEvent}`); this._context.initialTimestamp = earliestEvent; } } @@ -1103,7 +1104,7 @@ export class ReplayContainer implements ReplayContainerInterface { return; } - const start = this._context.initialTimestamp; + const start = this.session.started; const now = Date.now(); const duration = now - start; diff --git a/packages/replay/src/util/handleRecordingEmit.ts b/packages/replay/src/util/handleRecordingEmit.ts index 987f589412ed..4039fad5caa8 100644 --- a/packages/replay/src/util/handleRecordingEmit.ts +++ b/packages/replay/src/util/handleRecordingEmit.ts @@ -72,6 +72,11 @@ export function getHandleRecordingEmit(replay: ReplayContainer): RecordingEmitCa if (replay.recordingMode === 'buffer' && replay.session && replay.eventBuffer) { const earliestEvent = replay.eventBuffer.getEarliestTimestamp(); if (earliestEvent) { + // eslint-disable-next-line no-console + const log = replay.getOptions()._experiments.traceInternals ? console.info : logger.info; + __DEBUG_BUILD__ && + log(`[Replay] Updating session start time to earliest event in buffer at ${earliestEvent}`); + replay.session.started = earliestEvent; if (replay.getOptions().stickySession) { From 69b316a437d82d0846302ee363ffeb61dd0239a3 Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Tue, 25 Jul 2023 09:55:24 +0200 Subject: [PATCH 2/3] Update packages/browser-integration-tests/suites/replay/maxReplayDuration/test.ts Co-authored-by: Billy Vong --- .../suites/replay/maxReplayDuration/test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/browser-integration-tests/suites/replay/maxReplayDuration/test.ts b/packages/browser-integration-tests/suites/replay/maxReplayDuration/test.ts index b5a940e62ace..59152992eede 100644 --- a/packages/browser-integration-tests/suites/replay/maxReplayDuration/test.ts +++ b/packages/browser-integration-tests/suites/replay/maxReplayDuration/test.ts @@ -6,7 +6,7 @@ import { getReplayEvent, shouldSkipReplayTest, waitForReplayRequest } from '../. const SESSION_MAX_AGE = 2000; -sentryTest('keeps track of max duration accross reloads', async ({ getLocalTestPath, page }) => { +sentryTest('keeps track of max duration across reloads', async ({ getLocalTestPath, page }) => { if (shouldSkipReplayTest()) { sentryTest.skip(); } From 36807b9a41d147bd44d610d8a18453fbdd89a7a9 Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Tue, 25 Jul 2023 11:15:36 +0200 Subject: [PATCH 3/3] try unflake --- .../suites/replay/maxReplayDuration/test.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/browser-integration-tests/suites/replay/maxReplayDuration/test.ts b/packages/browser-integration-tests/suites/replay/maxReplayDuration/test.ts index 59152992eede..1e27aef149e2 100644 --- a/packages/browser-integration-tests/suites/replay/maxReplayDuration/test.ts +++ b/packages/browser-integration-tests/suites/replay/maxReplayDuration/test.ts @@ -29,6 +29,7 @@ sentryTest('keeps track of max duration across reloads', async ({ getLocalTestPa await new Promise(resolve => setTimeout(resolve, SESSION_MAX_AGE / 2)); await page.reload(); + await page.click('#button1'); // After the second reload, we should have a new session (because we exceeded max age) const reqPromise3 = waitForReplayRequest(page, 0);