From 3c00c684d2d682db70bfd228576cc1f2a95f7a42 Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Thu, 4 May 2023 17:01:33 +0200 Subject: [PATCH 01/10] fix(replay: Keep session active on key press (#8037) --- packages/replay/src/replay.ts | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/packages/replay/src/replay.ts b/packages/replay/src/replay.ts index 3cbb7f0002da..3d2fe4a6d7af 100644 --- a/packages/replay/src/replay.ts +++ b/packages/replay/src/replay.ts @@ -620,6 +620,7 @@ export class ReplayContainer implements ReplayContainerInterface { WINDOW.document.addEventListener('visibilitychange', this._handleVisibilityChange); WINDOW.addEventListener('blur', this._handleWindowBlur); WINDOW.addEventListener('focus', this._handleWindowFocus); + WINDOW.addEventListener('keydown', this._handleKeyboardEvent); // There is no way to remove these listeners, so ensure they are only added once if (!this._hasInitializedCoreListeners) { @@ -648,6 +649,7 @@ export class ReplayContainer implements ReplayContainerInterface { WINDOW.removeEventListener('blur', this._handleWindowBlur); WINDOW.removeEventListener('focus', this._handleWindowFocus); + WINDOW.removeEventListener('keydown', this._handleKeyboardEvent); if (this._performanceObserver) { this._performanceObserver.disconnect(); @@ -698,6 +700,11 @@ export class ReplayContainer implements ReplayContainerInterface { this._doChangeToForegroundTasks(breadcrumb); }; + /** Ensure page remains active when a key is pressed. */ + private _handleKeyboardEvent: (event: KeyboardEvent) => void = () => { + this.triggerUserActivity(); + }; + /** * Tasks to run when we consider a page to be hidden (via blurring and/or visibility) */ From 2cc7708c0a3ab526716fc4998f59d2104e6f3be3 Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Thu, 4 May 2023 18:49:36 +0200 Subject: [PATCH 02/10] doc(sveltekit): Update README with auto instrumentation configuration (#8038) --- packages/sveltekit/README.md | 131 ++++++++++++++++++++++++----------- 1 file changed, 91 insertions(+), 40 deletions(-) diff --git a/packages/sveltekit/README.md b/packages/sveltekit/README.md index 506e134168a3..3951ffcac04f 100644 --- a/packages/sveltekit/README.md +++ b/packages/sveltekit/README.md @@ -127,47 +127,25 @@ The Sentry SvelteKit SDK mostly relies on [SvelteKit Hooks](https://kit.svelte.d // export const handle = sequence(sentryHandle(), yourHandler()); ``` -### 4. Configuring `load` Functions +### 4. Vite Setup -5. To catch errors and performance data in your universal `load` functions (e.g. in `+page.(js|ts)`), wrap our `wrapLoadWithSentry` function around your load code: +Add `sentrySvelteKit` to your Vite plugins in `vite.config.(js|ts)` file so that the Sentry SDK can apply build-time features. +Make sure that it is added _before_ the `sveltekit` plugin: - ```javascript - // +page.(js|ts) - import { wrapLoadWithSentry } from '@sentry/sveltekit'; - - export const load = wrapLoadWithSentry((event) => { - //... your load code - }); - ``` - -6. To catch errors and performance data in your server `load` functions (e.g. in `+page.server.(js|ts)`), wrap our `wrapServerLoadWithSentry` function around your load code: - - ```javascript - // +page.server.(js|ts) - import { wrapServerLoadWithSentry } from '@sentry/sveltekit'; - - export const load = wrapServerLoadWithSentry((event) => { - //... your server load code - }); - ``` - -### 5. Vite Setup +```javascript +// vite.config.(js|ts) +import { sveltekit } from '@sveltejs/kit/vite'; +import { sentrySvelteKit } from '@sentry/sveltekit'; -1. Add our `sentrySvelteKit` plugins to your `vite.config.(js|ts)` file so that the Sentry SDK can apply build-time features. - Make sure that it is added before the `sveltekit` plugin: +export default { + plugins: [sentrySvelteKit(), sveltekit()], + // ... rest of your Vite config +}; +``` - ```javascript - // vite.config.(js|ts) - import { sveltekit } from '@sveltejs/kit/vite'; - import { sentrySvelteKit } from '@sentry/sveltekit'; - - export default { - plugins: [sentrySvelteKit(), sveltekit()], - // ... rest of your Vite config - }; - ``` +This adds the [Sentry Vite Plugin](https://github.com/getsentry/sentry-javascript-bundler-plugins/tree/main/packages/vite-plugin) to your Vite config to automatically upload source maps to Sentry. - This adds the [Sentry Vite Plugin](https://github.com/getsentry/sentry-javascript-bundler-plugins/tree/main/packages/vite-plugin) to your Vite config to automatically upload source maps to Sentry. +--- ## Uploading Source Maps @@ -252,14 +230,87 @@ export default { }; ``` +## Configure Auto-Instrumentation + +The SDK mostly relies on [SvelteKit's hooks](https://kit.svelte.dev/docs/hooks) to collect error and performance data. However, SvelteKit doesn't yet offer a hook for universal or server-only `load` function calls. Therefore, the SDK uses a Vite plugin to auto-instrument `load` functions so that you don't have to add a Sentry wrapper to each function manually. Auto-instrumentation is enabled by default, as soon as you add the `sentrySvelteKit()` function call to your `vite.config.(js|ts)`. However, you can customize the behavior, or disable it entirely. In this case, you can still manually wrap specific `load` functions with the `withSentry` function. + +Note: The SDK will only auto-instrument `load` functions in `+page` or `+layout` files that do not yet contain any Sentry code. +If you already have custom Sentry code in such files, you'll have to [manually](#instrument-load-functions-manually) add our wrapper to your `load` functions. + + +### Customize Auto-instrumentation + +By passing the `autoInstrument` option to `sentrySvelteKit` you can disable auto-instrumentation entirely, or customize which `load` functions should be instrumented: + +```javascript +// vite.config.(js|ts) +import { sveltekit } from '@sveltejs/kit/vite'; +import { sentrySvelteKit } from '@sentry/sveltekit'; + +export default { + plugins: [ + sentrySvelteKit({ + autoInstrument: { + load: true, // universal load functions + serverLoad: false, // server-only load functions + }, + }), + sveltekit(), + ], + // ... rest of your Vite config +}; +``` + +### Disable Auto-instrumentation + +If you set the `autoInstrument` option to `false`, the SDK won't auto-instrument any `load` function. You can still [manually instrument](#instrument-load-functions-manually) specific `load` functions. + +```javascript +// vite.config.(js|ts) +import { sveltekit } from '@sveltejs/kit/vite'; +import { sentrySvelteKit } from '@sentry/sveltekit'; + +export default { + plugins: [ + sentrySvelteKit({ + autoInstrument: false; + }), + sveltekit(), + ], + // ... rest of your Vite config +}; +``` + +### Instrument `load` Functions Manually + +If you don't want to use auto-instrumentation, you can also manually instrument specific `load` functions with our load function wrappers: + +To instrument your universal `load` functions in `+(page|layout).(js|ts)`, wrap our `wrapLoadWithSentry` function around your load code: + +```javascript +import { wrapLoadWithSentry } from '@sentry/sveltekit'; + +export const load = wrapLoadWithSentry((event) => { + //... your load code +}); +``` + +To instrument server `load` functions in `+(page|layout).server.(js|ts)`, wrap our `wrapServerLoadWithSentry` function around your load code: + +```javascript +import { wrapServerLoadWithSentry } from '@sentry/sveltekit'; + +export const load = wrapServerLoadWithSentry((event) => { + //... your server load code +}); +``` + + ## Known Limitations This SDK is still under active development. -Take a look at our [SvelteKit SDK Development Roadmap](https://github.com/getsentry/sentry-javascript/issues/6692) to follow the progress: +Take a look at our [SvelteKit SDK Development Roadmap](https://github.com/getsentry/sentry-javascript/issues/6692) to follow the progress. - **Adapters** other than `@sveltejs/adapter-node` are currently not supported. We haven't yet tested other platforms like Vercel. This is on our roadmap but it will come at a later time. - -- We're aiming to **simplify SDK setup** in the future so that you don't have to go in and manually add our wrappers to all your `load` functions. - This will be addressed once the SDK supports all Sentry features. From 194130e4d98caf248e8774a169076a153c8cb026 Mon Sep 17 00:00:00 2001 From: Sreetam Das Date: Fri, 5 May 2023 13:28:18 +0530 Subject: [PATCH 03/10] fix(sveltekit): Wrap `load` when typed explicitly (#8049) When `load` is explicitly typed (using `export const load: LayoutLoad = ...`), auto-instrumentation failed. This patch extends the regex to handle this case and adds a test for it. --- packages/sveltekit/src/vite/autoInstrument.ts | 4 +++- packages/sveltekit/test/vite/autoInstrument.test.ts | 6 ++++++ 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/packages/sveltekit/src/vite/autoInstrument.ts b/packages/sveltekit/src/vite/autoInstrument.ts index c30299c6b8a3..136d1fef0148 100644 --- a/packages/sveltekit/src/vite/autoInstrument.ts +++ b/packages/sveltekit/src/vite/autoInstrument.ts @@ -98,7 +98,9 @@ export async function canWrapLoad(id: string, debug: boolean): Promise debug && console.log(`Skipping wrapping ${id} because it already contains Sentry code`); } - const hasLoadDeclaration = /((const|let|var|function)\s+load\s*(=|\())|as\s+load\s*(,|})/gm.test(codeWithoutComments); + const hasLoadDeclaration = /((const|let|var|function)\s+load\s*(=|\(|:))|as\s+load\s*(,|})/gm.test( + codeWithoutComments, + ); if (!hasLoadDeclaration) { // eslint-disable-next-line no-console debug && console.log(`Skipping wrapping ${id} because it doesn't declare a \`load\` function`); diff --git a/packages/sveltekit/test/vite/autoInstrument.test.ts b/packages/sveltekit/test/vite/autoInstrument.test.ts index bc1f8d13adb8..0b5599912e7a 100644 --- a/packages/sveltekit/test/vite/autoInstrument.test.ts +++ b/packages/sveltekit/test/vite/autoInstrument.test.ts @@ -176,6 +176,12 @@ describe('canWrapLoad', () => { async function somethingElse(){}; export { somethingElse as load, foo }`, ], + + [ + 'export variable declaration - inline function with assigned type', + `import type { LayoutLoad } from './$types'; + export const load : LayoutLoad = async () => { return { props: { msg: "hi" } } }`, + ], ])('returns `true` if a load declaration (%s) exists and no Sentry code was found', async (_, code) => { fileContent = code; expect(await canWrapLoad('+page.ts', false)).toEqual(true); From 49837acaaaae7d1172d12dd115744dcf563727fc Mon Sep 17 00:00:00 2001 From: Billy Vong Date: Fri, 5 May 2023 19:21:02 +0200 Subject: [PATCH 04/10] test(replay): Update jest custom matchers for replay to use differ (#8047) Use `printDiffOrStringify()` util function from jest to generate a pretty diff of the received vs expected. Also when searching for *any* calls for `toHaveSentReplay`, stop at the first call where *any* of the keys have a successful match, instead of always falling through to the last call. --- packages/replay/jest.setup.ts | 38 +++++++++++++++++++++++++---------- 1 file changed, 27 insertions(+), 11 deletions(-) diff --git a/packages/replay/jest.setup.ts b/packages/replay/jest.setup.ts index 778e057f93ea..b485f3c882d1 100644 --- a/packages/replay/jest.setup.ts +++ b/packages/replay/jest.setup.ts @@ -50,9 +50,12 @@ const toHaveSameSession = function (received: jest.Mocked, expe return { pass, message: () => - `${this.utils.matcherHint('toHaveSameSession', undefined, undefined, options)}\n\n` + - `Expected: ${pass ? 'not ' : ''}${this.utils.printExpected(expected)}\n` + - `Received: ${this.utils.printReceived(received.session)}`, + `${this.utils.matcherHint( + 'toHaveSameSession', + undefined, + undefined, + options, + )}\n\n${this.utils.printDiffOrStringify(expected, received.session, 'Expected', 'Received')}`, }; }; @@ -138,11 +141,18 @@ const toHaveSentReplay = function ( let result: CheckCallForSentReplayResult; + const expectedKeysLength = expected ? ('sample' in expected ? Object.keys(expected.sample) : Object.keys(expected)).length : 0; + for (const currentCall of calls) { result = checkCallForSentReplay.call(this, currentCall[0], expected); if (result.pass) { break; } + + // stop on the first call where any of the expected obj passes + if (result.results.length < expectedKeysLength) { + break; + } } // @ts-ignore use before assigned @@ -161,10 +171,13 @@ const toHaveSentReplay = function ( ? 'Expected Replay to not have been sent, but a request was attempted' : 'Expected Replay to have been sent, but a request was not attempted' : `${this.utils.matcherHint('toHaveSentReplay', undefined, undefined, options)}\n\n${results - .map( - ({ key, expectedVal, actualVal }: Result) => - `Expected (key: ${key}): ${pass ? 'not ' : ''}${this.utils.printExpected(expectedVal)}\n` + - `Received (key: ${key}): ${this.utils.printReceived(actualVal)}`, + .map(({ key, expectedVal, actualVal }: Result) => + this.utils.printDiffOrStringify( + expectedVal, + actualVal, + `Expected (key: ${key})`, + `Received (key: ${key})`, + ), ) .join('\n')}`, }; @@ -197,10 +210,13 @@ const toHaveLastSentReplay = function ( ? 'Expected Replay to not have been sent, but a request was attempted' : 'Expected Replay to have last been sent, but a request was not attempted' : `${this.utils.matcherHint('toHaveSentReplay', undefined, undefined, options)}\n\n${results - .map( - ({ key, expectedVal, actualVal }: Result) => - `Expected (key: ${key}): ${pass ? 'not ' : ''}${this.utils.printExpected(expectedVal)}\n` + - `Received (key: ${key}): ${this.utils.printReceived(actualVal)}`, + .map(({ key, expectedVal, actualVal }: Result) => + this.utils.printDiffOrStringify( + expectedVal, + actualVal, + `Expected (key: ${key})`, + `Received (key: ${key})`, + ), ) .join('\n')}`, }; From d400d697fe24fcd7e61f1a9b0974353176ca8683 Mon Sep 17 00:00:00 2001 From: Billy Vong Date: Fri, 5 May 2023 19:12:34 -0230 Subject: [PATCH 05/10] feat(replay): Add event to capture options on checkouts (#8011) Add a custom event that captures configuration options on checkout + segment 0. --- .../suites/replay/bufferMode/test.ts | 5 -- .../suites/replay/captureReplay/test.ts | 2 - .../captureReplayFromReplayPackage/test.ts | 2 - .../suites/replay/customEvents/test.ts | 57 +++++++++++++++++++ .../suites/replay/errors/errorMode/test.ts | 3 - .../utils/replayEventTemplates.ts | 1 - .../utils/replayHelpers.ts | 9 ++- .../tests/fixtures/ReplayRecordingData.ts | 20 +++++++ .../src/eventBuffer/EventBufferArray.ts | 7 ++- .../EventBufferCompressionWorker.ts | 7 ++- .../src/eventBuffer/EventBufferProxy.ts | 7 ++- packages/replay/src/integration.ts | 2 + packages/replay/src/types.ts | 17 ++++++ packages/replay/src/types/rrweb.ts | 2 +- .../replay/src/util/handleRecordingEmit.ts | 51 ++++++++++++++++- packages/replay/src/util/sendReplayRequest.ts | 14 ----- .../test/integration/errorSampleRate.test.ts | 52 ++++++++--------- .../replay/test/integration/events.test.ts | 6 -- .../replay/test/integration/session.test.ts | 8 ++- packages/replay/test/integration/stop.test.ts | 5 ++ .../unit/util/handleRecordingEmit.test.ts | 25 +++++--- .../replay/test/utils/setupReplayContainer.ts | 44 ++++++++++---- 22 files changed, 258 insertions(+), 88 deletions(-) diff --git a/packages/browser-integration-tests/suites/replay/bufferMode/test.ts b/packages/browser-integration-tests/suites/replay/bufferMode/test.ts index c5a4e9ae2526..2a835c444550 100644 --- a/packages/browser-integration-tests/suites/replay/bufferMode/test.ts +++ b/packages/browser-integration-tests/suites/replay/bufferMode/test.ts @@ -115,7 +115,6 @@ sentryTest( expect(event0).toEqual( getExpectedReplayEvent({ - contexts: { replay: { error_sample_rate: 0, session_sample_rate: 0 } }, error_ids: [errorEventId!], replay_type: 'buffer', }), @@ -150,7 +149,6 @@ sentryTest( expect(event1).toEqual( getExpectedReplayEvent({ - contexts: { replay: { error_sample_rate: 0, session_sample_rate: 0 } }, replay_type: 'buffer', // although we're in session mode, we still send 'buffer' as replay_type segment_id: 1, urls: [], @@ -162,7 +160,6 @@ sentryTest( expect(event2).toEqual( getExpectedReplayEvent({ - contexts: { replay: { error_sample_rate: 0, session_sample_rate: 0 } }, replay_type: 'buffer', // although we're in session mode, we still send 'buffer' as replay_type segment_id: 2, urls: [], @@ -266,7 +263,6 @@ sentryTest( expect(event0).toEqual( getExpectedReplayEvent({ - contexts: { replay: { error_sample_rate: 0, session_sample_rate: 0 } }, error_ids: [errorEventId!], replay_type: 'buffer', }), @@ -372,7 +368,6 @@ sentryTest('[buffer-mode] can sample on each error event', async ({ getLocalTest expect(event0).toEqual( getExpectedReplayEvent({ - contexts: { replay: { error_sample_rate: 1, session_sample_rate: 0 } }, error_ids: errorEventIds, replay_type: 'buffer', }), diff --git a/packages/browser-integration-tests/suites/replay/captureReplay/test.ts b/packages/browser-integration-tests/suites/replay/captureReplay/test.ts index 473d88ea53db..72cbc47efe99 100644 --- a/packages/browser-integration-tests/suites/replay/captureReplay/test.ts +++ b/packages/browser-integration-tests/suites/replay/captureReplay/test.ts @@ -64,7 +64,6 @@ sentryTest('should capture replays (@sentry/browser export)', async ({ getLocalT }, }, platform: 'javascript', - contexts: { replay: { session_sample_rate: 1, error_sample_rate: 0 } }, }); expect(replayEvent1).toBeDefined(); @@ -103,6 +102,5 @@ sentryTest('should capture replays (@sentry/browser export)', async ({ getLocalT }, }, platform: 'javascript', - contexts: { replay: { session_sample_rate: 1, error_sample_rate: 0 } }, }); }); diff --git a/packages/browser-integration-tests/suites/replay/captureReplayFromReplayPackage/test.ts b/packages/browser-integration-tests/suites/replay/captureReplayFromReplayPackage/test.ts index 03ee0f78e540..6caf1e4ea57c 100644 --- a/packages/browser-integration-tests/suites/replay/captureReplayFromReplayPackage/test.ts +++ b/packages/browser-integration-tests/suites/replay/captureReplayFromReplayPackage/test.ts @@ -64,7 +64,6 @@ sentryTest('should capture replays (@sentry/replay export)', async ({ getLocalTe }, }, platform: 'javascript', - contexts: { replay: { session_sample_rate: 1, error_sample_rate: 0 } }, }); expect(replayEvent1).toBeDefined(); @@ -103,6 +102,5 @@ sentryTest('should capture replays (@sentry/replay export)', async ({ getLocalTe }, }, platform: 'javascript', - contexts: { replay: { session_sample_rate: 1, error_sample_rate: 0 } }, }); }); diff --git a/packages/browser-integration-tests/suites/replay/customEvents/test.ts b/packages/browser-integration-tests/suites/replay/customEvents/test.ts index ce8f27bf4995..585266746365 100644 --- a/packages/browser-integration-tests/suites/replay/customEvents/test.ts +++ b/packages/browser-integration-tests/suites/replay/customEvents/test.ts @@ -174,3 +174,60 @@ sentryTest( ); }, ); + +sentryTest( + 'replay recording should contain an "options" breadcrumb for Replay SDK configuration', + async ({ forceFlushReplay, getLocalTestPath, page, browserName }) => { + // TODO(replay): This is flakey on firefox and webkit where clicks are flakey + if (shouldSkipReplayTest() || ['firefox', 'webkit'].includes(browserName)) { + 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 forceFlushReplay(); + + await page.click('#error'); + await forceFlushReplay(); + + const req0 = await reqPromise0; + const content0 = getReplayRecordingContent(req0); + + expect(content0.optionsEvents).toEqual([ + { + tag: 'options', + payload: { + sessionSampleRate: 1, + errorSampleRate: 0, + useCompressionOption: false, + blockAllMedia: false, + maskAllText: true, + maskAllInputs: true, + useCompression: false, + networkDetailHasUrls: false, + networkCaptureBodies: true, + networkRequestHasHeaders: true, + networkResponseHasHeaders: true, + }, + }, + ]); + + const req1 = await reqPromise1; + const content1 = getReplayRecordingContent(req1); + + // Should only be on first segment + expect(content1.optionsEvents).toEqual([]); + }, +); diff --git a/packages/browser-integration-tests/suites/replay/errors/errorMode/test.ts b/packages/browser-integration-tests/suites/replay/errors/errorMode/test.ts index fee9e05d4a49..aa452cdd9307 100644 --- a/packages/browser-integration-tests/suites/replay/errors/errorMode/test.ts +++ b/packages/browser-integration-tests/suites/replay/errors/errorMode/test.ts @@ -84,7 +84,6 @@ sentryTest( expect(event0).toEqual( getExpectedReplayEvent({ - contexts: { replay: { error_sample_rate: 1, session_sample_rate: 0 } }, error_ids: [errorEventId!], replay_type: 'buffer', }), @@ -119,7 +118,6 @@ sentryTest( expect(event1).toEqual( getExpectedReplayEvent({ - contexts: { replay: { error_sample_rate: 1, session_sample_rate: 0 } }, replay_type: 'buffer', // although we're in session mode, we still send 'error' as replay_type segment_id: 1, urls: [], @@ -134,7 +132,6 @@ sentryTest( // we continue recording everything expect(event2).toEqual( getExpectedReplayEvent({ - contexts: { replay: { error_sample_rate: 1, session_sample_rate: 0 } }, replay_type: 'buffer', segment_id: 2, urls: [], diff --git a/packages/browser-integration-tests/utils/replayEventTemplates.ts b/packages/browser-integration-tests/utils/replayEventTemplates.ts index e6ee4bda18d2..d88fbd1bf0e5 100644 --- a/packages/browser-integration-tests/utils/replayEventTemplates.ts +++ b/packages/browser-integration-tests/utils/replayEventTemplates.ts @@ -38,7 +38,6 @@ const DEFAULT_REPLAY_EVENT = { }, }, platform: 'javascript', - contexts: { replay: { session_sample_rate: 1, error_sample_rate: 0 } }, }; /** diff --git a/packages/browser-integration-tests/utils/replayHelpers.ts b/packages/browser-integration-tests/utils/replayHelpers.ts index b1448dc97d85..415dcd667414 100644 --- a/packages/browser-integration-tests/utils/replayHelpers.ts +++ b/packages/browser-integration-tests/utils/replayHelpers.ts @@ -160,6 +160,7 @@ type CustomRecordingContent = { type RecordingContent = { fullSnapshots: RecordingSnapshot[]; incrementalSnapshots: RecordingSnapshot[]; + optionsEvents: CustomRecordingEvent[]; } & CustomRecordingContent; /** @@ -207,6 +208,11 @@ export function getIncrementalRecordingSnapshots(resOrReq: Request | Response): return events.filter(isIncrementalSnapshot); } +function getOptionsEvents(replayRequest: Request): CustomRecordingEvent[] { + const events = getDecompressedRecordingEvents(replayRequest); + return getAllCustomRrwebRecordingEvents(events).filter(data => data.tag === 'options'); +} + function getDecompressedRecordingEvents(resOrReq: Request | Response): RecordingSnapshot[] { const replayRequest = getRequest(resOrReq); return ( @@ -227,8 +233,9 @@ export function getReplayRecordingContent(resOrReq: Request | Response): Recordi const fullSnapshots = getFullRecordingSnapshots(replayRequest); const incrementalSnapshots = getIncrementalRecordingSnapshots(replayRequest); const customEvents = getCustomRecordingEvents(replayRequest); + const optionsEvents = getOptionsEvents(replayRequest); - return { fullSnapshots, incrementalSnapshots, ...customEvents }; + return { fullSnapshots, incrementalSnapshots, optionsEvents, ...customEvents }; } /** diff --git a/packages/e2e-tests/test-applications/standard-frontend-react/tests/fixtures/ReplayRecordingData.ts b/packages/e2e-tests/test-applications/standard-frontend-react/tests/fixtures/ReplayRecordingData.ts index da5ba529edd7..a22694a64304 100644 --- a/packages/e2e-tests/test-applications/standard-frontend-react/tests/fixtures/ReplayRecordingData.ts +++ b/packages/e2e-tests/test-applications/standard-frontend-react/tests/fixtures/ReplayRecordingData.ts @@ -7,6 +7,26 @@ export const ReplayRecordingData = [ data: { href: expect.stringMatching(/http:\/\/localhost:\d+\//), width: 1280, height: 720 }, timestamp: expect.any(Number), }, + { + data: { + payload: { + blockAllMedia: true, + errorSampleRate: 0, + maskAllInputs: true, + maskAllText: true, + networkCaptureBodies: true, + networkDetailHasUrls: false, + networkRequestHasHeaders: true, + networkResponseHasHeaders: true, + sessionSampleRate: 1, + useCompression: false, + useCompressionOption: true, + }, + tag: 'options', + }, + timestamp: expect.any(Number), + type: 5, + }, { type: 2, data: { diff --git a/packages/replay/src/eventBuffer/EventBufferArray.ts b/packages/replay/src/eventBuffer/EventBufferArray.ts index 42ce00aabbc6..eaebd1b174e7 100644 --- a/packages/replay/src/eventBuffer/EventBufferArray.ts +++ b/packages/replay/src/eventBuffer/EventBufferArray.ts @@ -1,4 +1,4 @@ -import type { AddEventResult, EventBuffer, RecordingEvent } from '../types'; +import type { AddEventResult, EventBuffer, EventBufferType, RecordingEvent } from '../types'; import { timestampToMs } from '../util/timestampToMs'; /** @@ -18,6 +18,11 @@ export class EventBufferArray implements EventBuffer { return this.events.length > 0; } + /** @inheritdoc */ + public get type(): EventBufferType { + return 'sync'; + } + /** @inheritdoc */ public destroy(): void { this.events = []; diff --git a/packages/replay/src/eventBuffer/EventBufferCompressionWorker.ts b/packages/replay/src/eventBuffer/EventBufferCompressionWorker.ts index 57ad449dac55..45696ea46bc9 100644 --- a/packages/replay/src/eventBuffer/EventBufferCompressionWorker.ts +++ b/packages/replay/src/eventBuffer/EventBufferCompressionWorker.ts @@ -1,6 +1,6 @@ import type { ReplayRecordingData } from '@sentry/types'; -import type { AddEventResult, EventBuffer, RecordingEvent } from '../types'; +import type { AddEventResult, EventBuffer, EventBufferType, RecordingEvent } from '../types'; import { timestampToMs } from '../util/timestampToMs'; import { WorkerHandler } from './WorkerHandler'; @@ -22,6 +22,11 @@ export class EventBufferCompressionWorker implements EventBuffer { return !!this._earliestTimestamp; } + /** @inheritdoc */ + public get type(): EventBufferType { + return 'worker'; + } + /** * Ensure the worker is ready (or not). * This will either resolve when the worker is ready, or reject if an error occured. diff --git a/packages/replay/src/eventBuffer/EventBufferProxy.ts b/packages/replay/src/eventBuffer/EventBufferProxy.ts index 972fc9d58911..df50634bf3eb 100644 --- a/packages/replay/src/eventBuffer/EventBufferProxy.ts +++ b/packages/replay/src/eventBuffer/EventBufferProxy.ts @@ -1,7 +1,7 @@ import type { ReplayRecordingData } from '@sentry/types'; import { logger } from '@sentry/utils'; -import type { AddEventResult, EventBuffer, RecordingEvent } from '../types'; +import type { AddEventResult, EventBuffer, EventBufferType, RecordingEvent } from '../types'; import { EventBufferArray } from './EventBufferArray'; import { EventBufferCompressionWorker } from './EventBufferCompressionWorker'; @@ -24,6 +24,11 @@ export class EventBufferProxy implements EventBuffer { this._ensureWorkerIsLoadedPromise = this._ensureWorkerIsLoaded(); } + /** @inheritdoc */ + public get type(): EventBufferType { + return this._used.type; + } + /** @inheritDoc */ public get hasEvents(): boolean { return this._used.hasEvents; diff --git a/packages/replay/src/integration.ts b/packages/replay/src/integration.ts index 81279947b969..d5d4115fffc7 100644 --- a/packages/replay/src/integration.ts +++ b/packages/replay/src/integration.ts @@ -123,6 +123,8 @@ export class Replay implements Integration { errorSampleRate, useCompression, blockAllMedia, + maskAllInputs, + maskAllText, networkDetailAllowUrls, networkCaptureBodies, networkRequestHeaders: _getMergedNetworkHeaders(networkRequestHeaders), diff --git a/packages/replay/src/types.ts b/packages/replay/src/types.ts index 0d653f3a7817..aef61f57a07a 100644 --- a/packages/replay/src/types.ts +++ b/packages/replay/src/types.ts @@ -257,6 +257,16 @@ export interface ReplayPluginOptions extends ReplayNetworkOptions { */ blockAllMedia: boolean; + /** + * Mask all inputs in recordings + */ + maskAllInputs: boolean; + + /** + * Mask all text in recordings + */ + maskAllText: boolean; + /** * _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. @@ -435,12 +445,19 @@ export interface Session { shouldRefresh: boolean; } +export type EventBufferType = 'sync' | 'worker'; + export interface EventBuffer { /** * If any events have been added to the buffer. */ readonly hasEvents: boolean; + /** + * The buffer type + */ + readonly type: EventBufferType; + /** * Destroy the event buffer. */ diff --git a/packages/replay/src/types/rrweb.ts b/packages/replay/src/types/rrweb.ts index 7a794face4cb..7f2dfec78110 100644 --- a/packages/replay/src/types/rrweb.ts +++ b/packages/replay/src/types/rrweb.ts @@ -3,7 +3,7 @@ type blockClass = string | RegExp; type maskTextClass = string | RegExp; -enum EventType { +export enum EventType { DomContentLoaded = 0, Load = 1, FullSnapshot = 2, diff --git a/packages/replay/src/util/handleRecordingEmit.ts b/packages/replay/src/util/handleRecordingEmit.ts index edf9aa4946f1..f72850f5536c 100644 --- a/packages/replay/src/util/handleRecordingEmit.ts +++ b/packages/replay/src/util/handleRecordingEmit.ts @@ -1,7 +1,8 @@ import { logger } from '@sentry/utils'; import { saveSession } from '../session/saveSession'; -import type { RecordingEvent, ReplayContainer } from '../types'; +import type { AddEventResult, RecordingEvent, ReplayContainer } from '../types'; +import { EventType } from '../types/rrweb'; import { addEvent } from './addEvent'; type RecordingEmitCallback = (event: RecordingEvent, isCheckout?: boolean) => void; @@ -48,6 +49,14 @@ export function getHandleRecordingEmit(replay: ReplayContainer): RecordingEmitCa return false; } + // Additionally, create a meta event that will capture certain SDK settings. + // In order to handle buffer mode, this needs to either be done when we + // receive checkout events or at flush time. + // + // `isCheckout` is always true, but want to be explicit that it should + // only be added for checkouts + void addSettingsEvent(replay, isCheckout); + // 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 @@ -84,3 +93,43 @@ export function getHandleRecordingEmit(replay: ReplayContainer): RecordingEmitCa }); }; } + +/** + * Exported for tests + */ +export function createOptionsEvent(replay: ReplayContainer): RecordingEvent { + const options = replay.getOptions(); + return { + type: EventType.Custom, + timestamp: Date.now(), + data: { + tag: 'options', + payload: { + sessionSampleRate: options.sessionSampleRate, + errorSampleRate: options.errorSampleRate, + useCompressionOption: options.useCompression, + blockAllMedia: options.blockAllMedia, + maskAllText: options.maskAllText, + maskAllInputs: options.maskAllInputs, + useCompression: replay.eventBuffer ? replay.eventBuffer.type === 'worker' : false, + networkDetailHasUrls: options.networkDetailAllowUrls.length > 0, + networkCaptureBodies: options.networkCaptureBodies, + networkRequestHasHeaders: options.networkRequestHeaders.length > 0, + networkResponseHasHeaders: options.networkResponseHeaders.length > 0, + }, + }, + }; +} + +/** + * Add a "meta" event that contains a simplified view on current configuration + * options. This should only be included on the first segment of a recording. + */ +function addSettingsEvent(replay: ReplayContainer, isCheckout?: boolean): Promise { + // Only need to add this event when sending the first segment + if (!isCheckout || !replay.session || replay.session.segmentId !== 0) { + return Promise.resolve(null); + } + + return addEvent(replay, createOptionsEvent(replay), false); +} diff --git a/packages/replay/src/util/sendReplayRequest.ts b/packages/replay/src/util/sendReplayRequest.ts index 009fdc2067bf..65f217f857cd 100644 --- a/packages/replay/src/util/sendReplayRequest.ts +++ b/packages/replay/src/util/sendReplayRequest.ts @@ -18,7 +18,6 @@ export async function sendReplayRequest({ eventContext, timestamp, session, - options, }: SendReplayData): Promise { const preparedRecordingData = prepareRecordingData({ recordingData, @@ -60,15 +59,6 @@ export async function sendReplayRequest({ return; } - replayEvent.contexts = { - ...replayEvent.contexts, - replay: { - ...(replayEvent.contexts && replayEvent.contexts.replay), - session_sample_rate: options.sessionSampleRate, - error_sample_rate: options.errorSampleRate, - }, - }; - /* For reference, the fully built event looks something like this: { @@ -99,10 +89,6 @@ export async function sendReplayRequest({ }, "sdkProcessingMetadata": {}, "contexts": { - "replay": { - "session_sample_rate": 1, - "error_sample_rate": 0, - }, }, } */ diff --git a/packages/replay/test/integration/errorSampleRate.test.ts b/packages/replay/test/integration/errorSampleRate.test.ts index 16962bf5b2f8..74fde11f50f0 100644 --- a/packages/replay/test/integration/errorSampleRate.test.ts +++ b/packages/replay/test/integration/errorSampleRate.test.ts @@ -11,6 +11,7 @@ import { import type { ReplayContainer } from '../../src/replay'; import { clearSession } from '../../src/session/clearSession'; import { addEvent } from '../../src/util/addEvent'; +import { createOptionsEvent } from '../../src/util/handleRecordingEmit'; import { PerformanceEntryResource } from '../fixtures/performanceEntry/resource'; import type { RecordMock } from '../index'; import { BASE_TIMESTAMP } from '../index'; @@ -50,6 +51,7 @@ describe('Integration | errorSampleRate', () => { it('uploads a replay when `Sentry.captureException` is called and continues recording', async () => { const TEST_EVENT = { data: {}, timestamp: BASE_TIMESTAMP, type: 3 }; mockRecord._emitter(TEST_EVENT); + const optionsEvent = createOptionsEvent(replay); expect(mockRecord.takeFullSnapshot).not.toHaveBeenCalled(); expect(replay).not.toHaveLastSentReplay(); @@ -72,15 +74,10 @@ describe('Integration | errorSampleRate', () => { recordingPayloadHeader: { segment_id: 0 }, replayEventPayload: expect.objectContaining({ replay_type: 'buffer', - contexts: { - replay: { - error_sample_rate: 1, - session_sample_rate: 0, - }, - }, }), recordingData: JSON.stringify([ { data: { isCheckout: true }, timestamp: BASE_TIMESTAMP, type: 2 }, + optionsEvent, TEST_EVENT, { type: 5, @@ -104,12 +101,6 @@ describe('Integration | errorSampleRate', () => { recordingPayloadHeader: { segment_id: 1 }, replayEventPayload: expect.objectContaining({ replay_type: 'buffer', - contexts: { - replay: { - error_sample_rate: 1, - session_sample_rate: 0, - }, - }, }), recordingData: JSON.stringify([ { data: { isCheckout: true }, timestamp: BASE_TIMESTAMP + DEFAULT_FLUSH_MIN_DELAY + 40, type: 2 }, @@ -161,6 +152,7 @@ describe('Integration | errorSampleRate', () => { it('manually flushes replay and does not continue to record', async () => { const TEST_EVENT = { data: {}, timestamp: BASE_TIMESTAMP, type: 3 }; mockRecord._emitter(TEST_EVENT); + const optionsEvent = createOptionsEvent(replay); expect(mockRecord.takeFullSnapshot).not.toHaveBeenCalled(); expect(replay).not.toHaveLastSentReplay(); @@ -183,15 +175,10 @@ describe('Integration | errorSampleRate', () => { recordingPayloadHeader: { segment_id: 0 }, replayEventPayload: expect.objectContaining({ replay_type: 'buffer', - contexts: { - replay: { - error_sample_rate: 1, - session_sample_rate: 0, - }, - }, }), recordingData: JSON.stringify([ { data: { isCheckout: true }, timestamp: BASE_TIMESTAMP, type: 2 }, + optionsEvent, TEST_EVENT, { type: 5, @@ -224,15 +211,10 @@ describe('Integration | errorSampleRate', () => { recordingPayloadHeader: { segment_id: 0 }, replayEventPayload: expect.objectContaining({ replay_type: 'buffer', - contexts: { - replay: { - error_sample_rate: 1, - session_sample_rate: 0, - }, - }, }), recordingData: JSON.stringify([ { data: { isCheckout: true }, timestamp: BASE_TIMESTAMP, type: 2 }, + optionsEvent, TEST_EVENT, { type: 5, @@ -538,6 +520,7 @@ describe('Integration | errorSampleRate', () => { it('has the correct timestamps with deferred root event and last replay update', async () => { const TEST_EVENT = { data: {}, timestamp: BASE_TIMESTAMP, type: 3 }; mockRecord._emitter(TEST_EVENT); + const optionsEvent = createOptionsEvent(replay); expect(mockRecord.takeFullSnapshot).not.toHaveBeenCalled(); expect(replay).not.toHaveLastSentReplay(); @@ -554,7 +537,11 @@ describe('Integration | errorSampleRate', () => { await new Promise(process.nextTick); expect(replay).toHaveSentReplay({ - recordingData: JSON.stringify([{ data: { isCheckout: true }, timestamp: BASE_TIMESTAMP, type: 2 }, TEST_EVENT]), + recordingData: JSON.stringify([ + { data: { isCheckout: true }, timestamp: BASE_TIMESTAMP, type: 2 }, + optionsEvent, + TEST_EVENT, + ]), replayEventPayload: expect.objectContaining({ replay_start_timestamp: BASE_TIMESTAMP / 1000, // the exception happens roughly 10 seconds after BASE_TIMESTAMP @@ -590,6 +577,7 @@ describe('Integration | errorSampleRate', () => { // in production, this happens at a time interval // session started time should be updated to this current timestamp mockRecord.takeFullSnapshot(true); + const optionsEvent = createOptionsEvent(replay); jest.runAllTimers(); jest.advanceTimersByTime(20); @@ -617,6 +605,7 @@ describe('Integration | errorSampleRate', () => { timestamp: BASE_TIMESTAMP + ELAPSED + 20, type: 2, }, + optionsEvent, ]), }); }); @@ -732,8 +721,9 @@ it('sends a replay after loading the session multiple times', async () => { }, autoStart: false, }); - // @ts-ignore this is protected, but we want to call it for this test - integration._initialize(); + integration['_initialize'](); + + const optionsEvent = createOptionsEvent(replay); jest.runAllTimers(); @@ -750,12 +740,18 @@ it('sends a replay after loading the session multiple times', async () => { await new Promise(process.nextTick); expect(replay).toHaveSentReplay({ - recordingData: JSON.stringify([{ data: { isCheckout: true }, timestamp: BASE_TIMESTAMP, type: 2 }, TEST_EVENT]), + recordingPayloadHeader: { segment_id: 0 }, + recordingData: JSON.stringify([ + { data: { isCheckout: true }, timestamp: BASE_TIMESTAMP, type: 2 }, + optionsEvent, + TEST_EVENT, + ]), }); // 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({ + recordingPayloadHeader: { segment_id: 1 }, recordingData: JSON.stringify([{ data: { isCheckout: true }, timestamp: BASE_TIMESTAMP + 5040, type: 2 }]), }); }); diff --git a/packages/replay/test/integration/events.test.ts b/packages/replay/test/integration/events.test.ts index e0b229e1c82d..b95faffa59da 100644 --- a/packages/replay/test/integration/events.test.ts +++ b/packages/replay/test/integration/events.test.ts @@ -130,12 +130,6 @@ describe('Integration | events', () => { expect(replay).toHaveLastSentReplay({ replayEventPayload: expect.objectContaining({ replay_start_timestamp: (BASE_TIMESTAMP - 10000) / 1000, - contexts: { - replay: { - error_sample_rate: 0, - session_sample_rate: 1, - }, - }, urls: ['http://localhost/'], // this doesn't truly test if we are capturing the right URL as we don't change URLs, but good enough }), }); diff --git a/packages/replay/test/integration/session.test.ts b/packages/replay/test/integration/session.test.ts index 6b0f942af616..304059659078 100644 --- a/packages/replay/test/integration/session.test.ts +++ b/packages/replay/test/integration/session.test.ts @@ -14,6 +14,7 @@ import { clearSession } from '../../src/session/clearSession'; import type { Session } from '../../src/types'; import { addEvent } from '../../src/util/addEvent'; import { createPerformanceSpans } from '../../src/util/createPerformanceSpans'; +import { createOptionsEvent } from '../../src/util/handleRecordingEmit'; import { BASE_TIMESTAMP } from '../index'; import type { RecordMock } from '../mocks/mockRrweb'; import { resetSdkMock } from '../mocks/resetSdkMock'; @@ -196,6 +197,8 @@ describe('Integration | session', () => { // Replay does not send immediately because checkout was due to expired session expect(replay).not.toHaveLastSentReplay(); + const optionsEvent = createOptionsEvent(replay); + await advanceTimers(DEFAULT_FLUSH_MIN_DELAY); const newTimestamp = BASE_TIMESTAMP + ELAPSED + 20; @@ -204,6 +207,7 @@ describe('Integration | session', () => { recordingPayloadHeader: { segment_id: 0 }, recordingData: JSON.stringify([ { data: { isCheckout: true }, timestamp: newTimestamp, type: 2 }, + optionsEvent, { type: 5, timestamp: newTimestamp, @@ -381,6 +385,7 @@ describe('Integration | session', () => { type: 3, }; mockRecord._emitter(NEW_TEST_EVENT); + const optionsEvent = createOptionsEvent(replay); jest.runAllTimers(); await advanceTimers(DEFAULT_FLUSH_MIN_DELAY); @@ -388,7 +393,8 @@ describe('Integration | session', () => { expect(replay).toHaveLastSentReplay({ recordingPayloadHeader: { segment_id: 0 }, recordingData: JSON.stringify([ - { data: { isCheckout: true }, timestamp: newTimestamp, type: 2 }, + { data: { isCheckout: true }, timestamp: BASE_TIMESTAMP + ELAPSED, type: 2 }, + optionsEvent, { type: 5, timestamp: newTimestamp, diff --git a/packages/replay/test/integration/stop.test.ts b/packages/replay/test/integration/stop.test.ts index a477ee8e044f..cc0e28195244 100644 --- a/packages/replay/test/integration/stop.test.ts +++ b/packages/replay/test/integration/stop.test.ts @@ -5,6 +5,7 @@ import { WINDOW } from '../../src/constants'; import type { ReplayContainer } from '../../src/replay'; import { clearSession } from '../../src/session/clearSession'; import { addEvent } from '../../src/util/addEvent'; +import { createOptionsEvent } from '../../src/util/handleRecordingEmit'; // mock functions need to be imported first import { BASE_TIMESTAMP, mockRrweb, mockSdk } from '../index'; import { useFakeTimers } from '../utils/use-fake-timers'; @@ -95,6 +96,7 @@ describe('Integration | stop', () => { // re-enable replay integration.start(); + const optionsEvent = createOptionsEvent(replay); // will be different session expect(replay.session?.id).not.toEqual(previousSessionId); @@ -121,6 +123,7 @@ describe('Integration | stop', () => { jest.runAllTimers(); await new Promise(process.nextTick); expect(replay).toHaveLastSentReplay({ + recordingPayloadHeader: { segment_id: 0 }, recordingData: JSON.stringify([ // This event happens when we call `replay.start` { @@ -128,10 +131,12 @@ describe('Integration | stop', () => { timestamp: BASE_TIMESTAMP + ELAPSED + EXTRA_TICKS, type: 2, }, + optionsEvent, TEST_EVENT, hiddenBreadcrumb, ]), }); + // Session's last activity is last updated when we call `setup()` and *NOT* // when tab is blurred expect(replay.session?.lastActivity).toBe(BASE_TIMESTAMP + ELAPSED + 20); diff --git a/packages/replay/test/unit/util/handleRecordingEmit.test.ts b/packages/replay/test/unit/util/handleRecordingEmit.test.ts index 4762b875ce5b..a4c7f82c425d 100644 --- a/packages/replay/test/unit/util/handleRecordingEmit.test.ts +++ b/packages/replay/test/unit/util/handleRecordingEmit.test.ts @@ -1,13 +1,16 @@ import { EventType } from '@sentry-internal/rrweb'; import { BASE_TIMESTAMP } from '../..'; +import type { RecordingEvent } from '../../../src/types'; import * as SentryAddEvent from '../../../src/util/addEvent'; -import { getHandleRecordingEmit } from '../../../src/util/handleRecordingEmit'; +import { createOptionsEvent, getHandleRecordingEmit } from '../../../src/util/handleRecordingEmit'; import { setupReplayContainer } from '../../utils/setupReplayContainer'; import { useFakeTimers } from '../../utils/use-fake-timers'; useFakeTimers(); +let optionsEvent: RecordingEvent; + describe('Unit | util | handleRecordingEmit', () => { let addEventMock: jest.SpyInstance; @@ -29,6 +32,7 @@ describe('Unit | util | handleRecordingEmit', () => { sessionSampleRate: 1, }, }); + optionsEvent = createOptionsEvent(replay); const handler = getHandleRecordingEmit(replay); @@ -43,13 +47,14 @@ describe('Unit | util | handleRecordingEmit', () => { handler(event); await new Promise(process.nextTick); - expect(addEventMock).toBeCalledTimes(1); - expect(addEventMock).toHaveBeenLastCalledWith(replay, event, true); + expect(addEventMock).toBeCalledTimes(2); + expect(addEventMock).toHaveBeenNthCalledWith(1, replay, event, true); + expect(addEventMock).toHaveBeenLastCalledWith(replay, optionsEvent, false); handler(event); await new Promise(process.nextTick); - expect(addEventMock).toBeCalledTimes(2); + expect(addEventMock).toBeCalledTimes(3); expect(addEventMock).toHaveBeenLastCalledWith(replay, event, false); }); @@ -60,6 +65,7 @@ describe('Unit | util | handleRecordingEmit', () => { sessionSampleRate: 1, }, }); + optionsEvent = createOptionsEvent(replay); const handler = getHandleRecordingEmit(replay); @@ -74,13 +80,16 @@ describe('Unit | util | handleRecordingEmit', () => { handler(event, true); await new Promise(process.nextTick); - expect(addEventMock).toBeCalledTimes(1); - expect(addEventMock).toHaveBeenLastCalledWith(replay, event, true); + // Called twice, once for event and once for settings on checkout only + expect(addEventMock).toBeCalledTimes(2); + expect(addEventMock).toHaveBeenNthCalledWith(1, replay, event, true); + expect(addEventMock).toHaveBeenLastCalledWith(replay, optionsEvent, false); handler(event, true); await new Promise(process.nextTick); - expect(addEventMock).toBeCalledTimes(2); - expect(addEventMock).toHaveBeenLastCalledWith(replay, event, true); + expect(addEventMock).toBeCalledTimes(4); + expect(addEventMock).toHaveBeenNthCalledWith(3, replay, event, true); + expect(addEventMock).toHaveBeenLastCalledWith(replay, { ...optionsEvent, timestamp: BASE_TIMESTAMP + 20 }, false); }); }); diff --git a/packages/replay/test/utils/setupReplayContainer.ts b/packages/replay/test/utils/setupReplayContainer.ts index cf4812da5f2f..83ced117c464 100644 --- a/packages/replay/test/utils/setupReplayContainer.ts +++ b/packages/replay/test/utils/setupReplayContainer.ts @@ -3,24 +3,30 @@ import { ReplayContainer } from '../../src/replay'; import { clearSession } from '../../src/session/clearSession'; import type { RecordingOptions, ReplayPluginOptions } from '../../src/types'; +const DEFAULT_OPTIONS = { + flushMinDelay: 100, + flushMaxDelay: 100, + stickySession: false, + sessionSampleRate: 0, + errorSampleRate: 1, + useCompression: false, + blockAllMedia: true, + networkDetailAllowUrls: [], + networkCaptureBodies: true, + networkRequestHeaders: [], + networkResponseHeaders: [], + _experiments: {}, +}; + export function setupReplayContainer({ options, recordingOptions, }: { options?: Partial; recordingOptions?: Partial } = {}): ReplayContainer { const replay = new ReplayContainer({ options: { - flushMinDelay: 100, - flushMaxDelay: 100, - stickySession: false, - sessionSampleRate: 0, - errorSampleRate: 1, - useCompression: false, - blockAllMedia: true, - networkDetailAllowUrls: [], - networkCaptureBodies: true, - networkRequestHeaders: [], - networkResponseHeaders: [], - _experiments: {}, + ...DEFAULT_OPTIONS, + maskAllInputs: !!recordingOptions?.maskAllInputs, + maskAllText: !!recordingOptions?.maskAllText, ...options, }, recordingOptions: { @@ -39,3 +45,17 @@ export function setupReplayContainer({ return replay; } + +export const DEFAULT_OPTIONS_EVENT_PAYLOAD = { + sessionSampleRate: DEFAULT_OPTIONS.sessionSampleRate, + errorSampleRate: DEFAULT_OPTIONS.errorSampleRate, + useCompressionOption: false, + blockAllMedia: DEFAULT_OPTIONS.blockAllMedia, + maskAllText: false, + maskAllInputs: false, + useCompression: DEFAULT_OPTIONS.useCompression, + networkDetailHasUrls: DEFAULT_OPTIONS.networkDetailAllowUrls.length > 0, + networkCaptureBodies: DEFAULT_OPTIONS.networkCaptureBodies, + networkRequestHeaders: DEFAULT_OPTIONS.networkRequestHeaders.length > 0, + networkResponseHeaders: DEFAULT_OPTIONS.networkResponseHeaders.length > 0, +}; From 79e8e102c2c7a27f2444c0eb4f1dc9dbb65fb345 Mon Sep 17 00:00:00 2001 From: Billy Vong Date: Mon, 8 May 2023 05:38:19 -0230 Subject: [PATCH 06/10] feat(replay): Upgrade `rrweb` to 1.108.0 (#8056) - fix: Fix some input masking (esp for radio buttons) (getsentry/rrweb#85) - fix: Unescaped `:` in CSS rule from Safari (getsentry/rrweb#86) - feat: Define custom elements (web components) (getsentry/rrweb#87) --- packages/replay/package.json | 4 ++-- yarn.lock | 18 +++++++++--------- 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/packages/replay/package.json b/packages/replay/package.json index 1fbaa10f3f69..4d78217eb14e 100644 --- a/packages/replay/package.json +++ b/packages/replay/package.json @@ -45,8 +45,8 @@ "devDependencies": { "@babel/core": "^7.17.5", "@sentry-internal/replay-worker": "7.51.0", - "@sentry-internal/rrweb": "1.106.0", - "@sentry-internal/rrweb-snapshot": "1.106.0", + "@sentry-internal/rrweb": "1.108.0", + "@sentry-internal/rrweb-snapshot": "1.108.0", "jsdom-worker": "^0.2.1", "tslib": "^1.9.3" }, diff --git a/yarn.lock b/yarn.lock index e3fadfbb2260..eee0eb3cd55f 100644 --- a/yarn.lock +++ b/yarn.lock @@ -4046,17 +4046,17 @@ semver "7.3.2" semver-intersect "1.4.0" -"@sentry-internal/rrweb-snapshot@1.106.0": - version "1.106.0" - resolved "https://registry.yarnpkg.com/@sentry-internal/rrweb-snapshot/-/rrweb-snapshot-1.106.0.tgz#24714d4005a918855eeb09d4deda457198f77caf" - integrity sha512-jLX6uGAW8StwPlQOScLhERVa6VOPmbQRFdhTx70Flkt0ocxg2vBJSdaI4Efu3jU0mtA0gHR01LB5xd1ODMRXow== +"@sentry-internal/rrweb-snapshot@1.108.0": + version "1.108.0" + resolved "https://registry.yarnpkg.com/@sentry-internal/rrweb-snapshot/-/rrweb-snapshot-1.108.0.tgz#9b09b7e5d6b13d4d7493017ee190b097f9916284" + integrity sha512-ypR/4oBB8s7d5+7JTkdk+VvlMPRRhbuz3xSFMXShCH2LJ6kINGfYBAYr6rr6o2Bko9j5rVHjYDDrVWkTw4CXSg== -"@sentry-internal/rrweb@1.106.0": - version "1.106.0" - resolved "https://registry.yarnpkg.com/@sentry-internal/rrweb/-/rrweb-1.106.0.tgz#251026e42cf142119eed0616808a279099aa0573" - integrity sha512-/YtV8EoWmMoTE592aVdrgGH5EBiCILdKOt228pM+juGaxRqFxsX26UIeTJG7fihSweUz6NLbKXZjS0+cFYmhrQ== +"@sentry-internal/rrweb@1.108.0": + version "1.108.0" + resolved "https://registry.yarnpkg.com/@sentry-internal/rrweb/-/rrweb-1.108.0.tgz#4b724c1fff44fb4705723c121ca424c00fabc398" + integrity sha512-IuRuA1k2N23e6oTRnV9866mauoOvesYFZFlQHgOvt7p3pJDfXhDUZj1DKaQZJrbooTUUIh7YrpZ2Vukoq0wCFw== dependencies: - "@sentry-internal/rrweb-snapshot" "1.106.0" + "@sentry-internal/rrweb-snapshot" "1.108.0" "@types/css-font-loading-module" "0.0.7" "@xstate/fsm" "^1.4.0" base64-arraybuffer "^1.0.1" From 458895493680c14445a2a2dc911630c5fc3dc453 Mon Sep 17 00:00:00 2001 From: Abhijeet Prasad Date: Mon, 8 May 2023 10:20:25 +0200 Subject: [PATCH 07/10] fix(node): Make sure we use same ID for checkIns (#8050) --- packages/node/src/client.ts | 14 ++++++++++---- packages/node/src/sdk.ts | 10 ++++++++-- packages/node/test/client.test.ts | 31 +++++++++++++++++++++++++------ packages/node/test/sdk.test.ts | 20 ++++++++++++++++++++ packages/types/src/checkin.ts | 15 +++++++++++++-- 5 files changed, 76 insertions(+), 14 deletions(-) diff --git a/packages/node/src/client.ts b/packages/node/src/client.ts index fd0e94de595b..af39f786ac3c 100644 --- a/packages/node/src/client.ts +++ b/packages/node/src/client.ts @@ -153,25 +153,30 @@ export class NodeClient extends BaseClient { * @param checkIn An object that describes a check in. * @param upsertMonitorConfig An optional object that describes a monitor config. Use this if you want * to create a monitor automatically when sending a check in. + * @returns A string representing the id of the check in. */ - public captureCheckIn(checkIn: CheckIn, monitorConfig?: MonitorConfig): void { + public captureCheckIn(checkIn: CheckIn, monitorConfig?: MonitorConfig): string { + const id = checkIn.status !== 'in_progress' && checkIn.checkInId ? checkIn.checkInId : uuid4(); if (!this._isEnabled()) { __DEBUG_BUILD__ && logger.warn('SDK not enabled, will not capture checkin.'); - return; + return id; } const options = this.getOptions(); const { release, environment, tunnel } = options; const serializedCheckIn: SerializedCheckIn = { - check_in_id: uuid4(), + check_in_id: id, monitor_slug: checkIn.monitorSlug, status: checkIn.status, - duration: checkIn.duration, release, environment, }; + if (checkIn.status !== 'in_progress') { + serializedCheckIn.duration = checkIn.duration; + } + if (monitorConfig) { serializedCheckIn.monitor_config = { schedule: monitorConfig.schedule, @@ -183,6 +188,7 @@ export class NodeClient extends BaseClient { const envelope = createCheckInEnvelope(serializedCheckIn, this.getSdkMetadata(), tunnel, this.getDsn()); void this._sendEnvelope(envelope); + return id; } /** diff --git a/packages/node/src/sdk.ts b/packages/node/src/sdk.ts index d8bb6c25c989..d4e2df4ac5f9 100644 --- a/packages/node/src/sdk.ts +++ b/packages/node/src/sdk.ts @@ -13,6 +13,7 @@ import { logger, nodeStackLineParser, stackParserFromStackParserOptions, + uuid4, } from '@sentry/utils'; import { setNodeAsyncContextStrategy } from './async'; @@ -273,12 +274,17 @@ export function captureCheckIn( checkIn: CheckIn, upsertMonitorConfig?: MonitorConfig, ): ReturnType { + const capturedCheckIn = + checkIn.status !== 'in_progress' && checkIn.checkInId ? checkIn : { ...checkIn, checkInId: uuid4() }; + const client = getCurrentHub().getClient(); if (client) { - return client.captureCheckIn(checkIn, upsertMonitorConfig); + client.captureCheckIn(capturedCheckIn, upsertMonitorConfig); + } else { + __DEBUG_BUILD__ && logger.warn('Cannot capture check in. No client defined.'); } - __DEBUG_BUILD__ && logger.warn('Cannot capture check in. No client defined.'); + return capturedCheckIn.checkInId; } /** Node.js stack parser */ diff --git a/packages/node/test/client.test.ts b/packages/node/test/client.test.ts index c29627accb2e..ee5fd5bdd957 100644 --- a/packages/node/test/client.test.ts +++ b/packages/node/test/client.test.ts @@ -294,8 +294,8 @@ describe('NodeClient', () => { // @ts-ignore accessing private method const sendEnvelopeSpy = jest.spyOn(client, '_sendEnvelope'); - client.captureCheckIn( - { monitorSlug: 'foo', status: 'ok', duration: 1222 }, + const id = client.captureCheckIn( + { monitorSlug: 'foo', status: 'in_progress' }, { schedule: { type: 'crontab', @@ -314,10 +314,9 @@ describe('NodeClient', () => { [ expect.any(Object), { - check_in_id: expect.any(String), - duration: 1222, + check_in_id: id, monitor_slug: 'foo', - status: 'ok', + status: 'in_progress', release: '1.0.0', environment: 'dev', monitor_config: { @@ -333,6 +332,26 @@ describe('NodeClient', () => { ], ], ]); + + client.captureCheckIn({ monitorSlug: 'foo', status: 'ok', duration: 1222, checkInId: id }); + + expect(sendEnvelopeSpy).toHaveBeenCalledTimes(2); + expect(sendEnvelopeSpy).toHaveBeenCalledWith([ + expect.any(Object), + [ + [ + expect.any(Object), + { + check_in_id: id, + monitor_slug: 'foo', + duration: 1222, + status: 'ok', + release: '1.0.0', + environment: 'dev', + }, + ], + ], + ]); }); it('does not send a checkIn envelope if disabled', () => { @@ -342,7 +361,7 @@ describe('NodeClient', () => { // @ts-ignore accessing private method const sendEnvelopeSpy = jest.spyOn(client, '_sendEnvelope'); - client.captureCheckIn({ monitorSlug: 'foo', status: 'ok', duration: 1222 }); + client.captureCheckIn({ monitorSlug: 'foo', status: 'in_progress' }); expect(sendEnvelopeSpy).toHaveBeenCalledTimes(0); }); diff --git a/packages/node/test/sdk.test.ts b/packages/node/test/sdk.test.ts index abd0265b62c4..f7c2595c66a5 100644 --- a/packages/node/test/sdk.test.ts +++ b/packages/node/test/sdk.test.ts @@ -1,5 +1,7 @@ +import { getCurrentHub } from '@sentry/core'; import type { Integration } from '@sentry/types'; +import type { NodeClient } from '../build/types'; import { init } from '../src/sdk'; import * as sdk from '../src/sdk'; @@ -90,3 +92,21 @@ describe('init()', () => { expect(newIntegration.setupOnce as jest.Mock).toHaveBeenCalledTimes(1); }); }); + +describe('captureCheckIn', () => { + it('always returns an id', () => { + const hub = getCurrentHub(); + const client = hub.getClient(); + expect(client).toBeDefined(); + + const captureCheckInSpy = jest.spyOn(client!, 'captureCheckIn'); + + // test if captureCheckIn returns an id even if client is not defined + hub.bindClient(undefined); + + expect(captureCheckInSpy).toHaveBeenCalledTimes(0); + expect(sdk.captureCheckIn({ monitorSlug: 'gogogo', status: 'in_progress' })).toBeTruthy(); + + hub.bindClient(client); + }); +}); diff --git a/packages/types/src/checkin.ts b/packages/types/src/checkin.ts index 67537e46d390..a316c0c7a375 100644 --- a/packages/types/src/checkin.ts +++ b/packages/types/src/checkin.ts @@ -38,15 +38,26 @@ export interface SerializedCheckIn { }; } -export interface CheckIn { +interface InProgressCheckIn { // The distinct slug of the monitor. monitorSlug: SerializedCheckIn['monitor_slug']; // The status of the check-in. - status: SerializedCheckIn['status']; + status: 'in_progress'; +} + +export interface FinishedCheckIn { + // The distinct slug of the monitor. + monitorSlug: SerializedCheckIn['monitor_slug']; + // The status of the check-in. + status: 'ok' | 'error'; + // Check-In ID (unique and client generated). + checkInId: SerializedCheckIn['check_in_id']; // The duration of the check-in in seconds. Will only take effect if the status is ok or error. duration?: SerializedCheckIn['duration']; } +export type CheckIn = InProgressCheckIn | FinishedCheckIn; + type SerializedMonitorConfig = NonNullable; export interface MonitorConfig { From d8cf8d3cc0b641bf994d1c1a83b0b69b80a69337 Mon Sep 17 00:00:00 2001 From: Billy Vong Date: Mon, 8 May 2023 05:50:42 -0230 Subject: [PATCH 08/10] fix(replay): Move error sampling to before send (#8057) --------- Co-authored-by: Francesco Novy --- .../suites/replay/bufferMode/test.ts | 219 +++++++++--------- .../utils/fixtures.ts | 8 + .../src/coreHandlers/handleAfterSendEvent.ts | 20 +- .../src/coreHandlers/handleGlobalEvent.ts | 13 +- .../util/shouldSampleForBufferEvent.ts | 29 +++ .../coreHandlers/handleAfterSendEvent.test.ts | 2 +- 6 files changed, 170 insertions(+), 121 deletions(-) create mode 100644 packages/replay/src/coreHandlers/util/shouldSampleForBufferEvent.ts diff --git a/packages/browser-integration-tests/suites/replay/bufferMode/test.ts b/packages/browser-integration-tests/suites/replay/bufferMode/test.ts index 2a835c444550..a9a9dbbe86e5 100644 --- a/packages/browser-integration-tests/suites/replay/bufferMode/test.ts +++ b/packages/browser-integration-tests/suites/replay/bufferMode/test.ts @@ -299,117 +299,126 @@ sentryTest( // Doing this in buffer mode to test changing error sample rate after first // error happens. -sentryTest('[buffer-mode] can sample on each error event', async ({ getLocalTestPath, page, browserName }) => { - // This was sometimes flaky on firefox/webkit, so skipping for now - if (shouldSkipReplayTest() || ['firefox', 'webkit'].includes(browserName)) { - sentryTest.skip(); - } - - let callsToSentry = 0; - const errorEventIds: string[] = []; - const reqPromise0 = waitForReplayRequest(page, 0); - const reqErrorPromise = waitForErrorRequest(page); - - await page.route('https://dsn.ingest.sentry.io/**/*', route => { - const event = envelopeRequestParser(route.request()); - // error events have no type field - if (event && !event.type && event.event_id) { - errorEventIds.push(event.event_id); - } - // We only want to count errors & replays here - if (event && (!event.type || isReplayEvent(event))) { - callsToSentry++; +sentryTest( + '[buffer-mode] can sample on each error event', + async ({ getLocalTestPath, page, browserName, enableConsole }) => { + // This was sometimes flaky on firefox/webkit, so skipping for now + if (shouldSkipReplayTest() || ['firefox', 'webkit'].includes(browserName)) { + sentryTest.skip(); } - return route.fulfill({ - status: 200, - contentType: 'application/json', - body: JSON.stringify({ id: 'test-id' }), + enableConsole(); + + let callsToSentry = 0; + const errorEventIds: string[] = []; + const reqPromise0 = waitForReplayRequest(page, 0); + const reqErrorPromise0 = waitForErrorRequest(page); + + await page.route('https://dsn.ingest.sentry.io/**/*', route => { + const event = envelopeRequestParser(route.request()); + // error events have no type field + if (event && !event.type && event.event_id) { + errorEventIds.push(event.event_id); + } + // We only want to count errors & replays here + if (event && (!event.type || isReplayEvent(event))) { + callsToSentry++; + } + + return route.fulfill({ + status: 200, + contentType: 'application/json', + body: JSON.stringify({ id: 'test-id' }), + }); + }); + + const url = await getLocalTestPath({ testDir: __dirname }); + + await page.goto(url); + // Start buffering and assert that it is enabled + expect( + await page.evaluate(() => { + const replayIntegration = (window as unknown as Window & { Replay: InstanceType }).Replay; + const replay = replayIntegration['_replay']; + replayIntegration.startBuffering(); + return replay.isEnabled(); + }), + ).toBe(true); + + await page.click('#go-background'); + await page.click('#error'); + await new Promise(resolve => setTimeout(resolve, 1000)); + + // 1 unsampled error, no replay + const reqError0 = await reqErrorPromise0; + const errorEvent0 = envelopeRequestParser(reqError0); + expect(callsToSentry).toEqual(1); + expect(errorEvent0.tags?.replayId).toBeUndefined(); + + await page.evaluate(async () => { + const replayIntegration = (window as unknown as Window & { Replay: Replay }).Replay; + replayIntegration['_replay'].getOptions().errorSampleRate = 1.0; }); - }); - - const url = await getLocalTestPath({ testDir: __dirname }); - - await page.goto(url); - // Start buffering and assert that it is enabled - expect( - await page.evaluate(() => { - const replayIntegration = (window as unknown as Window & { Replay: InstanceType }).Replay; - const replay = replayIntegration['_replay']; - replayIntegration.startBuffering(); - return replay.isEnabled(); - }), - ).toBe(true); - - await page.click('#go-background'); - await page.click('#error'); - await new Promise(resolve => setTimeout(resolve, 1000)); - - // 1 error, no replay - await reqErrorPromise; - expect(callsToSentry).toEqual(1); - - await page.evaluate(async () => { - const replayIntegration = (window as unknown as Window & { Replay: Replay }).Replay; - replayIntegration['_replay'].getOptions().errorSampleRate = 1.0; - }); - - // Error sample rate is now at 1.0, this error should create a replay - await page.click('#error2'); - - const req0 = await reqPromise0; - - // 2 errors, 1 flush - await reqErrorPromise; - expect(callsToSentry).toEqual(3); - - const event0 = getReplayEvent(req0); - const content0 = getReplayRecordingContent(req0); - - expect(event0).toEqual( - getExpectedReplayEvent({ - error_ids: errorEventIds, - replay_type: 'buffer', - }), - ); - - // The first event should have both, full and incremental snapshots, - // as we recorded and kept all events in the buffer - expect(content0.fullSnapshots).toHaveLength(1); - // We want to make sure that the event that triggered the error was - // recorded, as well as the first error that did not get sampled. - expect(content0.breadcrumbs).toEqual( - expect.arrayContaining([ - { - ...expectedClickBreadcrumb, - message: 'body > button#error', - data: { - nodeId: expect.any(Number), - node: { - attributes: { - id: 'error', + + // Error sample rate is now at 1.0, this error should create a replay + const reqErrorPromise1 = waitForErrorRequest(page); + await page.click('#error2'); + // 1 unsampled error, 1 sampled error -> 1 flush + const req0 = await reqPromise0; + const reqError1 = await reqErrorPromise1; + const errorEvent1 = envelopeRequestParser(reqError1); + expect(callsToSentry).toEqual(3); + expect(errorEvent0.event_id).not.toEqual(errorEvent1.event_id); + expect(errorEvent1.tags?.replayId).toBeDefined(); + + const event0 = getReplayEvent(req0); + const content0 = getReplayRecordingContent(req0); + + expect(event0).toEqual( + getExpectedReplayEvent({ + error_ids: errorEventIds, + replay_type: 'buffer', + }), + ); + + // The first event should have both, full and incremental snapshots, + // as we recorded and kept all events in the buffer + expect(content0.fullSnapshots).toHaveLength(1); + // We want to make sure that the event that triggered the error was + // recorded, as well as the first error that did not get sampled. + expect(content0.breadcrumbs).toEqual( + expect.arrayContaining([ + { + ...expectedClickBreadcrumb, + message: 'body > button#error', + data: { + nodeId: expect.any(Number), + node: { + attributes: { + id: 'error', + }, + id: expect.any(Number), + tagName: 'button', + textContent: '***** *****', }, - id: expect.any(Number), - tagName: 'button', - textContent: '***** *****', }, }, - }, - { - ...expectedClickBreadcrumb, - message: 'body > button#error2', - data: { - nodeId: expect.any(Number), - node: { - attributes: { - id: 'error2', + { + ...expectedClickBreadcrumb, + message: 'body > button#error2', + data: { + nodeId: expect.any(Number), + node: { + attributes: { + id: 'error2', + }, + id: expect.any(Number), + tagName: 'button', + textContent: '******* *****', }, - id: expect.any(Number), - tagName: 'button', - textContent: '******* *****', }, }, - }, - ]), - ); -}); + ]), + ); + }, +); diff --git a/packages/browser-integration-tests/utils/fixtures.ts b/packages/browser-integration-tests/utils/fixtures.ts index 274f42ab3fd4..ae0c8eab3df7 100644 --- a/packages/browser-integration-tests/utils/fixtures.ts +++ b/packages/browser-integration-tests/utils/fixtures.ts @@ -29,6 +29,7 @@ export type TestFixtures = { getLocalTestPath: (options: { testDir: string }) => Promise; getLocalTestUrl: (options: { testDir: string; skipRouteHandler?: boolean }) => Promise; forceFlushReplay: () => Promise; + enableConsole: () => void; runInChromium: (fn: (...args: unknown[]) => unknown, args?: unknown[]) => unknown; runInFirefox: (fn: (...args: unknown[]) => unknown, args?: unknown[]) => unknown; runInWebkit: (fn: (...args: unknown[]) => unknown, args?: unknown[]) => unknown; @@ -109,6 +110,13 @@ const sentryTest = base.extend({ `), ); }, + + enableConsole: ({ page }, use) => { + return use(() => + // eslint-disable-next-line no-console + page.on('console', msg => console.log(msg.text())), + ); + }, }); export { sentryTest }; diff --git a/packages/replay/src/coreHandlers/handleAfterSendEvent.ts b/packages/replay/src/coreHandlers/handleAfterSendEvent.ts index 5c8e59d6be4e..c90f90c2ae28 100644 --- a/packages/replay/src/coreHandlers/handleAfterSendEvent.ts +++ b/packages/replay/src/coreHandlers/handleAfterSendEvent.ts @@ -1,10 +1,8 @@ import { getCurrentHub } from '@sentry/core'; import type { Event, Transport, TransportMakeRequestResponse } from '@sentry/types'; -import { UNABLE_TO_SEND_REPLAY } from '../constants'; import type { ReplayContainer } from '../types'; import { isErrorEvent, isTransactionEvent } from '../util/eventUtils'; -import { isSampled } from '../util/isSampled'; type AfterSendEventCallback = (event: Event, sendResponse: TransportMakeRequestResponse | void) => void; @@ -42,22 +40,18 @@ export function handleAfterSendEvent(replay: ReplayContainer): AfterSendEventCal return; } - // Add error to list of errorIds of replay + // Add error to list of errorIds of replay. This is ok to do even if not + // sampled because context will get reset at next checkout. + // XXX: There is also a race condition where it's possible to capture an + // error to Sentry before Replay SDK has loaded, but response returns after + // it was loaded, and this gets called. if (event.event_id) { replay.getContext().errorIds.add(event.event_id); } - // Trigger error recording + // If error event is tagged with replay id it means it was sampled (when in buffer mode) // Need to be very careful that this does not cause an infinite loop - if ( - replay.recordingMode === 'buffer' && - event.exception && - event.message !== UNABLE_TO_SEND_REPLAY // ignore this error because otherwise we could loop indefinitely with trying to capture replay and failing - ) { - if (!isSampled(replay.getOptions().errorSampleRate)) { - return; - } - + if (replay.recordingMode === 'buffer' && event.tags && event.tags.replayId) { setTimeout(() => { // Capture current event buffer as new replay void replay.sendBufferedReplayOrFlush(); diff --git a/packages/replay/src/coreHandlers/handleGlobalEvent.ts b/packages/replay/src/coreHandlers/handleGlobalEvent.ts index ba539b661387..26fb5f4a633a 100644 --- a/packages/replay/src/coreHandlers/handleGlobalEvent.ts +++ b/packages/replay/src/coreHandlers/handleGlobalEvent.ts @@ -6,6 +6,7 @@ import type { ReplayContainer } from '../types'; import { isErrorEvent, isReplayEvent, isTransactionEvent } from '../util/eventUtils'; import { isRrwebError } from '../util/isRrwebError'; import { handleAfterSendEvent } from './handleAfterSendEvent'; +import { shouldSampleForBufferEvent } from './util/shouldSampleForBufferEvent'; /** * Returns a listener to be added to `addGlobalEventProcessor(listener)`. @@ -36,8 +37,16 @@ export function handleGlobalEventListener( return null; } - // Only tag transactions with replayId if not waiting for an error - if (isErrorEvent(event) || (isTransactionEvent(event) && replay.recordingMode === 'session')) { + // When in buffer mode, we decide to sample here. + // Later, in `handleAfterSendEvent`, if the replayId is set, we know that we sampled + // And convert the buffer session to a full session + const isErrorEventSampled = shouldSampleForBufferEvent(replay, event); + + // Tag errors if it has been sampled in buffer mode, or if it is session mode + // Only tag transactions if in session mode + const shouldTagReplayId = isErrorEventSampled || replay.recordingMode === 'session'; + + if (shouldTagReplayId) { event.tags = { ...event.tags, replayId: replay.getSessionId() }; } diff --git a/packages/replay/src/coreHandlers/util/shouldSampleForBufferEvent.ts b/packages/replay/src/coreHandlers/util/shouldSampleForBufferEvent.ts new file mode 100644 index 000000000000..736d296e2d95 --- /dev/null +++ b/packages/replay/src/coreHandlers/util/shouldSampleForBufferEvent.ts @@ -0,0 +1,29 @@ +import type { Event } from '@sentry/types'; + +import { UNABLE_TO_SEND_REPLAY } from '../../constants'; +import type { ReplayContainer } from '../../types'; +import { isSampled } from '../../util/isSampled'; + +/** + * Determine if event should be sampled (only applies in buffer mode). + * When an event is captured by `hanldleGlobalEvent`, when in buffer mode + * we determine if we want to sample the error or not. + */ +export function shouldSampleForBufferEvent(replay: ReplayContainer, event: Event): boolean { + if (replay.recordingMode !== 'buffer') { + return false; + } + + // ignore this error because otherwise we could loop indefinitely with + // trying to capture replay and failing + if (event.message === UNABLE_TO_SEND_REPLAY) { + return false; + } + + // Require the event to be an error event & to have an exception + if (!event.exception || event.type) { + return false; + } + + return isSampled(replay.getOptions().errorSampleRate); +} diff --git a/packages/replay/test/integration/coreHandlers/handleAfterSendEvent.test.ts b/packages/replay/test/integration/coreHandlers/handleAfterSendEvent.test.ts index 9f4748253110..1e59a4f7eef0 100644 --- a/packages/replay/test/integration/coreHandlers/handleAfterSendEvent.test.ts +++ b/packages/replay/test/integration/coreHandlers/handleAfterSendEvent.test.ts @@ -136,7 +136,7 @@ describe('Integration | coreHandlers | handleAfterSendEvent', () => { const mockSend = getCurrentHub().getClient()!.getTransport()!.send as unknown as jest.SpyInstance; - const error1 = Error({ event_id: 'err1' }); + const error1 = Error({ event_id: 'err1', tags: { replayId: 'replayid1' } }); const handler = handleAfterSendEvent(replay); From be129db7f9402b5eb987d8e284b40b6f5ba033fb Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Mon, 8 May 2023 10:30:46 +0200 Subject: [PATCH 09/10] feat(replay): Improve click target detection (#8026) --- .../suites/replay/customEvents/template.html | 10 +- .../suites/replay/customEvents/test.ts | 7 +- packages/replay/src/coreHandlers/handleDom.ts | 36 +++-- .../test/unit/coreHandlers/handleDom.test.ts | 130 ++++++++++++++++++ 4 files changed, 164 insertions(+), 19 deletions(-) create mode 100644 packages/replay/test/unit/coreHandlers/handleDom.test.ts diff --git a/packages/browser-integration-tests/suites/replay/customEvents/template.html b/packages/browser-integration-tests/suites/replay/customEvents/template.html index 56a956a95d24..e988c5ed1666 100644 --- a/packages/browser-integration-tests/suites/replay/customEvents/template.html +++ b/packages/browser-integration-tests/suites/replay/customEvents/template.html @@ -5,13 +5,9 @@
An Error
- - + diff --git a/packages/browser-integration-tests/suites/replay/customEvents/test.ts b/packages/browser-integration-tests/suites/replay/customEvents/test.ts index 585266746365..92bcd081ea8f 100644 --- a/packages/browser-integration-tests/suites/replay/customEvents/test.ts +++ b/packages/browser-integration-tests/suites/replay/customEvents/test.ts @@ -135,16 +135,15 @@ sentryTest( expect.arrayContaining([ { ...expectedClickBreadcrumb, - message: 'body > button > img#img[alt="Alt Text"]', + message: 'body > button[title="Button title"]', data: { nodeId: expect.any(Number), node: { attributes: { - alt: 'Alt Text', - id: 'img', + title: '****** *****', }, id: expect.any(Number), - tagName: 'img', + tagName: 'button', textContent: '', }, }, diff --git a/packages/replay/src/coreHandlers/handleDom.ts b/packages/replay/src/coreHandlers/handleDom.ts index 8878f2b71966..f98a92725861 100644 --- a/packages/replay/src/coreHandlers/handleDom.ts +++ b/packages/replay/src/coreHandlers/handleDom.ts @@ -8,7 +8,7 @@ import { createBreadcrumb } from '../util/createBreadcrumb'; import { addBreadcrumbEvent } from './util/addBreadcrumbEvent'; import { getAttributesToRecord } from './util/getAttributesToRecord'; -interface DomHandlerData { +export interface DomHandlerData { name: string; event: Node | { target: Node }; } @@ -31,15 +31,18 @@ export const handleDomListener: (replay: ReplayContainer) => (handlerData: DomHa /** * An event handler to react to DOM events. + * Exported for tests only. */ -function handleDom(handlerData: DomHandlerData): Breadcrumb | null { +export function handleDom(handlerData: DomHandlerData): Breadcrumb | null { let target; let targetNode: Node | INode | undefined; + const isClick = handlerData.name === 'click'; + // Accessing event.target can throw (see getsentry/raven-js#838, #768) try { - targetNode = getTargetNode(handlerData); - target = htmlTreeAsString(targetNode); + targetNode = isClick ? getClickTargetNode(handlerData.event) : getTargetNode(handlerData.event); + target = htmlTreeAsString(targetNode, { maxStringLength: 200 }); } catch (e) { target = ''; } @@ -73,12 +76,29 @@ function handleDom(handlerData: DomHandlerData): Breadcrumb | null { }); } -function getTargetNode(handlerData: DomHandlerData): Node { - if (isEventWithTarget(handlerData.event)) { - return handlerData.event.target; +function getTargetNode(event: DomHandlerData['event']): Node { + if (isEventWithTarget(event)) { + return event.target; + } + + return event; +} + +const INTERACTIVE_SELECTOR = 'button,a'; + +// For clicks, we check if the target is inside of a button or link +// If so, we use this as the target instead +// This is useful because if you click on the image in , +// The target will be the image, not the button, which we don't want here +function getClickTargetNode(event: DomHandlerData['event']): Node { + const target = getTargetNode(event); + + if (!target || !(target instanceof Element)) { + return target; } - return handlerData.event; + const closestInteractive = target.closest(INTERACTIVE_SELECTOR); + return closestInteractive || target; } function isEventWithTarget(event: unknown): event is { target: Node } { diff --git a/packages/replay/test/unit/coreHandlers/handleDom.test.ts b/packages/replay/test/unit/coreHandlers/handleDom.test.ts new file mode 100644 index 000000000000..dc1fff0b5ff2 --- /dev/null +++ b/packages/replay/test/unit/coreHandlers/handleDom.test.ts @@ -0,0 +1,130 @@ +import type { DomHandlerData } from '../../../src/coreHandlers/handleDom'; +import { handleDom } from '../../../src/coreHandlers/handleDom'; + +describe('Unit | coreHandlers | handleDom', () => { + test('it works with a basic click event on a div', () => { + const parent = document.createElement('body'); + const target = document.createElement('div'); + target.classList.add('my-class', 'other-class'); + parent.appendChild(target); + + const handlerData: DomHandlerData = { + name: 'click', + event: { + target, + }, + }; + const actual = handleDom(handlerData); + expect(actual).toEqual({ + category: 'ui.click', + data: {}, + message: 'body > div.my-class.other-class', + timestamp: expect.any(Number), + type: 'default', + }); + }); + + test('it works with a basic click event on a button', () => { + const parent = document.createElement('body'); + const target = document.createElement('button'); + target.classList.add('my-class', 'other-class'); + parent.appendChild(target); + + const handlerData: DomHandlerData = { + name: 'click', + event: { + target, + }, + }; + const actual = handleDom(handlerData); + expect(actual).toEqual({ + category: 'ui.click', + data: {}, + message: 'body > button.my-class.other-class', + timestamp: expect.any(Number), + type: 'default', + }); + }); + + test('it works with a basic click event on a span inside of