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(); diff --git a/packages/browser/src/tracing/browserTracingIntegration.ts b/packages/browser/src/tracing/browserTracingIntegration.ts index 8c01e2aa7e5b..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, { isRedirect }); 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..e39da8cdda39 100644 --- a/packages/browser/test/tracing/browserTracingIntegration.test.ts +++ b/packages/browser/test/tracing/browserTracingIntegration.test.ts @@ -788,6 +788,27 @@ 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' }, + { isRedirect: undefined }, + ); + }); }); describe('using the tag data', () => { diff --git a/packages/core/src/client.ts b/packages/core/src/client.ts index 9fbd5804f87c..e31ab482e0b3 100644 --- a/packages/core/src/client.ts +++ b/packages/core/src/client.ts @@ -603,6 +603,15 @@ 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, navigationOptions?: { isRedirect?: boolean }) => 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 +791,15 @@ 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, + navigationOptions?: { isRedirect?: boolean }, + ): void; + /** * Emit a hook event for browser tracing integrations to trigger a span for a navigation. */ 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, + }); }, ); });