Skip to content

Commit 1864b22

Browse files
authored
feat(browser): Add beforeStartNavigationSpan lifecycle hook (#16863)
This change adds a new `beforeStartNavigationSpan` lifecycle hook that gets triggered directly before the `startNavigationSpan` hook by the `startBrowserTracingNavigationSpan` helper function. The reason we need this is because we have logic for standalone CLS and LCP spans that needs to run _before_ we start preparing any work that's executed at the `startNavigationSpan` hook: - `beforeStartNavigationSpan`: send standalone span with the pageload trace id (i.e. the currently still active trace id on the propagation context - `startNavigationSpan`: first recycles the propagation context, then starts the new span This change does not yet include switching over LCP and CLS collection to the new hook (coming in a follow-up commit)
1 parent 0df39c6 commit 1864b22

File tree

6 files changed

+69
-18
lines changed

6 files changed

+69
-18
lines changed

packages/browser-utils/src/metrics/cls.ts

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -72,9 +72,12 @@ export function trackClsAsStandaloneSpan(): void {
7272
return;
7373
}
7474

75-
const unsubscribeStartNavigation = client.on('startNavigationSpan', () => {
76-
_collectClsOnce();
77-
unsubscribeStartNavigation?.();
75+
const unsubscribeStartNavigation = client.on('startNavigationSpan', (_, options) => {
76+
// we only want to collect LCP if we actually navigate. Redirects should be ignored.
77+
if (!options?.isRedirect) {
78+
_collectClsOnce();
79+
unsubscribeStartNavigation?.();
80+
}
7881
});
7982

8083
const activeSpan = getActiveSpan();

packages/browser-utils/src/metrics/lcp.ts

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -72,9 +72,12 @@ export function trackLcpAsStandaloneSpan(): void {
7272
return;
7373
}
7474

75-
const unsubscribeStartNavigation = client.on('startNavigationSpan', () => {
76-
_collectLcpOnce();
77-
unsubscribeStartNavigation?.();
75+
const unsubscribeStartNavigation = client.on('startNavigationSpan', (_, options) => {
76+
// we only want to collect LCP if we actually navigate. Redirects should be ignored.
77+
if (!options?.isRedirect) {
78+
_collectLcpOnce();
79+
unsubscribeStartNavigation?.();
80+
}
7881
});
7982

8083
const activeSpan = getActiveSpan();

packages/browser/src/tracing/browserTracingIntegration.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -657,7 +657,7 @@ export function startBrowserTracingNavigationSpan(
657657
options?: { url?: string; isRedirect?: boolean },
658658
): Span | undefined {
659659
const { url, isRedirect } = options || {};
660-
660+
client.emit('beforeStartNavigationSpan', spanOptions, { isRedirect });
661661
client.emit('startNavigationSpan', spanOptions, { isRedirect });
662662

663663
const scope = getCurrentScope();

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

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -788,6 +788,27 @@ describe('browserTracingIntegration', () => {
788788
},
789789
});
790790
});
791+
792+
it('triggers beforeStartNavigationSpan hook listeners', () => {
793+
const client = new BrowserClient(
794+
getDefaultBrowserClientOptions({
795+
tracesSampleRate: 1,
796+
integrations: [browserTracingIntegration()],
797+
}),
798+
);
799+
setCurrentClient(client);
800+
801+
const mockBeforeStartNavigationSpanCallback = vi.fn((options: StartSpanOptions) => options);
802+
803+
client.on('beforeStartNavigationSpan', mockBeforeStartNavigationSpanCallback);
804+
805+
startBrowserTracingNavigationSpan(client, { name: 'test span', op: 'navigation' });
806+
807+
expect(mockBeforeStartNavigationSpanCallback).toHaveBeenCalledWith(
808+
{ name: 'test span', op: 'navigation' },
809+
{ isRedirect: undefined },
810+
);
811+
});
791812
});
792813

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

packages/core/src/client.ts

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -603,6 +603,15 @@ export abstract class Client<O extends ClientOptions = ClientOptions> {
603603
) => void,
604604
): () => void;
605605

606+
/**
607+
* A hook for triggering right before a navigation span is started.
608+
* @returns {() => void} A function that, when executed, removes the registered callback.
609+
*/
610+
public on(
611+
hook: 'beforeStartNavigationSpan',
612+
callback: (options: StartSpanOptions, navigationOptions?: { isRedirect?: boolean }) => void,
613+
): () => void;
614+
606615
/**
607616
* A hook for browser tracing integrations to trigger a span for a navigation.
608617
* @returns {() => void} A function that, when executed, removes the registered callback.
@@ -782,6 +791,15 @@ export abstract class Client<O extends ClientOptions = ClientOptions> {
782791
traceOptions?: { sentryTrace?: string | undefined; baggage?: string | undefined },
783792
): void;
784793

794+
/**
795+
* Emit a hook event for triggering right before a navigation span is started.
796+
*/
797+
public emit(
798+
hook: 'beforeStartNavigationSpan',
799+
options: StartSpanOptions,
800+
navigationOptions?: { isRedirect?: boolean },
801+
): void;
802+
785803
/**
786804
* Emit a hook event for browser tracing integrations to trigger a span for a navigation.
787805
*/

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

Lines changed: 17 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -321,19 +321,25 @@ describe('pagesRouterInstrumentNavigation', () => {
321321

322322
Router.events.emit('routeChangeStart', targetLocation);
323323

324-
expect(emit).toHaveBeenCalledTimes(1);
324+
expect(emit).toHaveBeenCalledTimes(2);
325+
const expectedStartSpanOptions = {
326+
name: expectedTransactionName,
327+
attributes: {
328+
'sentry.op': 'navigation',
329+
'sentry.origin': 'auto.navigation.nextjs.pages_router_instrumentation',
330+
'sentry.source': expectedTransactionSource,
331+
},
332+
};
325333
expect(emit).toHaveBeenCalledWith(
326-
'startNavigationSpan',
327-
expect.objectContaining({
328-
name: expectedTransactionName,
329-
attributes: {
330-
'sentry.op': 'navigation',
331-
'sentry.origin': 'auto.navigation.nextjs.pages_router_instrumentation',
332-
'sentry.source': expectedTransactionSource,
333-
},
334-
}),
335-
{ isRedirect: undefined },
334+
'beforeStartNavigationSpan',
335+
expect.objectContaining(expectedStartSpanOptions),
336+
{
337+
isRedirect: undefined,
338+
},
336339
);
340+
expect(emit).toHaveBeenCalledWith('startNavigationSpan', expect.objectContaining(expectedStartSpanOptions), {
341+
isRedirect: undefined,
342+
});
337343
},
338344
);
339345
});

0 commit comments

Comments
 (0)