From 3b8c22fada430da319451cf2a2fd8f64ecd24087 Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Mon, 21 Sep 2026 17:26:44 +0200 Subject: [PATCH] fix(react-router): Use low-cardinality names for `function` spans The loader, action and fetcher spans from the instrumentation API still named themselves after the value they had at hand: the route pattern, which falls back to the raw request path when React Router matches no pattern, or the fetcher key, which is unique per fetcher instance. Under span streaming both are high cardinality. Name these spans after the function they wrap, matching the `code.function.name` they already set. The `function` op's description template is `{{code.function.name}}` and nothing else, so Relay cannot infer the route from any attribute we could add - the previous name is kept on `sentry.description` instead. Co-Authored-By: Claude Opus 5 (1M context) --- MIGRATION.md | 1 + .../tests/performance/lazy.server.test.ts | 3 +- .../performance/performance.server.test.ts | 6 +- .../src/client/createClientInstrumentation.ts | 29 +++++++- .../src/server/createServerInstrumentation.ts | 19 ++++- .../createClientInstrumentation.test.ts | 70 ++++++++++++++++++- .../createServerInstrumentation.test.ts | 58 +++++++++++++++ 7 files changed, 175 insertions(+), 11 deletions(-) diff --git a/MIGRATION.md b/MIGRATION.md index aadb4dd7793a..04a25bbca366 100644 --- a/MIGRATION.md +++ b/MIGRATION.md @@ -1027,6 +1027,7 @@ The following span names were adjusted: | `function` (Angular `TraceMethod`) | The decorator's `name` option in angle brackets | ``, `` | The decorator's `name` option, or `Function execution` if it has none | `Login.ngOnInit`, `getUsers`, `Function execution` | | `function` (SvelteKit) | The route the wrapped function ran for, or the raw URL path if the SDK couldn't resolve one | `/users/[id]`, `/users/123`, `GET /api/users/[id]` | The name of the wrapped function | `load`, `GET` | | `function` (Ember route hooks) | The full route name | `slow-loading-route.index` | The hook the span wraps, matching its `code.function.name`. The route moves to `sentry.description` | `beforeModel`, `model`, `setupController` | +| `function` (React Router route hooks) | The route the hook ran for, the raw URL path if React Router matched no pattern, or the fetcher key | `/users/:id`, `/users/123`, `Fetcher fetcher-1` | The hook the span wraps, matching its `code.function.name`. The previous name moves to `sentry.description` | `loader`, `action`, `clientLoader`, `fetcher` | | `function.gcp` | The request method and path for HTTP functions, otherwise the trigger's event or trigger type | `POST /users`, `google.pubsub.topic.publish`, `firebase.function.http.request` | The function name, or `Serverless function execution` if the SDK cannot resolve one | `myFunction`, `Serverless function execution` | | `function.aws` | The Lambda function name | `my-function` | Unchanged, except that the SDK now falls back to `Serverless function execution` if it cannot resolve the function name | `my-function`, `Serverless function execution` | | `graphql` | The graphql phase and, for operations, the operation name | `query GetUser`, `graphql.parse`, `graphql.resolve user.0.name` | The operation type, or the processing type where there is none | `GraphQL query`, `GraphQL parse`, `GraphQL resolve` | diff --git a/dev-packages/e2e-tests/test-applications/react-router-7-framework-instrumentation/tests/performance/lazy.server.test.ts b/dev-packages/e2e-tests/test-applications/react-router-7-framework-instrumentation/tests/performance/lazy.server.test.ts index ca252bc8c706..a0c163661234 100644 --- a/dev-packages/e2e-tests/test-applications/react-router-7-framework-instrumentation/tests/performance/lazy.server.test.ts +++ b/dev-packages/e2e-tests/test-applications/react-router-7-framework-instrumentation/tests/performance/lazy.server.test.ts @@ -60,13 +60,14 @@ test.describe('server - instrumentation API lazy loading', () => { expect(loaderSpan).toMatchObject({ span_id: expect.any(String), trace_id: expect.any(String), - name: '/performance/lazy-route', + name: 'loader', }); expect(loaderSpan!.attributes).toMatchObject({ 'sentry.origin': { value: 'auto.function.react_router.instrumentation_api', type: 'string' }, 'sentry.op': { value: 'function', type: 'string' }, 'code.function.name': { value: 'loader', type: 'string' }, + 'sentry.description': { value: '/performance/lazy-route', type: 'string' }, }); }); diff --git a/dev-packages/e2e-tests/test-applications/react-router-7-framework-instrumentation/tests/performance/performance.server.test.ts b/dev-packages/e2e-tests/test-applications/react-router-7-framework-instrumentation/tests/performance/performance.server.test.ts index e895b2e93f01..bf3cb1682ca2 100644 --- a/dev-packages/e2e-tests/test-applications/react-router-7-framework-instrumentation/tests/performance/performance.server.test.ts +++ b/dev-packages/e2e-tests/test-applications/react-router-7-framework-instrumentation/tests/performance/performance.server.test.ts @@ -86,7 +86,7 @@ test.describe('server - instrumentation API performance', () => { expect(loaderSpan).toMatchObject({ span_id: expect.any(String), trace_id: expect.any(String), - name: '/performance/server-loader', + name: 'loader', parent_span_id: expect.any(String), start_timestamp: expect.any(Number), end_timestamp: expect.any(Number), @@ -97,6 +97,7 @@ test.describe('server - instrumentation API performance', () => { 'sentry.origin': { value: 'auto.function.react_router.instrumentation_api', type: 'string' }, 'sentry.op': { value: 'function', type: 'string' }, 'code.function.name': { value: 'loader', type: 'string' }, + 'sentry.description': { value: '/performance/server-loader', type: 'string' }, }); }); @@ -113,7 +114,7 @@ test.describe('server - instrumentation API performance', () => { expect(actionSpan).toMatchObject({ span_id: expect.any(String), trace_id: expect.any(String), - name: '/performance/server-action', + name: 'action', parent_span_id: expect.any(String), start_timestamp: expect.any(Number), end_timestamp: expect.any(Number), @@ -124,6 +125,7 @@ test.describe('server - instrumentation API performance', () => { 'sentry.origin': { value: 'auto.function.react_router.instrumentation_api', type: 'string' }, 'sentry.op': { value: 'function', type: 'string' }, 'code.function.name': { value: 'action', type: 'string' }, + 'sentry.description': { value: '/performance/server-action', type: 'string' }, }); }); diff --git a/packages/react-router/src/client/createClientInstrumentation.ts b/packages/react-router/src/client/createClientInstrumentation.ts index ff6bbf609abe..faa39b4fbe75 100644 --- a/packages/react-router/src/client/createClientInstrumentation.ts +++ b/packages/react-router/src/client/createClientInstrumentation.ts @@ -28,6 +28,7 @@ import { import { SENTRY_SEGMENT_NAME_SOURCE, CODE_FUNCTION_NAME, + SENTRY_DESCRIPTION, SENTRY_OP, URL_FULL, URL_TEMPLATE, @@ -231,13 +232,21 @@ export function createSentryClientInstrumentation( }, async fetch(callFetch, info) { + const client = getClient(); + const hasSpanStreaming = !!client && hasSpanStreamingEnabled(client); + const description = `Fetcher ${info.fetcherKey}`; + await startSpan( { - name: `Fetcher ${info.fetcherKey}`, + // With span streaming, a `function` span is named after the function it wraps. The + // fetcher key identifies a single fetcher instance and would be high cardinality. + name: hasSpanStreaming ? 'fetcher' : description, attributes: { [SENTRY_OP]: FUNCTION, [CODE_FUNCTION_NAME]: 'fetcher', [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.function.react_router.instrumentation_api', + // Relay infers a `function` span's description from `code.function.name` alone, which drops the key. + ...(hasSpanStreaming && { [SENTRY_DESCRIPTION]: description }), }, }, async span => { @@ -266,13 +275,20 @@ export function createSentryClientInstrumentation( // pageload, so this only affects navigations.) updateRootSpanRoute(routePattern, !!pattern); + const client = getClient(); + const hasSpanStreaming = !!client && hasSpanStreamingEnabled(client); + await startSpan( { - name: routePattern, + // With span streaming, a `function` span is named after the function it wraps, because + // `routePattern` falls back to the raw request path for routes without a pattern. + name: hasSpanStreaming ? 'clientLoader' : routePattern, attributes: { [SENTRY_OP]: FUNCTION, [CODE_FUNCTION_NAME]: 'clientLoader', [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.function.react_router.instrumentation_api', + // Relay infers a `function` span's description from `code.function.name` alone, which drops the route. + ...(hasSpanStreaming && { [SENTRY_DESCRIPTION]: routePattern }), }, }, async span => { @@ -293,13 +309,20 @@ export function createSentryClientInstrumentation( const routePattern = pattern || urlPath; updateRootSpanRoute(routePattern, !!pattern); + const client = getClient(); + const hasSpanStreaming = !!client && hasSpanStreamingEnabled(client); + await startSpan( { - name: routePattern, + // With span streaming, a `function` span is named after the function it wraps, because + // `routePattern` falls back to the raw request path for routes without a pattern. + name: hasSpanStreaming ? 'clientAction' : routePattern, attributes: { [SENTRY_OP]: FUNCTION, [CODE_FUNCTION_NAME]: 'clientAction', [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.function.react_router.instrumentation_api', + // Relay infers a `function` span's description from `code.function.name` alone, which drops the route. + ...(hasSpanStreaming && { [SENTRY_DESCRIPTION]: routePattern }), }, }, async span => { diff --git a/packages/react-router/src/server/createServerInstrumentation.ts b/packages/react-router/src/server/createServerInstrumentation.ts index 436ade2dd6d9..f6fdee0e0133 100644 --- a/packages/react-router/src/server/createServerInstrumentation.ts +++ b/packages/react-router/src/server/createServerInstrumentation.ts @@ -3,6 +3,7 @@ import { CODE_FUNCTION_NAME, HTTP_REQUEST_METHOD, HTTP_ROUTE, + SENTRY_DESCRIPTION, SENTRY_OP, URL_FULL, URL_PATH, @@ -144,13 +145,20 @@ export function createSentryServerInstrumentation( const routePattern = normalizeRoutePath(pattern) || urlPath; updateRootSpanWithRoute(info.request.method, pattern, urlPath); + const client = getClient(); + const hasSpanStreaming = !!client && hasSpanStreamingEnabled(client); + await startSpan( { - name: routePattern, + // With span streaming, a `function` span is named after the function it wraps, because + // `routePattern` falls back to the raw request path for routes without a pattern. + name: hasSpanStreaming ? 'loader' : routePattern, attributes: { [SENTRY_OP]: FUNCTION, [CODE_FUNCTION_NAME]: 'loader', [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.function.react_router.instrumentation_api', + // Relay infers a `function` span's description from `code.function.name` alone, which drops the route. + ...(hasSpanStreaming && { [SENTRY_DESCRIPTION]: routePattern }), }, }, async span => { @@ -172,13 +180,20 @@ export function createSentryServerInstrumentation( const routePattern = normalizeRoutePath(pattern) || urlPath; updateRootSpanWithRoute(info.request.method, pattern, urlPath); + const client = getClient(); + const hasSpanStreaming = !!client && hasSpanStreamingEnabled(client); + await startSpan( { - name: routePattern, + // With span streaming, a `function` span is named after the function it wraps, because + // `routePattern` falls back to the raw request path for routes without a pattern. + name: hasSpanStreaming ? 'action' : routePattern, attributes: { [SENTRY_OP]: FUNCTION, [CODE_FUNCTION_NAME]: 'action', [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.function.react_router.instrumentation_api', + // Relay infers a `function` span's description from `code.function.name` alone, which drops the route. + ...(hasSpanStreaming && { [SENTRY_DESCRIPTION]: routePattern }), }, }, async span => { diff --git a/packages/react-router/test/client/createClientInstrumentation.test.ts b/packages/react-router/test/client/createClientInstrumentation.test.ts index 5ab217787727..bf400872ab5e 100644 --- a/packages/react-router/test/client/createClientInstrumentation.test.ts +++ b/packages/react-router/test/client/createClientInstrumentation.test.ts @@ -45,6 +45,7 @@ vi.mock('@sentry/browser', () => ({ // Span streaming is the default trace lifecycle, and it's what makes span names low cardinality. const mockStreamingClient = { getOptions: () => ({ traceLifecycle: 'stream' }) }; +const mockStaticClient = { getOptions: () => ({ traceLifecycle: 'static' }) }; describe('createSentryClientInstrumentation', () => { beforeEach(() => { @@ -206,6 +207,7 @@ describe('createSentryClientInstrumentation', () => { const mockInstrument = vi.fn(); (coreBrowser.startSpan as any).mockImplementation((_opts: any, fn: any) => fn()); + (core.getClient as any).mockReturnValue(mockStreamingClient); const instrumentation = createSentryClientInstrumentation(); instrumentation.router?.({ instrument: mockInstrument }); @@ -221,10 +223,11 @@ describe('createSentryClientInstrumentation', () => { expect(coreBrowser.startSpan).toHaveBeenCalledWith( expect.objectContaining({ - name: 'Fetcher fetcher-1', + name: 'fetcher', attributes: expect.objectContaining({ 'sentry.op': 'function', 'code.function.name': 'fetcher', + 'sentry.description': 'Fetcher fetcher-1', 'sentry.origin': 'auto.function.react_router.instrumentation_api', }), }), @@ -233,11 +236,35 @@ describe('createSentryClientInstrumentation', () => { expect(mockCallFetch).toHaveBeenCalled(); }); + it('keeps the fetcher key in the span name without span streaming', async () => { + const mockCallFetch = vi.fn().mockResolvedValue({ status: 'success', error: undefined }); + const mockInstrument = vi.fn(); + + (coreBrowser.startSpan as any).mockImplementation((_opts: any, fn: any) => fn()); + (core.getClient as any).mockReturnValue(mockStaticClient); + + const instrumentation = createSentryClientInstrumentation(); + instrumentation.router?.({ instrument: mockInstrument }); + + const hooks = mockInstrument.mock.calls[0]![0]; + + await hooks.fetch(mockCallFetch, { href: '/api/data', currentUrl: '/home', fetcherKey: 'fetcher-1' }); + + expect(coreBrowser.startSpan).toHaveBeenCalledWith( + expect.objectContaining({ + name: 'Fetcher fetcher-1', + attributes: expect.not.objectContaining({ 'sentry.description': expect.anything() }), + }), + expect.any(Function), + ); + }); + it('should instrument route loader with spans', async () => { const mockCallLoader = vi.fn().mockResolvedValue({ status: 'success', error: undefined }); const mockInstrument = vi.fn(); (coreBrowser.startSpan as any).mockImplementation((_opts: any, fn: any) => fn()); + (core.getClient as any).mockReturnValue(mockStreamingClient); const instrumentation = createSentryClientInstrumentation(); // Route has id, index, path as required properties @@ -261,10 +288,11 @@ describe('createSentryClientInstrumentation', () => { expect(coreBrowser.startSpan).toHaveBeenCalledWith( expect.objectContaining({ - name: '/users/:id', + name: 'clientLoader', attributes: expect.objectContaining({ 'sentry.op': 'function', 'code.function.name': 'clientLoader', + 'sentry.description': '/users/:id', 'sentry.origin': 'auto.function.react_router.instrumentation_api', }), }), @@ -278,6 +306,7 @@ describe('createSentryClientInstrumentation', () => { const mockInstrument = vi.fn(); (coreBrowser.startSpan as any).mockImplementation((_opts: any, fn: any) => fn()); + (core.getClient as any).mockReturnValue(mockStreamingClient); const instrumentation = createSentryClientInstrumentation(); instrumentation.route?.({ @@ -299,10 +328,11 @@ describe('createSentryClientInstrumentation', () => { expect(coreBrowser.startSpan).toHaveBeenCalledWith( expect.objectContaining({ - name: '/users/:id', + name: 'clientAction', attributes: expect.objectContaining({ 'sentry.op': 'function', 'code.function.name': 'clientAction', + 'sentry.description': '/users/:id', 'sentry.origin': 'auto.function.react_router.instrumentation_api', }), }), @@ -593,6 +623,7 @@ describe('createSentryClientInstrumentation', () => { const mockInstrument = vi.fn(); (coreBrowser.startSpan as any).mockImplementation((_opts: any, fn: any) => fn()); + (core.getClient as any).mockReturnValue(mockStaticClient); const instrumentation = createSentryClientInstrumentation(); instrumentation.route?.({ @@ -620,6 +651,39 @@ describe('createSentryClientInstrumentation', () => { ); }); + it('keeps the raw pathname out of the span name with span streaming', async () => { + const mockCallLoader = vi.fn().mockResolvedValue({ status: 'success', error: undefined }); + const mockInstrument = vi.fn(); + + (coreBrowser.startSpan as any).mockImplementation((_opts: any, fn: any) => fn()); + (core.getClient as any).mockReturnValue(mockStreamingClient); + + const instrumentation = createSentryClientInstrumentation(); + instrumentation.route?.({ + id: 'test-route', + index: false, + path: '/test', + instrument: mockInstrument, + }); + + const hooks = mockInstrument.mock.calls[0]![0]; + + await hooks.loader(mockCallLoader, { + request: { method: 'GET', url: 'http://example.com/users/123', headers: { get: () => null } }, + params: { id: '123' }, + unstable_pattern: undefined, + context: undefined, + }); + + expect(coreBrowser.startSpan).toHaveBeenCalledWith( + expect.objectContaining({ + name: 'clientLoader', + attributes: expect.objectContaining({ 'sentry.description': '/users/123' }), + }), + expect.any(Function), + ); + }); + it('should instrument route middleware with spans', async () => { const mockCallMiddleware = vi.fn().mockResolvedValue({ status: 'success', error: undefined }); const mockInstrument = vi.fn(); diff --git a/packages/react-router/test/server/createServerInstrumentation.test.ts b/packages/react-router/test/server/createServerInstrumentation.test.ts index c7b82679430e..05844562895f 100644 --- a/packages/react-router/test/server/createServerInstrumentation.test.ts +++ b/packages/react-router/test/server/createServerInstrumentation.test.ts @@ -101,6 +101,64 @@ describe('createSentryServerInstrumentation', () => { expect(core.updateSpanName).toHaveBeenCalledWith(mockRootSpan, 'GET /users/:id'); }); + + it.each([ + ['loader', 'loader', '/users/:id'], + ['action', 'action', '/users/:id'], + ])('names the %s span after the function it wraps', async (hookName, functionName, pattern) => { + const mockCall = vi.fn().mockResolvedValue({ status: 'success', error: undefined }); + const mockInstrument = vi.fn(); + + (core.startSpan as any).mockImplementation((_opts: any, fn: any) => fn()); + + const instrumentation = createSentryServerInstrumentation(); + instrumentation.route?.({ id: 'test-route', index: false, path: '/test', instrument: mockInstrument }); + const hooks = mockInstrument.mock.calls[0]![0]; + + await hooks[hookName](mockCall, { + request: { method: 'GET', url: 'http://example.com/users/123', headers: { get: () => null } }, + params: { id: '123' }, + unstable_pattern: pattern, + context: undefined, + }); + + expect(core.startSpan).toHaveBeenCalledWith( + expect.objectContaining({ + name: functionName, + attributes: expect.objectContaining({ + 'code.function.name': functionName, + 'sentry.description': pattern, + }), + }), + expect.any(Function), + ); + }); + + it('keeps the raw pathname out of the span name for an unmatched route', async () => { + const mockCall = vi.fn().mockResolvedValue({ status: 'success', error: undefined }); + const mockInstrument = vi.fn(); + + (core.startSpan as any).mockImplementation((_opts: any, fn: any) => fn()); + + const instrumentation = createSentryServerInstrumentation(); + instrumentation.route?.({ id: 'test-route', index: false, path: '/test', instrument: mockInstrument }); + const hooks = mockInstrument.mock.calls[0]![0]; + + await hooks.loader(mockCall, { + request: { method: 'GET', url: 'http://example.com/users/123', headers: { get: () => null } }, + params: { id: '123' }, + unstable_pattern: undefined, + context: undefined, + }); + + expect(core.startSpan).toHaveBeenCalledWith( + expect.objectContaining({ + name: 'loader', + attributes: expect.objectContaining({ 'sentry.description': '/users/123' }), + }), + expect.any(Function), + ); + }); }); it('should set the global flag when React Router invokes the handler registration', () => {