diff --git a/packages/react-router/src/client/createClientInstrumentation.ts b/packages/react-router/src/client/createClientInstrumentation.ts index 97f0c0670bce..01621de85e61 100644 --- a/packages/react-router/src/client/createClientInstrumentation.ts +++ b/packages/react-router/src/client/createClientInstrumentation.ts @@ -2,13 +2,17 @@ import { startBrowserTracingNavigationSpan } from '@sentry/browser'; import type { Span } from '@sentry/core'; import { debug, + getActiveSpan, getClient, + getRootSpan, GLOBAL_OBJ, SEMANTIC_ATTRIBUTE_SENTRY_OP, SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, + spanToJSON, SPAN_STATUS_ERROR, startSpan, + updateSpanName, } from '@sentry/core'; import { DEBUG_BUILD } from '../common/debug-build'; import type { ClientInstrumentation, InstrumentableRoute, InstrumentableRouter } from '../common/types'; @@ -223,7 +227,11 @@ export function createSentryClientInstrumentation( route.instrument({ async loader(callLoader, info) { const urlPath = getPathFromRequest(info.request); - const routePattern = normalizeRoutePath(getPattern(info)) || urlPath; + const pattern = normalizeRoutePath(getPattern(info)); + const routePattern = pattern || urlPath; + // Parameterize the active navigation root span. (Route hooks don't fire on initial + // pageload, so this only affects navigations.) + updateRootSpanRoute(routePattern, !!pattern); await startSpan( { @@ -247,7 +255,9 @@ export function createSentryClientInstrumentation( async action(callAction, info) { const urlPath = getPathFromRequest(info.request); - const routePattern = normalizeRoutePath(getPattern(info)) || urlPath; + const pattern = normalizeRoutePath(getPattern(info)); + const routePattern = pattern || urlPath; + updateRootSpanRoute(routePattern, !!pattern); await startSpan( { @@ -328,6 +338,30 @@ export function createSentryClientInstrumentation( }; } +/** + * Updates the active navigation/pageload root span name with the parameterized route, so the + * transaction reflects the parameterized route pattern (e.g. `/users/:id`). + */ +function updateRootSpanRoute(routeName: string, hasPattern: boolean): void { + if (!hasPattern) { + return; + } + + const activeSpan = getActiveSpan(); + const rootSpan = activeSpan && getRootSpan(activeSpan); + if (!rootSpan) { + return; + } + + const { op } = spanToJSON(rootSpan); + if (op !== 'navigation' && op !== 'pageload') { + return; + } + + updateSpanName(rootSpan, routeName); + rootSpan.setAttribute(SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, 'route'); +} + /** * Check if React Router's instrumentation API is being used on the client. * @experimental diff --git a/packages/react-router/src/client/hydratedRouter.ts b/packages/react-router/src/client/hydratedRouter.ts index adce54afc2d2..ca07a0e50885 100644 --- a/packages/react-router/src/client/hydratedRouter.ts +++ b/packages/react-router/src/client/hydratedRouter.ts @@ -76,7 +76,19 @@ export function instrumentHydratedRouter(): void { return; } - const rootSpanName = spanToJSON(rootSpan).description; + const rootSpanJson = spanToJSON(rootSpan); + + // When the instrumentation API is active, navigation roots are parameterized + // by the native route hooks + if ( + rootSpanJson.op === 'navigation' && + isClientInstrumentationApiUsed() && + rootSpanJson.data?.[SEMANTIC_ATTRIBUTE_SENTRY_SOURCE] === 'route' + ) { + return; + } + + const rootSpanName = rootSpanJson.description; const parameterizedRoute = getParameterizedRoute(newState); if ( diff --git a/packages/react-router/test/client/createClientInstrumentation.test.ts b/packages/react-router/test/client/createClientInstrumentation.test.ts index 00323eb17629..7180615a5730 100644 --- a/packages/react-router/test/client/createClientInstrumentation.test.ts +++ b/packages/react-router/test/client/createClientInstrumentation.test.ts @@ -14,6 +14,10 @@ vi.mock('@sentry/core', async () => { startSpan: vi.fn(), captureException: vi.fn(), getClient: vi.fn(), + getActiveSpan: vi.fn(), + getRootSpan: vi.fn(), + spanToJSON: vi.fn(), + updateSpanName: vi.fn(), GLOBAL_OBJ: globalThis, SEMANTIC_ATTRIBUTE_SENTRY_OP: 'sentry.op', SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN: 'sentry.origin', @@ -750,3 +754,72 @@ describe('isNavigateHookInvoked', () => { expect(browser.startBrowserTracingNavigationSpan).not.toHaveBeenCalled(); }); }); + +describe('navigation root parameterization', () => { + beforeEach(() => { + vi.clearAllMocks(); + (core.startSpan as any).mockImplementation((_opts: any, fn: any) => fn({ setStatus: vi.fn() })); + }); + + it('renames the active navigation/pageload root span with the route pattern from the loader hook', async () => { + const mockRootSpan = { setAttribute: vi.fn() }; + (core.getActiveSpan as any).mockReturnValue({}); + (core.getRootSpan as any).mockReturnValue(mockRootSpan); + (core.spanToJSON as any).mockReturnValue({ op: 'navigation' }); + + const mockInstrument = vi.fn(); + const instrumentation = createSentryClientInstrumentation(); + instrumentation.route?.({ id: 'r', index: false, path: '/users/:id', instrument: mockInstrument }); + const hooks = mockInstrument.mock.calls[0]![0]; + + await hooks.loader(vi.fn().mockResolvedValue({ status: 'success', error: undefined }), { + request: { method: 'GET', url: 'http://localhost/users/123', headers: { get: () => null } }, + params: { id: '123' }, + unstable_pattern: '/users/:id', + context: undefined, + }); + + expect(core.updateSpanName).toHaveBeenCalledWith(mockRootSpan, '/users/:id'); + expect(mockRootSpan.setAttribute).toHaveBeenCalledWith('sentry.source', 'route'); + }); + + it('does not rename the root span when the route has no pattern', async () => { + const mockRootSpan = { setAttribute: vi.fn() }; + (core.getActiveSpan as any).mockReturnValue({}); + (core.getRootSpan as any).mockReturnValue(mockRootSpan); + (core.spanToJSON as any).mockReturnValue({ op: 'navigation' }); + + const mockInstrument = vi.fn(); + const instrumentation = createSentryClientInstrumentation(); + instrumentation.route?.({ id: 'r', index: false, path: undefined, instrument: mockInstrument }); + const hooks = mockInstrument.mock.calls[0]![0]; + + await hooks.loader(vi.fn().mockResolvedValue({ status: 'success', error: undefined }), { + request: { method: 'GET', url: 'http://localhost/unknown', headers: { get: () => null } }, + params: {}, + context: undefined, + }); + + expect(core.updateSpanName).not.toHaveBeenCalled(); + }); + + it('does not rename root spans that are not pageload/navigation', async () => { + (core.getActiveSpan as any).mockReturnValue({}); + (core.getRootSpan as any).mockReturnValue({ setAttribute: vi.fn() }); + (core.spanToJSON as any).mockReturnValue({ op: 'http.server' }); + + const mockInstrument = vi.fn(); + const instrumentation = createSentryClientInstrumentation(); + instrumentation.route?.({ id: 'r', index: false, path: '/users/:id', instrument: mockInstrument }); + const hooks = mockInstrument.mock.calls[0]![0]; + + await hooks.loader(vi.fn().mockResolvedValue({ status: 'success', error: undefined }), { + request: { method: 'GET', url: 'http://localhost/users/123', headers: { get: () => null } }, + params: { id: '123' }, + unstable_pattern: '/users/:id', + context: undefined, + }); + + expect(core.updateSpanName).not.toHaveBeenCalled(); + }); +}); diff --git a/packages/react-router/test/client/hydratedRouter.test.ts b/packages/react-router/test/client/hydratedRouter.test.ts index e02af8e9f55d..b4e0449e2d46 100644 --- a/packages/react-router/test/client/hydratedRouter.test.ts +++ b/packages/react-router/test/client/hydratedRouter.test.ts @@ -119,6 +119,57 @@ describe('instrumentHydratedRouter', () => { expect(mockPageloadSpan.setAttribute).toHaveBeenLastCalledWith(SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, 'route'); }); + it('skips the subscribe heuristic when the API is active and a route hook already set source:route', () => { + // When a native route hook has parameterized the navigation root (source:route), the legacy + // heuristic must not re-rename it. + (globalThis as any).__sentryReactRouterClientInstrumentationUsed = true; + (core.spanToJSON as any).mockImplementation((span: any) => ({ + description: '/foo/bar', + op: span === mockNavigationSpan ? 'navigation' : 'pageload', + data: { source: 'route' }, + })); + + instrumentHydratedRouter(); + const callback = mockRouter.subscribe.mock.calls[0][0]; + const newState = { + location: { pathname: '/foo/bar' }, + matches: [{ route: { path: '/foo/:id' } }], + navigation: { state: 'idle' }, + }; + (core.getActiveSpan as any).mockReturnValue(mockNavigationSpan); + callback(newState); + + expect(mockNavigationSpan.updateName).not.toHaveBeenCalled(); + + delete (globalThis as any).__sentryReactRouterClientInstrumentationUsed; + }); + + it('still parameterizes a navigation root via subscribe (backstop) when the API is active but the route had no hook (source:url)', () => { + // Routes without a loader/action never trigger a route hook, so the navigation root is still + // source:url. The heuristic must still parameterize it instead of leaving the raw URL. + (globalThis as any).__sentryReactRouterClientInstrumentationUsed = true; + (core.spanToJSON as any).mockImplementation((span: any) => ({ + description: '/foo/bar', + op: span === mockNavigationSpan ? 'navigation' : 'pageload', + data: { source: 'url' }, + })); + + instrumentHydratedRouter(); + const callback = mockRouter.subscribe.mock.calls[0][0]; + const newState = { + location: { pathname: '/foo/bar' }, + matches: [{ route: { path: '/foo/:id' } }], + navigation: { state: 'idle' }, + }; + (core.getActiveSpan as any).mockReturnValue(mockNavigationSpan); + callback(newState); + + expect(mockNavigationSpan.updateName).toHaveBeenCalledWith('/foo/:id'); + expect(mockNavigationSpan.setAttribute).toHaveBeenCalledWith(SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, 'route'); + + delete (globalThis as any).__sentryReactRouterClientInstrumentationUsed; + }); + it('does not update navigation transaction on state change to loading', () => { instrumentHydratedRouter(); // Simulate a state change to loading (non-idle)