From e6f64a739cf0d8978dde44a11ef4ee1ca0ee1bd5 Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Fri, 27 Jan 2023 10:06:29 -0500 Subject: [PATCH 1/3] feat(replay): Add "start recording" breadcrumb --- packages/replay/src/replay.ts | 27 ++++++++- packages/replay/src/util/addEvent.ts | 2 + packages/replay/src/util/getFullUrl.ts | 10 ++++ .../test/integration/errorSampleRate.test.ts | 35 ++++++++++- .../test/integration/rateLimiting.test.ts | 58 +++++++++++++++++-- .../replay/test/integration/session.test.ts | 39 +++++++++++++ packages/replay/test/integration/stop.test.ts | 16 ++++- .../replay/test/unit/util/getFullUrl.test.ts | 20 +++++++ 8 files changed, 195 insertions(+), 12 deletions(-) create mode 100644 packages/replay/src/util/getFullUrl.ts create mode 100644 packages/replay/test/unit/util/getFullUrl.test.ts diff --git a/packages/replay/src/replay.ts b/packages/replay/src/replay.ts index 2c648e13b786..35e71214b95c 100644 --- a/packages/replay/src/replay.ts +++ b/packages/replay/src/replay.ts @@ -36,6 +36,7 @@ import { createBreadcrumb } from './util/createBreadcrumb'; import { createPerformanceEntries } from './util/createPerformanceEntries'; import { createPerformanceSpans } from './util/createPerformanceSpans'; import { debounce } from './util/debounce'; +import { getFullURL } from './util/getFullUrl'; import { isExpired } from './util/isExpired'; import { isSessionExpired } from './util/isSessionExpired'; import { overwriteRecordDroppedEvent, restoreRecordDroppedEvent } from './util/monkeyPatchRecordDroppedEvent'; @@ -111,6 +112,8 @@ export class ReplayContainer implements ReplayContainerInterface { initialUrl: '', }; + private _startRecordingbreadcrumb: Breadcrumb | undefined; + public constructor({ options, recordingOptions, @@ -209,6 +212,7 @@ export class ReplayContainer implements ReplayContainerInterface { }); } catch (err) { this._handleException(err); + return; } } @@ -441,8 +445,7 @@ export class ReplayContainer implements ReplayContainerInterface { * first flush. */ private _setInitialState(): void { - const urlPath = `${WINDOW.location.pathname}${WINDOW.location.hash}${WINDOW.location.search}`; - const url = `${WINDOW.location.origin}${urlPath}`; + const url = getFullURL(); this.performanceEvents = []; @@ -541,6 +544,12 @@ export class ReplayContainer implements ReplayContainerInterface { return false; } + // We only do this for session mode, as error mode will be changed into session mode + // (which is when startRecording() is called again) once the error occurs. + if (this.recordingMode === 'session') { + this._addStartRecordingBreadcrumb(); + } + // If there is a previousSessionId after a full snapshot occurs, then // the replay session was started due to session expiration. The new session // is started before triggering a new checkout and contains the id @@ -898,4 +907,18 @@ export class ReplayContainer implements ReplayContainerInterface { }, rateLimitDuration); } } + + /** + * Creates a breadcrumb indicating the start of a replay recording + */ + private _addStartRecordingBreadcrumb(): void { + // if (this._startRecordingbreadcrumb) { + // return; + // } + this._startRecordingbreadcrumb = createBreadcrumb({ + category: 'replay.recording.start', + data: { url: getFullURL() }, + }); + this._createCustomBreadcrumb(this._startRecordingbreadcrumb); + } } diff --git a/packages/replay/src/util/addEvent.ts b/packages/replay/src/util/addEvent.ts index 2b957823bb0f..2d7138aacf61 100644 --- a/packages/replay/src/util/addEvent.ts +++ b/packages/replay/src/util/addEvent.ts @@ -19,6 +19,8 @@ export async function addEvent( return null; } + console.log(event); + // TODO: sadness -- we will want to normalize timestamps to be in ms - // requires coordination with frontend const isMs = event.timestamp > 9999999999; diff --git a/packages/replay/src/util/getFullUrl.ts b/packages/replay/src/util/getFullUrl.ts new file mode 100644 index 000000000000..e8322d5b2060 --- /dev/null +++ b/packages/replay/src/util/getFullUrl.ts @@ -0,0 +1,10 @@ +import { WINDOW } from '../constants'; + +/** + * Takes the full URL from `window.location` and returns it as a string. + */ +export function getFullURL(): string { + const urlPath = `${WINDOW.location.pathname}${WINDOW.location.hash}${WINDOW.location.search}`; + const url = `${WINDOW.location.origin}${urlPath}`; + return url; +} diff --git a/packages/replay/test/integration/errorSampleRate.test.ts b/packages/replay/test/integration/errorSampleRate.test.ts index 5ba806734cae..a6de6a6f3f69 100644 --- a/packages/replay/test/integration/errorSampleRate.test.ts +++ b/packages/replay/test/integration/errorSampleRate.test.ts @@ -109,20 +109,33 @@ describe('Integration | errorSampleRate', () => { }, }, }), - recordingData: JSON.stringify([{ data: { isCheckout: true }, timestamp: BASE_TIMESTAMP + 5020, type: 2 }]), }); jest.advanceTimersByTime(DEFAULT_FLUSH_MIN_DELAY); // New checkout when we call `startRecording` again after uploading segment // after an error occurs + const timestamp = BASE_TIMESTAMP + DEFAULT_FLUSH_MIN_DELAY + 20; expect(replay).toHaveLastSentReplay({ recordingData: JSON.stringify([ { data: { isCheckout: true }, - timestamp: BASE_TIMESTAMP + DEFAULT_FLUSH_MIN_DELAY + 20, + timestamp, type: 2, }, + { + type: 5, + timestamp: timestamp / 1000, + data: { + tag: 'breadcrumb', + payload: { + timestamp: timestamp / 1000, + type: 'default', + category: 'replay.recording.start', + data: { url: 'http://localhost/' }, + }, + }, + }, ]), }); @@ -460,9 +473,25 @@ it('sends a replay after loading the session multiple times', async () => { recordingData: JSON.stringify([{ data: { isCheckout: true }, timestamp: BASE_TIMESTAMP, type: 2 }, TEST_EVENT]), }); + const timestamp = BASE_TIMESTAMP + 5020; // Latest checkout when we call `startRecording` again after uploading segment // after an error occurs (e.g. when we switch to session replay recording) expect(replay).toHaveLastSentReplay({ - recordingData: JSON.stringify([{ data: { isCheckout: true }, timestamp: BASE_TIMESTAMP + 5020, type: 2 }]), + recordingData: JSON.stringify([ + { data: { isCheckout: true }, timestamp: timestamp, type: 2 }, + { + type: 5, + timestamp: timestamp / 1000, + data: { + tag: 'breadcrumb', + payload: { + timestamp: timestamp / 1000, + type: 'default', + category: 'replay.recording.start', + data: { url: 'http://localhost/' }, + }, + }, + }, + ]), }); }); diff --git a/packages/replay/test/integration/rateLimiting.test.ts b/packages/replay/test/integration/rateLimiting.test.ts index 379efb9b430d..cd214d5b2d1b 100644 --- a/packages/replay/test/integration/rateLimiting.test.ts +++ b/packages/replay/test/integration/rateLimiting.test.ts @@ -95,7 +95,7 @@ describe('Integration | rate-limiting behaviour', () => { // @ts-ignore private API jest.spyOn(replay, '_handleRateLimit'); - const TEST_EVENT = { data: {}, timestamp: BASE_TIMESTAMP, type: 2 }; + const TEST_EVENT = { data: {}, timestamp: BASE_TIMESTAMP, type: 3 }; mockTransportSend.mockImplementationOnce(() => { return Promise.resolve(rateLimitResponse); @@ -142,9 +142,23 @@ describe('Integration | rate-limiting behaviour', () => { expect(replay.isPaused()).toBe(false); expect(mockSendReplayRequest).toHaveBeenCalledTimes(2); + const checkoutTimestamp = BASE_TIMESTAMP + DEFAULT_FLUSH_MIN_DELAY * 7; expect(replay).toHaveLastSentReplay({ recordingData: JSON.stringify([ - { data: { isCheckout: true }, timestamp: BASE_TIMESTAMP + DEFAULT_FLUSH_MIN_DELAY * 7, type: 2 }, + { data: { isCheckout: true }, timestamp: checkoutTimestamp, type: 2 }, + { + type: 5, + timestamp: checkoutTimestamp / 1000, + data: { + tag: 'breadcrumb', + payload: { + timestamp: checkoutTimestamp / 1000, + type: 'default', + category: 'replay.recording.start', + data: { url: 'http://localhost/' }, + }, + }, + }, ]), }); @@ -179,7 +193,7 @@ describe('Integration | rate-limiting behaviour', () => { // @ts-ignore private API jest.spyOn(replay, '_handleRateLimit'); - const TEST_EVENT = { data: {}, timestamp: BASE_TIMESTAMP, type: 2 }; + const TEST_EVENT = { data: {}, timestamp: BASE_TIMESTAMP, type: 3 }; mockTransportSend.mockImplementationOnce(() => { return Promise.resolve({ statusCode: 429 }); @@ -227,9 +241,24 @@ describe('Integration | rate-limiting behaviour', () => { expect(replay.isPaused()).toBe(false); expect(mockSendReplayRequest).toHaveBeenCalledTimes(2); + + const checkoutTimestamp = BASE_TIMESTAMP + DEFAULT_FLUSH_MIN_DELAY * 13; expect(replay).toHaveLastSentReplay({ recordingData: JSON.stringify([ - { data: { isCheckout: true }, timestamp: BASE_TIMESTAMP + DEFAULT_FLUSH_MIN_DELAY * 13, type: 2 }, + { data: { isCheckout: true }, timestamp: checkoutTimestamp, type: 2 }, + { + type: 5, + timestamp: checkoutTimestamp / 1000, + data: { + tag: 'breadcrumb', + payload: { + timestamp: checkoutTimestamp / 1000, + type: 'default', + category: 'replay.recording.start', + data: { url: 'http://localhost/' }, + }, + }, + }, ]), }); @@ -282,11 +311,28 @@ describe('Integration | rate-limiting behaviour', () => { expect(mockRecord.takeFullSnapshot).not.toHaveBeenCalled(); expect(mockTransportSend).toHaveBeenCalledTimes(1); - expect(replay).toHaveLastSentReplay({ recordingData: JSON.stringify([TEST_EVENT]) }); + expect(replay).toHaveLastSentReplay({ + recordingData: JSON.stringify([ + TEST_EVENT, + { + type: 5, + timestamp: BASE_TIMESTAMP / 1000, + data: { + tag: 'breadcrumb', + payload: { + timestamp: BASE_TIMESTAMP / 1000, + type: 'default', + category: 'replay.recording.start', + data: { url: 'http://localhost/' }, + }, + }, + }, + ]), + }); expect(replay['_handleRateLimit']).toHaveBeenCalledTimes(1); expect(replay.resume).not.toHaveBeenCalled(); - expect(replay.isPaused).toHaveBeenCalledTimes(2); + expect(replay.isPaused).toHaveBeenCalledTimes(3); expect(replay.pause).not.toHaveBeenCalled(); }); }); diff --git a/packages/replay/test/integration/session.test.ts b/packages/replay/test/integration/session.test.ts index dc54503e922d..f501ad3c64df 100644 --- a/packages/replay/test/integration/session.test.ts +++ b/packages/replay/test/integration/session.test.ts @@ -151,6 +151,19 @@ describe('Integration | session', () => { expect(replay).toHaveLastSentReplay({ recordingData: JSON.stringify([ { data: { isCheckout: true }, timestamp: newTimestamp, type: 2 }, + { + type: 5, + timestamp: newTimestamp / 1000, + data: { + tag: 'breadcrumb', + payload: { + timestamp: newTimestamp / 1000, + type: 'default', + category: 'replay.recording.start', + data: { url: 'http://localhost/' }, + }, + }, + }, { type: 5, timestamp: breadcrumbTimestamp, @@ -311,6 +324,19 @@ describe('Integration | session', () => { recordingPayloadHeader: { segment_id: 0 }, recordingData: JSON.stringify([ { data: { isCheckout: true }, timestamp: newTimestamp, type: 2 }, + { + type: 5, + timestamp: newTimestamp / 1000, + data: { + tag: 'breadcrumb', + payload: { + timestamp: newTimestamp / 1000, + type: 'default', + category: 'replay.recording.start', + data: { url: 'http://dummy/' }, + }, + }, + }, { type: 5, timestamp: breadcrumbTimestamp, @@ -422,6 +448,19 @@ describe('Integration | session', () => { recordingPayloadHeader: { segment_id: 0 }, recordingData: JSON.stringify([ { data: { isCheckout: true }, timestamp: newTimestamp, type: 2 }, + { + type: 5, + timestamp: newTimestamp / 1000, + data: { + tag: 'breadcrumb', + payload: { + timestamp: newTimestamp / 1000, + type: 'default', + category: 'replay.recording.start', + data: { url: 'http://dummy/' }, + }, + }, + }, { type: 5, timestamp: breadcrumbTimestamp, diff --git a/packages/replay/test/integration/stop.test.ts b/packages/replay/test/integration/stop.test.ts index 651a21f7e5bc..46371847abad 100644 --- a/packages/replay/test/integration/stop.test.ts +++ b/packages/replay/test/integration/stop.test.ts @@ -109,14 +109,28 @@ describe('Integration | stop', () => { WINDOW.dispatchEvent(new Event('blur')); jest.runAllTimers(); await new Promise(process.nextTick); + const checkoutTimestamp = BASE_TIMESTAMP + ELAPSED + EXTRA_TICKS; expect(replay).toHaveLastSentReplay({ recordingData: JSON.stringify([ // This event happens when we call `replay.start` { data: { isCheckout: true }, - timestamp: BASE_TIMESTAMP + ELAPSED + EXTRA_TICKS, + timestamp: checkoutTimestamp, type: 2, }, + { + type: 5, + timestamp: checkoutTimestamp / 1000, + data: { + tag: 'breadcrumb', + payload: { + timestamp: checkoutTimestamp / 1000, + type: 'default', + category: 'replay.recording.start', + data: { url: 'http://localhost/' }, + }, + }, + }, TEST_EVENT, hiddenBreadcrumb, ]), diff --git a/packages/replay/test/unit/util/getFullUrl.test.ts b/packages/replay/test/unit/util/getFullUrl.test.ts new file mode 100644 index 000000000000..9775f818e3b0 --- /dev/null +++ b/packages/replay/test/unit/util/getFullUrl.test.ts @@ -0,0 +1,20 @@ +import { getFullURL } from '../../../src/util/getFullUrl'; + +jest.mock('../../../src/constants', () => { + return { + WINDOW: { + location: { + origin: 'https://myDomain.com', + pathname: '/users/123', + hash: '#edit', + search: '?mode=admin', + }, + }, + }; +}); + +describe('Unit | util | getFullURL', () => { + it('returns the concatenated full URL form `window.location`', () => { + expect(getFullURL()).toBe('https://myDomain.com/users/123#edit?mode=admin'); + }); +}); From e81048f2a3640fed19c3aa76b9a88c8433bd4a2f Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Tue, 31 Jan 2023 16:25:00 +0100 Subject: [PATCH 2/3] add integration test --- .../replay/startRecordingBreadcrumb/init.js | 18 ++++++++ .../startRecordingBreadcrumb/template.html | 9 ++++ .../replay/startRecordingBreadcrumb/test.ts | 42 +++++++++++++++++++ packages/integration-tests/utils/helpers.ts | 4 +- .../integration-tests/utils/replayHelpers.ts | 13 +++++- 5 files changed, 82 insertions(+), 4 deletions(-) create mode 100644 packages/integration-tests/suites/replay/startRecordingBreadcrumb/init.js create mode 100644 packages/integration-tests/suites/replay/startRecordingBreadcrumb/template.html create mode 100644 packages/integration-tests/suites/replay/startRecordingBreadcrumb/test.ts diff --git a/packages/integration-tests/suites/replay/startRecordingBreadcrumb/init.js b/packages/integration-tests/suites/replay/startRecordingBreadcrumb/init.js new file mode 100644 index 000000000000..ccb9689a14d6 --- /dev/null +++ b/packages/integration-tests/suites/replay/startRecordingBreadcrumb/init.js @@ -0,0 +1,18 @@ +import * as Sentry from '@sentry/browser'; +import { Replay } from '@sentry/replay'; + +window.Sentry = Sentry; +window.Replay = new Replay({ + flushMinDelay: 200, + flushMaxDelay: 200, + useCompression: false, +}); + +Sentry.init({ + dsn: 'https://public@dsn.ingest.sentry.io/1337', + sampleRate: 0, + replaysSessionSampleRate: 1.0, + replaysOnErrorSampleRate: 0.0, + + integrations: [window.Replay], +}); diff --git a/packages/integration-tests/suites/replay/startRecordingBreadcrumb/template.html b/packages/integration-tests/suites/replay/startRecordingBreadcrumb/template.html new file mode 100644 index 000000000000..2b3e2f0b27b4 --- /dev/null +++ b/packages/integration-tests/suites/replay/startRecordingBreadcrumb/template.html @@ -0,0 +1,9 @@ + + + + + + + + + diff --git a/packages/integration-tests/suites/replay/startRecordingBreadcrumb/test.ts b/packages/integration-tests/suites/replay/startRecordingBreadcrumb/test.ts new file mode 100644 index 000000000000..fb10bb82aceb --- /dev/null +++ b/packages/integration-tests/suites/replay/startRecordingBreadcrumb/test.ts @@ -0,0 +1,42 @@ +import { expect } from '@playwright/test'; +import { SDK_VERSION } from '@sentry/browser'; +import type { RecordingEvent } from '@sentry/replay/build/npm/types/types'; +import type { ReplayEvent } from '@sentry/types'; + +import { sentryTest } from '../../../utils/fixtures'; +import { envelopeRequestParser } from '../../../utils/helpers'; +import { getReplayBreadcrumbs, waitForReplayRequest } from '../../../utils/replayHelpers'; + +sentryTest('adds a start recording breadcrumb to the replay', async ({ getLocalTestPath, page }) => { + // Replay bundles are es6 only + if (process.env.PW_BUNDLE && process.env.PW_BUNDLE.startsWith('bundle_es5')) { + sentryTest.skip(); + } + + const reqPromise = waitForReplayRequest(page, 0); + + 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); + + const replayRecording = envelopeRequestParser(await reqPromise, 5) as RecordingEvent[]; + const breadCrumbs = getReplayBreadcrumbs(replayRecording, 'replay.recording.start'); + + expect(breadCrumbs.length).toBe(1); + expect(breadCrumbs[0]).toEqual({ + category: 'replay.recording.start', + data: { + url: expect.stringContaining('replay/startRecordingBreadcrumb/dist/index.html'), + }, + timestamp: expect.any(Number), + type: 'default', + }); +}); diff --git a/packages/integration-tests/utils/helpers.ts b/packages/integration-tests/utils/helpers.ts index 443e3e0e57af..5fb2aafff3e1 100644 --- a/packages/integration-tests/utils/helpers.ts +++ b/packages/integration-tests/utils/helpers.ts @@ -17,8 +17,8 @@ export const envelopeParser = (request: Request | null): unknown[] => { }); }; -export const envelopeRequestParser = (request: Request | null): Event => { - return envelopeParser(request)[2] as Event; +export const envelopeRequestParser = (request: Request | null, envelopeIndex = 2): Event => { + return envelopeParser(request)[envelopeIndex] as Event; }; export const envelopeHeaderRequestParser = (request: Request | null): EventEnvelopeHeaders => { diff --git a/packages/integration-tests/utils/replayHelpers.ts b/packages/integration-tests/utils/replayHelpers.ts index 601285c266b9..54c16f7bec06 100644 --- a/packages/integration-tests/utils/replayHelpers.ts +++ b/packages/integration-tests/utils/replayHelpers.ts @@ -1,5 +1,5 @@ -import type { ReplayContainer } from '@sentry/replay/build/npm/types/types'; -import type { Event, ReplayEvent } from '@sentry/types'; +import type { RecordingEvent, ReplayContainer } from '@sentry/replay/build/npm/types/types'; +import type { Breadcrumb, Event, ReplayEvent } from '@sentry/types'; import type { Page, Request } from 'playwright'; import { envelopeRequestParser } from './helpers'; @@ -56,3 +56,12 @@ export async function getReplaySnapshot(page: Page): Promise { } export const REPLAY_DEFAULT_FLUSH_MAX_DELAY = 5_000; + +export function getReplayBreadcrumbs(rrwebEvents: RecordingEvent[], category?: string): Breadcrumb[] { + return rrwebEvents + .filter(event => event.type === 5) + .map(event => event.data as { tag: string; payload: { category: string } }) + .filter(data => data.tag === 'breadcrumb') + .map(data => data.payload) + .filter(payload => !category || payload.category === category); +} From 05076ac3505e8d8d55c4e1f6b520a056c0c4fb8c Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Tue, 31 Jan 2023 17:19:26 +0100 Subject: [PATCH 3/3] cleanup --- .../suites/replay/startRecordingBreadcrumb/test.ts | 2 -- packages/replay/src/replay.ts | 11 +++-------- packages/replay/src/util/addEvent.ts | 2 -- 3 files changed, 3 insertions(+), 12 deletions(-) diff --git a/packages/integration-tests/suites/replay/startRecordingBreadcrumb/test.ts b/packages/integration-tests/suites/replay/startRecordingBreadcrumb/test.ts index fb10bb82aceb..6892727c0c6b 100644 --- a/packages/integration-tests/suites/replay/startRecordingBreadcrumb/test.ts +++ b/packages/integration-tests/suites/replay/startRecordingBreadcrumb/test.ts @@ -1,7 +1,5 @@ import { expect } from '@playwright/test'; -import { SDK_VERSION } from '@sentry/browser'; import type { RecordingEvent } from '@sentry/replay/build/npm/types/types'; -import type { ReplayEvent } from '@sentry/types'; import { sentryTest } from '../../../utils/fixtures'; import { envelopeRequestParser } from '../../../utils/helpers'; diff --git a/packages/replay/src/replay.ts b/packages/replay/src/replay.ts index 35e71214b95c..f00ea81b15ca 100644 --- a/packages/replay/src/replay.ts +++ b/packages/replay/src/replay.ts @@ -112,8 +112,6 @@ export class ReplayContainer implements ReplayContainerInterface { initialUrl: '', }; - private _startRecordingbreadcrumb: Breadcrumb | undefined; - public constructor({ options, recordingOptions, @@ -212,7 +210,6 @@ export class ReplayContainer implements ReplayContainerInterface { }); } catch (err) { this._handleException(err); - return; } } @@ -910,15 +907,13 @@ export class ReplayContainer implements ReplayContainerInterface { /** * Creates a breadcrumb indicating the start of a replay recording + * and adds the current, full URL to the breadcrumb data. */ private _addStartRecordingBreadcrumb(): void { - // if (this._startRecordingbreadcrumb) { - // return; - // } - this._startRecordingbreadcrumb = createBreadcrumb({ + const breadcrumb = createBreadcrumb({ category: 'replay.recording.start', data: { url: getFullURL() }, }); - this._createCustomBreadcrumb(this._startRecordingbreadcrumb); + this._createCustomBreadcrumb(breadcrumb); } } diff --git a/packages/replay/src/util/addEvent.ts b/packages/replay/src/util/addEvent.ts index 2d7138aacf61..2b957823bb0f 100644 --- a/packages/replay/src/util/addEvent.ts +++ b/packages/replay/src/util/addEvent.ts @@ -19,8 +19,6 @@ export async function addEvent( return null; } - console.log(event); - // TODO: sadness -- we will want to normalize timestamps to be in ms - // requires coordination with frontend const isMs = event.timestamp > 9999999999;