From 4670a6c72927f372134d5a3721930dc63ff99dcf Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Tue, 4 Mar 2025 14:15:53 +0100 Subject: [PATCH 01/15] feat(browser): Add `previous_trace` span links --- .../previous-trace-links/default/test.ts | 80 +++++++++++++ .../previous-trace-links/init.js | 9 ++ .../previous-trace-links/meta/template.html | 9 ++ .../previous-trace-links/meta/test.ts | 46 +++++++ .../negatively-sampled/init.js | 14 +++ .../negatively-sampled/test.ts | 36 ++++++ .../session-storage/init.js | 9 ++ .../session-storage/test.ts | 42 +++++++ .../src/tracing/browserTracingIntegration.ts | 48 ++++++++ packages/browser/src/tracing/previousTrace.ts | 70 +++++++++++ .../tracing/browserTracingIntegration.test.ts | 38 +++++- .../test/tracing/previousTrace.test.ts | 113 ++++++++++++++++++ packages/core/src/semanticAttributes.ts | 12 ++ 13 files changed, 525 insertions(+), 1 deletion(-) create mode 100644 dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/previous-trace-links/default/test.ts create mode 100644 dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/previous-trace-links/init.js create mode 100644 dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/previous-trace-links/meta/template.html create mode 100644 dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/previous-trace-links/meta/test.ts create mode 100644 dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/previous-trace-links/negatively-sampled/init.js create mode 100644 dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/previous-trace-links/negatively-sampled/test.ts create mode 100644 dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/previous-trace-links/session-storage/init.js create mode 100644 dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/previous-trace-links/session-storage/test.ts create mode 100644 packages/browser/src/tracing/previousTrace.ts create mode 100644 packages/browser/test/tracing/previousTrace.test.ts diff --git a/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/previous-trace-links/default/test.ts b/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/previous-trace-links/default/test.ts new file mode 100644 index 000000000000..5e6fdd0ad48b --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/previous-trace-links/default/test.ts @@ -0,0 +1,80 @@ +import { expect } from '@playwright/test'; +import { SEMANTIC_LINK_ATTRIBUTE_LINK_TYPE, type Event } from '@sentry/core'; + +import { sentryTest } from '../../../../../utils/fixtures'; +import { + envelopeRequestParser, + getFirstSentryEnvelopeRequest, + shouldSkipTracingTest, + waitForTransactionRequest, +} from '../../../../../utils/helpers'; + +sentryTest("navigation spans link back to previous trace's root span", async ({ getLocalTestUrl, page }) => { + if (shouldSkipTracingTest()) { + sentryTest.skip(); + } + + const url = await getLocalTestUrl({ testDir: __dirname }); + + const pageloadRequest = await getFirstSentryEnvelopeRequest(page, url); + const navigationRequest = await getFirstSentryEnvelopeRequest(page, `${url}#foo`); + const navigation2Request = await getFirstSentryEnvelopeRequest(page, `${url}#bar`); + + const pageloadTraceContext = pageloadRequest.contexts?.trace; + const navigationTraceContext = navigationRequest.contexts?.trace; + const navigation2TraceContext = navigation2Request.contexts?.trace; + + const pageloadTraceId = pageloadTraceContext?.trace_id; + const navigationTraceId = navigationTraceContext?.trace_id; + const navigation2TraceId = navigation2TraceContext?.trace_id; + + expect(pageloadTraceContext?.op).toBe('pageload'); + expect(navigationTraceContext?.op).toBe('navigation'); + expect(navigation2TraceContext?.op).toBe('navigation'); + + expect(pageloadTraceContext?.links).toBeUndefined(); + expect(navigationTraceContext?.links).toEqual([ + { + trace_id: pageloadTraceId, + span_id: pageloadTraceContext?.span_id, + sampled: true, + attributes: { + [SEMANTIC_LINK_ATTRIBUTE_LINK_TYPE]: 'previous_trace', + }, + }, + ]); + + expect(navigation2TraceContext?.links).toEqual([ + { + trace_id: navigationTraceId, + span_id: navigationTraceContext?.span_id, + sampled: true, + attributes: { + [SEMANTIC_LINK_ATTRIBUTE_LINK_TYPE]: 'previous_trace', + }, + }, + ]); + + expect(pageloadTraceId).not.toEqual(navigationTraceId); + expect(navigationTraceId).not.toEqual(navigation2TraceId); + expect(pageloadTraceId).not.toEqual(navigation2TraceId); +}); + +sentryTest("doesn't link between hard page reloads by default", async ({ getLocalTestUrl, page }) => { + if (shouldSkipTracingTest()) { + sentryTest.skip(); + } + + const url = await getLocalTestUrl({ testDir: __dirname }); + + const pageload1Event = await getFirstSentryEnvelopeRequest(page, url); + + const pageload2RequestPromise = waitForTransactionRequest(page, evt => evt.contexts?.trace?.op === 'pageload'); + await page.reload(); + const pageload2Event = envelopeRequestParser(await pageload2RequestPromise); + + expect(pageload1Event.contexts?.trace).toBeDefined(); + expect(pageload2Event.contexts?.trace).toBeDefined(); + expect(pageload1Event.contexts?.trace?.links).toBeUndefined(); + expect(pageload2Event.contexts?.trace?.links).toBeUndefined(); +}); diff --git a/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/previous-trace-links/init.js b/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/previous-trace-links/init.js new file mode 100644 index 000000000000..83076460599f --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/previous-trace-links/init.js @@ -0,0 +1,9 @@ +import * as Sentry from '@sentry/browser'; + +window.Sentry = Sentry; + +Sentry.init({ + dsn: 'https://public@dsn.ingest.sentry.io/1337', + integrations: [Sentry.browserTracingIntegration()], + tracesSampleRate: 1, +}); diff --git a/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/previous-trace-links/meta/template.html b/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/previous-trace-links/meta/template.html new file mode 100644 index 000000000000..f8024594da10 --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/previous-trace-links/meta/template.html @@ -0,0 +1,9 @@ + + + + + + + + diff --git a/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/previous-trace-links/meta/test.ts b/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/previous-trace-links/meta/test.ts new file mode 100644 index 000000000000..b0041c0db13a --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/previous-trace-links/meta/test.ts @@ -0,0 +1,46 @@ +import { expect } from '@playwright/test'; +import { SEMANTIC_LINK_ATTRIBUTE_LINK_TYPE, type Event } from '@sentry/core'; + +import { sentryTest } from '../../../../../utils/fixtures'; +import { getFirstSentryEnvelopeRequest, shouldSkipTracingTest } from '../../../../../utils/helpers'; + +sentryTest( + "links back to previous trace's local root span if continued from meta tags", + async ({ getLocalTestUrl, page }) => { + if (shouldSkipTracingTest()) { + sentryTest.skip(); + } + + const url = await getLocalTestUrl({ testDir: __dirname }); + + const pageloadRequest = await getFirstSentryEnvelopeRequest(page, url); + const navigationRequest = await getFirstSentryEnvelopeRequest(page, `${url}#foo`); + + const pageloadTraceContext = pageloadRequest.contexts?.trace; + const navigationTraceContext = navigationRequest.contexts?.trace; + + const metaTagTraceId = '12345678901234567890123456789012'; + + const navigationTraceId = navigationTraceContext?.trace_id; + + expect(pageloadTraceContext?.op).toBe('pageload'); + expect(navigationTraceContext?.op).toBe('navigation'); + + // sanity check + expect(pageloadTraceContext?.trace_id).toBe(metaTagTraceId); + expect(pageloadTraceContext?.links).toBeUndefined(); + + expect(navigationTraceContext?.links).toEqual([ + { + trace_id: metaTagTraceId, + span_id: pageloadTraceContext?.span_id, + sampled: true, + attributes: { + [SEMANTIC_LINK_ATTRIBUTE_LINK_TYPE]: 'previous_trace', + }, + }, + ]); + + expect(navigationTraceId).not.toEqual(metaTagTraceId); + }, +); diff --git a/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/previous-trace-links/negatively-sampled/init.js b/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/previous-trace-links/negatively-sampled/init.js new file mode 100644 index 000000000000..6c884b0632c8 --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/previous-trace-links/negatively-sampled/init.js @@ -0,0 +1,14 @@ +import * as Sentry from '@sentry/browser'; + +window.Sentry = Sentry; + +Sentry.init({ + dsn: 'https://public@dsn.ingest.sentry.io/1337', + integrations: [Sentry.browserTracingIntegration()], + tracesSampler: (ctx) => { + if (ctx.attributes['sentry.origin'] === 'auto.pageload.browser') { + return 0; + } + return 1; + } +}); diff --git a/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/previous-trace-links/negatively-sampled/test.ts b/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/previous-trace-links/negatively-sampled/test.ts new file mode 100644 index 000000000000..f3d94b1ed28c --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/previous-trace-links/negatively-sampled/test.ts @@ -0,0 +1,36 @@ +import { expect } from '@playwright/test'; +import { SEMANTIC_LINK_ATTRIBUTE_LINK_TYPE, type Event } from '@sentry/core'; + +import { sentryTest } from '../../../../../utils/fixtures'; +import { getFirstSentryEnvelopeRequest, shouldSkipTracingTest } from '../../../../../utils/helpers'; + +sentryTest('includes a span link to a previously negatively sampled span', async ({ getLocalTestUrl, page }) => { + if (shouldSkipTracingTest()) { + sentryTest.skip(); + } + + const url = await getLocalTestUrl({ testDir: __dirname }); + + await page.goto(url); + + const navigationRequest = await getFirstSentryEnvelopeRequest(page, `${url}#foo`); + + const navigationTraceContext = navigationRequest.contexts?.trace; + + const navigationTraceId = navigationTraceContext?.trace_id; + + expect(navigationTraceContext?.op).toBe('navigation'); + + expect(navigationTraceContext?.links).toEqual([ + { + trace_id: expect.stringMatching(/[a-f0-9]{32}/), + span_id: expect.stringMatching(/[a-f0-9]{16}/), + sampled: false, + attributes: { + [SEMANTIC_LINK_ATTRIBUTE_LINK_TYPE]: 'previous_trace', + }, + }, + ]); + + expect(navigationTraceId).not.toEqual(navigationTraceContext?.links![0].trace_id); +}); diff --git a/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/previous-trace-links/session-storage/init.js b/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/previous-trace-links/session-storage/init.js new file mode 100644 index 000000000000..c278e15a1883 --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/previous-trace-links/session-storage/init.js @@ -0,0 +1,9 @@ +import * as Sentry from '@sentry/browser'; + +window.Sentry = Sentry; + +Sentry.init({ + dsn: 'https://public@dsn.ingest.sentry.io/1337', + integrations: [Sentry.browserTracingIntegration({persistPreviousTrace: true})], + tracesSampleRate: 1, +}); diff --git a/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/previous-trace-links/session-storage/test.ts b/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/previous-trace-links/session-storage/test.ts new file mode 100644 index 000000000000..1b08f2b6ed6d --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/previous-trace-links/session-storage/test.ts @@ -0,0 +1,42 @@ +import { expect } from '@playwright/test'; +import { SEMANTIC_LINK_ATTRIBUTE_LINK_TYPE, type Event } from '@sentry/core'; + +import { sentryTest } from '../../../../../utils/fixtures'; +import { + envelopeRequestParser, + getFirstSentryEnvelopeRequest, + shouldSkipTracingTest, + waitForTransactionRequest, +} from '../../../../../utils/helpers'; + +sentryTest( + 'adds link between hard page reload traces when opting into sessionStorage', + async ({ getLocalTestUrl, page }) => { + if (shouldSkipTracingTest()) { + sentryTest.skip(); + } + + const url = await getLocalTestUrl({ testDir: __dirname }); + + const pageload1Event = await getFirstSentryEnvelopeRequest(page, url); + + const pageload2RequestPromise = waitForTransactionRequest(page, evt => evt.contexts?.trace?.op === 'pageload'); + await page.reload(); + const pageload2Event = envelopeRequestParser(await pageload2RequestPromise); + + const pageload1TraceContext = pageload1Event.contexts?.trace; + expect(pageload1TraceContext).toBeDefined(); + expect(pageload1TraceContext?.links).toBeUndefined(); + + expect(pageload2Event.contexts?.trace?.links).toEqual([ + { + trace_id: pageload1TraceContext?.trace_id, + span_id: pageload1TraceContext?.span_id, + sampled: true, + attributes: { [SEMANTIC_LINK_ATTRIBUTE_LINK_TYPE]: 'previous_trace' }, + }, + ]); + + expect(pageload1TraceContext?.trace_id).not.toEqual(pageload2Event.contexts?.trace?.trace_id); + }, +); diff --git a/packages/browser/src/tracing/browserTracingIntegration.ts b/packages/browser/src/tracing/browserTracingIntegration.ts index f34a37542d29..7faa730914cc 100644 --- a/packages/browser/src/tracing/browserTracingIntegration.ts +++ b/packages/browser/src/tracing/browserTracingIntegration.ts @@ -35,6 +35,12 @@ import { DEBUG_BUILD } from '../debug-build'; import { WINDOW } from '../helpers'; import { registerBackgroundTabDetection } from './backgroundtab'; import { defaultRequestInstrumentationOptions, instrumentOutgoingRequests } from './request'; +import type { PreviousTraceInfo } from './previousTrace'; +import { + addPreviousTraceSpanLink, + getPreviousTraceFromSessionStorage, + storePreviousTraceInSessionStorage, +} from './previousTrace'; export const BROWSER_TRACING_INTEGRATION_ID = 'BrowserTracing'; @@ -142,6 +148,25 @@ export interface BrowserTracingOptions { */ enableHTTPTimings: boolean; + /** + * If enabled, previously started traces (e.g. pageload or navigation spans) will be linked + * to the current trace. This lets you navigate across traces within a user journey in the + * Sentry UI. + * + * Set `persistPreviousTrace` to `true` to connect traces across hard page reloads. + * + * @default true, this is turned on by default. + */ + enablePreviousTrace?: boolean; + + /** + * If set to true, the previous trace will be stored in `sessionStorage`, so that + * traces can be linked across hard page refreshes. + * + * @default false, by default, previous trace data is only stored in-memory. + */ + persistPreviousTrace?: boolean; + /** * _experiments allows the user to send options to define how this integration works. * @@ -175,6 +200,8 @@ const DEFAULT_BROWSER_TRACING_OPTIONS: BrowserTracingOptions = { enableLongTask: true, enableLongAnimationFrame: true, enableInp: true, + enablePreviousTrace: true, + persistPreviousTrace: false, _experiments: {}, ...defaultRequestInstrumentationOptions, }; @@ -214,6 +241,8 @@ export const browserTracingIntegration = ((_options: Partial { [SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE]: 1, [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'url', }, + links: [ + { + attributes: { + 'sentry.link.type': 'previous_trace', + }, + sampled: true, + span_id: span?.spanContext().spanId, + trace_id: span?.spanContext().traceId, + }, + ], span_id: expect.stringMatching(/[a-f0-9]{16}/), start_timestamp: expect.any(Number), trace_id: expect.stringMatching(/[a-f0-9]{32}/), @@ -230,6 +240,16 @@ describe('browserTracingIntegration', () => { [SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE]: 1, [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'url', }, + links: [ + { + attributes: { + 'sentry.link.type': 'previous_trace', + }, + sampled: true, + span_id: span2?.spanContext().spanId, + trace_id: span2?.spanContext().traceId, + }, + ], span_id: expect.stringMatching(/[a-f0-9]{16}/), start_timestamp: expect.any(Number), trace_id: expect.stringMatching(/[a-f0-9]{32}/), @@ -483,6 +503,16 @@ describe('browserTracingIntegration', () => { [SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE]: 1, [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'custom', }, + links: [ + { + attributes: { + 'sentry.link.type': 'previous_trace', + }, + sampled: true, + span_id: expect.stringMatching(/[a-f0-9]{16}/), + trace_id: expect.stringMatching(/[a-f0-9]{32}/), + }, + ], span_id: expect.stringMatching(/[a-f0-9]{16}/), start_timestamp: expect.any(Number), trace_id: expect.stringMatching(/[a-f0-9]{32}/), @@ -494,7 +524,13 @@ describe('browserTracingIntegration', () => { const client = new BrowserClient( getDefaultBrowserClientOptions({ tracesSampleRate: 1, - integrations: [browserTracingIntegration({ instrumentNavigation: false })], + integrations: [ + browserTracingIntegration({ + instrumentNavigation: false, + // disabling previous trace b/c not relevant for this test + enablePreviousTrace: false, + }), + ], }), ); setCurrentClient(client); diff --git a/packages/browser/test/tracing/previousTrace.test.ts b/packages/browser/test/tracing/previousTrace.test.ts new file mode 100644 index 000000000000..870abb497ae7 --- /dev/null +++ b/packages/browser/test/tracing/previousTrace.test.ts @@ -0,0 +1,113 @@ +import { describe, it, expect, vi } from 'vitest'; +import type { PreviousTraceInfo } from '../../src/tracing/previousTrace'; +import { addPreviousTraceSpanLink, PREVIOUS_TRACE_MAX_DURATION } from '../../src/tracing/previousTrace'; +import type { StartSpanOptions } from '@sentry/core'; + +describe('addPreviousTraceSpanLink', () => { + it(`adds a previous_trace span link to startSpanOptions if the previous trace was created within ${PREVIOUS_TRACE_MAX_DURATION}M`, () => { + const previousTraceInfo: PreviousTraceInfo = { + spanContext: { + traceId: '123', + spanId: '456', + traceFlags: 1, + }, + // max time reached exactly + startTimestamp: Date.now() / 1000 - PREVIOUS_TRACE_MAX_DURATION, + }; + + const startSpanOptions: StartSpanOptions = { + name: '/dashboard', + }; + + const updatedPreviousTraceInfo = addPreviousTraceSpanLink(previousTraceInfo, startSpanOptions); + + expect(updatedPreviousTraceInfo).toBe(previousTraceInfo); + expect(startSpanOptions.links).toEqual([ + { + attributes: { + 'sentry.link.type': 'previous_trace', + }, + context: { + spanId: '456', + traceFlags: 1, + traceId: '123', + }, + }, + ]); + }); + + it("doesn't overwrite previously existing span links", () => { + const previousTraceInfo: PreviousTraceInfo = { + spanContext: { + traceId: '123', + spanId: '456', + traceFlags: 1, + }, + startTimestamp: Date.now() / 1000, + }; + + const startSpanOptions: StartSpanOptions = { + name: '/dashboard', + links: [ + { + context: { + traceId: '789', + spanId: '101', + traceFlags: 1, + }, + attributes: { + someKey: 'someValue', + }, + }, + ], + }; + + const updatedPreviousTraceInfo = addPreviousTraceSpanLink(previousTraceInfo, startSpanOptions); + + expect(updatedPreviousTraceInfo).toBe(previousTraceInfo); + expect(startSpanOptions.links).toEqual([ + { + context: { + traceId: '789', + spanId: '101', + traceFlags: 1, + }, + attributes: { + someKey: 'someValue', + }, + }, + { + attributes: { + 'sentry.link.type': 'previous_trace', + }, + context: { + spanId: '456', + traceFlags: 1, + traceId: '123', + }, + }, + ]); + }); + + it(`doesn't add a previous_trace span link if the previous trace was created more than ${PREVIOUS_TRACE_MAX_DURATION}M ago`, () => { + const previousTraceInfo: PreviousTraceInfo = { + spanContext: { + traceId: '123', + spanId: '456', + traceFlags: 0, + }, + startTimestamp: Date.now() / 1000 - PREVIOUS_TRACE_MAX_DURATION - 1, + }; + + const startSpanOptions: StartSpanOptions = { + name: '/dashboard', + }; + + const updatedPreviousTraceInfo = addPreviousTraceSpanLink(previousTraceInfo, startSpanOptions); + + expect(updatedPreviousTraceInfo).toBeUndefined(); + expect(startSpanOptions.links).toBeUndefined(); + }); +}); + +// TODO: Add tests for sessionstorage helpers diff --git a/packages/core/src/semanticAttributes.ts b/packages/core/src/semanticAttributes.ts index dea57836d3bc..aa25b70f7304 100644 --- a/packages/core/src/semanticAttributes.ts +++ b/packages/core/src/semanticAttributes.ts @@ -57,3 +57,15 @@ export const SEMANTIC_ATTRIBUTE_CACHE_ITEM_SIZE = 'cache.item_size'; /** TODO: Remove these once we update to latest semantic conventions */ export const SEMANTIC_ATTRIBUTE_HTTP_REQUEST_METHOD = 'http.request.method'; export const SEMANTIC_ATTRIBUTE_URL_FULL = 'url.full'; + +/** + * A span link attribute to mark the link as a special span link. + * + * Known values: + * - `previous_trace`: The span links to the frontend root span of the previous trace. + * - `next_trace`: The span links to the frontend root span of the next trace. (Not set by the SDK) + * + * Other values may be set as appropriate. + * @see https://develop.sentry.dev/sdk/telemetry/traces/span-links/#link-types + */ +export const SEMANTIC_LINK_ATTRIBUTE_LINK_TYPE = 'sentry.link.type'; From ee29ca45cc7d1404a98d8a0ba633b1d5e683a3ef Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Mon, 10 Mar 2025 12:28:17 +0100 Subject: [PATCH 02/15] wip --- .../src/tracing/browserTracingIntegration.ts | 37 +++++++++---------- packages/browser/src/tracing/previousTrace.ts | 32 ++++++++-------- 2 files changed, 33 insertions(+), 36 deletions(-) diff --git a/packages/browser/src/tracing/browserTracingIntegration.ts b/packages/browser/src/tracing/browserTracingIntegration.ts index 7faa730914cc..b585fab589f3 100644 --- a/packages/browser/src/tracing/browserTracingIntegration.ts +++ b/packages/browser/src/tracing/browserTracingIntegration.ts @@ -24,6 +24,7 @@ import { getDynamicSamplingContextFromSpan, getIsolationScope, getLocationHref, + getRootSpan, logger, propagationContextFromHeaders, registerSpanErrorInstrumentation, @@ -35,7 +36,6 @@ import { DEBUG_BUILD } from '../debug-build'; import { WINDOW } from '../helpers'; import { registerBackgroundTabDetection } from './backgroundtab'; import { defaultRequestInstrumentationOptions, instrumentOutgoingRequests } from './request'; -import type { PreviousTraceInfo } from './previousTrace'; import { addPreviousTraceSpanLink, getPreviousTraceFromSessionStorage, @@ -274,19 +274,10 @@ export const browserTracingIntegration = ((_options: Partial { + if (getRootSpan(span) !== span) { + return; + } + + previousTraceInfo = addPreviousTraceSpanLink(previousTraceInfo, span); + + if (persistPreviousTrace) { + storePreviousTraceInSessionStorage(previousTraceInfo); + } + }); + } }, }; }) satisfies IntegrationFn; diff --git a/packages/browser/src/tracing/previousTrace.ts b/packages/browser/src/tracing/previousTrace.ts index 6beeb3c0c1d4..b402edb34e06 100644 --- a/packages/browser/src/tracing/previousTrace.ts +++ b/packages/browser/src/tracing/previousTrace.ts @@ -1,4 +1,5 @@ -import { logger, SEMANTIC_LINK_ATTRIBUTE_LINK_TYPE, type SpanContextData, type StartSpanOptions } from '@sentry/core'; +import type { Span } from '@sentry/core'; +import { logger, SEMANTIC_LINK_ATTRIBUTE_LINK_TYPE, spanToJSON, type SpanContextData } from '@sentry/core'; import { WINDOW } from '../exports'; import { DEBUG_BUILD } from '../debug-build'; @@ -25,23 +26,22 @@ const PREVIOUS_TRACE_KEY = 'sentry_previous_trace'; * Returns @param previousTraceInfo if the previous trace is still valid, otherwise returns undefined. */ export function addPreviousTraceSpanLink( - previousTraceInfo: PreviousTraceInfo, - startSpanOptions: StartSpanOptions, -): PreviousTraceInfo | undefined { - if (Date.now() / 1000 - previousTraceInfo.startTimestamp <= PREVIOUS_TRACE_MAX_DURATION) { - startSpanOptions.links = [ - ...(startSpanOptions.links || []), - { - context: previousTraceInfo.spanContext, - attributes: { - [SEMANTIC_LINK_ATTRIBUTE_LINK_TYPE]: 'previous_trace', - }, + previousTraceInfo: PreviousTraceInfo | undefined, + span: Span, +): PreviousTraceInfo { + if (previousTraceInfo && Date.now() / 1000 - previousTraceInfo.startTimestamp <= PREVIOUS_TRACE_MAX_DURATION) { + span.addLink({ + context: previousTraceInfo.spanContext, + attributes: { + [SEMANTIC_LINK_ATTRIBUTE_LINK_TYPE]: 'previous_trace', }, - ]; - } else if (previousTraceInfo) { - return undefined; + }); } - return previousTraceInfo; + + return { + spanContext: span.spanContext(), + startTimestamp: spanToJSON(span).start_timestamp, + }; } /** From a8fba247fa52a96ee6e2082c0de1e0080cbd2c4f Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Wed, 19 Mar 2025 16:39:52 +0100 Subject: [PATCH 03/15] use spanStart client hook instead; adjust tests --- .../previous-trace-links/default/test.ts | 74 +++++++++++-------- .../previous-trace-links/init.js | 1 + .../previous-trace-links/meta/test.ts | 35 +++++---- .../negatively-sampled/test.ts | 47 ++++++------ .../session-storage/test.ts | 62 ++++++++-------- .../src/tracing/browserTracingIntegration.ts | 32 ++++---- packages/browser/src/tracing/previousTrace.ts | 10 ++- packages/core/src/client.ts | 4 +- 8 files changed, 148 insertions(+), 117 deletions(-) diff --git a/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/previous-trace-links/default/test.ts b/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/previous-trace-links/default/test.ts index 5e6fdd0ad48b..bf1d9f78e308 100644 --- a/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/previous-trace-links/default/test.ts +++ b/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/previous-trace-links/default/test.ts @@ -1,13 +1,8 @@ import { expect } from '@playwright/test'; -import { SEMANTIC_LINK_ATTRIBUTE_LINK_TYPE, type Event } from '@sentry/core'; +import { SEMANTIC_LINK_ATTRIBUTE_LINK_TYPE } from '@sentry/core'; import { sentryTest } from '../../../../../utils/fixtures'; -import { - envelopeRequestParser, - getFirstSentryEnvelopeRequest, - shouldSkipTracingTest, - waitForTransactionRequest, -} from '../../../../../utils/helpers'; +import { envelopeRequestParser, shouldSkipTracingTest, waitForTransactionRequest } from '../../../../../utils/helpers'; sentryTest("navigation spans link back to previous trace's root span", async ({ getLocalTestUrl, page }) => { if (shouldSkipTracingTest()) { @@ -16,24 +11,34 @@ sentryTest("navigation spans link back to previous trace's root span", async ({ const url = await getLocalTestUrl({ testDir: __dirname }); - const pageloadRequest = await getFirstSentryEnvelopeRequest(page, url); - const navigationRequest = await getFirstSentryEnvelopeRequest(page, `${url}#foo`); - const navigation2Request = await getFirstSentryEnvelopeRequest(page, `${url}#bar`); + const pageloadTraceContext = await sentryTest.step('Initial pageload', async () => { + const pageloadRequestPromise = waitForTransactionRequest(page, evt => evt.contexts?.trace?.op === 'pageload'); + await page.goto(url); + const pageloadRequest = envelopeRequestParser(await pageloadRequestPromise); + return pageloadRequest.contexts?.trace; + }); - const pageloadTraceContext = pageloadRequest.contexts?.trace; - const navigationTraceContext = navigationRequest.contexts?.trace; - const navigation2TraceContext = navigation2Request.contexts?.trace; + const navigation1TraceContext = await sentryTest.step('First navigation', async () => { + const navigation1RequestPromise = waitForTransactionRequest(page, evt => evt.contexts?.trace?.op === 'navigation'); + await page.goto(`${url}#foo`); + const navigation1Request = envelopeRequestParser(await navigation1RequestPromise); + return navigation1Request.contexts?.trace; + }); + + const navigation2TraceContext = await sentryTest.step('Second navigation', async () => { + const navigation2RequestPromise = waitForTransactionRequest(page, evt => evt.contexts?.trace?.op === 'navigation'); + await page.goto(`${url}#bar`); + const navigation2Request = envelopeRequestParser(await navigation2RequestPromise); + return navigation2Request.contexts?.trace; + }); const pageloadTraceId = pageloadTraceContext?.trace_id; - const navigationTraceId = navigationTraceContext?.trace_id; + const navigation1TraceId = navigation1TraceContext?.trace_id; const navigation2TraceId = navigation2TraceContext?.trace_id; - expect(pageloadTraceContext?.op).toBe('pageload'); - expect(navigationTraceContext?.op).toBe('navigation'); - expect(navigation2TraceContext?.op).toBe('navigation'); - expect(pageloadTraceContext?.links).toBeUndefined(); - expect(navigationTraceContext?.links).toEqual([ + + expect(navigation1TraceContext?.links).toEqual([ { trace_id: pageloadTraceId, span_id: pageloadTraceContext?.span_id, @@ -46,8 +51,8 @@ sentryTest("navigation spans link back to previous trace's root span", async ({ expect(navigation2TraceContext?.links).toEqual([ { - trace_id: navigationTraceId, - span_id: navigationTraceContext?.span_id, + trace_id: navigation1TraceId, + span_id: navigation1TraceContext?.span_id, sampled: true, attributes: { [SEMANTIC_LINK_ATTRIBUTE_LINK_TYPE]: 'previous_trace', @@ -55,8 +60,8 @@ sentryTest("navigation spans link back to previous trace's root span", async ({ }, ]); - expect(pageloadTraceId).not.toEqual(navigationTraceId); - expect(navigationTraceId).not.toEqual(navigation2TraceId); + expect(pageloadTraceId).not.toEqual(navigation1TraceId); + expect(navigation1TraceId).not.toEqual(navigation2TraceId); expect(pageloadTraceId).not.toEqual(navigation2TraceId); }); @@ -67,14 +72,21 @@ sentryTest("doesn't link between hard page reloads by default", async ({ getLoca const url = await getLocalTestUrl({ testDir: __dirname }); - const pageload1Event = await getFirstSentryEnvelopeRequest(page, url); + await sentryTest.step('First pageload', async () => { + const pageloadRequestPromise = waitForTransactionRequest(page, evt => evt.contexts?.trace?.op === 'pageload'); + await page.goto(url); + const pageload1Event = envelopeRequestParser(await pageloadRequestPromise); + + expect(pageload1Event.contexts?.trace).toBeDefined(); + expect(pageload1Event.contexts?.trace?.links).toBeUndefined(); + }); - const pageload2RequestPromise = waitForTransactionRequest(page, evt => evt.contexts?.trace?.op === 'pageload'); - await page.reload(); - const pageload2Event = envelopeRequestParser(await pageload2RequestPromise); + await sentryTest.step('Second pageload', async () => { + const pageload2RequestPromise = waitForTransactionRequest(page, evt => evt.contexts?.trace?.op === 'pageload'); + await page.reload(); + const pageload2Event = envelopeRequestParser(await pageload2RequestPromise); - expect(pageload1Event.contexts?.trace).toBeDefined(); - expect(pageload2Event.contexts?.trace).toBeDefined(); - expect(pageload1Event.contexts?.trace?.links).toBeUndefined(); - expect(pageload2Event.contexts?.trace?.links).toBeUndefined(); + expect(pageload2Event.contexts?.trace).toBeDefined(); + expect(pageload2Event.contexts?.trace?.links).toBeUndefined(); + }); }); diff --git a/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/previous-trace-links/init.js b/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/previous-trace-links/init.js index 83076460599f..1ce4125ee422 100644 --- a/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/previous-trace-links/init.js +++ b/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/previous-trace-links/init.js @@ -6,4 +6,5 @@ Sentry.init({ dsn: 'https://public@dsn.ingest.sentry.io/1337', integrations: [Sentry.browserTracingIntegration()], tracesSampleRate: 1, + debug: true, }); diff --git a/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/previous-trace-links/meta/test.ts b/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/previous-trace-links/meta/test.ts index b0041c0db13a..f5e2c7d613e0 100644 --- a/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/previous-trace-links/meta/test.ts +++ b/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/previous-trace-links/meta/test.ts @@ -1,8 +1,8 @@ import { expect } from '@playwright/test'; -import { SEMANTIC_LINK_ATTRIBUTE_LINK_TYPE, type Event } from '@sentry/core'; +import { SEMANTIC_LINK_ATTRIBUTE_LINK_TYPE } from '@sentry/core'; import { sentryTest } from '../../../../../utils/fixtures'; -import { getFirstSentryEnvelopeRequest, shouldSkipTracingTest } from '../../../../../utils/helpers'; +import { envelopeRequestParser, shouldSkipTracingTest, waitForTransactionRequest } from '../../../../../utils/helpers'; sentryTest( "links back to previous trace's local root span if continued from meta tags", @@ -13,22 +13,31 @@ sentryTest( const url = await getLocalTestUrl({ testDir: __dirname }); - const pageloadRequest = await getFirstSentryEnvelopeRequest(page, url); - const navigationRequest = await getFirstSentryEnvelopeRequest(page, `${url}#foo`); + const metaTagTraceId = '12345678901234567890123456789012'; - const pageloadTraceContext = pageloadRequest.contexts?.trace; - const navigationTraceContext = navigationRequest.contexts?.trace; + const pageloadTraceContext = await sentryTest.step('Initial pageload', async () => { + const pageloadRequestPromise = waitForTransactionRequest(page, evt => evt.contexts?.trace?.op === 'pageload'); + await page.goto(url); + const pageloadRequest = envelopeRequestParser(await pageloadRequestPromise); - const metaTagTraceId = '12345678901234567890123456789012'; + const traceContext = pageloadRequest.contexts?.trace; - const navigationTraceId = navigationTraceContext?.trace_id; + // sanity check + expect(traceContext?.trace_id).toBe(metaTagTraceId); - expect(pageloadTraceContext?.op).toBe('pageload'); - expect(navigationTraceContext?.op).toBe('navigation'); + expect(traceContext?.links).toBeUndefined(); - // sanity check - expect(pageloadTraceContext?.trace_id).toBe(metaTagTraceId); - expect(pageloadTraceContext?.links).toBeUndefined(); + return traceContext; + }); + + const navigationTraceContext = await sentryTest.step('Navigation', async () => { + const navigationRequestPromise = waitForTransactionRequest(page, evt => evt.contexts?.trace?.op === 'navigation'); + await page.goto(`${url}#foo`); + const navigationRequest = envelopeRequestParser(await navigationRequestPromise); + return navigationRequest.contexts?.trace; + }); + + const navigationTraceId = navigationTraceContext?.trace_id; expect(navigationTraceContext?.links).toEqual([ { diff --git a/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/previous-trace-links/negatively-sampled/test.ts b/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/previous-trace-links/negatively-sampled/test.ts index f3d94b1ed28c..2563b22ad701 100644 --- a/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/previous-trace-links/negatively-sampled/test.ts +++ b/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/previous-trace-links/negatively-sampled/test.ts @@ -1,8 +1,8 @@ import { expect } from '@playwright/test'; -import { SEMANTIC_LINK_ATTRIBUTE_LINK_TYPE, type Event } from '@sentry/core'; +import { SEMANTIC_LINK_ATTRIBUTE_LINK_TYPE } from '@sentry/core'; import { sentryTest } from '../../../../../utils/fixtures'; -import { getFirstSentryEnvelopeRequest, shouldSkipTracingTest } from '../../../../../utils/helpers'; +import { envelopeRequestParser, shouldSkipTracingTest, waitForTransactionRequest } from '../../../../../utils/helpers'; sentryTest('includes a span link to a previously negatively sampled span', async ({ getLocalTestUrl, page }) => { if (shouldSkipTracingTest()) { @@ -11,26 +11,29 @@ sentryTest('includes a span link to a previously negatively sampled span', async const url = await getLocalTestUrl({ testDir: __dirname }); - await page.goto(url); - - const navigationRequest = await getFirstSentryEnvelopeRequest(page, `${url}#foo`); - - const navigationTraceContext = navigationRequest.contexts?.trace; - - const navigationTraceId = navigationTraceContext?.trace_id; - - expect(navigationTraceContext?.op).toBe('navigation'); - - expect(navigationTraceContext?.links).toEqual([ - { - trace_id: expect.stringMatching(/[a-f0-9]{32}/), - span_id: expect.stringMatching(/[a-f0-9]{16}/), - sampled: false, - attributes: { - [SEMANTIC_LINK_ATTRIBUTE_LINK_TYPE]: 'previous_trace', + await sentryTest.step('Initial pageload', async () => { + // No event to check for an event here because this pageload span is sampled negatively! + await page.goto(url); + }); + + await sentryTest.step('Navigation', async () => { + const navigationRequestPromise = waitForTransactionRequest(page, evt => evt.contexts?.trace?.op === 'navigation'); + await page.goto(`${url}#foo`); + const navigationEvent = envelopeRequestParser(await navigationRequestPromise); + const navigationTraceContext = navigationEvent.contexts?.trace; + + expect(navigationTraceContext?.op).toBe('navigation'); + expect(navigationTraceContext?.links).toEqual([ + { + trace_id: expect.stringMatching(/[a-f0-9]{32}/), + span_id: expect.stringMatching(/[a-f0-9]{16}/), + sampled: false, + attributes: { + [SEMANTIC_LINK_ATTRIBUTE_LINK_TYPE]: 'previous_trace', + }, }, - }, - ]); + ]); - expect(navigationTraceId).not.toEqual(navigationTraceContext?.links![0].trace_id); + expect(navigationTraceContext?.trace_id).not.toEqual(navigationTraceContext?.links![0].trace_id); + }); }); diff --git a/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/previous-trace-links/session-storage/test.ts b/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/previous-trace-links/session-storage/test.ts index 1b08f2b6ed6d..7489f528a0e8 100644 --- a/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/previous-trace-links/session-storage/test.ts +++ b/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/previous-trace-links/session-storage/test.ts @@ -1,42 +1,40 @@ import { expect } from '@playwright/test'; -import { SEMANTIC_LINK_ATTRIBUTE_LINK_TYPE, type Event } from '@sentry/core'; +import { SEMANTIC_LINK_ATTRIBUTE_LINK_TYPE } from '@sentry/core'; import { sentryTest } from '../../../../../utils/fixtures'; -import { - envelopeRequestParser, - getFirstSentryEnvelopeRequest, - shouldSkipTracingTest, - waitForTransactionRequest, -} from '../../../../../utils/helpers'; +import { envelopeRequestParser, shouldSkipTracingTest, waitForTransactionRequest } from '../../../../../utils/helpers'; -sentryTest( - 'adds link between hard page reload traces when opting into sessionStorage', - async ({ getLocalTestUrl, page }) => { - if (shouldSkipTracingTest()) { - sentryTest.skip(); - } +sentryTest('adds link between hard page reloads when opting into sessionStorage', async ({ getLocalTestUrl, page }) => { + if (shouldSkipTracingTest()) { + sentryTest.skip(); + } - const url = await getLocalTestUrl({ testDir: __dirname }); - - const pageload1Event = await getFirstSentryEnvelopeRequest(page, url); - - const pageload2RequestPromise = waitForTransactionRequest(page, evt => evt.contexts?.trace?.op === 'pageload'); - await page.reload(); - const pageload2Event = envelopeRequestParser(await pageload2RequestPromise); + const url = await getLocalTestUrl({ testDir: __dirname }); + const pageload1TraceContext = await sentryTest.step('First pageload', async () => { + const pageloadRequestPromise = waitForTransactionRequest(page, evt => evt.contexts?.trace?.op === 'pageload'); + await page.goto(url); + const pageload1Event = envelopeRequestParser(await pageloadRequestPromise); const pageload1TraceContext = pageload1Event.contexts?.trace; expect(pageload1TraceContext).toBeDefined(); expect(pageload1TraceContext?.links).toBeUndefined(); + return pageload1TraceContext; + }); - expect(pageload2Event.contexts?.trace?.links).toEqual([ - { - trace_id: pageload1TraceContext?.trace_id, - span_id: pageload1TraceContext?.span_id, - sampled: true, - attributes: { [SEMANTIC_LINK_ATTRIBUTE_LINK_TYPE]: 'previous_trace' }, - }, - ]); - - expect(pageload1TraceContext?.trace_id).not.toEqual(pageload2Event.contexts?.trace?.trace_id); - }, -); + const pageload2Event = await sentryTest.step('Hard page reload', async () => { + const pageload2RequestPromise = waitForTransactionRequest(page, evt => evt.contexts?.trace?.op === 'pageload'); + await page.reload(); + return envelopeRequestParser(await pageload2RequestPromise); + }); + + expect(pageload2Event.contexts?.trace?.links).toEqual([ + { + trace_id: pageload1TraceContext?.trace_id, + span_id: pageload1TraceContext?.span_id, + sampled: true, + attributes: { [SEMANTIC_LINK_ATTRIBUTE_LINK_TYPE]: 'previous_trace' }, + }, + ]); + + expect(pageload1TraceContext?.trace_id).not.toEqual(pageload2Event.contexts?.trace?.trace_id); +}); diff --git a/packages/browser/src/tracing/browserTracingIntegration.ts b/packages/browser/src/tracing/browserTracingIntegration.ts index b585fab589f3..2fe78fb0d503 100644 --- a/packages/browser/src/tracing/browserTracingIntegration.ts +++ b/packages/browser/src/tracing/browserTracingIntegration.ts @@ -385,6 +385,22 @@ export const browserTracingIntegration = ((_options: Partial { + if (getRootSpan(span) !== span) { + return; + } + + previousTraceInfo = addPreviousTraceSpanLink(previousTraceInfo, span); + + if (persistPreviousTrace) { + storePreviousTraceInSessionStorage(previousTraceInfo); + } + }); + } + if (WINDOW.location) { if (instrumentPageLoad) { const origin = browserPerformanceTimeOrigin(); @@ -449,22 +465,6 @@ export const browserTracingIntegration = ((_options: Partial { - if (getRootSpan(span) !== span) { - return; - } - - previousTraceInfo = addPreviousTraceSpanLink(previousTraceInfo, span); - - if (persistPreviousTrace) { - storePreviousTraceInSessionStorage(previousTraceInfo); - } - }); - } }, }; }) satisfies IntegrationFn; diff --git a/packages/browser/src/tracing/previousTrace.ts b/packages/browser/src/tracing/previousTrace.ts index b402edb34e06..8ef7d93227fe 100644 --- a/packages/browser/src/tracing/previousTrace.ts +++ b/packages/browser/src/tracing/previousTrace.ts @@ -29,7 +29,15 @@ export function addPreviousTraceSpanLink( previousTraceInfo: PreviousTraceInfo | undefined, span: Span, ): PreviousTraceInfo { + const spanJson = spanToJSON(span); + if (previousTraceInfo && Date.now() / 1000 - previousTraceInfo.startTimestamp <= PREVIOUS_TRACE_MAX_DURATION) { + if (DEBUG_BUILD) { + logger.info( + `Adding previous_trace ${previousTraceInfo.spanContext} to span ${{ op: spanJson.op, ...span.spanContext() }}`, + ); + } + span.addLink({ context: previousTraceInfo.spanContext, attributes: { @@ -61,7 +69,7 @@ export function storePreviousTraceInSessionStorage(previousTraceInfo: PreviousTr */ export function getPreviousTraceFromSessionStorage(): PreviousTraceInfo | undefined { try { - const previousTraceInfo = WINDOW.sessionStorage.getItem(PREVIOUS_TRACE_KEY); + const previousTraceInfo = WINDOW.sessionStorage?.getItem(PREVIOUS_TRACE_KEY); // @ts-expect-error - intentionally risking JSON.parse throwing when previousTraceInfo is null to save bundle size return JSON.parse(previousTraceInfo); } catch (e) { diff --git a/packages/core/src/client.ts b/packages/core/src/client.ts index af13ea2d691f..a564b9caecef 100644 --- a/packages/core/src/client.ts +++ b/packages/core/src/client.ts @@ -773,8 +773,8 @@ export abstract class Client { */ public emit(hook: string, ...rest: unknown[]): void { const callbacks = this._hooks[hook]; - if (callbacks) { - callbacks.forEach(callback => callback(...rest)); + for (const callback of callbacks || []) { + callback(...rest); } } From b9dba9f788f2a582b9d09c8dacdd506efba79c22 Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Wed, 19 Mar 2025 16:52:38 +0100 Subject: [PATCH 04/15] revert client change --- packages/core/src/client.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/core/src/client.ts b/packages/core/src/client.ts index a564b9caecef..af13ea2d691f 100644 --- a/packages/core/src/client.ts +++ b/packages/core/src/client.ts @@ -773,8 +773,8 @@ export abstract class Client { */ public emit(hook: string, ...rest: unknown[]): void { const callbacks = this._hooks[hook]; - for (const callback of callbacks || []) { - callback(...rest); + if (callbacks) { + callbacks.forEach(callback => callback(...rest)); } } From 8b66941c11265884f1b35e418b4f810626207a78 Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Wed, 19 Mar 2025 18:42:52 +0100 Subject: [PATCH 05/15] account for custom traces and traces with more than 1 root span --- .../custom-trace/subject.js | 15 ++++ .../custom-trace/template.html | 9 ++ .../previous-trace-links/custom-trace/test.ts | 63 +++++++++++++ .../interaction-spans/init.js | 9 ++ .../interaction-spans/template.html | 8 ++ .../interaction-spans/test.ts | 90 +++++++++++++++++++ packages/browser/src/tracing/previousTrace.ts | 27 +++++- 7 files changed, 217 insertions(+), 4 deletions(-) create mode 100644 dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/previous-trace-links/custom-trace/subject.js create mode 100644 dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/previous-trace-links/custom-trace/template.html create mode 100644 dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/previous-trace-links/custom-trace/test.ts create mode 100644 dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/previous-trace-links/interaction-spans/init.js create mode 100644 dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/previous-trace-links/interaction-spans/template.html create mode 100644 dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/previous-trace-links/interaction-spans/test.ts diff --git a/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/previous-trace-links/custom-trace/subject.js b/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/previous-trace-links/custom-trace/subject.js new file mode 100644 index 000000000000..c6fbf085140a --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/previous-trace-links/custom-trace/subject.js @@ -0,0 +1,15 @@ +const btn1 = document.getElementById('btn1'); +const btn2 = document.getElementById('btn2'); + +btn1.addEventListener('click', () => { + Sentry.startNewTrace(() => { + Sentry.startSpan({name: 'custom root span 1', op: 'custom'}, () => {}); + }); +}); + + +btn2.addEventListener('click', () => { + Sentry.startNewTrace(() => { + Sentry.startSpan({name: 'custom root span 2', op: 'custom'}, () => {}); + }); +}); diff --git a/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/previous-trace-links/custom-trace/template.html b/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/previous-trace-links/custom-trace/template.html new file mode 100644 index 000000000000..d5b66b29965d --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/previous-trace-links/custom-trace/template.html @@ -0,0 +1,9 @@ + + + + + + + diff --git a/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/previous-trace-links/custom-trace/test.ts b/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/previous-trace-links/custom-trace/test.ts new file mode 100644 index 000000000000..ab2d8ae2c8af --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/previous-trace-links/custom-trace/test.ts @@ -0,0 +1,63 @@ +import { expect } from '@playwright/test'; +import { SEMANTIC_LINK_ATTRIBUTE_LINK_TYPE } from '@sentry/core'; + +import { sentryTest } from '../../../../../utils/fixtures'; +import { envelopeRequestParser, shouldSkipTracingTest, waitForTransactionRequest } from '../../../../../utils/helpers'; + +sentryTest('manually started custom traces are linked correctly in the chain', async ({ getLocalTestUrl, page }) => { + if (shouldSkipTracingTest()) { + sentryTest.skip(); + } + + const url = await getLocalTestUrl({ testDir: __dirname }); + + const pageloadTraceContext = await sentryTest.step('Initial pageload', async () => { + const pageloadRequestPromise = waitForTransactionRequest(page, evt => evt.contexts?.trace?.op === 'pageload'); + await page.goto(url); + const pageloadRequest = envelopeRequestParser(await pageloadRequestPromise); + return pageloadRequest.contexts?.trace; + }); + + const customTrace1Context = await sentryTest.step('Custom trace', async () => { + const customTrace1RequestPromise = waitForTransactionRequest(page, evt => evt.contexts?.trace?.op === 'custom'); + await page.locator('#btn1').click(); + const customTrace1Event = envelopeRequestParser(await customTrace1RequestPromise); + + const customTraceCtx = customTrace1Event.contexts?.trace; + + expect(customTraceCtx?.trace_id).not.toEqual(pageloadTraceContext?.trace_id); + expect(customTraceCtx?.links).toEqual([ + { + trace_id: pageloadTraceContext?.trace_id, + span_id: pageloadTraceContext?.span_id, + sampled: true, + attributes: { + [SEMANTIC_LINK_ATTRIBUTE_LINK_TYPE]: 'previous_trace', + }, + }, + ]); + + return customTraceCtx; + }); + + await sentryTest.step('Navigation', async () => { + const navigation1RequestPromise = waitForTransactionRequest(page, evt => evt.contexts?.trace?.op === 'navigation'); + await page.goto(`${url}#foo`); + const navigationEvent = envelopeRequestParser(await navigation1RequestPromise); + const navTraceContext = navigationEvent.contexts?.trace; + + expect(navTraceContext?.trace_id).not.toEqual(customTrace1Context?.trace_id); + expect(navTraceContext?.trace_id).not.toEqual(pageloadTraceContext?.trace_id); + + expect(navTraceContext?.links).toEqual([ + { + trace_id: customTrace1Context?.trace_id, + span_id: customTrace1Context?.span_id, + sampled: true, + attributes: { + [SEMANTIC_LINK_ATTRIBUTE_LINK_TYPE]: 'previous_trace', + }, + }, + ]); + }); +}); diff --git a/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/previous-trace-links/interaction-spans/init.js b/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/previous-trace-links/interaction-spans/init.js new file mode 100644 index 000000000000..5d32c7af1e43 --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/previous-trace-links/interaction-spans/init.js @@ -0,0 +1,9 @@ +import * as Sentry from '@sentry/browser'; + +window.Sentry = Sentry; + +Sentry.init({ + dsn: 'https://public@dsn.ingest.sentry.io/1337', + tracesSampleRate: 1, + integrations: [Sentry.browserTracingIntegration({_experiments: {enableInteractions: true}})], +}); diff --git a/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/previous-trace-links/interaction-spans/template.html b/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/previous-trace-links/interaction-spans/template.html new file mode 100644 index 000000000000..05c7fc4b2417 --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/previous-trace-links/interaction-spans/template.html @@ -0,0 +1,8 @@ + + + + + + + diff --git a/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/previous-trace-links/interaction-spans/test.ts b/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/previous-trace-links/interaction-spans/test.ts new file mode 100644 index 000000000000..ca2a184f6680 --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/previous-trace-links/interaction-spans/test.ts @@ -0,0 +1,90 @@ +import { expect } from '@playwright/test'; +import { SEMANTIC_LINK_ATTRIBUTE_LINK_TYPE } from '@sentry/core'; + +import { sentryTest } from '../../../../../utils/fixtures'; +import { envelopeRequestParser, shouldSkipTracingTest, waitForTransactionRequest } from '../../../../../utils/helpers'; + +/* + This is quite peculiar behavior but it's a result of the route-based trace lifetime. + Once we shortened trace lifetime, this whole scenario will change as the interaction + spans will be their own trace. So most likely, we can replace this test with a new one + that covers the new default behavior. +*/ +sentryTest( + 'only the first root spans in the trace link back to the previous trace', + async ({ getLocalTestUrl, page }) => { + if (shouldSkipTracingTest()) { + sentryTest.skip(); + } + + const url = await getLocalTestUrl({ testDir: __dirname }); + + const pageloadTraceContext = await sentryTest.step('Initial pageload', async () => { + const pageloadRequestPromise = waitForTransactionRequest(page, evt => evt.contexts?.trace?.op === 'pageload'); + await page.goto(url); + + const pageloadEvent = envelopeRequestParser(await pageloadRequestPromise); + const traceContext = pageloadEvent.contexts?.trace; + + expect(traceContext).toBeDefined(); + expect(traceContext?.links).toBeUndefined(); + + return traceContext; + }); + + await sentryTest.step('Click Before navigation', async () => { + const interactionRequestPromise = waitForTransactionRequest(page, evt => { + return evt.contexts?.trace?.op === 'ui.action.click'; + }); + await page.click('#btn'); + + const interactionEvent = envelopeRequestParser(await interactionRequestPromise); + const interactionTraceContext = interactionEvent.contexts?.trace; + + // sanity check: route-based trace lifetime means the trace_id should be the same + expect(interactionTraceContext?.trace_id).toBe(pageloadTraceContext?.trace_id); + + // no links yet as previous root span belonged to same trace + expect(interactionTraceContext?.links).toBeUndefined(); + }); + + const navigationTraceContext = await sentryTest.step('Navigation', async () => { + const navigationRequestPromise = waitForTransactionRequest(page, evt => evt.contexts?.trace?.op === 'navigation'); + await page.goto(`${url}#foo`); + const navigationEvent = envelopeRequestParser(await navigationRequestPromise); + + const traceContext = navigationEvent.contexts?.trace; + + expect(traceContext?.op).toBe('navigation'); + expect(traceContext?.links).toEqual([ + { + trace_id: pageloadTraceContext?.trace_id, + span_id: pageloadTraceContext?.span_id, + sampled: true, + attributes: { + [SEMANTIC_LINK_ATTRIBUTE_LINK_TYPE]: 'previous_trace', + }, + }, + ]); + + expect(traceContext?.trace_id).not.toEqual(traceContext?.links![0].trace_id); + return traceContext; + }); + + await sentryTest.step('Click After navigation', async () => { + const interactionRequestPromise = waitForTransactionRequest(page, evt => { + return evt.contexts?.trace?.op === 'ui.action.click'; + }); + await page.click('#btn'); + const interactionEvent = envelopeRequestParser(await interactionRequestPromise); + + const interactionTraceContext = interactionEvent.contexts?.trace; + + // sanity check: route-based trace lifetime means the trace_id should be the same + expect(interactionTraceContext?.trace_id).toBe(navigationTraceContext?.trace_id); + + // since this is the second root span in the trace, it doesn't link back to the previous trace + expect(interactionTraceContext?.links).toBeUndefined(); + }); + }, +); diff --git a/packages/browser/src/tracing/previousTrace.ts b/packages/browser/src/tracing/previousTrace.ts index 8ef7d93227fe..17c4e1b81b65 100644 --- a/packages/browser/src/tracing/previousTrace.ts +++ b/packages/browser/src/tracing/previousTrace.ts @@ -22,8 +22,10 @@ export const PREVIOUS_TRACE_MAX_DURATION = 216_000; const PREVIOUS_TRACE_KEY = 'sentry_previous_trace'; /** - * Adds a previous_trace span link to @param startSpanOptions if the previous trace from @param previousTraceInfo is still valid. - * Returns @param previousTraceInfo if the previous trace is still valid, otherwise returns undefined. + * Adds a previous_trace span link to the passed span if the passed + * previousTraceInfo is still valid. + * + * @returns the updated previous trace info (based on the current span/trace) or `undefined` the */ export function addPreviousTraceSpanLink( previousTraceInfo: PreviousTraceInfo | undefined, @@ -31,10 +33,27 @@ export function addPreviousTraceSpanLink( ): PreviousTraceInfo { const spanJson = spanToJSON(span); - if (previousTraceInfo && Date.now() / 1000 - previousTraceInfo.startTimestamp <= PREVIOUS_TRACE_MAX_DURATION) { + if (!previousTraceInfo) { + return { + spanContext: span.spanContext(), + startTimestamp: spanJson.start_timestamp, + }; + } + + if (previousTraceInfo.spanContext.traceId === spanJson.trace_id) { + // This means, we're still in the same trace so let's not update the previous trace info + // or add a link to the current span. + // Once we move away from the long-lived, route-based trace model, we can remove this cases + return previousTraceInfo; + } + + if (Date.now() / 1000 - previousTraceInfo.startTimestamp <= PREVIOUS_TRACE_MAX_DURATION) { if (DEBUG_BUILD) { logger.info( - `Adding previous_trace ${previousTraceInfo.spanContext} to span ${{ op: spanJson.op, ...span.spanContext() }}`, + `Adding previous_trace ${previousTraceInfo.spanContext} link to span ${{ + op: spanJson.op, + ...span.spanContext(), + }}`, ); } From 94a5f4410ae3c4c19e686c34d5283baf4e31a068 Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Thu, 20 Mar 2025 12:05:47 +0100 Subject: [PATCH 06/15] add more unit tests --- packages/browser/src/tracing/previousTrace.ts | 2 +- .../test/tracing/previousTrace.test.ts | 197 ++++++++++++++---- 2 files changed, 156 insertions(+), 43 deletions(-) diff --git a/packages/browser/src/tracing/previousTrace.ts b/packages/browser/src/tracing/previousTrace.ts index 17c4e1b81b65..ed8fb8df3f31 100644 --- a/packages/browser/src/tracing/previousTrace.ts +++ b/packages/browser/src/tracing/previousTrace.ts @@ -19,7 +19,7 @@ export interface PreviousTraceInfo { export const PREVIOUS_TRACE_MAX_DURATION = 216_000; // session storage key -const PREVIOUS_TRACE_KEY = 'sentry_previous_trace'; +export const PREVIOUS_TRACE_KEY = 'sentry_previous_trace'; /** * Adds a previous_trace span link to the passed span if the passed diff --git a/packages/browser/test/tracing/previousTrace.test.ts b/packages/browser/test/tracing/previousTrace.test.ts index 870abb497ae7..c2fe55fa5af3 100644 --- a/packages/browser/test/tracing/previousTrace.test.ts +++ b/packages/browser/test/tracing/previousTrace.test.ts @@ -1,39 +1,83 @@ -import { describe, it, expect, vi } from 'vitest'; +import { describe, it, expect, beforeEach, vi } from 'vitest'; import type { PreviousTraceInfo } from '../../src/tracing/previousTrace'; -import { addPreviousTraceSpanLink, PREVIOUS_TRACE_MAX_DURATION } from '../../src/tracing/previousTrace'; -import type { StartSpanOptions } from '@sentry/core'; +import { + addPreviousTraceSpanLink, + getPreviousTraceFromSessionStorage, + PREVIOUS_TRACE_KEY, + PREVIOUS_TRACE_MAX_DURATION, +} from '../../src/tracing/previousTrace'; +import { SentrySpan, spanToJSON, timestampInSeconds, type StartSpanOptions } from '@sentry/core'; +import { storePreviousTraceInSessionStorage } from '../../src/tracing/previousTrace'; +import { get } from 'http'; describe('addPreviousTraceSpanLink', () => { - it(`adds a previous_trace span link to startSpanOptions if the previous trace was created within ${PREVIOUS_TRACE_MAX_DURATION}M`, () => { + it(`adds a previous_trace span link to startSpanOptions if the previous trace was created within ${PREVIOUS_TRACE_MAX_DURATION}s`, () => { + const currentSpanStart = timestampInSeconds(); + const previousTraceInfo: PreviousTraceInfo = { spanContext: { traceId: '123', spanId: '456', traceFlags: 1, }, - // max time reached exactly - startTimestamp: Date.now() / 1000 - PREVIOUS_TRACE_MAX_DURATION, + // max time reached almost exactly + startTimestamp: currentSpanStart - PREVIOUS_TRACE_MAX_DURATION + 1, }; - const startSpanOptions: StartSpanOptions = { - name: '/dashboard', - }; + const currentSpan = new SentrySpan({ + name: 'test', + startTimestamp: currentSpanStart, + parentSpanId: '789', + spanId: 'abc', + traceId: 'def', + sampled: true, + }); - const updatedPreviousTraceInfo = addPreviousTraceSpanLink(previousTraceInfo, startSpanOptions); + const updatedPreviousTraceInfo = addPreviousTraceSpanLink(previousTraceInfo, currentSpan); - expect(updatedPreviousTraceInfo).toBe(previousTraceInfo); - expect(startSpanOptions.links).toEqual([ + expect(spanToJSON(currentSpan).links).toEqual([ { attributes: { 'sentry.link.type': 'previous_trace', }, - context: { - spanId: '456', - traceFlags: 1, - traceId: '123', - }, + trace_id: '123', + span_id: '456', + sampled: true, }, ]); + + expect(updatedPreviousTraceInfo).toEqual({ + spanContext: currentSpan.spanContext(), + startTimestamp: currentSpanStart, + }); + }); + + it(`doesn't add a previous_trace span link if the previous trace was created more than ${PREVIOUS_TRACE_MAX_DURATION}s ago`, () => { + const currentSpanStart = timestampInSeconds(); + + const previousTraceInfo: PreviousTraceInfo = { + spanContext: { + traceId: '123', + spanId: '456', + traceFlags: 0, + }, + startTimestamp: Date.now() / 1000 - PREVIOUS_TRACE_MAX_DURATION - 1, + }; + + const currentSpan = new SentrySpan({ + name: '/dashboard', + startTimestamp: currentSpanStart, + }); + + const updatedPreviousTraceInfo = addPreviousTraceSpanLink(previousTraceInfo, currentSpan); + + expect(spanToJSON(currentSpan).links).toBeUndefined(); + + // but still updates the previousTraceInfo to the current span + expect(updatedPreviousTraceInfo).toEqual({ + spanContext: currentSpan.spanContext(), + startTimestamp: currentSpanStart, + }); }); it("doesn't overwrite previously existing span links", () => { @@ -46,7 +90,9 @@ describe('addPreviousTraceSpanLink', () => { startTimestamp: Date.now() / 1000, }; - const startSpanOptions: StartSpanOptions = { + const currentSpanStart = timestampInSeconds(); + + const currentSpan = new SentrySpan({ name: '/dashboard', links: [ { @@ -60,18 +106,16 @@ describe('addPreviousTraceSpanLink', () => { }, }, ], - }; + startTimestamp: currentSpanStart, + }); - const updatedPreviousTraceInfo = addPreviousTraceSpanLink(previousTraceInfo, startSpanOptions); + const updatedPreviousTraceInfo = addPreviousTraceSpanLink(previousTraceInfo, currentSpan); - expect(updatedPreviousTraceInfo).toBe(previousTraceInfo); - expect(startSpanOptions.links).toEqual([ + expect(spanToJSON(currentSpan).links).toEqual([ { - context: { - traceId: '789', - spanId: '101', - traceFlags: 1, - }, + trace_id: '789', + span_id: '101', + sampled: true, attributes: { someKey: 'someValue', }, @@ -80,34 +124,103 @@ describe('addPreviousTraceSpanLink', () => { attributes: { 'sentry.link.type': 'previous_trace', }, - context: { - spanId: '456', - traceFlags: 1, - traceId: '123', - }, + trace_id: '123', + span_id: '456', + sampled: true, }, ]); + + expect(updatedPreviousTraceInfo).toEqual({ + spanContext: currentSpan.spanContext(), + startTimestamp: currentSpanStart, + }); + }); + + it("doesn't add a link and returns the current span's data as previous trace info, if previous trace info was undefined", () => { + const currentSpanStart = timestampInSeconds(); + const currentSpan = new SentrySpan({ name: 'test', startTimestamp: currentSpanStart }); + + const updatedPreviousTraceInfo = addPreviousTraceSpanLink(undefined, currentSpan); + + expect(spanToJSON(currentSpan).links).toBeUndefined(); + + expect(updatedPreviousTraceInfo).toEqual({ + spanContext: currentSpan.spanContext(), + startTimestamp: currentSpanStart, + }); }); - it(`doesn't add a previous_trace span link if the previous trace was created more than ${PREVIOUS_TRACE_MAX_DURATION}M ago`, () => { + it("doesn't add a link and returns the unchanged previous trace info if the current span is part of the same trace", () => { + const currentSpanStart = timestampInSeconds(); + const currentSpan = new SentrySpan({ + name: 'test', + startTimestamp: currentSpanStart, + traceId: '123', + spanId: '456', + }); + const previousTraceInfo: PreviousTraceInfo = { spanContext: { traceId: '123', spanId: '456', - traceFlags: 0, + traceFlags: 1, }, - startTimestamp: Date.now() / 1000 - PREVIOUS_TRACE_MAX_DURATION - 1, + startTimestamp: currentSpanStart - 1, }; - const startSpanOptions: StartSpanOptions = { - name: '/dashboard', - }; + const updatedPreviousTraceInfo = addPreviousTraceSpanLink(previousTraceInfo, currentSpan); - const updatedPreviousTraceInfo = addPreviousTraceSpanLink(previousTraceInfo, startSpanOptions); + expect(spanToJSON(currentSpan).links).toBeUndefined(); - expect(updatedPreviousTraceInfo).toBeUndefined(); - expect(startSpanOptions.links).toBeUndefined(); + expect(updatedPreviousTraceInfo).toBe(previousTraceInfo); }); }); -// TODO: Add tests for sessionstorage helpers +describe('store and retrieve previous trace data via sessionStorage ', () => { + const storage: Record = {}; + const sessionStorageMock = { + setItem: vi.fn((key, value) => { + storage[key] = value; + }), + getItem: vi.fn(key => storage[key]), + }; + + beforeEach(() => { + vi.clearAllMocks(); + // @ts-expect-error - mock contains only necessary API + globalThis.sessionStorage = sessionStorageMock; + }); + + it('stores the previous trace info in sessionStorage', () => { + const previousTraceInfo: PreviousTraceInfo = { + spanContext: { + traceId: '123', + spanId: '456', + traceFlags: 1, + }, + startTimestamp: Date.now() / 1000, + }; + + storePreviousTraceInSessionStorage(previousTraceInfo); + expect(sessionStorageMock.setItem).toHaveBeenCalledWith(PREVIOUS_TRACE_KEY, JSON.stringify(previousTraceInfo)); + expect(getPreviousTraceFromSessionStorage()).toEqual(previousTraceInfo); + }); + + it("doesn't throw if accessing sessionStorage fails and returns undefined", () => { + // @ts-expect-error - this is fine + globalThis.sessionStorage = undefined; + + const previousTraceInfo: PreviousTraceInfo = { + spanContext: { + traceId: '123', + spanId: '456', + traceFlags: 1, + }, + startTimestamp: Date.now() / 1000, + }; + + expect(() => storePreviousTraceInSessionStorage(previousTraceInfo)).not.toThrow(); + expect(getPreviousTraceFromSessionStorage).not.toThrow(); + expect(getPreviousTraceFromSessionStorage()).toBeUndefined(); + }); +}); From 55e3ac37116559297a9959995d3f89b0437995fd Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Thu, 20 Mar 2025 12:06:03 +0100 Subject: [PATCH 07/15] cleanup --- packages/browser/test/tracing/previousTrace.test.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/packages/browser/test/tracing/previousTrace.test.ts b/packages/browser/test/tracing/previousTrace.test.ts index c2fe55fa5af3..f5815cbedc68 100644 --- a/packages/browser/test/tracing/previousTrace.test.ts +++ b/packages/browser/test/tracing/previousTrace.test.ts @@ -6,9 +6,8 @@ import { PREVIOUS_TRACE_KEY, PREVIOUS_TRACE_MAX_DURATION, } from '../../src/tracing/previousTrace'; -import { SentrySpan, spanToJSON, timestampInSeconds, type StartSpanOptions } from '@sentry/core'; +import { SentrySpan, spanToJSON, timestampInSeconds } from '@sentry/core'; import { storePreviousTraceInSessionStorage } from '../../src/tracing/previousTrace'; -import { get } from 'http'; describe('addPreviousTraceSpanLink', () => { it(`adds a previous_trace span link to startSpanOptions if the previous trace was created within ${PREVIOUS_TRACE_MAX_DURATION}s`, () => { From e1630d624c8a974589056a72e81d7b96adf3cac7 Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Thu, 20 Mar 2025 12:51:17 +0100 Subject: [PATCH 08/15] more tests --- packages/browser/src/tracing/previousTrace.ts | 3 +- .../tracing/browserTracingIntegration.test.ts | 70 +++++++++++++++++++ 2 files changed, 72 insertions(+), 1 deletion(-) diff --git a/packages/browser/src/tracing/previousTrace.ts b/packages/browser/src/tracing/previousTrace.ts index ed8fb8df3f31..1b6b8216ec4b 100644 --- a/packages/browser/src/tracing/previousTrace.ts +++ b/packages/browser/src/tracing/previousTrace.ts @@ -25,7 +25,8 @@ export const PREVIOUS_TRACE_KEY = 'sentry_previous_trace'; * Adds a previous_trace span link to the passed span if the passed * previousTraceInfo is still valid. * - * @returns the updated previous trace info (based on the current span/trace) or `undefined` the + * @returns the updated previous trace info (based on the current span/trace) to + * be used on the next call */ export function addPreviousTraceSpanLink( previousTraceInfo: PreviousTraceInfo | undefined, diff --git a/packages/browser/test/tracing/browserTracingIntegration.test.ts b/packages/browser/test/tracing/browserTracingIntegration.test.ts index 3371a7533766..e26c8b9981b0 100644 --- a/packages/browser/test/tracing/browserTracingIntegration.test.ts +++ b/packages/browser/test/tracing/browserTracingIntegration.test.ts @@ -29,6 +29,7 @@ import { spanIsSampled, spanToJSON, startInactiveSpan, + withScope, } from '@sentry/core'; import type { Span, StartSpanOptions } from '@sentry/core'; import { JSDOM } from 'jsdom'; @@ -1028,6 +1029,75 @@ describe('browserTracingIntegration', () => { }); }); + describe('enablePreviousTrace', () => { + it('registers the previous trace listener on span start by default', () => { + const client = new BrowserClient( + getDefaultBrowserClientOptions({ + tracesSampleRate: 1, + integrations: [browserTracingIntegration({ instrumentPageLoad: false, instrumentNavigation: false })], + }), + ); + setCurrentClient(client); + client.init(); + + const span1 = startInactiveSpan({ name: 'test span 1', forceTransaction: true }); + span1.end(); + const span1Json = spanToJSON(span1); + + expect(span1Json.links).toBeUndefined(); + + // ensure we start a new trace + getCurrentScope().setPropagationContext({ traceId: '123', sampleRand: 0.2 }); + + const span2 = startInactiveSpan({ name: 'test span 2', forceTransaction: true }); + span2.end(); + const spanJson2 = spanToJSON(span2); + + expect(spanJson2.links).toEqual([ + { + attributes: { + 'sentry.link.type': 'previous_trace', + }, + sampled: true, + span_id: span1Json.span_id, + trace_id: span1Json.trace_id, + }, + ]); + }); + + it("doesn't register the previous trace listener on span start if disabled", () => { + const client = new BrowserClient( + getDefaultBrowserClientOptions({ + tracesSampleRate: 1, + integrations: [ + browserTracingIntegration({ + instrumentPageLoad: false, + instrumentNavigation: false, + enablePreviousTrace: false, + }), + ], + }), + ); + setCurrentClient(client); + client.init(); + + const span1 = startInactiveSpan({ name: 'test span 1', forceTransaction: true }); + span1.end(); + const span1Json = spanToJSON(span1); + + expect(span1Json.links).toBeUndefined(); + + // ensure we start a new trace + getCurrentScope().setPropagationContext({ traceId: '123', sampleRand: 0.2 }); + + const span2 = startInactiveSpan({ name: 'test span 2', forceTransaction: true }); + span2.end(); + const spanJson2 = spanToJSON(span2); + + expect(spanJson2.links).toBeUndefined(); + }); + }); + // TODO(lforst): I cannot manage to get this test to pass. /* it('heartbeatInterval can be a custom value', () => { From 0a6ca94e1862ae697d260b702b4e554febce5d1d Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Thu, 20 Mar 2025 13:20:00 +0100 Subject: [PATCH 09/15] size limit --- .size-limit.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.size-limit.js b/.size-limit.js index 203e8cecb6ce..7ecd54ab92f4 100644 --- a/.size-limit.js +++ b/.size-limit.js @@ -47,7 +47,7 @@ module.exports = [ path: 'packages/browser/build/npm/esm/index.js', import: createImport('init', 'browserTracingIntegration', 'replayIntegration'), gzip: true, - limit: '75.5 KB', + limit: '76 KB', }, { name: '@sentry/browser (incl. Tracing, Replay) - with treeshaking flags', From 3320e74bc09b40a3c6a6115a9733e99406da4286 Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Thu, 20 Mar 2025 13:20:45 +0100 Subject: [PATCH 10/15] lint --- packages/browser/test/tracing/browserTracingIntegration.test.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/browser/test/tracing/browserTracingIntegration.test.ts b/packages/browser/test/tracing/browserTracingIntegration.test.ts index e26c8b9981b0..81534b33eb3c 100644 --- a/packages/browser/test/tracing/browserTracingIntegration.test.ts +++ b/packages/browser/test/tracing/browserTracingIntegration.test.ts @@ -29,7 +29,6 @@ import { spanIsSampled, spanToJSON, startInactiveSpan, - withScope, } from '@sentry/core'; import type { Span, StartSpanOptions } from '@sentry/core'; import { JSDOM } from 'jsdom'; From 6c42afd4be107a591b3f05ae097f42116ea36346 Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Thu, 20 Mar 2025 13:31:07 +0100 Subject: [PATCH 11/15] fix e2e tests --- .../tests/client-transactions.test.ts | 14 ++++++++++++-- .../tests/transactions.test.ts | 10 ++++++++++ .../tests/transactions.test.ts | 10 ++++++++++ .../tests/transactions.test.ts | 10 ++++++++++ 4 files changed, 42 insertions(+), 2 deletions(-) diff --git a/dev-packages/e2e-tests/test-applications/create-next-app/tests/client-transactions.test.ts b/dev-packages/e2e-tests/test-applications/create-next-app/tests/client-transactions.test.ts index 8721a4d086bb..cbb2cae29265 100644 --- a/dev-packages/e2e-tests/test-applications/create-next-app/tests/client-transactions.test.ts +++ b/dev-packages/e2e-tests/test-applications/create-next-app/tests/client-transactions.test.ts @@ -43,7 +43,7 @@ test('Sends a pageload transaction to Sentry', async ({ page }) => { ); }); -test('captures a navigation transcation to Sentry', async ({ page }) => { +test('captures a navigation transaction to Sentry', async ({ page }) => { const clientNavigationTxnEventPromise = waitForTransaction('create-next-app', txnEvent => { return txnEvent?.transaction === '/user/[id]'; }); @@ -53,7 +53,7 @@ test('captures a navigation transcation to Sentry', async ({ page }) => { // navigation to page const clickPromise = page.getByText('navigate').click(); - const [clientTxnEvent, serverTxnEvent, _1] = await Promise.all([clientNavigationTxnEventPromise, clickPromise]); + const [clientTxnEvent, serverTxnEvent] = await Promise.all([clientNavigationTxnEventPromise, clickPromise]); expect(clientTxnEvent).toEqual( expect.objectContaining({ @@ -76,6 +76,16 @@ test('captures a navigation transcation to Sentry', async ({ page }) => { 'sentry.sample_rate': 1, 'sentry.source': 'route', }), + links: [ + { + attributes: { + 'sentry.link.type': 'previous_trace', + }, + sampled: true, + span_id: expect.stringMatching(/[a-f0-9]{16}/), + trace_id: expect.stringMatching(/[a-f0-9]{32}/), + }, + ], }, }, request: { diff --git a/dev-packages/e2e-tests/test-applications/react-create-browser-router/tests/transactions.test.ts b/dev-packages/e2e-tests/test-applications/react-create-browser-router/tests/transactions.test.ts index c35d731915d6..0529a7debb63 100644 --- a/dev-packages/e2e-tests/test-applications/react-create-browser-router/tests/transactions.test.ts +++ b/dev-packages/e2e-tests/test-applications/react-create-browser-router/tests/transactions.test.ts @@ -145,6 +145,16 @@ test('Captures a lazy navigation transaction', async ({ page }) => { 'sentry.sample_rate': 1, 'sentry.source': 'route', }), + links: [ + { + attributes: { + 'sentry.link.type': 'previous_trace', + }, + sampled: true, + span_id: expect.stringMatching(/[a-f0-9]{16}/), + trace_id: expect.stringMatching(/[a-f0-9]{32}/), + }, + ], op: 'navigation', span_id: expect.stringMatching(/[a-f0-9]{16}/), trace_id: expect.stringMatching(/[a-f0-9]{32}/), diff --git a/dev-packages/e2e-tests/test-applications/react-create-hash-router/tests/transactions.test.ts b/dev-packages/e2e-tests/test-applications/react-create-hash-router/tests/transactions.test.ts index 7abb269d15b0..920506838080 100644 --- a/dev-packages/e2e-tests/test-applications/react-create-hash-router/tests/transactions.test.ts +++ b/dev-packages/e2e-tests/test-applications/react-create-hash-router/tests/transactions.test.ts @@ -122,6 +122,16 @@ test('Captures a navigation transaction', async ({ page }) => { 'sentry.sample_rate': 1, 'sentry.source': 'route', }), + links: [ + { + attributes: { + 'sentry.link.type': 'previous_trace', + }, + sampled: true, + span_id: expect.stringMatching(/[a-f0-9]{16}/), + trace_id: expect.stringMatching(/[a-f0-9]{32}/), + }, + ], op: 'navigation', span_id: expect.stringMatching(/[a-f0-9]{16}/), trace_id: expect.stringMatching(/[a-f0-9]{32}/), diff --git a/dev-packages/e2e-tests/test-applications/react-create-memory-router/tests/transactions.test.ts b/dev-packages/e2e-tests/test-applications/react-create-memory-router/tests/transactions.test.ts index 7c75c395c3af..c5d53a26af86 100644 --- a/dev-packages/e2e-tests/test-applications/react-create-memory-router/tests/transactions.test.ts +++ b/dev-packages/e2e-tests/test-applications/react-create-memory-router/tests/transactions.test.ts @@ -55,6 +55,16 @@ test('Captures a navigation transaction', async ({ page }) => { 'sentry.sample_rate': 1, 'sentry.source': 'route', }), + links: [ + { + attributes: { + 'sentry.link.type': 'previous_trace', + }, + sampled: true, + span_id: 'ab3fd8c9ce9d134a', + trace_id: 'ee68b12db60a4aca9da1ac2ceac3f55d', + }, + ], op: 'navigation', span_id: expect.stringMatching(/[a-f0-9]{16}/), trace_id: expect.stringMatching(/[a-f0-9]{32}/), From b44e790babb1f8d4365f667047d6afa309ebb044 Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Fri, 21 Mar 2025 10:50:03 +0100 Subject: [PATCH 12/15] change to only one option (`linkPreviousTrace`) --- .../session-storage/init.js | 2 +- .../src/tracing/browserTracingIntegration.ts | 50 ++++++++++--------- .../tracing/browserTracingIntegration.test.ts | 4 +- 3 files changed, 30 insertions(+), 26 deletions(-) diff --git a/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/previous-trace-links/session-storage/init.js b/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/previous-trace-links/session-storage/init.js index c278e15a1883..c346a6e054b3 100644 --- a/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/previous-trace-links/session-storage/init.js +++ b/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/previous-trace-links/session-storage/init.js @@ -4,6 +4,6 @@ window.Sentry = Sentry; Sentry.init({ dsn: 'https://public@dsn.ingest.sentry.io/1337', - integrations: [Sentry.browserTracingIntegration({persistPreviousTrace: true})], + integrations: [Sentry.browserTracingIntegration({linkPreviousTrace: 'session-storage'})], tracesSampleRate: 1, }); diff --git a/packages/browser/src/tracing/browserTracingIntegration.ts b/packages/browser/src/tracing/browserTracingIntegration.ts index 2fe78fb0d503..062b308527d6 100644 --- a/packages/browser/src/tracing/browserTracingIntegration.ts +++ b/packages/browser/src/tracing/browserTracingIntegration.ts @@ -36,6 +36,7 @@ import { DEBUG_BUILD } from '../debug-build'; import { WINDOW } from '../helpers'; import { registerBackgroundTabDetection } from './backgroundtab'; import { defaultRequestInstrumentationOptions, instrumentOutgoingRequests } from './request'; +import type { PreviousTraceInfo } from './previousTrace'; import { addPreviousTraceSpanLink, getPreviousTraceFromSessionStorage, @@ -149,23 +150,27 @@ export interface BrowserTracingOptions { enableHTTPTimings: boolean; /** - * If enabled, previously started traces (e.g. pageload or navigation spans) will be linked - * to the current trace. This lets you navigate across traces within a user journey in the - * Sentry UI. + * Link the currently started trace to a previous trace (e.g. a prior pageload, navigation or + * manually started span). When enabled, this option will allow you to navigate between traces + * in the Sentry UI. * - * Set `persistPreviousTrace` to `true` to connect traces across hard page reloads. + * You can set this option to the following values: * - * @default true, this is turned on by default. - */ - enablePreviousTrace?: boolean; - - /** - * If set to true, the previous trace will be stored in `sessionStorage`, so that - * traces can be linked across hard page refreshes. + * - `'in-memory'`: The previous trace data will be stored in memory. + * This is useful for single-page applications and enabled by default. + * + * - `'session-storage'`: The previous trace data will be stored in the `sessionStorage`. + * This is useful for multi-page applications or static sites but it means that the + * Sentry SDK writes to the browser's `sessionStorage`. * - * @default false, by default, previous trace data is only stored in-memory. + * - `'off'`: The previous trace data will not be stored or linked. + * + * Note that your `tracesSampleRate` or `tracesSampler` config significantly influences + * how often traces will be linked. + * + * @default 'in-memory' - see explanation above */ - persistPreviousTrace?: boolean; + linkPreviousTrace: 'in-memory' | 'session-storage' | 'off'; /** * _experiments allows the user to send options to define how this integration works. @@ -200,8 +205,7 @@ const DEFAULT_BROWSER_TRACING_OPTIONS: BrowserTracingOptions = { enableLongTask: true, enableLongAnimationFrame: true, enableInp: true, - enablePreviousTrace: true, - persistPreviousTrace: false, + linkPreviousTrace: 'in-memory', _experiments: {}, ...defaultRequestInstrumentationOptions, }; @@ -241,8 +245,7 @@ export const browserTracingIntegration = ((_options: Partial { if (getRootSpan(span) !== span) { return; } - previousTraceInfo = addPreviousTraceSpanLink(previousTraceInfo, span); - - if (persistPreviousTrace) { - storePreviousTraceInSessionStorage(previousTraceInfo); + if (linkPreviousTrace === 'session-storage') { + const updatedPreviousTraceInfo = addPreviousTraceSpanLink(getPreviousTraceFromSessionStorage(), span); + storePreviousTraceInSessionStorage(updatedPreviousTraceInfo); + } else { + inMemoryPreviousTraceInfo = addPreviousTraceSpanLink(inMemoryPreviousTraceInfo, span); } }); } diff --git a/packages/browser/test/tracing/browserTracingIntegration.test.ts b/packages/browser/test/tracing/browserTracingIntegration.test.ts index 81534b33eb3c..fe524bbf0895 100644 --- a/packages/browser/test/tracing/browserTracingIntegration.test.ts +++ b/packages/browser/test/tracing/browserTracingIntegration.test.ts @@ -528,7 +528,7 @@ describe('browserTracingIntegration', () => { browserTracingIntegration({ instrumentNavigation: false, // disabling previous trace b/c not relevant for this test - enablePreviousTrace: false, + linkPreviousTrace: 'off', }), ], }), @@ -1072,7 +1072,7 @@ describe('browserTracingIntegration', () => { browserTracingIntegration({ instrumentPageLoad: false, instrumentNavigation: false, - enablePreviousTrace: false, + linkPreviousTrace: 'off', }), ], }), From 5e8300b45977b6eac1b1b0b7a75ed9fcdbeb575a Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Fri, 21 Mar 2025 10:51:55 +0100 Subject: [PATCH 13/15] fix 2 more e2e tests --- .../tests/transactions.test.ts | 10 ++++++++++ .../tests/transactions.test.ts | 4 ++-- 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/dev-packages/e2e-tests/test-applications/react-create-browser-router/tests/transactions.test.ts b/dev-packages/e2e-tests/test-applications/react-create-browser-router/tests/transactions.test.ts index 0529a7debb63..ee0c507076fa 100644 --- a/dev-packages/e2e-tests/test-applications/react-create-browser-router/tests/transactions.test.ts +++ b/dev-packages/e2e-tests/test-applications/react-create-browser-router/tests/transactions.test.ts @@ -58,6 +58,16 @@ test('Captures a navigation transaction', async ({ page }) => { 'sentry.sample_rate': 1, 'sentry.source': 'route', }), + links: [ + { + attributes: { + 'sentry.link.type': 'previous_trace', + }, + sampled: true, + span_id: expect.stringMatching(/[a-f0-9]{16}/), + trace_id: expect.stringMatching(/[a-f0-9]{32}/), + }, + ], op: 'navigation', span_id: expect.stringMatching(/[a-f0-9]{16}/), trace_id: expect.stringMatching(/[a-f0-9]{32}/), diff --git a/dev-packages/e2e-tests/test-applications/react-create-memory-router/tests/transactions.test.ts b/dev-packages/e2e-tests/test-applications/react-create-memory-router/tests/transactions.test.ts index c5d53a26af86..61a583a7bf55 100644 --- a/dev-packages/e2e-tests/test-applications/react-create-memory-router/tests/transactions.test.ts +++ b/dev-packages/e2e-tests/test-applications/react-create-memory-router/tests/transactions.test.ts @@ -61,8 +61,8 @@ test('Captures a navigation transaction', async ({ page }) => { 'sentry.link.type': 'previous_trace', }, sampled: true, - span_id: 'ab3fd8c9ce9d134a', - trace_id: 'ee68b12db60a4aca9da1ac2ceac3f55d', + span_id: expect.stringMatching(/[a-f0-9]{16}/), + trace_id: expect.stringMatching(/[a-f0-9]{32}/), }, ], op: 'navigation', From be5b5261bd3c3352b88e8721218e0876b28bda40 Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Fri, 21 Mar 2025 10:55:37 +0100 Subject: [PATCH 14/15] add comment for max duratio --- packages/browser/src/tracing/previousTrace.ts | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/packages/browser/src/tracing/previousTrace.ts b/packages/browser/src/tracing/previousTrace.ts index 1b6b8216ec4b..36b6936abf3d 100644 --- a/packages/browser/src/tracing/previousTrace.ts +++ b/packages/browser/src/tracing/previousTrace.ts @@ -48,6 +48,11 @@ export function addPreviousTraceSpanLink( return previousTraceInfo; } + // Only add the link if the startTimeStamp of the previous trace's root span is within + // PREVIOUS_TRACE_MAX_DURATION (1h) of the current root span's startTimestamp + // This is done to + // - avoid adding links to "stale" traces + // - enable more efficient querying for previous/next traces in Sentry if (Date.now() / 1000 - previousTraceInfo.startTimestamp <= PREVIOUS_TRACE_MAX_DURATION) { if (DEBUG_BUILD) { logger.info( From 0a465f88caf2c094d341288c4b0de49f9d27ac34 Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Fri, 21 Mar 2025 10:58:41 +0100 Subject: [PATCH 15/15] cleanup --- packages/browser/test/tracing/browserTracingIntegration.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/browser/test/tracing/browserTracingIntegration.test.ts b/packages/browser/test/tracing/browserTracingIntegration.test.ts index fe524bbf0895..ee43935cd531 100644 --- a/packages/browser/test/tracing/browserTracingIntegration.test.ts +++ b/packages/browser/test/tracing/browserTracingIntegration.test.ts @@ -1028,7 +1028,7 @@ describe('browserTracingIntegration', () => { }); }); - describe('enablePreviousTrace', () => { + describe('linkPreviousTrace', () => { it('registers the previous trace listener on span start by default', () => { const client = new BrowserClient( getDefaultBrowserClientOptions({