From d25b179f0dac03078b5c480f4d0ea46f550ba1bc Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Wed, 2 Apr 2025 12:33:10 +0200 Subject: [PATCH 1/5] ref(browser): Temporarily add `sentry.previous_trace` span attribute --- .../previous-trace-links/default/test.ts | 8 +++++++ .../test-applications/vue-3/src/main.ts | 5 ++++ packages/browser/src/tracing/previousTrace.ts | 19 ++++++++++++++- .../test/tracing/previousTrace.test.ts | 23 +++++++++++++++---- 4 files changed, 50 insertions(+), 5 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 bf1d9f78e308..f36aa22e6a12 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 @@ -49,6 +49,10 @@ sentryTest("navigation spans link back to previous trace's root span", async ({ }, ]); + expect(navigation1TraceContext?.data).toMatchObject({ + 'sentry.previous_trace': `${pageloadTraceId}-${pageloadTraceContext?.span_id}-1`, + }); + expect(navigation2TraceContext?.links).toEqual([ { trace_id: navigation1TraceId, @@ -60,6 +64,10 @@ sentryTest("navigation spans link back to previous trace's root span", async ({ }, ]); + expect(navigation2TraceContext?.data).toMatchObject({ + 'sentry.previous_trace': `${navigation1TraceId}-${navigation1TraceContext?.span_id}-1`, + }); + expect(pageloadTraceId).not.toEqual(navigation1TraceId); expect(navigation1TraceId).not.toEqual(navigation2TraceId); expect(pageloadTraceId).not.toEqual(navigation2TraceId); diff --git a/dev-packages/e2e-tests/test-applications/vue-3/src/main.ts b/dev-packages/e2e-tests/test-applications/vue-3/src/main.ts index 340820d333ec..ac5e4a4fef76 100644 --- a/dev-packages/e2e-tests/test-applications/vue-3/src/main.ts +++ b/dev-packages/e2e-tests/test-applications/vue-3/src/main.ts @@ -24,6 +24,11 @@ Sentry.init({ }), browserTracingIntegration({ router, + instrumentPageLoad: false, + instrumentNavigation: false, + enableInp: false, + enableLongTask: false, + enableLongAnimationFrame: false, }), ], tunnel: `http://localhost:3031/`, // proxy server diff --git a/packages/browser/src/tracing/previousTrace.ts b/packages/browser/src/tracing/previousTrace.ts index fd968c5a5cc3..0d8b5b766e81 100644 --- a/packages/browser/src/tracing/previousTrace.ts +++ b/packages/browser/src/tracing/previousTrace.ts @@ -1,5 +1,11 @@ import type { Span } from '@sentry/core'; -import { logger, SEMANTIC_LINK_ATTRIBUTE_LINK_TYPE, spanToJSON, type SpanContextData } from '@sentry/core'; +import { + logger, + SEMANTIC_LINK_ATTRIBUTE_LINK_TYPE, + spanIsSampled, + spanToJSON, + type SpanContextData, +} from '@sentry/core'; import { WINDOW } from '../exports'; import { DEBUG_BUILD } from '../debug-build'; @@ -21,6 +27,8 @@ export const PREVIOUS_TRACE_MAX_DURATION = 3600; // session storage key export const PREVIOUS_TRACE_KEY = 'sentry_previous_trace'; +export const PREVIOUS_TRACE_TMP_SPAN_ATTRIBUTE = 'sentry.previous_trace'; + /** * Adds a previous_trace span link to the passed span if the passed * previousTraceInfo is still valid. @@ -69,6 +77,15 @@ export function addPreviousTraceSpanLink( [SEMANTIC_LINK_ATTRIBUTE_LINK_TYPE]: 'previous_trace', }, }); + + // TODO: Remove this once EAP can store span links. We currently only set this attribute so that we + // can obtain the previous trace information from the EAP store. Long-term, EAP will handle + // span links and then we should remove this again. Also throwing in a TODO(v10), to remind us + // to check this at v10 time :) + span.setAttribute( + PREVIOUS_TRACE_TMP_SPAN_ATTRIBUTE, + `${previousTraceInfo.spanContext.traceId}-${previousTraceInfo.spanContext.spanId}-${spanIsSampled(span) ? 1 : 0}`, + ); } return { diff --git a/packages/browser/test/tracing/previousTrace.test.ts b/packages/browser/test/tracing/previousTrace.test.ts index f5815cbedc68..e3e3e3cc597e 100644 --- a/packages/browser/test/tracing/previousTrace.test.ts +++ b/packages/browser/test/tracing/previousTrace.test.ts @@ -5,6 +5,7 @@ import { getPreviousTraceFromSessionStorage, PREVIOUS_TRACE_KEY, PREVIOUS_TRACE_MAX_DURATION, + PREVIOUS_TRACE_TMP_SPAN_ATTRIBUTE, } from '../../src/tracing/previousTrace'; import { SentrySpan, spanToJSON, timestampInSeconds } from '@sentry/core'; import { storePreviousTraceInSessionStorage } from '../../src/tracing/previousTrace'; @@ -34,7 +35,9 @@ describe('addPreviousTraceSpanLink', () => { const updatedPreviousTraceInfo = addPreviousTraceSpanLink(previousTraceInfo, currentSpan); - expect(spanToJSON(currentSpan).links).toEqual([ + const spanJson = spanToJSON(currentSpan); + + expect(spanJson.links).toEqual([ { attributes: { 'sentry.link.type': 'previous_trace', @@ -45,6 +48,10 @@ describe('addPreviousTraceSpanLink', () => { }, ]); + expect(spanJson.data).toMatchObject({ + [PREVIOUS_TRACE_TMP_SPAN_ATTRIBUTE]: '123-456-1', + }); + expect(updatedPreviousTraceInfo).toEqual({ spanContext: currentSpan.spanContext(), startTimestamp: currentSpanStart, @@ -70,7 +77,11 @@ describe('addPreviousTraceSpanLink', () => { const updatedPreviousTraceInfo = addPreviousTraceSpanLink(previousTraceInfo, currentSpan); - expect(spanToJSON(currentSpan).links).toBeUndefined(); + const spanJson = spanToJSON(currentSpan); + + expect(spanJson.links).toBeUndefined(); + + expect(Object.keys(spanJson.data)).not.toContain(PREVIOUS_TRACE_TMP_SPAN_ATTRIBUTE); // but still updates the previousTraceInfo to the current span expect(updatedPreviousTraceInfo).toEqual({ @@ -141,7 +152,9 @@ describe('addPreviousTraceSpanLink', () => { const updatedPreviousTraceInfo = addPreviousTraceSpanLink(undefined, currentSpan); - expect(spanToJSON(currentSpan).links).toBeUndefined(); + const spanJson = spanToJSON(currentSpan); + expect(spanJson.links).toBeUndefined(); + expect(Object.keys(spanJson.data)).not.toContain(PREVIOUS_TRACE_TMP_SPAN_ATTRIBUTE); expect(updatedPreviousTraceInfo).toEqual({ spanContext: currentSpan.spanContext(), @@ -169,7 +182,9 @@ describe('addPreviousTraceSpanLink', () => { const updatedPreviousTraceInfo = addPreviousTraceSpanLink(previousTraceInfo, currentSpan); - expect(spanToJSON(currentSpan).links).toBeUndefined(); + const spanJson = spanToJSON(currentSpan); + expect(spanJson.links).toBeUndefined(); + expect(Object.keys(spanJson.data)).not.toContain(PREVIOUS_TRACE_TMP_SPAN_ATTRIBUTE); expect(updatedPreviousTraceInfo).toBe(previousTraceInfo); }); From bac5dfb7e4de3f1bf4ddd0a4812e63c4f5032a69 Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Wed, 2 Apr 2025 12:50:12 +0200 Subject: [PATCH 2/5] fix sampled flag --- .../negatively-sampled/test.ts | 4 ++++ packages/browser/src/tracing/previousTrace.ts | 19 ++++++++----------- 2 files changed, 12 insertions(+), 11 deletions(-) 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 2563b22ad701..6c0ec874684b 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 @@ -34,6 +34,10 @@ sentryTest('includes a span link to a previously negatively sampled span', async }, ]); + expect(navigationTraceContext?.data).toMatchObject({ + 'sentry.previous_trace': expect.stringMatching(/[a-f0-9]{32}-[a-f0-9]{16}-0/), + }); + expect(navigationTraceContext?.trace_id).not.toEqual(navigationTraceContext?.links![0].trace_id); }); }); diff --git a/packages/browser/src/tracing/previousTrace.ts b/packages/browser/src/tracing/previousTrace.ts index 0d8b5b766e81..91e52b519dad 100644 --- a/packages/browser/src/tracing/previousTrace.ts +++ b/packages/browser/src/tracing/previousTrace.ts @@ -1,11 +1,5 @@ import type { Span } from '@sentry/core'; -import { - logger, - SEMANTIC_LINK_ATTRIBUTE_LINK_TYPE, - spanIsSampled, - spanToJSON, - type SpanContextData, -} 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'; @@ -49,7 +43,8 @@ export function addPreviousTraceSpanLink( }; } - if (previousTraceInfo.spanContext.traceId === spanJson.trace_id) { + const previousTraceSpanCtx = previousTraceInfo.spanContext; + if (previousTraceSpanCtx.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 @@ -64,7 +59,7 @@ export function addPreviousTraceSpanLink( if (Date.now() / 1000 - previousTraceInfo.startTimestamp <= PREVIOUS_TRACE_MAX_DURATION) { if (DEBUG_BUILD) { logger.info( - `Adding previous_trace ${previousTraceInfo.spanContext} link to span ${{ + `Adding previous_trace ${previousTraceSpanCtx} link to span ${{ op: spanJson.op, ...span.spanContext(), }}`, @@ -72,7 +67,7 @@ export function addPreviousTraceSpanLink( } span.addLink({ - context: previousTraceInfo.spanContext, + context: previousTraceSpanCtx, attributes: { [SEMANTIC_LINK_ATTRIBUTE_LINK_TYPE]: 'previous_trace', }, @@ -84,7 +79,9 @@ export function addPreviousTraceSpanLink( // to check this at v10 time :) span.setAttribute( PREVIOUS_TRACE_TMP_SPAN_ATTRIBUTE, - `${previousTraceInfo.spanContext.traceId}-${previousTraceInfo.spanContext.spanId}-${spanIsSampled(span) ? 1 : 0}`, + `${previousTraceSpanCtx.traceId}-${previousTraceSpanCtx.spanId}-${ + previousTraceSpanCtx.traceFlags === 0x1 ? 1 : 0 + }`, ); } From de9427248fae8ad65d65d46ae47872531cd479b6 Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Wed, 2 Apr 2025 12:53:35 +0200 Subject: [PATCH 3/5] fix unit tests --- .../browser/test/tracing/browserTracingIntegration.test.ts | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/packages/browser/test/tracing/browserTracingIntegration.test.ts b/packages/browser/test/tracing/browserTracingIntegration.test.ts index fe9005d47215..6fd155b36e35 100644 --- a/packages/browser/test/tracing/browserTracingIntegration.test.ts +++ b/packages/browser/test/tracing/browserTracingIntegration.test.ts @@ -40,6 +40,7 @@ import { startBrowserTracingPageLoadSpan, } from '../../src/tracing/browserTracingIntegration'; import { getDefaultBrowserClientOptions } from '../helper/browser-client-options'; +import { PREVIOUS_TRACE_TMP_SPAN_ATTRIBUTE } from '../../src/tracing/previousTrace'; // We're setting up JSDom here because the Next.js routing instrumentations requires a few things to be present on pageload: // 1. Access to window.document API for `window.document.getElementById` @@ -202,6 +203,7 @@ describe('browserTracingIntegration', () => { [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.navigation.browser', [SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE]: 1, [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'url', + [PREVIOUS_TRACE_TMP_SPAN_ATTRIBUTE]: `${span?.spanContext().traceId}-${span?.spanContext().spanId}-1`, }, links: [ { @@ -239,6 +241,7 @@ describe('browserTracingIntegration', () => { [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.navigation.browser', [SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE]: 1, [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'url', + [PREVIOUS_TRACE_TMP_SPAN_ATTRIBUTE]: `${span2?.spanContext().traceId}-${span2?.spanContext().spanId}-1`, }, links: [ { @@ -502,6 +505,7 @@ describe('browserTracingIntegration', () => { [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'manual', [SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE]: 1, [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'custom', + [PREVIOUS_TRACE_TMP_SPAN_ATTRIBUTE]: expect.stringMatching(/[a-f0-9]{32}-[a-f0-9]{16}-1/), }, links: [ { From af7affc78336e07d13c18568118c121b4e84afc1 Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Wed, 2 Apr 2025 16:26:12 +0200 Subject: [PATCH 4/5] revert unintended change --- dev-packages/e2e-tests/test-applications/vue-3/src/main.ts | 5 ----- 1 file changed, 5 deletions(-) diff --git a/dev-packages/e2e-tests/test-applications/vue-3/src/main.ts b/dev-packages/e2e-tests/test-applications/vue-3/src/main.ts index ac5e4a4fef76..340820d333ec 100644 --- a/dev-packages/e2e-tests/test-applications/vue-3/src/main.ts +++ b/dev-packages/e2e-tests/test-applications/vue-3/src/main.ts @@ -24,11 +24,6 @@ Sentry.init({ }), browserTracingIntegration({ router, - instrumentPageLoad: false, - instrumentNavigation: false, - enableInp: false, - enableLongTask: false, - enableLongAnimationFrame: false, }), ], tunnel: `http://localhost:3031/`, // proxy server From 643d7121475c386d893bfe5182d7bb5281677ce9 Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Wed, 2 Apr 2025 18:13:14 +0200 Subject: [PATCH 5/5] size limit ... --- .size-limit.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.size-limit.js b/.size-limit.js index 9107bc637491..eed705e16da6 100644 --- a/.size-limit.js +++ b/.size-limit.js @@ -210,7 +210,7 @@ module.exports = [ import: createImport('init'), ignore: ['next/router', 'next/constants'], gzip: true, - limit: '41 KB', + limit: '42 KB', }, // SvelteKit SDK (ESM) {