Skip to content

Commit 639ef2a

Browse files
Lms24cadesalaberry
authored andcommitted
feat(tracing-internal): Reset propagation context on navigation (getsentry#11401)
Add propagationContext resetting to our `browserTracing` navigation span handler. Whenever our `browserTracingIntegration`s (the default one or the framework/router-specific ones) start a navigation idle span (via `startBrowserTracingNavigationSpan`), we'll now set a new propagation context (i.e. new traceId and spanId) onto the scopes.
1 parent 35f81d3 commit 639ef2a

File tree

7 files changed

+156
-3
lines changed

7 files changed

+156
-3
lines changed

dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/navigation/test.ts

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,3 +49,25 @@ sentryTest('should create a navigation transaction on page navigation', async ({
4949

5050
expect(pageloadSpanId).not.toEqual(navigationSpanId);
5151
});
52+
53+
sentryTest('should create a new trace for for multiple navigations', async ({ getLocalTestPath, page }) => {
54+
if (shouldSkipTracingTest()) {
55+
sentryTest.skip();
56+
}
57+
58+
const url = await getLocalTestPath({ testDir: __dirname });
59+
60+
await getFirstSentryEnvelopeRequest<Event>(page, url);
61+
const navigationEvent1 = await getFirstSentryEnvelopeRequest<Event>(page, `${url}#foo`);
62+
const navigationEvent2 = await getFirstSentryEnvelopeRequest<Event>(page, `${url}#bar`);
63+
64+
expect(navigationEvent1.contexts?.trace?.op).toBe('navigation');
65+
expect(navigationEvent2.contexts?.trace?.op).toBe('navigation');
66+
67+
const navigation1TraceId = navigationEvent1.contexts?.trace?.trace_id;
68+
const navigation2TraceId = navigationEvent2.contexts?.trace?.trace_id;
69+
70+
expect(navigation1TraceId).toBeDefined();
71+
expect(navigation2TraceId).toBeDefined();
72+
expect(navigation1TraceId).not.toEqual(navigation2TraceId);
73+
});

dev-packages/e2e-tests/test-applications/sveltekit-2/src/routes/+page.svelte

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,4 +29,10 @@
2929
<li>
3030
<a href="/server-load-fetch">Route with nested fetch in server load</a>
3131
</li>
32+
<li>
33+
<a href="/nav1">Nav 1</a>
34+
</li>
35+
<li>
36+
<a href="/nav2">Nav 2</a>
37+
</li>
3238
</ul>
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
<h1>Navigation 1</h1>
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
<h1>Navigation 2</h1>
Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,69 @@
1+
import { expect, test } from '@playwright/test';
2+
import { waitForTransaction } from '../event-proxy-server';
3+
import { waitForInitialPageload } from './utils';
4+
5+
test.describe('client-specific performance events', () => {
6+
test('multiple navigations have distinct traces', async ({ page }) => {
7+
const navigationTxn1EventPromise = waitForTransaction('sveltekit-2', txnEvent => {
8+
return txnEvent?.transaction === '/nav1' && txnEvent.contexts?.trace?.op === 'navigation';
9+
});
10+
11+
const navigationTxn2EventPromise = waitForTransaction('sveltekit-2', txnEvent => {
12+
return txnEvent?.transaction === '/' && txnEvent.contexts?.trace?.op === 'navigation';
13+
});
14+
15+
const navigationTxn3EventPromise = waitForTransaction('sveltekit-2', txnEvent => {
16+
return txnEvent?.transaction === '/nav2' && txnEvent.contexts?.trace?.op === 'navigation';
17+
});
18+
19+
await waitForInitialPageload(page);
20+
21+
const [navigationTxn1Event] = await Promise.all([navigationTxn1EventPromise, page.getByText('Nav 1').click()]);
22+
const [navigationTxn2Event] = await Promise.all([navigationTxn2EventPromise, page.goBack()]);
23+
const [navigationTxn3Event] = await Promise.all([navigationTxn3EventPromise, page.getByText('Nav 2').click()]);
24+
25+
expect(navigationTxn1Event).toMatchObject({
26+
transaction: '/nav1',
27+
transaction_info: { source: 'route' },
28+
type: 'transaction',
29+
contexts: {
30+
trace: {
31+
op: 'navigation',
32+
origin: 'auto.navigation.sveltekit',
33+
trace_id: expect.stringMatching(/[a-f0-9]{32}/),
34+
},
35+
},
36+
});
37+
38+
expect(navigationTxn2Event).toMatchObject({
39+
transaction: '/',
40+
transaction_info: { source: 'route' },
41+
type: 'transaction',
42+
contexts: {
43+
trace: {
44+
op: 'navigation',
45+
origin: 'auto.navigation.sveltekit',
46+
trace_id: expect.stringMatching(/[a-f0-9]{32}/),
47+
},
48+
},
49+
});
50+
51+
expect(navigationTxn3Event).toMatchObject({
52+
transaction: '/nav2',
53+
transaction_info: { source: 'route' },
54+
type: 'transaction',
55+
contexts: {
56+
trace: {
57+
op: 'navigation',
58+
origin: 'auto.navigation.sveltekit',
59+
trace_id: expect.stringMatching(/[a-f0-9]{32}/),
60+
},
61+
},
62+
});
63+
64+
// traces should NOT be connected
65+
expect(navigationTxn1Event.contexts?.trace?.trace_id).not.toBe(navigationTxn2Event.contexts?.trace?.trace_id);
66+
expect(navigationTxn2Event.contexts?.trace?.trace_id).not.toBe(navigationTxn3Event.contexts?.trace?.trace_id);
67+
expect(navigationTxn1Event.contexts?.trace?.trace_id).not.toBe(navigationTxn3Event.contexts?.trace?.trace_id);
68+
});
69+
});

packages/tracing-internal/src/browser/browserTracingIntegration.ts

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,14 +8,21 @@ import {
88
getActiveSpan,
99
getClient,
1010
getCurrentScope,
11+
getIsolationScope,
1112
getRootSpan,
1213
spanToJSON,
1314
startIdleSpan,
1415
withScope,
1516
} from '@sentry/core';
1617
import type { Client, IntegrationFn, StartSpanOptions, TransactionSource } from '@sentry/types';
1718
import type { Span } from '@sentry/types';
18-
import { addHistoryInstrumentationHandler, browserPerformanceTimeOrigin, getDomElement, logger } from '@sentry/utils';
19+
import {
20+
addHistoryInstrumentationHandler,
21+
browserPerformanceTimeOrigin,
22+
getDomElement,
23+
logger,
24+
uuid4,
25+
} from '@sentry/utils';
1926

2027
import { DEBUG_BUILD } from '../common/debug-build';
2128
import { registerBackgroundTabDetection } from './backgroundtab';
@@ -371,6 +378,15 @@ export function startBrowserTracingPageLoadSpan(
371378
* This will only do something if a browser tracing integration has been setup.
372379
*/
373380
export function startBrowserTracingNavigationSpan(client: Client, spanOptions: StartSpanOptions): Span | undefined {
381+
getCurrentScope().setPropagationContext({
382+
traceId: uuid4(),
383+
spanId: uuid4().substring(16),
384+
});
385+
getIsolationScope().setPropagationContext({
386+
traceId: uuid4(),
387+
spanId: uuid4().substring(16),
388+
});
389+
374390
client.emit('startNavigationSpan', spanOptions);
375391

376392
getCurrentScope().setTransactionName(spanOptions.name);

packages/tracing-internal/test/browser/browserTracingIntegration.test.ts

Lines changed: 40 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -603,7 +603,7 @@ describe('browserTracingIntegration', () => {
603603
expect(spanToJSON(pageloadSpan!).data?.[SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]).toBe('custom');
604604
});
605605

606-
it('sets the pageload span name on `scope.transactionName`', () => {
606+
it('sets the navigation span name on `scope.transactionName`', () => {
607607
const client = new TestClient(
608608
getDefaultClientOptions({
609609
integrations: [browserTracingIntegration()],
@@ -612,10 +612,48 @@ describe('browserTracingIntegration', () => {
612612
setCurrentClient(client);
613613
client.init();
614614

615-
startBrowserTracingPageLoadSpan(client, { name: 'test navigation span' });
615+
startBrowserTracingNavigationSpan(client, { name: 'test navigation span' });
616616

617617
expect(getCurrentScope().getScopeData().transactionName).toBe('test navigation span');
618618
});
619+
620+
it("resets the scopes' propagationContexts", () => {
621+
const client = new TestClient(
622+
getDefaultClientOptions({
623+
integrations: [browserTracingIntegration()],
624+
}),
625+
);
626+
setCurrentClient(client);
627+
client.init();
628+
629+
const oldIsolationScopePropCtx = getIsolationScope().getPropagationContext();
630+
const oldCurrentScopePropCtx = getCurrentScope().getPropagationContext();
631+
632+
startBrowserTracingNavigationSpan(client, { name: 'test navigation span' });
633+
634+
const newIsolationScopePropCtx = getIsolationScope().getPropagationContext();
635+
const newCurrentScopePropCtx = getCurrentScope().getPropagationContext();
636+
637+
expect(oldCurrentScopePropCtx).toEqual({
638+
spanId: expect.stringMatching(/[a-f0-9]{16}/),
639+
traceId: expect.stringMatching(/[a-f0-9]{32}/),
640+
});
641+
expect(oldIsolationScopePropCtx).toEqual({
642+
spanId: expect.stringMatching(/[a-f0-9]{16}/),
643+
traceId: expect.stringMatching(/[a-f0-9]{32}/),
644+
});
645+
expect(newCurrentScopePropCtx).toEqual({
646+
spanId: expect.stringMatching(/[a-f0-9]{16}/),
647+
traceId: expect.stringMatching(/[a-f0-9]{32}/),
648+
});
649+
expect(newIsolationScopePropCtx).toEqual({
650+
spanId: expect.stringMatching(/[a-f0-9]{16}/),
651+
traceId: expect.stringMatching(/[a-f0-9]{32}/),
652+
});
653+
654+
expect(newIsolationScopePropCtx?.traceId).not.toEqual(oldIsolationScopePropCtx?.traceId);
655+
expect(newCurrentScopePropCtx?.traceId).not.toEqual(oldCurrentScopePropCtx?.traceId);
656+
});
619657
});
620658

621659
describe('using the <meta> tag data', () => {

0 commit comments

Comments
 (0)