Skip to content

Commit 644da33

Browse files
mydeamydea
committed
fix continue trace
1 parent 3370b86 commit 644da33

File tree

7 files changed

+113
-40
lines changed

7 files changed

+113
-40
lines changed

‎packages/core/src/baseclient.ts‎

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -450,7 +450,13 @@ export abstract class BaseClient<O extends ClientOptions> implements Client<O> {
450450
): void;
451451

452452
/** @inheritdoc */
453-
public on(hook: 'startPageLoadSpan', callback: (options: StartSpanOptions) => void): void;
453+
public on(
454+
hook: 'startPageLoadSpan',
455+
callback: (
456+
options: StartSpanOptions,
457+
traceOptions?: { sentryTrace?: string | undefined; baggage?: string | undefined },
458+
) => void,
459+
): void;
454460

455461
/** @inheritdoc */
456462
public on(hook: 'startNavigationSpan', callback: (options: StartSpanOptions) => void): void;
@@ -500,7 +506,11 @@ export abstract class BaseClient<O extends ClientOptions> implements Client<O> {
500506
public emit(hook: 'beforeSendFeedback', feedback: FeedbackEvent, options?: { includeReplay: boolean }): void;
501507

502508
/** @inheritdoc */
503-
public emit(hook: 'startPageLoadSpan', options: StartSpanOptions): void;
509+
public emit(
510+
hook: 'startPageLoadSpan',
511+
options: StartSpanOptions,
512+
traceOptions?: { sentryTrace?: string | undefined; baggage?: string | undefined },
513+
): void;
504514

505515
/** @inheritdoc */
506516
public emit(hook: 'startNavigationSpan', options: StartSpanOptions): void;

‎packages/nextjs/src/client/routing/pagesRouterRoutingInstrumentation.ts‎

Lines changed: 15 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -3,9 +3,6 @@ import {
33
SEMANTIC_ATTRIBUTE_SENTRY_OP,
44
SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN,
55
SEMANTIC_ATTRIBUTE_SENTRY_SOURCE,
6-
continueTrace,
7-
getCurrentScope,
8-
withScope,
96
} from '@sentry/core';
107
import { WINDOW, startBrowserTracingNavigationSpan, startBrowserTracingPageLoadSpan } from '@sentry/react';
118
import type { Client, TransactionSource } from '@sentry/types';
@@ -110,31 +107,21 @@ export function pagesRouterInstrumentPageLoad(client: Client): void {
110107
const { route, params, sentryTrace, baggage } = extractNextDataTagInformation();
111108
const name = route || globalObject.location.pathname;
112109

113-
// Continue trace updates the _current_ scope, but we want to break out of it again...
114-
// This is a bit hacky, because we want to get the span to use both the correct scope _and_ the correct propagation context
115-
// but wards, we want to reset it to avoid this also applying to other spans
116-
const scope = getCurrentScope();
117-
const propagationContextBefore = scope.getPropagationContext();
118-
119-
continueTrace({ sentryTrace, baggage }, () => {
120-
// Ensure we are on the original current scope again, so the span is set as active on it
121-
return withScope(scope, () => {
122-
startBrowserTracingPageLoadSpan(client, {
123-
name,
124-
// pageload should always start at timeOrigin (and needs to be in s, not ms)
125-
startTime: browserPerformanceTimeOrigin ? browserPerformanceTimeOrigin / 1000 : undefined,
126-
127-
attributes: {
128-
[SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'pageload',
129-
[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.pageload.nextjs.pages_router_instrumentation',
130-
[SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: route ? 'route' : 'url',
131-
...(params && client.getOptions().sendDefaultPii && { ...params }),
132-
},
133-
});
134-
});
135-
});
136-
137-
scope.setPropagationContext(propagationContextBefore);
110+
startBrowserTracingPageLoadSpan(
111+
client,
112+
{
113+
name,
114+
// pageload should always start at timeOrigin (and needs to be in s, not ms)
115+
startTime: browserPerformanceTimeOrigin ? browserPerformanceTimeOrigin / 1000 : undefined,
116+
attributes: {
117+
[SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'pageload',
118+
[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.pageload.nextjs.pages_router_instrumentation',
119+
[SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: route ? 'route' : 'url',
120+
...(params && client.getOptions().sendDefaultPii && { ...params }),
121+
},
122+
},
123+
{ sentryTrace, baggage },
124+
);
138125
}
139126

140127
/**

‎packages/nextjs/test/performance/appRouterInstrumentation.test.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,7 @@ describe('appRouterInstrumentPageLoad', () => {
5151
'sentry.source': 'url',
5252
},
5353
}),
54+
undefined,
5455
);
5556
});
5657
});

‎packages/nextjs/test/performance/pagesRouterInstrumentation.test.ts‎

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -194,8 +194,18 @@ describe('pagesRouterInstrumentPageLoad', () => {
194194

195195
pagesRouterInstrumentPageLoad(client);
196196

197+
const sentryTrace = (props as any).pageProps?._sentryTraceData;
198+
const baggage = (props as any).pageProps?._sentryBaggage;
199+
197200
expect(emit).toHaveBeenCalledTimes(1);
198-
expect(emit).toHaveBeenCalledWith('startPageLoadSpan', expect.objectContaining(expectedStartTransactionArgument));
201+
expect(emit).toHaveBeenCalledWith(
202+
'startPageLoadSpan',
203+
expect.objectContaining(expectedStartTransactionArgument),
204+
{
205+
sentryTrace,
206+
baggage,
207+
},
208+
);
199209
},
200210
);
201211
});

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

Lines changed: 14 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -245,7 +245,7 @@ export const browserTracingIntegration = ((_options: Partial<BrowserTracingOptio
245245
let activeSpan: Span | undefined;
246246
let startingUrl: string | undefined = WINDOW.location && WINDOW.location.href;
247247

248-
client.on('startNavigationSpan', (startSpanOptions: StartSpanOptions) => {
248+
client.on('startNavigationSpan', startSpanOptions => {
249249
if (getClient() !== client) {
250250
return;
251251
}
@@ -261,7 +261,7 @@ export const browserTracingIntegration = ((_options: Partial<BrowserTracingOptio
261261
});
262262
});
263263

264-
client.on('startPageLoadSpan', (startSpanOptions: StartSpanOptions) => {
264+
client.on('startPageLoadSpan', (startSpanOptions, traceOptions) => {
265265
if (getClient() !== client) {
266266
return;
267267
}
@@ -272,10 +272,10 @@ export const browserTracingIntegration = ((_options: Partial<BrowserTracingOptio
272272
activeSpan.end();
273273
}
274274

275-
const sentryTrace = getMetaContent('sentry-trace');
276-
const baggage = getMetaContent('baggage');
275+
const sentryTrace = traceOptions?.sentryTrace || getMetaContent('sentry-trace');
276+
const baggage = traceOptions?.baggage || getMetaContent('baggage');
277277

278-
// Continue trace updates the _current_ scope, but we want to break out of it again...
278+
// Continue trace updates the scope in the callback only, but we want to break out of it again...
279279
// This is a bit hacky, because we want to get the span to use both the correct scope _and_ the correct propagation context
280280
// but afterwards, we want to reset it to avoid this also applying to other spans
281281
const scope = getCurrentScope();
@@ -364,9 +364,16 @@ export const browserTracingIntegration = ((_options: Partial<BrowserTracingOptio
364364
/**
365365
* Manually start a page load span.
366366
* This will only do something if a browser tracing integration integration has been setup.
367+
*
368+
* If you provide a custom `traceOptions` object, it will be used to continue the trace
369+
* instead of the default behavior, which is to look it up on the <meta> tags.
367370
*/
368-
export function startBrowserTracingPageLoadSpan(client: Client, spanOptions: StartSpanOptions): Span | undefined {
369-
client.emit('startPageLoadSpan', spanOptions);
371+
export function startBrowserTracingPageLoadSpan(
372+
client: Client,
373+
spanOptions: StartSpanOptions,
374+
traceOptions?: { sentryTrace?: string | undefined; baggage?: string | undefined },
375+
): Span | undefined {
376+
client.emit('startPageLoadSpan', spanOptions, traceOptions);
370377

371378
const span = getActiveSpan();
372379
const op = span && spanToJSON(span).op;

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

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -708,6 +708,54 @@ describe('browserTracingIntegration', () => {
708708
expect(propagationContext.traceId).not.toEqual('12312012123120121231201212312012');
709709
expect(propagationContext.parentSpanId).not.toEqual('1121201211212012');
710710
});
711+
712+
it('uses passed in tracing data for pageload span over meta tags', () => {
713+
// make sampled false here, so we can see that it's being used rather than the tracesSampleRate-dictated one
714+
document.head.innerHTML =
715+
'<meta name="sentry-trace" content="12312012123120121231201212312012-1121201211212012-1">' +
716+
'<meta name="baggage" content="sentry-release=2.1.14,foo=bar">';
717+
718+
const client = new TestClient(
719+
getDefaultClientOptions({
720+
tracesSampleRate: 1,
721+
integrations: [browserTracingIntegration({ instrumentPageLoad: false })],
722+
}),
723+
);
724+
setCurrentClient(client);
725+
726+
client.init();
727+
728+
// manually create a pageload span with tracing data
729+
startBrowserTracingPageLoadSpan(
730+
client,
731+
{
732+
name: 'test span',
733+
},
734+
{
735+
sentryTrace: '12312012123120121231201212312011-1121201211212011-1',
736+
baggage: 'sentry-release=2.2.14,foo=bar',
737+
},
738+
);
739+
740+
const idleSpan = getActiveSpan()!;
741+
expect(idleSpan).toBeDefined();
742+
743+
const dynamicSamplingContext = getDynamicSamplingContextFromSpan(idleSpan!);
744+
const propagationContext = getCurrentScope().getPropagationContext();
745+
746+
// Span is correct
747+
expect(spanToJSON(idleSpan).op).toBe('pageload');
748+
expect(spanToJSON(idleSpan).trace_id).toEqual('12312012123120121231201212312011');
749+
expect(spanToJSON(idleSpan).parent_span_id).toEqual('1121201211212011');
750+
expect(spanIsSampled(idleSpan)).toBe(true);
751+
752+
expect(dynamicSamplingContext).toBeDefined();
753+
expect(dynamicSamplingContext).toStrictEqual({ release: '2.2.14' });
754+
755+
// Propagation context is reset and does not contain the meta tag data
756+
expect(propagationContext.traceId).not.toEqual('12312012123120121231201212312012');
757+
expect(propagationContext.parentSpanId).not.toEqual('1121201211212012');
758+
});
711759
});
712760

713761
describe('idleTimeout', () => {

‎packages/types/src/client.ts‎

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -248,7 +248,13 @@ export interface Client<O extends ClientOptions = ClientOptions> {
248248
/**
249249
* A hook for the browser tracing integrations to trigger a span start for a page load.
250250
*/
251-
on(hook: 'startPageLoadSpan', callback: (options: StartSpanOptions) => void): void;
251+
on(
252+
hook: 'startPageLoadSpan',
253+
callback: (
254+
options: StartSpanOptions,
255+
traceOptions?: { sentryTrace?: string | undefined; baggage?: string | undefined },
256+
) => void,
257+
): void;
252258

253259
/**
254260
* A hook for browser tracing integrations to trigger a span for a navigation.
@@ -321,7 +327,11 @@ export interface Client<O extends ClientOptions = ClientOptions> {
321327
/**
322328
* Emit a hook event for browser tracing integrations to trigger a span start for a page load.
323329
*/
324-
emit(hook: 'startPageLoadSpan', options: StartSpanOptions): void;
330+
emit(
331+
hook: 'startPageLoadSpan',
332+
options: StartSpanOptions,
333+
traceOptions?: { sentryTrace?: string | undefined; baggage?: string | undefined },
334+
): void;
325335

326336
/**
327337
* Emit a hook event for browser tracing integrations to trigger a span for a navigation.

0 commit comments

Comments
 (0)