From d281b13c5e7366ba23f3bf8e33312bdd2ce77613 Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Mon, 24 Jul 2023 17:58:09 +0200 Subject: [PATCH] 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) {