Skip to content

feat(tracing-internal): Reset propagation context on navigation #11401

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Apr 3, 2024
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
Original file line number Diff line number Diff line change
Expand Up @@ -49,3 +49,25 @@ sentryTest('should create a navigation transaction on page navigation', async ({

expect(pageloadSpanId).not.toEqual(navigationSpanId);
});

sentryTest('should create a new trace for for multiple navigations', async ({ getLocalTestPath, page }) => {
if (shouldSkipTracingTest()) {
sentryTest.skip();
}

const url = await getLocalTestPath({ testDir: __dirname });

await getFirstSentryEnvelopeRequest<Event>(page, url);
const navigationEvent1 = await getFirstSentryEnvelopeRequest<Event>(page, `${url}#foo`);
const navigationEvent2 = await getFirstSentryEnvelopeRequest<Event>(page, `${url}#bar`);

expect(navigationEvent1.contexts?.trace?.op).toBe('navigation');
expect(navigationEvent2.contexts?.trace?.op).toBe('navigation');

const navigation1TraceId = navigationEvent1.contexts?.trace?.trace_id;
const navigation2TraceId = navigationEvent2.contexts?.trace?.trace_id;

expect(navigation1TraceId).toBeDefined();
expect(navigation2TraceId).toBeDefined();
expect(navigation1TraceId).not.toEqual(navigation2TraceId);
});
Original file line number Diff line number Diff line change
Expand Up @@ -29,4 +29,10 @@
<li>
<a href="/server-load-fetch">Route with nested fetch in server load</a>
</li>
<li>
<a href="/nav1">Nav 1</a>
</li>
<li>
<a href="/nav2">Nav 2</a>
</li>
</ul>
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
<h1>Navigation 1</h1>
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
<h1>Navigation 2</h1>
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
import { expect, test } from '@playwright/test';
import { waitForTransaction } from '../event-proxy-server';
import { waitForInitialPageload } from './utils';

test.describe('client-specific performance events', () => {
test('multiple navigations have distinct traces', async ({ page }) => {
const navigationTxn1EventPromise = waitForTransaction('sveltekit-2', txnEvent => {
return txnEvent?.transaction === '/nav1' && txnEvent.contexts?.trace?.op === 'navigation';
});

const navigationTxn2EventPromise = waitForTransaction('sveltekit-2', txnEvent => {
return txnEvent?.transaction === '/' && txnEvent.contexts?.trace?.op === 'navigation';
});

const navigationTxn3EventPromise = waitForTransaction('sveltekit-2', txnEvent => {
return txnEvent?.transaction === '/nav2' && txnEvent.contexts?.trace?.op === 'navigation';
});

await waitForInitialPageload(page);

const [navigationTxn1Event] = await Promise.all([navigationTxn1EventPromise, page.getByText('Nav 1').click()]);
const [navigationTxn2Event] = await Promise.all([navigationTxn2EventPromise, page.goBack()]);
const [navigationTxn3Event] = await Promise.all([navigationTxn3EventPromise, page.getByText('Nav 2').click()]);

expect(navigationTxn1Event).toMatchObject({
transaction: '/nav1',
transaction_info: { source: 'route' },
type: 'transaction',
contexts: {
trace: {
op: 'navigation',
origin: 'auto.navigation.sveltekit',
trace_id: expect.stringMatching(/[a-f0-9]{32}/),
},
},
});

expect(navigationTxn2Event).toMatchObject({
transaction: '/',
transaction_info: { source: 'route' },
type: 'transaction',
contexts: {
trace: {
op: 'navigation',
origin: 'auto.navigation.sveltekit',
trace_id: expect.stringMatching(/[a-f0-9]{32}/),
},
},
});

expect(navigationTxn3Event).toMatchObject({
transaction: '/nav2',
transaction_info: { source: 'route' },
type: 'transaction',
contexts: {
trace: {
op: 'navigation',
origin: 'auto.navigation.sveltekit',
trace_id: expect.stringMatching(/[a-f0-9]{32}/),
},
},
});

// traces should NOT be connected
expect(navigationTxn1Event.contexts?.trace?.trace_id).not.toBe(navigationTxn2Event.contexts?.trace?.trace_id);
expect(navigationTxn2Event.contexts?.trace?.trace_id).not.toBe(navigationTxn3Event.contexts?.trace?.trace_id);
expect(navigationTxn1Event.contexts?.trace?.trace_id).not.toBe(navigationTxn3Event.contexts?.trace?.trace_id);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -8,14 +8,21 @@ import {
getActiveSpan,
getClient,
getCurrentScope,
getIsolationScope,
getRootSpan,
spanToJSON,
startIdleSpan,
withScope,
} from '@sentry/core';
import type { Client, IntegrationFn, StartSpanOptions, TransactionSource } from '@sentry/types';
import type { Span } from '@sentry/types';
import { addHistoryInstrumentationHandler, browserPerformanceTimeOrigin, getDomElement, logger } from '@sentry/utils';
import {
addHistoryInstrumentationHandler,
browserPerformanceTimeOrigin,
getDomElement,
logger,
uuid4,
} from '@sentry/utils';

import { DEBUG_BUILD } from '../common/debug-build';
import { registerBackgroundTabDetection } from './backgroundtab';
Expand Down Expand Up @@ -373,6 +380,15 @@ export function startBrowserTracingPageLoadSpan(
* This will only do something if a browser tracing integration has been setup.
*/
export function startBrowserTracingNavigationSpan(client: Client, spanOptions: StartSpanOptions): Span | undefined {
getCurrentScope().setPropagationContext({
traceId: uuid4(),
spanId: uuid4().substring(16),
});
getIsolationScope().setPropagationContext({
traceId: uuid4(),
spanId: uuid4().substring(16),
});

client.emit('startNavigationSpan', spanOptions);

getCurrentScope().setTransactionName(spanOptions.name);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -603,7 +603,7 @@ describe('browserTracingIntegration', () => {
expect(spanToJSON(pageloadSpan!).data?.[SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]).toBe('custom');
});

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

startBrowserTracingPageLoadSpan(client, { name: 'test navigation span' });
startBrowserTracingNavigationSpan(client, { name: 'test navigation span' });

expect(getCurrentScope().getScopeData().transactionName).toBe('test navigation span');
});

it("resets the scopes' propagationContexts", () => {
const client = new TestClient(
getDefaultClientOptions({
integrations: [browserTracingIntegration()],
}),
);
setCurrentClient(client);
client.init();

const oldIsolationScopePropCtx = getIsolationScope().getPropagationContext();
const oldCurrentScopePropCtx = getCurrentScope().getPropagationContext();

startBrowserTracingNavigationSpan(client, { name: 'test navigation span' });

const newIsolationScopePropCtx = getIsolationScope().getPropagationContext();
const newCurrentScopePropCtx = getCurrentScope().getPropagationContext();

expect(oldCurrentScopePropCtx).toEqual({
spanId: expect.stringMatching(/[a-f0-9]{16}/),
traceId: expect.stringMatching(/[a-f0-9]{32}/),
});
expect(oldIsolationScopePropCtx).toEqual({
spanId: expect.stringMatching(/[a-f0-9]{16}/),
traceId: expect.stringMatching(/[a-f0-9]{32}/),
});
expect(newCurrentScopePropCtx).toEqual({
spanId: expect.stringMatching(/[a-f0-9]{16}/),
traceId: expect.stringMatching(/[a-f0-9]{32}/),
});
expect(newIsolationScopePropCtx).toEqual({
spanId: expect.stringMatching(/[a-f0-9]{16}/),
traceId: expect.stringMatching(/[a-f0-9]{32}/),
});

expect(newIsolationScopePropCtx?.traceId).not.toEqual(oldIsolationScopePropCtx?.traceId);
expect(newCurrentScopePropCtx?.traceId).not.toEqual(oldCurrentScopePropCtx?.traceId);
});
});

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