From bbfa3b2b2b833b7b65eb05dff891055d520a8033 Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Wed, 9 Jul 2025 16:55:26 +0200 Subject: [PATCH 1/5] feat(core): Add `beforeStartNavigationSpan` lifecycle hook --- .../src/tracing/browserTracingIntegration.ts | 2 +- .../tracing/browserTracingIntegration.test.ts | 18 ++++++++++++++++++ packages/core/src/client.ts | 11 +++++++++++ 3 files changed, 30 insertions(+), 1 deletion(-) diff --git a/packages/browser/src/tracing/browserTracingIntegration.ts b/packages/browser/src/tracing/browserTracingIntegration.ts index 8c01e2aa7e5b..5707723427cd 100644 --- a/packages/browser/src/tracing/browserTracingIntegration.ts +++ b/packages/browser/src/tracing/browserTracingIntegration.ts @@ -657,7 +657,7 @@ export function startBrowserTracingNavigationSpan( options?: { url?: string; isRedirect?: boolean }, ): Span | undefined { const { url, isRedirect } = options || {}; - + client.emit('beforeStartNavigationSpan', spanOptions); client.emit('startNavigationSpan', spanOptions, { isRedirect }); const scope = getCurrentScope(); diff --git a/packages/browser/test/tracing/browserTracingIntegration.test.ts b/packages/browser/test/tracing/browserTracingIntegration.test.ts index 0545c6618db8..18ff368060ae 100644 --- a/packages/browser/test/tracing/browserTracingIntegration.test.ts +++ b/packages/browser/test/tracing/browserTracingIntegration.test.ts @@ -788,6 +788,24 @@ describe('browserTracingIntegration', () => { }, }); }); + + it('triggers beforeStartNavigationSpan hook listeners', () => { + const client = new BrowserClient( + getDefaultBrowserClientOptions({ + tracesSampleRate: 1, + integrations: [browserTracingIntegration()], + }), + ); + setCurrentClient(client); + + const mockBeforeStartNavigationSpanCallback = vi.fn((options: StartSpanOptions) => options); + + client.on('beforeStartNavigationSpan', mockBeforeStartNavigationSpanCallback); + + startBrowserTracingNavigationSpan(client, { name: 'test span', op: 'navigation' }); + + expect(mockBeforeStartNavigationSpanCallback).toHaveBeenCalledWith({ name: 'test span', op: 'navigation' }); + }); }); describe('using the tag data', () => { diff --git a/packages/core/src/client.ts b/packages/core/src/client.ts index 9fbd5804f87c..a2f1bdf506e7 100644 --- a/packages/core/src/client.ts +++ b/packages/core/src/client.ts @@ -603,6 +603,12 @@ export abstract class Client { ) => void, ): () => void; + /** + * A hook for triggering right before a navigation span is started. + * @returns {() => void} A function that, when executed, removes the registered callback. + */ + public on(hook: 'beforeStartNavigationSpan', callback: (options: StartSpanOptions) => void): () => void; + /** * A hook for browser tracing integrations to trigger a span for a navigation. * @returns {() => void} A function that, when executed, removes the registered callback. @@ -782,6 +788,11 @@ export abstract class Client { traceOptions?: { sentryTrace?: string | undefined; baggage?: string | undefined }, ): void; + /** + * Emit a hook event for triggering right before a navigation span is started. + */ + public emit(hook: 'beforeStartNavigationSpan', options: StartSpanOptions): void; + /** * Emit a hook event for browser tracing integrations to trigger a span for a navigation. */ From ed3c762705e1171b6a548f4518d746541d71042e Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Wed, 9 Jul 2025 17:15:32 +0200 Subject: [PATCH 2/5] fix nextjs unit test --- .../pagesRouterInstrumentation.test.ts | 28 +++++++++++-------- 1 file changed, 17 insertions(+), 11 deletions(-) diff --git a/packages/nextjs/test/performance/pagesRouterInstrumentation.test.ts b/packages/nextjs/test/performance/pagesRouterInstrumentation.test.ts index ba4fdd0d9313..b58411ee2812 100644 --- a/packages/nextjs/test/performance/pagesRouterInstrumentation.test.ts +++ b/packages/nextjs/test/performance/pagesRouterInstrumentation.test.ts @@ -321,19 +321,25 @@ describe('pagesRouterInstrumentNavigation', () => { Router.events.emit('routeChangeStart', targetLocation); - expect(emit).toHaveBeenCalledTimes(1); + expect(emit).toHaveBeenCalledTimes(2); + const expectedStartSpanOptions = { + name: expectedTransactionName, + attributes: { + 'sentry.op': 'navigation', + 'sentry.origin': 'auto.navigation.nextjs.pages_router_instrumentation', + 'sentry.source': expectedTransactionSource, + }, + }; expect(emit).toHaveBeenCalledWith( - 'startNavigationSpan', - expect.objectContaining({ - name: expectedTransactionName, - attributes: { - 'sentry.op': 'navigation', - 'sentry.origin': 'auto.navigation.nextjs.pages_router_instrumentation', - 'sentry.source': expectedTransactionSource, - }, - }), - { isRedirect: undefined }, + 'beforeStartNavigationSpan', + expect.objectContaining(expectedStartSpanOptions), + { + isRedirect: undefined, + }, ); + expect(emit).toHaveBeenCalledWith('startNavigationSpan', expect.objectContaining(expectedStartSpanOptions), { + isRedirect: undefined, + }); }, ); }); From 6a5946f41a24789b5c17e82c33f91920ed31ceeb Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Thu, 10 Jul 2025 11:48:08 +0200 Subject: [PATCH 3/5] add new redirect condition to LCP/CLS spans --- .../browser/src/tracing/browserTracingIntegration.ts | 2 +- packages/core/src/client.ts | 11 +++++++++-- 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/packages/browser/src/tracing/browserTracingIntegration.ts b/packages/browser/src/tracing/browserTracingIntegration.ts index 5707723427cd..a1eb186d5c1e 100644 --- a/packages/browser/src/tracing/browserTracingIntegration.ts +++ b/packages/browser/src/tracing/browserTracingIntegration.ts @@ -657,7 +657,7 @@ export function startBrowserTracingNavigationSpan( options?: { url?: string; isRedirect?: boolean }, ): Span | undefined { const { url, isRedirect } = options || {}; - client.emit('beforeStartNavigationSpan', spanOptions); + client.emit('beforeStartNavigationSpan', spanOptions, { isRedirect }); client.emit('startNavigationSpan', spanOptions, { isRedirect }); const scope = getCurrentScope(); diff --git a/packages/core/src/client.ts b/packages/core/src/client.ts index a2f1bdf506e7..e31ab482e0b3 100644 --- a/packages/core/src/client.ts +++ b/packages/core/src/client.ts @@ -607,7 +607,10 @@ export abstract class Client { * A hook for triggering right before a navigation span is started. * @returns {() => void} A function that, when executed, removes the registered callback. */ - public on(hook: 'beforeStartNavigationSpan', callback: (options: StartSpanOptions) => void): () => void; + public on( + hook: 'beforeStartNavigationSpan', + callback: (options: StartSpanOptions, navigationOptions?: { isRedirect?: boolean }) => void, + ): () => void; /** * A hook for browser tracing integrations to trigger a span for a navigation. @@ -791,7 +794,11 @@ export abstract class Client { /** * Emit a hook event for triggering right before a navigation span is started. */ - public emit(hook: 'beforeStartNavigationSpan', options: StartSpanOptions): void; + public emit( + hook: 'beforeStartNavigationSpan', + options: StartSpanOptions, + navigationOptions?: { isRedirect?: boolean }, + ): void; /** * Emit a hook event for browser tracing integrations to trigger a span for a navigation. From a95ff210be5b0afc7b0a11daee350566a5891b94 Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Thu, 10 Jul 2025 11:57:42 +0200 Subject: [PATCH 4/5] add redirect check to CLS/LCP collection --- packages/browser-utils/src/metrics/cls.ts | 9 ++++++--- packages/browser-utils/src/metrics/lcp.ts | 9 ++++++--- 2 files changed, 12 insertions(+), 6 deletions(-) diff --git a/packages/browser-utils/src/metrics/cls.ts b/packages/browser-utils/src/metrics/cls.ts index 5e9b646fde89..dd3e1b11bd7b 100644 --- a/packages/browser-utils/src/metrics/cls.ts +++ b/packages/browser-utils/src/metrics/cls.ts @@ -72,9 +72,12 @@ export function trackClsAsStandaloneSpan(): void { return; } - const unsubscribeStartNavigation = client.on('startNavigationSpan', () => { - _collectClsOnce(); - unsubscribeStartNavigation?.(); + const unsubscribeStartNavigation = client.on('startNavigationSpan', (_, options) => { + // we only want to collect LCP if we actually navigate. Redirects should be ignored. + if (!options?.isRedirect) { + _collectClsOnce(); + unsubscribeStartNavigation?.(); + } }); const activeSpan = getActiveSpan(); diff --git a/packages/browser-utils/src/metrics/lcp.ts b/packages/browser-utils/src/metrics/lcp.ts index abbe4348fa6f..6519ced374ec 100644 --- a/packages/browser-utils/src/metrics/lcp.ts +++ b/packages/browser-utils/src/metrics/lcp.ts @@ -72,9 +72,12 @@ export function trackLcpAsStandaloneSpan(): void { return; } - const unsubscribeStartNavigation = client.on('startNavigationSpan', () => { - _collectLcpOnce(); - unsubscribeStartNavigation?.(); + const unsubscribeStartNavigation = client.on('startNavigationSpan', (_, options) => { + // we only want to collect LCP if we actually navigate. Redirects should be ignored. + if (!options?.isRedirect) { + _collectLcpOnce(); + unsubscribeStartNavigation?.(); + } }); const activeSpan = getActiveSpan(); From 5524f6031d462ec443e1e6e61602f1a311d4f7e4 Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Thu, 10 Jul 2025 12:54:11 +0200 Subject: [PATCH 5/5] fix browser unit test --- .../browser/test/tracing/browserTracingIntegration.test.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/packages/browser/test/tracing/browserTracingIntegration.test.ts b/packages/browser/test/tracing/browserTracingIntegration.test.ts index 18ff368060ae..e39da8cdda39 100644 --- a/packages/browser/test/tracing/browserTracingIntegration.test.ts +++ b/packages/browser/test/tracing/browserTracingIntegration.test.ts @@ -804,7 +804,10 @@ describe('browserTracingIntegration', () => { startBrowserTracingNavigationSpan(client, { name: 'test span', op: 'navigation' }); - expect(mockBeforeStartNavigationSpanCallback).toHaveBeenCalledWith({ name: 'test span', op: 'navigation' }); + expect(mockBeforeStartNavigationSpanCallback).toHaveBeenCalledWith( + { name: 'test span', op: 'navigation' }, + { isRedirect: undefined }, + ); }); });