diff --git a/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/linked-traces/consistent-sampling/meta-precedence/test.ts b/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/linked-traces/consistent-sampling/meta-precedence/test.ts index 1b55f5235088..ece2b1f85790 100644 --- a/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/linked-traces/consistent-sampling/meta-precedence/test.ts +++ b/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/linked-traces/consistent-sampling/meta-precedence/test.ts @@ -30,10 +30,13 @@ sentryTest.describe('When `consistentTraceSampling` is `true` and page contains const clientReportPromise = waitForClientReportRequest(page); await sentryTest.step('Initial pageload', async () => { + // negative sampling decision -> no pageload txn await page.goto(url); }); await sentryTest.step('Make fetch request', async () => { + // The fetch requests starts a new trace on purpose. So we only want the + // sampling decision and rand to be the same as from the meta tag but not the trace id or DSC const tracingHeadersPromise = waitForTracingHeadersOnUrl(page, 'http://sentry-test-external.io'); await page.locator('#btn2').click(); diff --git a/dev-packages/browser-integration-tests/suites/tracing/request/fetch-tracing-without-performance-propagateTraceparent/subject.js b/dev-packages/browser-integration-tests/suites/tracing/request/fetch-tracing-without-performance-propagateTraceparent/subject.js index e9a1ca98b5b0..60acaa1ebdf8 100644 --- a/dev-packages/browser-integration-tests/suites/tracing/request/fetch-tracing-without-performance-propagateTraceparent/subject.js +++ b/dev-packages/browser-integration-tests/suites/tracing/request/fetch-tracing-without-performance-propagateTraceparent/subject.js @@ -1 +1,2 @@ -fetch('http://sentry-test-site.example/0').then(); +fetch('http://sentry-test-site.example/0'); +fetch('http://sentry-test-site.example/1'); diff --git a/dev-packages/browser-integration-tests/suites/tracing/request/fetch-tracing-without-performance-propagateTraceparent/test.ts b/dev-packages/browser-integration-tests/suites/tracing/request/fetch-tracing-without-performance-propagateTraceparent/test.ts index 22cc7ba98eb8..718cca0c8701 100644 --- a/dev-packages/browser-integration-tests/suites/tracing/request/fetch-tracing-without-performance-propagateTraceparent/test.ts +++ b/dev-packages/browser-integration-tests/suites/tracing/request/fetch-tracing-without-performance-propagateTraceparent/test.ts @@ -12,21 +12,36 @@ sentryTest( const url = await getLocalTestUrl({ testDir: __dirname }); - const [, request] = await Promise.all([page.goto(url), page.waitForRequest('http://sentry-test-site.example/0')]); + const [, request0, request1] = await Promise.all([ + page.goto(url), + page.waitForRequest('http://sentry-test-site.example/0'), + page.waitForRequest('http://sentry-test-site.example/1'), + ]); - const requestHeaders = request.headers(); + const requestHeaders0 = request0.headers(); - const traceparentData = extractTraceparentData(requestHeaders['sentry-trace']); + const traceparentData = extractTraceparentData(requestHeaders0['sentry-trace']); expect(traceparentData).toMatchObject({ traceId: expect.stringMatching(/^([a-f0-9]{32})$/), parentSpanId: expect.stringMatching(/^([a-f0-9]{16})$/), parentSampled: undefined, }); - expect(requestHeaders).toMatchObject({ + expect(requestHeaders0).toMatchObject({ 'sentry-trace': `${traceparentData?.traceId}-${traceparentData?.parentSpanId}`, baggage: expect.stringContaining(`sentry-trace_id=${traceparentData?.traceId}`), traceparent: `00-${traceparentData?.traceId}-${traceparentData?.parentSpanId}-00`, }); + + const requestHeaders1 = request1.headers(); + expect(requestHeaders1).toMatchObject({ + 'sentry-trace': `${traceparentData?.traceId}-${traceparentData?.parentSpanId}`, + baggage: expect.stringContaining(`sentry-trace_id=${traceparentData?.traceId}`), + traceparent: `00-${traceparentData?.traceId}-${traceparentData?.parentSpanId}-00`, + }); + + expect(requestHeaders1['sentry-trace']).toBe(requestHeaders0['sentry-trace']); + expect(requestHeaders1['baggage']).toBe(requestHeaders0['baggage']); + expect(requestHeaders1['traceparent']).toBe(requestHeaders0['traceparent']); }, ); diff --git a/dev-packages/browser-integration-tests/suites/tracing/trace-lifetime/tracing-without-performance-propagateTraceparent/init.js b/dev-packages/browser-integration-tests/suites/tracing/trace-lifetime/tracing-without-performance-propagateTraceparent/init.js new file mode 100644 index 000000000000..b5b7d0354f77 --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/tracing/trace-lifetime/tracing-without-performance-propagateTraceparent/init.js @@ -0,0 +1,11 @@ +import * as Sentry from '@sentry/browser'; + +window.Sentry = Sentry; + +Sentry.init({ + // in browser TwP means not setting tracesSampleRate but adding browserTracingIntegration, + dsn: 'https://public@dsn.ingest.sentry.io/1337', + integrations: [Sentry.browserTracingIntegration()], + tracePropagationTargets: ['http://sentry-test-site.example'], + propagateTraceparent: true, +}); diff --git a/dev-packages/browser-integration-tests/suites/tracing/trace-lifetime/tracing-without-performance-propagateTraceparent/template.html b/dev-packages/browser-integration-tests/suites/tracing/trace-lifetime/tracing-without-performance-propagateTraceparent/template.html new file mode 100644 index 000000000000..755df10f3a1a --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/tracing/trace-lifetime/tracing-without-performance-propagateTraceparent/template.html @@ -0,0 +1,17 @@ + + + + + + + + + + + + + + diff --git a/dev-packages/browser-integration-tests/suites/tracing/trace-lifetime/tracing-without-performance-propagateTraceparent/test.ts b/dev-packages/browser-integration-tests/suites/tracing/trace-lifetime/tracing-without-performance-propagateTraceparent/test.ts new file mode 100644 index 000000000000..d2106b892b7b --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/tracing/trace-lifetime/tracing-without-performance-propagateTraceparent/test.ts @@ -0,0 +1,130 @@ +import { expect } from '@playwright/test'; +import { extractTraceparentData } from '@sentry/core'; +import { sentryTest } from '../../../../utils/fixtures'; +import { shouldSkipTracingTest } from '../../../../utils/helpers'; + +const META_TAG_TRACE_ID = '12345678901234567890123456789012'; +const META_TAG_PARENT_SPAN_ID = '1234567890123456'; +const META_TAG_BAGGAGE = + 'sentry-trace_id=12345678901234567890123456789012,sentry-public_key=public,sentry-release=1.0.0,sentry-environment=prod,sentry-sample_rand=0.42'; + +sentryTest( + 'outgoing fetch requests have new traceId after navigation (with propagateTraceparent)', + async ({ getLocalTestUrl, page }) => { + if (shouldSkipTracingTest()) { + sentryTest.skip(); + } + + const url = await getLocalTestUrl({ testDir: __dirname }); + + await page.route('http://sentry-test-site.example/**', route => { + return route.fulfill({ + status: 200, + contentType: 'application/json', + body: JSON.stringify({}), + }); + }); + + await page.goto(url); + + const requestPromise = page.waitForRequest('http://sentry-test-site.example/*'); + await page.locator('#fetchBtn').click(); + const request = await requestPromise; + const headers = request.headers(); + + const sentryTraceParentData = extractTraceparentData(headers['sentry-trace']); + // sampling decision is deferred because TwP means we didn't sample any span + expect(sentryTraceParentData).toEqual({ + traceId: META_TAG_TRACE_ID, + parentSpanId: expect.stringMatching(/^[0-9a-f]{16}$/), + parentSampled: undefined, + }); + expect(headers['baggage']).toBe(META_TAG_BAGGAGE); + // but traceparent propagates a negative sampling decision because it has no concept of deferred sampling + expect(headers['traceparent']).toBe( + `00-${sentryTraceParentData?.traceId}-${sentryTraceParentData?.parentSpanId}-00`, + ); + + const requestPromise2 = page.waitForRequest('http://sentry-test-site.example/*'); + await page.locator('#fetchBtn').click(); + const request2 = await requestPromise2; + const headers2 = request2.headers(); + + const sentryTraceParentData2 = extractTraceparentData(headers2['sentry-trace']); + expect(sentryTraceParentData2).toEqual(sentryTraceParentData); + + await page.goto(`${url}#navigation`); + + const requestPromise3 = page.waitForRequest('http://sentry-test-site.example/*'); + await page.locator('#fetchBtn').click(); + const request3 = await requestPromise3; + const headers3 = request3.headers(); + + const sentryTraceParentData3 = extractTraceparentData(headers3['sentry-trace']); + // sampling decision is deferred because TwP means we didn't sample any span + expect(sentryTraceParentData3).toEqual({ + traceId: expect.not.stringContaining(sentryTraceParentData!.traceId!), + parentSpanId: expect.not.stringContaining(sentryTraceParentData!.parentSpanId!), + parentSampled: undefined, + }); + + expect(headers3['baggage']).toMatch( + /sentry-environment=production,sentry-public_key=public,sentry-trace_id=[0-9a-f]{32}/, + ); + expect(headers3['baggage']).not.toContain(`sentry-trace_id=${META_TAG_TRACE_ID}`); + // but traceparent propagates a negative sampling decision because it has no concept of deferred sampling + expect(headers3['traceparent']).toBe( + `00-${sentryTraceParentData3!.traceId}-${sentryTraceParentData3!.parentSpanId}-00`, + ); + + const requestPromise4 = page.waitForRequest('http://sentry-test-site.example/*'); + await page.locator('#fetchBtn').click(); + const request4 = await requestPromise4; + const headers4 = request4.headers(); + + const sentryTraceParentData4 = extractTraceparentData(headers4['sentry-trace']); + expect(sentryTraceParentData4).toEqual(sentryTraceParentData3); + }, +); + +sentryTest('outgoing XHR requests have new traceId after navigation', async ({ getLocalTestUrl, page }) => { + if (shouldSkipTracingTest()) { + sentryTest.skip(); + } + + const url = await getLocalTestUrl({ testDir: __dirname }); + + await page.route('http://sentry-test-site.example/**', route => { + return route.fulfill({ + status: 200, + contentType: 'application/json', + body: JSON.stringify({}), + }); + }); + + await page.goto(url); + + const requestPromise = page.waitForRequest('http://sentry-test-site.example/*'); + await page.locator('#xhrBtn').click(); + const request = await requestPromise; + const headers = request.headers(); + + // sampling decision is deferred because TwP means we didn't sample any span + expect(headers['sentry-trace']).toMatch(new RegExp(`^${META_TAG_TRACE_ID}-[0-9a-f]{16}$`)); + expect(headers['baggage']).toBe(META_TAG_BAGGAGE); + + await page.goto(`${url}#navigation`); + + const requestPromise2 = page.waitForRequest('http://sentry-test-site.example/*'); + await page.locator('#xhrBtn').click(); + const request2 = await requestPromise2; + const headers2 = request2.headers(); + + // sampling decision is deferred because TwP means we didn't sample any span + expect(headers2['sentry-trace']).toMatch(/^[0-9a-f]{32}-[0-9a-f]{16}$/); + expect(headers2['baggage']).not.toBe(`${META_TAG_TRACE_ID}-${META_TAG_PARENT_SPAN_ID}`); + expect(headers2['baggage']).toMatch( + /sentry-environment=production,sentry-public_key=public,sentry-trace_id=[0-9a-f]{32}/, + ); + expect(headers2['baggage']).not.toContain(`sentry-trace_id=${META_TAG_TRACE_ID}`); +}); diff --git a/packages/browser/src/tracing/browserTracingIntegration.ts b/packages/browser/src/tracing/browserTracingIntegration.ts index cd63678de16f..edec1b1617b6 100644 --- a/packages/browser/src/tracing/browserTracingIntegration.ts +++ b/packages/browser/src/tracing/browserTracingIntegration.ts @@ -5,6 +5,7 @@ import { browserPerformanceTimeOrigin, dateTimestampInSeconds, debug, + generateSpanId, generateTraceId, getClient, getCurrentScope, @@ -12,6 +13,7 @@ import { getIsolationScope, getLocationHref, GLOBAL_OBJ, + hasSpansEnabled, parseStringToURLObject, propagationContextFromHeaders, registerSpanErrorInstrumentation, @@ -512,10 +514,19 @@ export const browserTracingIntegration = ((_options: Partial { expect(oldCurrentScopePropCtx).toEqual({ traceId: expect.stringMatching(/[a-f0-9]{32}/), + propagationSpanId: expect.stringMatching(/[a-f0-9]{16}/), sampleRand: expect.any(Number), }); expect(oldIsolationScopePropCtx).toEqual({ @@ -731,15 +732,18 @@ describe('browserTracingIntegration', () => { }); expect(newCurrentScopePropCtx).toEqual({ traceId: expect.stringMatching(/[a-f0-9]{32}/), + propagationSpanId: expect.stringMatching(/[a-f0-9]{16}/), sampleRand: expect.any(Number), }); expect(newIsolationScopePropCtx).toEqual({ traceId: expect.stringMatching(/[a-f0-9]{32}/), + propagationSpanId: expect.stringMatching(/[a-f0-9]{16}/), sampleRand: expect.any(Number), }); expect(newIsolationScopePropCtx.traceId).not.toEqual(oldIsolationScopePropCtx.traceId); expect(newCurrentScopePropCtx.traceId).not.toEqual(oldCurrentScopePropCtx.traceId); + expect(newIsolationScopePropCtx.propagationSpanId).not.toEqual(oldIsolationScopePropCtx.propagationSpanId); }); it("saves the span's positive sampling decision and its DSC on the propagationContext when the span finishes", () => { @@ -758,7 +762,7 @@ describe('browserTracingIntegration', () => { }); const propCtxBeforeEnd = getCurrentScope().getPropagationContext(); - expect(propCtxBeforeEnd).toStrictEqual({ + expect(propCtxBeforeEnd).toEqual({ sampleRand: expect.any(Number), traceId: expect.stringMatching(/[a-f0-9]{32}/), }); @@ -766,7 +770,7 @@ describe('browserTracingIntegration', () => { navigationSpan!.end(); const propCtxAfterEnd = getCurrentScope().getPropagationContext(); - expect(propCtxAfterEnd).toStrictEqual({ + expect(propCtxAfterEnd).toEqual({ traceId: propCtxBeforeEnd.traceId, sampled: true, sampleRand: expect.any(Number), @@ -800,7 +804,7 @@ describe('browserTracingIntegration', () => { }); const propCtxBeforeEnd = getCurrentScope().getPropagationContext(); - expect(propCtxBeforeEnd).toStrictEqual({ + expect(propCtxBeforeEnd).toEqual({ traceId: expect.stringMatching(/[a-f0-9]{32}/), sampleRand: expect.any(Number), }); @@ -808,7 +812,7 @@ describe('browserTracingIntegration', () => { navigationSpan!.end(); const propCtxAfterEnd = getCurrentScope().getPropagationContext(); - expect(propCtxAfterEnd).toStrictEqual({ + expect(propCtxAfterEnd).toEqual({ traceId: propCtxBeforeEnd.traceId, sampled: false, sampleRand: expect.any(Number), diff --git a/packages/core/src/utils/spanUtils.ts b/packages/core/src/utils/spanUtils.ts index b6761c9930e7..89ecc6872efb 100644 --- a/packages/core/src/utils/spanUtils.ts +++ b/packages/core/src/utils/spanUtils.ts @@ -17,7 +17,7 @@ import type { SpanStatus } from '../types-hoist/spanStatus'; import { addNonEnumerableProperty } from '../utils/object'; import { generateSpanId } from '../utils/propagationContext'; import { timestampInSeconds } from '../utils/time'; -import { generateSentryTraceHeader } from '../utils/tracing'; +import { generateSentryTraceHeader, generateTraceparentHeader } from '../utils/tracing'; import { consoleSandbox } from './debug-logger'; import { _getSpanForScope } from './spanOnScope'; @@ -77,6 +77,15 @@ export function spanToTraceHeader(span: Span): string { return generateSentryTraceHeader(traceId, spanId, sampled); } +/** + * Convert a Span to a W3C traceparent header. + */ +export function spanToTraceparentHeader(span: Span): string { + const { traceId, spanId } = span.spanContext(); + const sampled = spanIsSampled(span); + return generateTraceparentHeader(traceId, spanId, sampled); +} + /** * Converts the span links array to a flattened version to be sent within an envelope. * diff --git a/packages/core/src/utils/traceData.ts b/packages/core/src/utils/traceData.ts index aa335dbd37bd..325e45a038e0 100644 --- a/packages/core/src/utils/traceData.ts +++ b/packages/core/src/utils/traceData.ts @@ -9,8 +9,8 @@ import type { Span } from '../types-hoist/span'; import type { SerializedTraceData } from '../types-hoist/tracing'; import { dynamicSamplingContextToSentryBaggageHeader } from './baggage'; import { debug } from './debug-logger'; -import { getActiveSpan, spanToTraceHeader } from './spanUtils'; -import { extractTraceparentData, generateSentryTraceHeader, TRACEPARENT_REGEXP } from './tracing'; +import { getActiveSpan, spanToTraceHeader, spanToTraceparentHeader } from './spanUtils'; +import { generateSentryTraceHeader, generateTraceparentHeader, TRACEPARENT_REGEXP } from './tracing'; /** * Extracts trace propagation data from the current span or from the client's scope (via transaction or propagation @@ -58,7 +58,7 @@ export function getTraceData( }; if (options.propagateTraceparent) { - const traceparent = _sentryTraceToTraceParentHeader(sentryTrace); + const traceparent = span ? spanToTraceparentHeader(span) : scopeToTraceparentHeader(scope); if (traceparent) { traceData.traceparent = traceparent; } @@ -75,22 +75,7 @@ function scopeToTraceHeader(scope: Scope): string { return generateSentryTraceHeader(traceId, propagationSpanId, sampled); } -/** - * Builds a W3C traceparent header from the given sentry-trace header. - * - * Why parse that header and not create traceparent from primitives? - * We want these two headers to always have the same ids. The easiest way to do this is to take - * one of them as the source of truth (sentry-trace) and derive the other from it. - * - * Most importantly, this guarantees parentSpanId consistency between sentry-trace and traceparent - * in tracing without performance (TwP) mode, where we always generate a random parentSpanId. - * - * Exported for testing - */ -export function _sentryTraceToTraceParentHeader(sentryTrace: string): string | undefined { - const { traceId, parentSpanId, parentSampled } = extractTraceparentData(sentryTrace) || {}; - if (!traceId || !parentSpanId) { - return undefined; - } - return `00-${traceId}-${parentSpanId}-${parentSampled ? '01' : '00'}`; +function scopeToTraceparentHeader(scope: Scope): string { + const { traceId, sampled, propagationSpanId } = scope.getPropagationContext(); + return generateTraceparentHeader(traceId, propagationSpanId, sampled); } diff --git a/packages/core/src/utils/tracing.ts b/packages/core/src/utils/tracing.ts index 0310cc8640e6..aa5a15153674 100644 --- a/packages/core/src/utils/tracing.ts +++ b/packages/core/src/utils/tracing.ts @@ -102,6 +102,17 @@ export function generateSentryTraceHeader( return `${traceId}-${spanId}${sampledString}`; } +/** + * Creates a W3C traceparent header from the given trace and span ids. + */ +export function generateTraceparentHeader( + traceId: string | undefined = generateTraceId(), + spanId: string | undefined = generateSpanId(), + sampled?: boolean, +): string { + return `00-${traceId}-${spanId}-${sampled ? '01' : '00'}`; +} + /** * Given any combination of an incoming trace, generate a sample rand based on its defined semantics. * diff --git a/packages/core/test/lib/utils/traceData.test.ts b/packages/core/test/lib/utils/traceData.test.ts index 37764cfc94a3..379103a8a48c 100644 --- a/packages/core/test/lib/utils/traceData.test.ts +++ b/packages/core/test/lib/utils/traceData.test.ts @@ -15,7 +15,6 @@ import { import { getAsyncContextStrategy } from '../../../src/asyncContext'; import { freezeDscOnSpan } from '../../../src/tracing/dynamicSamplingContext'; import type { Span } from '../../../src/types-hoist/span'; -import { _sentryTraceToTraceParentHeader } from '../../../src/utils/traceData'; import type { TestClientOptions } from '../../mocks/client'; import { getDefaultTestClientOptions, TestClient } from '../../mocks/client'; @@ -349,34 +348,3 @@ describe('getTraceData', () => { expect(traceData.traceparent).toMatch(/00-12345678901234567890123456789099-[0-9a-f]{16}-00/); }); }); - -describe('_sentryTraceToTraceParentHeader', () => { - it('returns positively sampled traceparent header for sentry-trace with positive sampling decision', () => { - const traceparent = _sentryTraceToTraceParentHeader('12345678901234567890123456789012-1234567890123456-1'); - expect(traceparent).toBe('00-12345678901234567890123456789012-1234567890123456-01'); - }); - - it('returns negatively sampled traceparent header for sentry-trace with negative sampling decision', () => { - const traceparent = _sentryTraceToTraceParentHeader('12345678901234567890123456789012-1234567890123456-0'); - expect(traceparent).toBe('00-12345678901234567890123456789012-1234567890123456-00'); - }); - - it('returns negatively sampled traceparent header for sentry-trace with no/deferred sampling decision', () => { - const traceparent = _sentryTraceToTraceParentHeader('12345678901234567890123456789012-1234567890123456'); - expect(traceparent).toBe('00-12345678901234567890123456789012-1234567890123456-00'); - }); - - it.each([ - '12345678901234567890123456789012--0', - '-12345678901234567890123456789012-0', - '--1', - '0', - '1', - '', - '00-12345678901234567890123456789012-1234567890123456-01', - '00-12345678901234567890123456789012-1234567890123456-00', - ])('returns undefined if the sentry-trace header is invalid (%s)', sentryTrace => { - const traceparent = _sentryTraceToTraceParentHeader(sentryTrace); - expect(traceparent).toBeUndefined(); - }); -}); diff --git a/packages/core/test/lib/utils/tracing.test.ts b/packages/core/test/lib/utils/tracing.test.ts index ea41190f3bb3..c950679a21be 100644 --- a/packages/core/test/lib/utils/tracing.test.ts +++ b/packages/core/test/lib/utils/tracing.test.ts @@ -1,5 +1,10 @@ import { describe, expect, it, test } from 'vitest'; -import { extractTraceparentData, propagationContextFromHeaders, shouldContinueTrace } from '../../../src/utils/tracing'; +import { + extractTraceparentData, + generateTraceparentHeader, + propagationContextFromHeaders, + shouldContinueTrace, +} from '../../../src/utils/tracing'; import { getDefaultTestClientOptions, TestClient } from '../../mocks/client'; const EXAMPLE_SENTRY_TRACE = '12312012123120121231201212312012-1121201211212012-1'; @@ -177,3 +182,20 @@ describe('shouldContinueTrace', () => { expect(result).toBe(false); }); }); + +describe('generateTraceparentHeader', () => { + test('returns a traceparent header with the given ids and positive sampling decision', () => { + const traceparent = generateTraceparentHeader('12345678901234567890123456789012', '1234567890123456', true); + expect(traceparent).toBe('00-12345678901234567890123456789012-1234567890123456-01'); + }); + + test('returns a traceparent header with the given ids and negative sampling decision', () => { + const traceparent = generateTraceparentHeader('12345678901234567890123456789012', '1234567890123456', false); + expect(traceparent).toBe('00-12345678901234567890123456789012-1234567890123456-00'); + }); + + test('no sampling decision passed, creates a negatively sampled traceparent header', () => { + const traceparent = generateTraceparentHeader('12345678901234567890123456789012', '1234567890123456'); + expect(traceparent).toBe('00-12345678901234567890123456789012-1234567890123456-00'); + }); +});