From fdb9b4dc52e1c730849c87a6bac158a7c3ee0408 Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Wed, 22 Mar 2023 16:49:36 +0100 Subject: [PATCH 1/6] feat: Add `replay_id` to transaction DSC --- .../suites/replay/dsc/init.js | 23 ++++++++++++++++ .../suites/replay/dsc/test.ts | 27 +++++++++++++++++++ packages/core/src/baseclient.ts | 7 +++++ packages/core/src/tracing/transaction.ts | 2 ++ .../replay/src/util/addGlobalListeners.ts | 23 +++++++++++----- packages/types/src/client.ts | 14 ++++++++-- packages/types/src/envelope.ts | 1 + 7 files changed, 88 insertions(+), 9 deletions(-) create mode 100644 packages/browser-integration-tests/suites/replay/dsc/init.js create mode 100644 packages/browser-integration-tests/suites/replay/dsc/test.ts diff --git a/packages/browser-integration-tests/suites/replay/dsc/init.js b/packages/browser-integration-tests/suites/replay/dsc/init.js new file mode 100644 index 000000000000..dc0a1170413d --- /dev/null +++ b/packages/browser-integration-tests/suites/replay/dsc/init.js @@ -0,0 +1,23 @@ +import * as Sentry from '@sentry/browser'; +import { Integrations } from '@sentry/tracing'; + +window.Sentry = Sentry; +window.Replay = new Sentry.Replay({ + flushMinDelay: 200, + flushMaxDelay: 200, +}); + +Sentry.init({ + dsn: 'https://public@dsn.ingest.sentry.io/1337', + integrations: [new Integrations.BrowserTracing({ tracingOrigins: [/.*/] }), window.Replay], + environment: 'production', + tracesSampleRate: 1, + replaysSessionSampleRate: 0.0, + replaysOnErrorSampleRate: 1.0, + debug: true, +}); + +Sentry.configureScope(scope => { + scope.setUser({ id: 'user123', segment: 'segmentB' }); + scope.setTransactionName('testTransactionDSC'); +}); diff --git a/packages/browser-integration-tests/suites/replay/dsc/test.ts b/packages/browser-integration-tests/suites/replay/dsc/test.ts new file mode 100644 index 000000000000..f345ca2036fa --- /dev/null +++ b/packages/browser-integration-tests/suites/replay/dsc/test.ts @@ -0,0 +1,27 @@ +import { expect } from '@playwright/test'; +import type { EventEnvelopeHeaders } from '@sentry/types'; + +import { sentryTest } from '../../../utils/fixtures'; +import { envelopeHeaderRequestParser, getFirstSentryEnvelopeRequest } from '../../../utils/helpers'; +import { getReplaySnapshot } from '../../../utils/replayHelpers'; + +sentryTest('should add replay_id to dsc of transactions', async ({ getLocalTestPath, page }) => { + const url = await getLocalTestPath({ testDir: __dirname }); + await page.goto(url); + + const replay = await getReplaySnapshot(page); + + expect(replay.session?.id).toBeDefined(); + + const envHeader = await getFirstSentryEnvelopeRequest(page, url, envelopeHeaderRequestParser); + + expect(envHeader.trace).toBeDefined(); + expect(envHeader.trace).toEqual({ + environment: 'production', + user_segment: 'segmentB', + sample_rate: '1', + trace_id: expect.any(String), + public_key: 'public', + replay_id: replay.session?.id, + }); +}); diff --git a/packages/core/src/baseclient.ts b/packages/core/src/baseclient.ts index a6db2788aa52..80be9dc037bd 100644 --- a/packages/core/src/baseclient.ts +++ b/packages/core/src/baseclient.ts @@ -6,6 +6,7 @@ import type { ClientOptions, DataCategory, DsnComponents, + DynamicSamplingContext, Envelope, ErrorEvent, Event, @@ -378,6 +379,9 @@ export abstract class BaseClient implements Client { /** @inheritdoc */ public on(hook: 'beforeAddBreadcrumb', callback: (breadcrumb: Breadcrumb, hint?: BreadcrumbHint) => void): void; + /** @inheritdoc */ + public on(hook: 'createDsc', callback: (dsc: DynamicSamplingContext) => void): void; + /** @inheritdoc */ public on(hook: string, callback: unknown): void { if (!this._hooks[hook]) { @@ -400,6 +404,9 @@ export abstract class BaseClient implements Client { /** @inheritdoc */ public emit(hook: 'beforeAddBreadcrumb', breadcrumb: Breadcrumb, hint?: BreadcrumbHint): void; + /** @inheritdoc */ + public emit(hook: 'createDsc', dsc: DynamicSamplingContext): void; + /** @inheritdoc */ public emit(hook: string, ...rest: unknown[]): void { if (this._hooks[hook]) { diff --git a/packages/core/src/tracing/transaction.ts b/packages/core/src/tracing/transaction.ts index b1e6e74195be..9dcee22fb888 100644 --- a/packages/core/src/tracing/transaction.ts +++ b/packages/core/src/tracing/transaction.ts @@ -276,6 +276,8 @@ export class Transaction extends SpanClass implements TransactionInterface { // Uncomment if we want to make DSC immutable // this._frozenDynamicSamplingContext = dsc; + client.emit && client.emit('createDsc', dsc); + return dsc; } } diff --git a/packages/replay/src/util/addGlobalListeners.ts b/packages/replay/src/util/addGlobalListeners.ts index fc68f322d090..46ba18bb9ed3 100644 --- a/packages/replay/src/util/addGlobalListeners.ts +++ b/packages/replay/src/util/addGlobalListeners.ts @@ -1,5 +1,6 @@ import type { BaseClient } from '@sentry/core'; import { addGlobalEventProcessor, getCurrentHub } from '@sentry/core'; +import type { Client, DynamicSamplingContext } from '@sentry/types'; import { addInstrumentationHandler } from '@sentry/utils'; import { handleAfterSendEvent } from '../coreHandlers/handleAfterSendEvent'; @@ -25,15 +26,23 @@ export function addGlobalListeners(replay: ReplayContainer): void { addInstrumentationHandler('history', handleHistorySpanListener(replay)); handleNetworkBreadcrumbs(replay); - // If a custom client has no hooks yet, we continue to use the "old" implementation - const hasHooks = !!(client && client.on); - // Tag all (non replay) events that get sent to Sentry with the current // replay ID so that we can reference them later in the UI - addGlobalEventProcessor(handleGlobalEventListener(replay, !hasHooks)); + addGlobalEventProcessor(handleGlobalEventListener(replay, !hasHooks(client))); - if (hasHooks) { - // eslint-disable-next-line @typescript-eslint/no-explicit-any - (client as BaseClient).on('afterSendEvent', handleAfterSendEvent(replay)); + // If a custom client has no hooks yet, we continue to use the "old" implementation + if (hasHooks(client)) { + client.on('afterSendEvent', handleAfterSendEvent(replay)); + client.on('createDsc', (dsc: DynamicSamplingContext) => { + const replayId = replay.getSessionId(); + if (replayId) { + dsc.replay_id = replayId; + } + }); } } + +// eslint-disable-next-line @typescript-eslint/no-explicit-any +function hasHooks(client: Client | undefined): client is BaseClient { + return !!(client && client.on); +} diff --git a/packages/types/src/client.ts b/packages/types/src/client.ts index 35b37f868d85..d455c4a7590b 100644 --- a/packages/types/src/client.ts +++ b/packages/types/src/client.ts @@ -2,7 +2,7 @@ import type { Breadcrumb, BreadcrumbHint } from './breadcrumb'; import type { EventDropReason } from './clientreport'; import type { DataCategory } from './datacategory'; import type { DsnComponents } from './dsn'; -import type { Envelope } from './envelope'; +import type { DynamicSamplingContext, Envelope } from './envelope'; import type { Event, EventHint } from './event'; import type { Integration, IntegrationClass } from './integration'; import type { ClientOptions } from './options'; @@ -177,6 +177,11 @@ export interface Client { */ on?(hook: 'beforeAddBreadcrumb', callback: (breadcrumb: Breadcrumb, hint?: BreadcrumbHint) => void): void; + /** + * Register a callback whena DSC (Dynamic Sampling Context) is created. + */ + on?(hook: 'createDsc', callback: (dsc: DynamicSamplingContext) => void): void; + /** * Fire a hook event for transaction start and finish. Expects to be given a transaction as the * second argument. @@ -196,7 +201,12 @@ export interface Client { emit?(hook: 'afterSendEvent', event: Event, sendResponse: TransportMakeRequestResponse | void): void; /** - * Fire a hook for when a bredacrumb is added. Expects the breadcrumb as second argument. + * Fire a hook for when a breadcrumb is added. Expects the breadcrumb as second argument. */ emit?(hook: 'beforeAddBreadcrumb', breadcrumb: Breadcrumb, hint?: BreadcrumbHint): void; + + /** + * Fire a hook for when a DSC (Dynamic Sampling Context) is created. Expects the DSC as second argument. + */ + emit?(hook: 'createDsc', dsc: DynamicSamplingContext): void; } diff --git a/packages/types/src/envelope.ts b/packages/types/src/envelope.ts index 60d67b89d0da..2234317ef8ce 100644 --- a/packages/types/src/envelope.ts +++ b/packages/types/src/envelope.ts @@ -18,6 +18,7 @@ export type DynamicSamplingContext = { environment?: string; transaction?: string; user_segment?: string; + replay_id?: string; }; export type EnvelopeItemType = From cdfee796a88bdd552292d1cf188ab2847427301f Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Wed, 22 Mar 2023 17:26:36 +0100 Subject: [PATCH 2/6] reduce flakyness --- packages/browser-integration-tests/suites/replay/dsc/init.js | 1 + packages/browser-integration-tests/suites/replay/dsc/test.ts | 4 ++-- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/packages/browser-integration-tests/suites/replay/dsc/init.js b/packages/browser-integration-tests/suites/replay/dsc/init.js index dc0a1170413d..75306a733bc0 100644 --- a/packages/browser-integration-tests/suites/replay/dsc/init.js +++ b/packages/browser-integration-tests/suites/replay/dsc/init.js @@ -5,6 +5,7 @@ window.Sentry = Sentry; window.Replay = new Sentry.Replay({ flushMinDelay: 200, flushMaxDelay: 200, + useCompression: false, }); Sentry.init({ diff --git a/packages/browser-integration-tests/suites/replay/dsc/test.ts b/packages/browser-integration-tests/suites/replay/dsc/test.ts index f345ca2036fa..8b253f22e3e4 100644 --- a/packages/browser-integration-tests/suites/replay/dsc/test.ts +++ b/packages/browser-integration-tests/suites/replay/dsc/test.ts @@ -9,12 +9,12 @@ sentryTest('should add replay_id to dsc of transactions', async ({ getLocalTestP const url = await getLocalTestPath({ testDir: __dirname }); await page.goto(url); + const envHeader = await getFirstSentryEnvelopeRequest(page, url, envelopeHeaderRequestParser); + const replay = await getReplaySnapshot(page); expect(replay.session?.id).toBeDefined(); - const envHeader = await getFirstSentryEnvelopeRequest(page, url, envelopeHeaderRequestParser); - expect(envHeader.trace).toBeDefined(); expect(envHeader.trace).toEqual({ environment: 'production', From 32e525a5ddb4aac577fd00fbb1e38dbeeebe16f2 Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Wed, 22 Mar 2023 17:37:55 +0100 Subject: [PATCH 3/6] skip without replay --- .../browser-integration-tests/suites/replay/dsc/test.ts | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/packages/browser-integration-tests/suites/replay/dsc/test.ts b/packages/browser-integration-tests/suites/replay/dsc/test.ts index 8b253f22e3e4..7440374efc29 100644 --- a/packages/browser-integration-tests/suites/replay/dsc/test.ts +++ b/packages/browser-integration-tests/suites/replay/dsc/test.ts @@ -3,9 +3,13 @@ import type { EventEnvelopeHeaders } from '@sentry/types'; import { sentryTest } from '../../../utils/fixtures'; import { envelopeHeaderRequestParser, getFirstSentryEnvelopeRequest } from '../../../utils/helpers'; -import { getReplaySnapshot } from '../../../utils/replayHelpers'; +import { getReplaySnapshot, shouldSkipReplayTest } from '../../../utils/replayHelpers'; sentryTest('should add replay_id to dsc of transactions', async ({ getLocalTestPath, page }) => { + if (shouldSkipReplayTest()) { + sentryTest.skip(); + } + const url = await getLocalTestPath({ testDir: __dirname }); await page.goto(url); From f7f483ff9957ba01f7062a2fb4ab98d9ccf01cf9 Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Wed, 22 Mar 2023 17:42:10 +0100 Subject: [PATCH 4/6] no debug --- packages/browser-integration-tests/suites/replay/dsc/init.js | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/browser-integration-tests/suites/replay/dsc/init.js b/packages/browser-integration-tests/suites/replay/dsc/init.js index 75306a733bc0..c43f001779eb 100644 --- a/packages/browser-integration-tests/suites/replay/dsc/init.js +++ b/packages/browser-integration-tests/suites/replay/dsc/init.js @@ -15,7 +15,6 @@ Sentry.init({ tracesSampleRate: 1, replaysSessionSampleRate: 0.0, replaysOnErrorSampleRate: 1.0, - debug: true, }); Sentry.configureScope(scope => { From 7e60dc8c973f8cf8dc2ed2a67a52b0bc8b40ca54 Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Thu, 23 Mar 2023 09:02:33 +0100 Subject: [PATCH 5/6] fix flakyness..?? --- .../suites/replay/dsc/test.ts | 3 ++- .../browser-integration-tests/utils/replayHelpers.ts | 10 ++++++++++ 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/packages/browser-integration-tests/suites/replay/dsc/test.ts b/packages/browser-integration-tests/suites/replay/dsc/test.ts index 7440374efc29..32bf88df73a0 100644 --- a/packages/browser-integration-tests/suites/replay/dsc/test.ts +++ b/packages/browser-integration-tests/suites/replay/dsc/test.ts @@ -3,7 +3,7 @@ import type { EventEnvelopeHeaders } from '@sentry/types'; import { sentryTest } from '../../../utils/fixtures'; import { envelopeHeaderRequestParser, getFirstSentryEnvelopeRequest } from '../../../utils/helpers'; -import { getReplaySnapshot, shouldSkipReplayTest } from '../../../utils/replayHelpers'; +import { getReplaySnapshot, shouldSkipReplayTest, waitForReplayRunning } from '../../../utils/replayHelpers'; sentryTest('should add replay_id to dsc of transactions', async ({ getLocalTestPath, page }) => { if (shouldSkipReplayTest()) { @@ -15,6 +15,7 @@ sentryTest('should add replay_id to dsc of transactions', async ({ getLocalTestP const envHeader = await getFirstSentryEnvelopeRequest(page, url, envelopeHeaderRequestParser); + await waitForReplayRunning(page); const replay = await getReplaySnapshot(page); expect(replay.session?.id).toBeDefined(); diff --git a/packages/browser-integration-tests/utils/replayHelpers.ts b/packages/browser-integration-tests/utils/replayHelpers.ts index cf21ce7b9c7b..bd7696ebd927 100644 --- a/packages/browser-integration-tests/utils/replayHelpers.ts +++ b/packages/browser-integration-tests/utils/replayHelpers.ts @@ -99,6 +99,16 @@ function isCustomSnapshot(event: RecordingEvent): event is RecordingEvent & { da return event.type === EventType.Custom; } +/** Wait for replay to be running & available. */ +export async function waitForReplayRunning(page: Page): Promise { + await page.waitForFunction(() => { + const replayIntegration = (window as unknown as Window & { Replay: { _replay: ReplayContainer } }).Replay; + const replay = replayIntegration._replay; + + return replay.isEnabled() && replay.session?.id !== undefined; + }); +} + /** * This returns the replay container (assuming it exists). * Note that due to how this works with playwright, this is a POJO copy of replay. From 30ee59315dfbd82bb7ed4fd1cd30ad4d00eab35a Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Thu, 23 Mar 2023 09:31:35 +0100 Subject: [PATCH 6/6] skip test on webkit --- packages/browser-integration-tests/suites/replay/dsc/test.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/packages/browser-integration-tests/suites/replay/dsc/test.ts b/packages/browser-integration-tests/suites/replay/dsc/test.ts index 32bf88df73a0..0819e9f7bf71 100644 --- a/packages/browser-integration-tests/suites/replay/dsc/test.ts +++ b/packages/browser-integration-tests/suites/replay/dsc/test.ts @@ -5,8 +5,9 @@ import { sentryTest } from '../../../utils/fixtures'; import { envelopeHeaderRequestParser, getFirstSentryEnvelopeRequest } from '../../../utils/helpers'; import { getReplaySnapshot, shouldSkipReplayTest, waitForReplayRunning } from '../../../utils/replayHelpers'; -sentryTest('should add replay_id to dsc of transactions', async ({ getLocalTestPath, page }) => { - if (shouldSkipReplayTest()) { +sentryTest('should add replay_id to dsc of transactions', async ({ getLocalTestPath, page, browserName }) => { + // This is flaky on webkit, so skipping there... + if (shouldSkipReplayTest() || browserName === 'webkit') { sentryTest.skip(); }