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', () => {