Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test confused me a bit, so I just added some comments. No change for consistent trace sampling!

Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
Original file line number Diff line number Diff line change
@@ -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');
Original file line number Diff line number Diff line change
Expand Up @@ -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']);
},
);
Original file line number Diff line number Diff line change
@@ -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,
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
<!doctype html>
<html>
<head>
<meta charset="utf-8" />
<!-- Purposefully emitting the `sampled` flag in the sentry-trace tag -->
<meta name="sentry-trace" content="12345678901234567890123456789012-1234567890123456" />
<meta
name="baggage"
content="sentry-trace_id=12345678901234567890123456789012,sentry-public_key=public,sentry-release=1.0.0,sentry-environment=prod,sentry-sample_rand=0.42"
/>
</head>
<body>
<button id="errorBtn">Throw Error</button>
<button id="fetchBtn">Fetch Request</button>
<button id="xhrBtn">XHR Request</button>
</body>
</html>
Original file line number Diff line number Diff line change
@@ -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}`);
});
21 changes: 19 additions & 2 deletions packages/browser/src/tracing/browserTracingIntegration.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,15 @@ import {
browserPerformanceTimeOrigin,
dateTimestampInSeconds,
debug,
generateSpanId,
generateTraceId,
getClient,
getCurrentScope,
getDynamicSamplingContextFromSpan,
getIsolationScope,
getLocationHref,
GLOBAL_OBJ,
hasSpansEnabled,
parseStringToURLObject,
propagationContextFromHeaders,
registerSpanErrorInstrumentation,
Expand Down Expand Up @@ -512,10 +514,19 @@ export const browserTracingIntegration = ((_options: Partial<BrowserTracingOptio

maybeEndActiveSpan();

getIsolationScope().setPropagationContext({ traceId: generateTraceId(), sampleRand: Math.random() });
getIsolationScope().setPropagationContext({
traceId: generateTraceId(),
sampleRand: Math.random(),
propagationSpanId: hasSpansEnabled() ? undefined : generateSpanId(),
});

const scope = getCurrentScope();
scope.setPropagationContext({ traceId: generateTraceId(), sampleRand: Math.random() });
scope.setPropagationContext({
traceId: generateTraceId(),
sampleRand: Math.random(),
propagationSpanId: hasSpansEnabled() ? undefined : generateSpanId(),
});

// We reset this to ensure we do not have lingering incorrect data here
// places that call this hook may set this where appropriate - else, the URL at span sending time is used
scope.setSDKProcessingMetadata({
Expand All @@ -541,6 +552,12 @@ export const browserTracingIntegration = ((_options: Partial<BrowserTracingOptio

const scope = getCurrentScope();
scope.setPropagationContext(propagationContext);
if (!hasSpansEnabled()) {
// for browser, we wanna keep the spanIds consistent during the entire lifetime of the trace
// this works by setting the propagationSpanId to a random spanId so that we have a consistent
// span id to propagate in TwP mode (!hasSpansEnabled())
scope.getPropagationContext().propagationSpanId = generateSpanId();
}

// We store the normalized request data on the scope, so we get the request data at time of span creation
// otherwise, the URL etc. may already be of the following navigation, and we'd report the wrong URL
Expand Down
12 changes: 8 additions & 4 deletions packages/browser/test/tracing/browserTracingIntegration.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -723,6 +723,7 @@ describe('browserTracingIntegration', () => {

expect(oldCurrentScopePropCtx).toEqual({
traceId: expect.stringMatching(/[a-f0-9]{32}/),
propagationSpanId: expect.stringMatching(/[a-f0-9]{16}/),
sampleRand: expect.any(Number),
});
expect(oldIsolationScopePropCtx).toEqual({
Expand All @@ -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", () => {
Expand All @@ -758,15 +762,15 @@ describe('browserTracingIntegration', () => {
});

const propCtxBeforeEnd = getCurrentScope().getPropagationContext();
expect(propCtxBeforeEnd).toStrictEqual({
expect(propCtxBeforeEnd).toEqual({
sampleRand: expect.any(Number),
traceId: expect.stringMatching(/[a-f0-9]{32}/),
});

navigationSpan!.end();

const propCtxAfterEnd = getCurrentScope().getPropagationContext();
expect(propCtxAfterEnd).toStrictEqual({
expect(propCtxAfterEnd).toEqual({
traceId: propCtxBeforeEnd.traceId,
sampled: true,
sampleRand: expect.any(Number),
Expand Down Expand Up @@ -800,15 +804,15 @@ describe('browserTracingIntegration', () => {
});

const propCtxBeforeEnd = getCurrentScope().getPropagationContext();
expect(propCtxBeforeEnd).toStrictEqual({
expect(propCtxBeforeEnd).toEqual({
traceId: expect.stringMatching(/[a-f0-9]{32}/),
sampleRand: expect.any(Number),
});

navigationSpan!.end();

const propCtxAfterEnd = getCurrentScope().getPropagationContext();
expect(propCtxAfterEnd).toStrictEqual({
expect(propCtxAfterEnd).toEqual({
traceId: propCtxBeforeEnd.traceId,
sampled: false,
sampleRand: expect.any(Number),
Expand Down
11 changes: 10 additions & 1 deletion packages/core/src/utils/spanUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand Down Expand Up @@ -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.
*
Expand Down
Loading
Loading