diff --git a/MIGRATION.md b/MIGRATION.md index 8e1990930207..44f29afe32c4 100644 --- a/MIGRATION.md +++ b/MIGRATION.md @@ -912,6 +912,7 @@ The following span names were adjusted: | `http.server` | The request method and route, or the raw URL path if the SDK couldn't resolve one (`GET /users/123`) | `GET /users/:id` when a route is known, otherwise just the request method (`GET`) | | `http.client`, `http.client.stream` | The request method and sanitized URL (`GET https://api.example.com/users/123`) | The request method and the domain (`GET api.example.com`), or just the method if there is no domain (`GET`) | | `router` | Framework-specific, sometimes containing the raw URL (`/users/123`, `SvelteKit Route Change`) | The span's `http.route`, or `Router` if the SDK has none | +| `handler` | Framework-specific, often carrying the request method (`GET /users/:id`, `route-handler`, `getUser`) | The span's `http.route`, or `Request handler` if the SDK has none | | `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`) | | `gen_ai.chat`, `gen_ai.embeddings`, `gen_ai.generate_content` | `{operation} {model}`, or `{operation} unknown` if the model is missing (`chat unknown`) | `{operation} {model}`, or `{operation}` if the model is missing (`chat`) | | `gen_ai.invoke_agent` | The LangChain chain name, prefixed with `chain` rather than the operation (`chain format_prompt`) | `{operation} {name}`, where the name is the span's `gen_ai.agent.name`, `gen_ai.pipeline.name` or `gen_ai.function_id`, in that order (`invoke_agent format_prompt`), or `{operation}` if the span carries none | @@ -950,6 +951,8 @@ Because the URL path is gone from `http.client` names, `graphqlClientIntegration Only the Express, Koa and Hapi integrations resolve a route template for `router` spans. Angular, Ember and SvelteKit have none when the span starts, so their router spans are named `Router`. +The Express, Fastify, Hapi and Elysia integrations resolve a route template for `handler` spans. NestJS has none when the span starts, so its request handler spans are named `Request handler`. The handler function name is no longer part of these span names. It stays on an attribute: `nestjs.callback` for NestJS, and `code.function.name` for Elysia, which now sets it on its handler spans. Elysia request handler spans also carry `http.route` now. Both attributes are set in both trace lifecycles. + Messaging span names now read ` ` in every integration. The amqplib, kafkajs and NestJS BullMQ integrations used their own word order or verb, so their names change: `my-queue process` became `process my-queue`, amqplib's `publish` became `send`, and the kafkajs batch span's `poll` became `receive`. Cloudflare Queues and the kafkajs producer already matched the conventions, so their names are the same in both trace lifecycles. The operation name an integration reports upstream stays on `messaging.operation.name`. AWS SQS `SendMessage`, `SendMessageBatch` and `ReceiveMessage`, and SNS `Publish`, are messaging spans (e.g. `queue.publish`) rather than `rpc` ones now. Every other command on those clients, such as `DeleteMessage`, stays `rpc`. Their names follow the messaging conventions too, so the operation comes first (`my-queue receive` becomes `receive my-queue`, `my-topic send` becomes `send my-topic`). A streamed SNS `Publish` to a platform endpoint is named `send`, because the endpoint ARN it used to carry ends in a per-device id (`endpoint/GCM/myapp/ send`). The full ARN remains on `messaging.destination.name`. diff --git a/dev-packages/node-integration-tests/suites/express/tracing/instrument.mjs b/dev-packages/node-integration-tests/suites/express/tracing/instrument.mjs index c727f3046e61..2a81e8aef848 100644 --- a/dev-packages/node-integration-tests/suites/express/tracing/instrument.mjs +++ b/dev-packages/node-integration-tests/suites/express/tracing/instrument.mjs @@ -2,7 +2,7 @@ import * as Sentry from '@sentry/node'; import { loggingTransport } from '@sentry-internal/node-integration-tests'; Sentry.init({ - traceLifecycle: 'static', + traceLifecycle: process.env.STREAMED === 'true' ? 'stream' : 'static', dsn: 'https://public@dsn.ingest.sentry.io/1337', release: '1.0', // disable attaching headers to /test/* endpoints diff --git a/dev-packages/node-integration-tests/suites/express/tracing/test.ts b/dev-packages/node-integration-tests/suites/express/tracing/test.ts index 765a88c8dcea..8f86f80bbd04 100644 --- a/dev-packages/node-integration-tests/suites/express/tracing/test.ts +++ b/dev-packages/node-integration-tests/suites/express/tracing/test.ts @@ -51,6 +51,37 @@ describe('express tracing', () => { await runner.completed(); }); + test('names router and request handler spans after their route when span streaming is enabled', async () => { + const runner = createRunner() + .withEnv({ STREAMED: 'true' }) + .expect({ + span: container => { + const spanFor = (type: string): (typeof container.items)[number] | undefined => + container.items.find(item => item.attributes['express.type']?.value === type); + + const handlerSpan = spanFor('request_handler'); + expect(handlerSpan?.name).toBe('/test/router/user/:id'); + // The name has to stay in step with the attribute it comes from. + expect(handlerSpan?.attributes['http.route']?.value).toBe('/test/router/user/:id'); + expect(handlerSpan?.attributes['sentry.op']?.value).toBe('handler'); + + const routerSpan = spanFor('router'); + expect(routerSpan?.name).toBe('/test/router/user'); + + // Spans of other layer types keep their names. + expect(container.items.find(item => item.name === 'corsMiddleware')?.attributes['express.type']).toEqual({ + type: 'string', + value: 'middleware', + }); + }, + }) + .start(); + + await runner.makeRequest('get', '/test/router/user/123'); + + await runner.completed(); + }); + test('should set a correct transaction name for routes specified in RegEx', async () => { const runner = createRunner() .expect({ diff --git a/dev-packages/node-integration-tests/suites/tracing/fastify/instrument.mjs b/dev-packages/node-integration-tests/suites/tracing/fastify/instrument.mjs index 170ad6f6a702..22bf57f14364 100644 --- a/dev-packages/node-integration-tests/suites/tracing/fastify/instrument.mjs +++ b/dev-packages/node-integration-tests/suites/tracing/fastify/instrument.mjs @@ -2,7 +2,7 @@ import * as Sentry from '@sentry/node'; import { loggingTransport } from '@sentry-internal/node-integration-tests'; Sentry.init({ - traceLifecycle: 'static', + traceLifecycle: process.env.STREAMED === 'true' ? 'stream' : 'static', dsn: 'https://public@dsn.ingest.sentry.io/1337', release: '1.0', tracesSampleRate: 1.0, diff --git a/dev-packages/node-integration-tests/suites/tracing/fastify/test.ts b/dev-packages/node-integration-tests/suites/tracing/fastify/test.ts index 863f8de184c1..847aad7dc50c 100644 --- a/dev-packages/node-integration-tests/suites/tracing/fastify/test.ts +++ b/dev-packages/node-integration-tests/suites/tracing/fastify/test.ts @@ -57,6 +57,32 @@ describe('fastify v5 auto-instrumentation', () => { await runner.completed(); }); + test('names request handler spans after their route when span streaming is enabled', async () => { + const runner = createRunner() + .withEnv({ STREAMED: 'true' }) + .expect({ + span: container => { + const handlerSpans = container.items.filter(item => item.attributes['sentry.op']?.value === 'handler'); + + // The request span and the route handler span. + expect(handlerSpans).toHaveLength(2); + for (const span of handlerSpans) { + expect(span.name).toBe('/test-transaction'); + // The name has to stay in step with the attribute it comes from. + expect(span.attributes['http.route']?.value).toBe('/test-transaction'); + } + + // Spans of other ops keep their names. + expect(container.items.find(item => item.name === 'preHandler - routePreHandler')).toBeDefined(); + }, + }) + .start(); + + await runner.makeRequest('get', '/test-transaction'); + + await runner.completed(); + }); + test('captures errors thrown in route handlers', async () => { const runner = createRunner() .ignore('transaction') diff --git a/dev-packages/node-integration-tests/suites/tracing/hapi/instrument.mjs b/dev-packages/node-integration-tests/suites/tracing/hapi/instrument.mjs index 170ad6f6a702..22bf57f14364 100644 --- a/dev-packages/node-integration-tests/suites/tracing/hapi/instrument.mjs +++ b/dev-packages/node-integration-tests/suites/tracing/hapi/instrument.mjs @@ -2,7 +2,7 @@ import * as Sentry from '@sentry/node'; import { loggingTransport } from '@sentry-internal/node-integration-tests'; Sentry.init({ - traceLifecycle: 'static', + traceLifecycle: process.env.STREAMED === 'true' ? 'stream' : 'static', dsn: 'https://public@dsn.ingest.sentry.io/1337', release: '1.0', tracesSampleRate: 1.0, diff --git a/dev-packages/node-integration-tests/suites/tracing/hapi/test.ts b/dev-packages/node-integration-tests/suites/tracing/hapi/test.ts index 057abb980951..e6d0f3369c3a 100644 --- a/dev-packages/node-integration-tests/suites/tracing/hapi/test.ts +++ b/dev-packages/node-integration-tests/suites/tracing/hapi/test.ts @@ -82,6 +82,30 @@ describe('hapi auto-instrumentation', () => { await runner.completed(); }); + test('names request handler spans after their route when span streaming is enabled', async () => { + const runner = createRunner() + .withEnv({ STREAMED: 'true' }) + .expect({ + span: container => { + const handlerSpan = container.items.find(item => item.attributes['sentry.op']?.value === 'handler'); + + // The route alone, without the `GET ` prefix the static name carries. + expect(handlerSpan?.name).toBe('/plugin-route'); + // The name has to stay in step with the attribute it comes from. + expect(handlerSpan?.attributes['http.route']?.value).toBe('/plugin-route'); + expect(handlerSpan?.attributes['hapi.type']?.value).toBe('plugin'); + + // Spans of other ops keep their names. + expect(container.items.find(item => item.name === 'ext - onPreResponse')).toBeDefined(); + }, + }) + .start(); + + await runner.makeRequest('get', '/plugin-route'); + + await runner.completed(); + }); + test('should handle returned plain errors in routes.', async () => { const runner = createRunner() .expect({ diff --git a/packages/core/src/integrations/express/patch-layer.ts b/packages/core/src/integrations/express/patch-layer.ts index d30aed69a967..7c23c2e11654 100644 --- a/packages/core/src/integrations/express/patch-layer.ts +++ b/packages/core/src/integrations/express/patch-layer.ts @@ -43,7 +43,7 @@ import { DEBUG_BUILD } from '../../debug-build'; import { SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN } from '../../semanticAttributes'; import { SPAN_STATUS_ERROR, withActiveSpan } from '../../tracing'; import { hasSpanStreamingEnabled } from '../../tracing/spans/hasSpanStreamingEnabled'; -import { ROUTER_SPAN_NAME_FALLBACK } from '../../tracing/spans/spanNames'; +import { REQUEST_HANDLER_SPAN_NAME_FALLBACK, ROUTER_SPAN_NAME_FALLBACK } from '../../tracing/spans/spanNames'; import { startSpanManual } from '../../tracing/trace'; import { debug } from '../../utils/debug-logger'; import type { SpanAttributes } from '../../types/span'; @@ -184,10 +184,19 @@ export function patchLayer( } const client = getClient(); - // With span streaming, span names have to be low cardinality, so router spans are named after their route. - const isStreamedRouterSpan = type === ExpressLayerType_ROUTER && !!client && hasSpanStreamingEnabled(client); - - const spanName = isStreamedRouterSpan ? actualMatchedRoute || ROUTER_SPAN_NAME_FALLBACK : name; + // With span streaming, span names have to be low cardinality, so router + // and request handler spans are named after their route. A route that did + // not validate against the request URL can describe a different request, + // so those spans take the static fallback instead. + const isStreamedSpan = !!client && hasSpanStreamingEnabled(client); + const isStreamedRouterSpan = isStreamedSpan && type === ExpressLayerType_ROUTER; + const isStreamedRequestHandlerSpan = isStreamedSpan && type === ExpressLayerType_REQUEST_HANDLER; + + const spanName = isStreamedRouterSpan + ? actualMatchedRoute || ROUTER_SPAN_NAME_FALLBACK + : isStreamedRequestHandlerSpan + ? actualMatchedRoute || REQUEST_HANDLER_SPAN_NAME_FALLBACK + : name; return startSpanManual({ name: spanName, attributes }, span => { let spanHasEnded = false; diff --git a/packages/core/src/tracing/spans/spanNames.ts b/packages/core/src/tracing/spans/spanNames.ts index 6d3862a9856b..f2460a75d9c3 100644 --- a/packages/core/src/tracing/spans/spanNames.ts +++ b/packages/core/src/tracing/spans/spanNames.ts @@ -77,6 +77,6 @@ export const ROUTER_SPAN_NAME_FALLBACK = 'Router'; /** * Fallback name for request handler spans when no better-suited span name is available. - * @see https://getsentry.github.io/sentry-conventions/names/#resource-resources + * @see https://getsentry.github.io/sentry-conventions/names/#web_server-request-handler */ -export const REQUEST_HANDLER_SPAN_NAME_FALLBACK = 'Request Handler'; +export const REQUEST_HANDLER_SPAN_NAME_FALLBACK = 'Request handler'; diff --git a/packages/core/test/lib/integrations/express/patch-layer.test.ts b/packages/core/test/lib/integrations/express/patch-layer.test.ts index 63b42ac3ec7b..926ac08040b5 100644 --- a/packages/core/test/lib/integrations/express/patch-layer.test.ts +++ b/packages/core/test/lib/integrations/express/patch-layer.test.ts @@ -649,6 +649,79 @@ describe('patchLayer', () => { ]); }); + it('names request handler spans after their route when span streaming is enabled', () => { + spanStreamingEnabled = true; + const options: ExpressPatchLayerOptions = {}; + const req = Object.assign(new EventEmitter(), { + originalUrl: '/a/b/c', + }) as unknown as ExpressRequest; + + const layer = { + name: 'handle', + handle: vi.fn(), + } as unknown as ExpressLayer; + + const res = Object.assign(new EventEmitter(), {}) as unknown as ExpressResponse; + + storeLayer(req, '/a'); + storeLayer(req, '/b'); + + patchLayer(() => options, layer, '/c'); + layer.handle(req, res); + + checkSpans([ + { + status: { code: 0, message: 'OK' }, + data: { + 'express.name': '/a/b/c', + 'express.type': 'request_handler', + 'http.route': '/a/b/c', + 'sentry.op': 'handler', + 'sentry.origin': 'auto.http.express', + }, + description: '/a/b/c', + }, + ]); + res.emit('finish'); + checkSpans([]); + }); + + it('falls back to a static request handler span name when the route is unknown', () => { + spanStreamingEnabled = true; + const options: ExpressPatchLayerOptions = {}; + const req = Object.assign(new EventEmitter(), { + originalUrl: '/abcdef', + }) as unknown as ExpressRequest; + + const layer = { + name: 'handle', + handle: vi.fn(), + } as unknown as ExpressLayer; + + const res = Object.assign(new EventEmitter(), {}) as unknown as ExpressResponse; + + storeLayer(req, '/a'); + storeLayer(req, '/b'); + + patchLayer(() => options, layer, '/c'); + layer.handle(req, res); + + checkSpans([ + { + status: { code: 0, message: 'OK' }, + data: { + 'express.name': '/a/b/c', + 'express.type': 'request_handler', + 'sentry.op': 'handler', + 'sentry.origin': 'auto.http.express', + }, + description: 'Request handler', + }, + ]); + res.emit('finish'); + checkSpans([]); + }); + it('handles case when route does not match url', () => { const onRouteResolved = vi.fn(); const options: ExpressPatchLayerOptions = { onRouteResolved }; diff --git a/packages/elysia/src/withElysia.ts b/packages/elysia/src/withElysia.ts index 3032f5af06ad..c68c192e3fa4 100644 --- a/packages/elysia/src/withElysia.ts +++ b/packages/elysia/src/withElysia.ts @@ -1,4 +1,11 @@ -import { HTTP_ROUTE, SENTRY_OP, SENTRY_SEGMENT_NAME_SOURCE, URL_FULL, URL_PATH } from '@sentry/conventions/attributes'; +import { + CODE_FUNCTION_NAME, + HTTP_ROUTE, + SENTRY_OP, + SENTRY_SEGMENT_NAME_SOURCE, + URL_FULL, + URL_PATH, +} from '@sentry/conventions/attributes'; import { HANDLER, HTTP_SERVER, MIDDLEWARE } from '@sentry/conventions/op'; import type { Span } from '@sentry/core'; import { @@ -9,6 +16,8 @@ import { getIsolationScope, getRootSpan, getTraceData, + hasSpanStreamingEnabled, + REQUEST_HANDLER_SPAN_NAME_FALLBACK, SEMANTIC_ATTRIBUTE_SENTRY_OP, SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, setHttpStatus, @@ -18,11 +27,18 @@ import { winterCGRequestToRequestData, withIsolationScope, filterCollectedUrl, - hasSpanStreamingEnabled, HTTP_SPAN_NAME_FALLBACK, } from '@sentry/core'; import type { AnyElysia, Elysia, ErrorContext, TraceHandler, TraceListener } from 'elysia'; +/** + * The part of Elysia's request context that the lifecycle spans read. Elysia types + * `.trace()`'s context as an index signature, which a required property would reject. + */ +interface LifecycleContext { + route?: string; +} + interface ElysiaHandlerOptions { shouldHandleError?: (context: ErrorContext) => boolean; } @@ -109,20 +125,38 @@ function defaultShouldHandleError(context: ErrorContext): boolean { * @param rootSpan - The root server span to parent lifecycle spans under. * Must be passed explicitly because Elysia's .trace() listener callbacks run * in a different async context where getActiveSpan() returns undefined. + * @param context - The request context. Read `route` off it inside the listener: + * Elysia assigns the route when the request enters the compiled handler, which + * is after `.trace()` hands out its listeners. */ -function instrumentLifecyclePhase(phaseName: string, listener: TraceListener, rootSpan: Span | undefined): void { +function instrumentLifecyclePhase( + phaseName: string, + listener: TraceListener, + rootSpan: Span | undefined, + context: LifecycleContext, +): void { const op = ELYSIA_LIFECYCLE_OP_MAP[phaseName]; if (!op) { return; } void listener(process => { + const client = getClient(); + const isRequestHandlerSpan = op === HANDLER; + // With span streaming, span names have to be low cardinality, so request handler + // spans are named after their route. + const isStreamedRequestHandlerSpan = isRequestHandlerSpan && !!client && hasSpanStreamingEnabled(client); + // The route describes the span in both trace lifecycles, and the other server + // integrations put it on their request handler spans too. + const routeAttribute = isRequestHandlerSpan && context.route ? { [HTTP_ROUTE]: context.route } : {}; + const phaseSpan = startInactiveSpan({ - name: phaseName, + name: isStreamedRequestHandlerSpan ? context.route || REQUEST_HANDLER_SPAN_NAME_FALLBACK : phaseName, parentSpan: rootSpan, attributes: { [SEMANTIC_ATTRIBUTE_SENTRY_OP]: op, [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: ELYSIA_ORIGIN, + ...routeAttribute, }, }); @@ -132,11 +166,16 @@ function instrumentLifecyclePhase(phaseName: string, listener: TraceListener, ro void process.onEvent(child => { const handlerName = child.name || 'anonymous'; const childSpan = startInactiveSpan({ - name: handlerName, + name: isStreamedRequestHandlerSpan ? context.route || REQUEST_HANDLER_SPAN_NAME_FALLBACK : handlerName, parentSpan: phaseSpan, attributes: { [SEMANTIC_ATTRIBUTE_SENTRY_OP]: op, [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: ELYSIA_ORIGIN, + ...routeAttribute, + // Streamed request handler spans are named after the route, so the + // handler name has no other place to go. Anonymous handlers have no + // name to record. + ...(isRequestHandlerSpan && child.name ? { [CODE_FUNCTION_NAME]: child.name } : {}), }, }); @@ -285,7 +324,7 @@ export function withElysia(app: T, options: ElysiaHandlerOp for (const [phaseName, listener] of phases) { if (listener) { - instrumentLifecyclePhase(phaseName, listener, rootSpan); + instrumentLifecyclePhase(phaseName, listener, rootSpan, lifecycle.context); } } }; diff --git a/packages/elysia/test/withElysia.test.ts b/packages/elysia/test/withElysia.test.ts index db42a65d3fe4..960372751b49 100644 --- a/packages/elysia/test/withElysia.test.ts +++ b/packages/elysia/test/withElysia.test.ts @@ -5,12 +5,16 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; // Capture handlers registered by withElysia let onAfterHandleHandler: (context: unknown) => void; let onErrorHandler: (context: unknown) => void; +let traceHandler: (lifecycle: unknown) => void; function createMockApp() { const app: Record = {}; app.use = vi.fn().mockReturnValue(app); app.wrap = vi.fn().mockReturnValue(app); - app.trace = vi.fn().mockReturnValue(app); + app.trace = vi.fn((_opts: unknown, handler: (lifecycle: unknown) => void) => { + traceHandler = handler; + return app; + }); app.onRequest = vi.fn(() => app); app.onAfterHandle = vi.fn((_opts: unknown, handler: (context: unknown) => void) => { onAfterHandleHandler = handler; @@ -30,9 +34,16 @@ const mockGetIsolationScope = vi.fn(() => ({ setSDKProcessingMetadata: vi.fn(), setTransactionName: vi.fn(), })); +let traceLifecycle: 'static' | 'stream' = 'stream'; const mockGetClient = vi.fn(() => ({ on: vi.fn(), + getOptions: () => ({ traceLifecycle }), })); +const startedSpans: { name: string; attributes?: Record }[] = []; +const mockStartInactiveSpan = vi.fn((options: { name: string; attributes?: Record }) => { + startedSpans.push({ name: options.name, attributes: options.attributes }); + return { end: vi.fn() }; +}); const mockRootSpan = { setAttribute: vi.fn(), setAttributes: vi.fn(), @@ -56,6 +67,8 @@ vi.mock('@sentry/core', async importActual => { getClient: () => mockGetClient(), getRootSpan: () => mockGetRootSpan(), getTraceData: () => mockGetTraceData(), + startInactiveSpan: (options: { name: string; attributes?: Record }) => + mockStartInactiveSpan(options), }; }); @@ -65,6 +78,8 @@ const { withElysia } = await import('../src/withElysia'); describe('withElysia', () => { beforeEach(() => { mockApp = createMockApp(); + startedSpans.length = 0; + traceLifecycle = 'stream'; }); afterEach(() => { @@ -185,6 +200,113 @@ describe('withElysia', () => { }); }); + describe('request handler span names', () => { + /** Drive the registered trace handler through a single `Handle` phase. */ + function runHandlePhase(handlerNames: string[], route = '/users/:id'): void { + traceHandler({ + context: { request: new Request('http://localhost/users/123'), route }, + onHandle: (callback: (process: unknown) => void) => { + callback({ + total: handlerNames.length, + onEvent: (onChild: (child: unknown) => void) => { + for (const name of handlerNames) { + onChild({ name, onStop: () => {} }); + } + }, + onStop: () => {}, + }); + }, + }); + } + + it('names the spans after the route when span streaming is enabled', () => { + // @ts-expect-error - mock app + withElysia(mockApp); + runHandlePhase(['getUser']); + + expect(startedSpans.map(span => span.name)).toEqual(['/users/:id', '/users/:id']); + }); + + it('uses the low cardinality fallback when the context carries no route', () => { + // @ts-expect-error - mock app + withElysia(mockApp); + runHandlePhase(['getUser'], ''); + + expect(startedSpans.map(span => span.name)).toEqual(['Request handler', 'Request handler']); + }); + + it('keeps the phase and handler names in static mode', () => { + traceLifecycle = 'static'; + // @ts-expect-error - mock app + withElysia(mockApp); + runHandlePhase(['getUser']); + + expect(startedSpans.map(span => span.name)).toEqual(['Handle', 'getUser']); + }); + + it('records the route on the handler spans in both trace lifecycles', () => { + // @ts-expect-error - mock app + withElysia(mockApp); + runHandlePhase(['getUser']); + + traceLifecycle = 'static'; + // @ts-expect-error - mock app + withElysia(createMockApp()); + runHandlePhase(['getUser']); + + expect(startedSpans).toHaveLength(4); + for (const span of startedSpans) { + expect(span.attributes).toMatchObject({ 'http.route': '/users/:id' }); + } + }); + + it('records no route when the context carries none', () => { + // @ts-expect-error - mock app + withElysia(mockApp); + runHandlePhase(['getUser'], ''); + + expect(startedSpans[0]?.attributes).not.toHaveProperty('http.route'); + expect(startedSpans[1]?.attributes).not.toHaveProperty('http.route'); + }); + + it('records no route on the spans of other lifecycle phases', () => { + // @ts-expect-error - mock app + withElysia(mockApp); + traceHandler({ + context: { request: new Request('http://localhost/users/123'), route: '/users/:id' }, + onRequest: (callback: (process: unknown) => void) => { + callback({ total: 0, onEvent: () => {}, onStop: () => {} }); + }, + }); + + expect(startedSpans).toHaveLength(1); + expect(startedSpans[0]?.attributes).toMatchObject({ 'sentry.op': 'middleware' }); + expect(startedSpans[0]?.attributes).not.toHaveProperty('http.route'); + }); + + it('records the handler name on the handler child span in both trace lifecycles', () => { + // @ts-expect-error - mock app + withElysia(mockApp); + runHandlePhase(['getUser']); + + traceLifecycle = 'static'; + // @ts-expect-error - mock app + withElysia(createMockApp()); + runHandlePhase(['getUser']); + + expect(startedSpans[1]?.attributes).toMatchObject({ 'code.function.name': 'getUser' }); + expect(startedSpans[3]?.attributes).toMatchObject({ 'code.function.name': 'getUser' }); + }); + + it('records no handler name for an anonymous handler', () => { + // @ts-expect-error - mock app + withElysia(mockApp); + runHandlePhase(['']); + + expect(startedSpans[1]?.attributes).not.toHaveProperty('code.function.name'); + }); + }); + describe('custom shouldHandleError', () => { it('uses custom shouldHandleError when provided', () => { const customShouldHandle = vi.fn(() => false); diff --git a/packages/nestjs/src/integrations/wrap-route.ts b/packages/nestjs/src/integrations/wrap-route.ts index 212643ed1dd0..10971ab25441 100644 --- a/packages/nestjs/src/integrations/wrap-route.ts +++ b/packages/nestjs/src/integrations/wrap-route.ts @@ -1,7 +1,14 @@ import { HTTP_REQUEST_METHOD, HTTP_ROUTE, SENTRY_OP, URL_FULL } from '@sentry/conventions/attributes'; import { FUNCTION, HANDLER } from '@sentry/conventions/op'; import type { SpanAttributes } from '@sentry/core'; -import { SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, startSpan, filterCollectedUrl } from '@sentry/core'; +import { + getClient, + hasSpanStreamingEnabled, + REQUEST_HANDLER_SPAN_NAME_FALLBACK, + SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, + startSpan, + filterCollectedUrl, +} from '@sentry/core'; import type { AnyFn } from './helpers'; import { copyReflectMetadata, HTTP_ORIGIN, isWrapped, markWrapped } from './helpers'; import { AttributeNames, NestType } from './enums'; @@ -61,7 +68,17 @@ export function wrapRouteHandler(callback: AnyFn, moduleVersion?: string): AnyFn [AttributeNames.VERSION]: moduleVersion || undefined, }; const wrapped = function (this: unknown, ...args: unknown[]): unknown { - return startSpan({ name: spanName, attributes }, () => callback.apply(this, args)); + const client = getClient(); + // With span streaming, span names have to be low cardinality. This wrapper + // sees only the controller method, not the request, so it has no route to + // name the span after and takes the static fallback. The enclosing + // request-context span carries `http.route`, and the callback name stays on + // the `nestjs.callback` attribute. + const isStreamedSpan = !!client && hasSpanStreamingEnabled(client); + + return startSpan({ name: isStreamedSpan ? REQUEST_HANDLER_SPAN_NAME_FALLBACK : spanName, attributes }, () => + callback.apply(this, args), + ); }; if (callback.name) { Object.defineProperty(wrapped, 'name', { value: callback.name }); diff --git a/packages/nestjs/test/integrations/orchestrion-subscriber.test.ts b/packages/nestjs/test/integrations/orchestrion-subscriber.test.ts index fef86fbfedd4..0a75619770ad 100644 --- a/packages/nestjs/test/integrations/orchestrion-subscriber.test.ts +++ b/packages/nestjs/test/integrations/orchestrion-subscriber.test.ts @@ -269,7 +269,9 @@ describe('NestJS orchestrion subscriber: request_context / request_handler', () wrappedCallback.call(instance); expect(handlerSpanJson).toBeDefined(); - expect(handlerSpanJson!.name).toBe('getCats'); + // With span streaming, the span name is low cardinality and the callback name + // only stays on the `nestjs.callback` attribute. + expect(handlerSpanJson!.name).toBe('Request handler'); expect(handlerSpanJson!.attributes['sentry.op']).toBe('handler'); expect(handlerSpanJson!.attributes['sentry.origin']).toBe('auto.http.nestjs'); expect(handlerSpanJson!.attributes).toMatchObject({ @@ -280,6 +282,25 @@ describe('NestJS orchestrion subscriber: request_context / request_handler', () }); }); + it('names the request_handler span after the callback in static mode', () => { + installTestAsyncContextStrategy(); + initTestClient('static'); + subscribeToNestChannels(); + + class CatsController {} + const instance = new CatsController(); + let handlerSpanJson: ReturnType | undefined; + function getCats(): string { + handlerSpanJson = spanToJSON(getActiveSpan()!); + return 'cats'; + } + + const { wrappedCallback } = driveCreate(instance, getCats, '10.4.1', () => () => undefined); + wrappedCallback.call(instance); + + expect(handlerSpanJson!.name).toBe('getCats'); + }); + it('nests the request_handler span under the request_context span', () => { installTestAsyncContextStrategy(); initTestClient(); diff --git a/packages/server-utils/src/integrations/express/instrumentation.ts b/packages/server-utils/src/integrations/express/instrumentation.ts index ba3fc8d7c977..5dd568b2efe5 100644 --- a/packages/server-utils/src/integrations/express/instrumentation.ts +++ b/packages/server-utils/src/integrations/express/instrumentation.ts @@ -10,6 +10,7 @@ import { getDefaultIsolationScope, getIsolationScope, hasSpanStreamingEnabled, + REQUEST_HANDLER_SPAN_NAME_FALLBACK, ROUTER_SPAN_NAME_FALLBACK, SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, startInactiveSpan, @@ -301,11 +302,20 @@ function getSpanForLayer(data: HandleChannelContext, options: ExpressIntegration } const client = getClient(); - // With span streaming, span names have to be low cardinality, so router spans are named after their route. - const isStreamedRouterSpan = type === 'router' && !!client && hasSpanStreamingEnabled(client); + // With span streaming, span names have to be low cardinality, so router + // and request handler spans are named after their route. A route that did + // not validate against the request URL can describe a different request, + // so those spans take the static fallback instead. + const isStreamedSpan = !!client && hasSpanStreamingEnabled(client); + const isStreamedRouterSpan = isStreamedSpan && type === 'router'; + const isStreamedRequestHandlerSpan = isStreamedSpan && type === 'request_handler'; const span = startInactiveSpan({ - name: isStreamedRouterSpan ? matchedRoute || ROUTER_SPAN_NAME_FALLBACK : name, + name: isStreamedRouterSpan + ? matchedRoute || ROUTER_SPAN_NAME_FALLBACK + : isStreamedRequestHandlerSpan + ? matchedRoute || REQUEST_HANDLER_SPAN_NAME_FALLBACK + : name, attributes: { [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: ORIGIN, [SENTRY_OP]: EXPRESS_TYPE_TO_SPAN_OP[type], diff --git a/packages/server-utils/src/integrations/fastify/instrumentation.ts b/packages/server-utils/src/integrations/fastify/instrumentation.ts index 860e99ac9a15..387d10a10a3a 100644 --- a/packages/server-utils/src/integrations/fastify/instrumentation.ts +++ b/packages/server-utils/src/integrations/fastify/instrumentation.ts @@ -26,7 +26,10 @@ import { HANDLER, MIDDLEWARE } from '@sentry/conventions/op'; import type { Span } from '@sentry/core'; import { isObjectLike, + getClient, getIsolationScope, + hasSpanStreamingEnabled, + REQUEST_HANDLER_SPAN_NAME_FALLBACK, SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, SPAN_STATUS_ERROR, startInactiveSpan, @@ -178,8 +181,17 @@ function onRequest(this: any, request: any, _reply: any, hookDone: () => void): setHttpServerSpanRouteAttribute(route); } + const client = getClient(); + // With span streaming, span names have to be low cardinality, so request handler + // spans are named after their route alone, without the method prefix. + const isStreamedSpan = !!client && hasSpanStreamingEnabled(client); + const requestSpan = startInactiveSpan({ - name: route != null ? `${request.method} ${route}` : 'request', + name: isStreamedSpan + ? route || REQUEST_HANDLER_SPAN_NAME_FALLBACK + : route != null + ? `${request.method} ${route}` + : 'request', attributes, }); request[kRequestSpan] = requestSpan; @@ -328,9 +340,18 @@ function handlerWrapper(handler: AnyFn, hookName: string, spanAttributes: Record const hookType = spanAttributes[ATTRIBUTE_FASTIFY_TYPE]; const op = hookType === HOOK_TYPE_INSTANCE ? MIDDLEWARE : hookType === HOOK_TYPE_HANDLER ? HANDLER : undefined; + const client = getClient(); + // With span streaming, span names have to be low cardinality, so request handler + // spans are named after their route. + const isStreamedRequestHandlerSpan = hookType === HOOK_TYPE_HANDLER && !!client && hasSpanStreamingEnabled(client); + const attributeHookName = spanAttributes[ATTRIBUTE_HOOK_NAME]; - const name = op && typeof attributeHookName === 'string' ? attributeHookName : `${hookName} - ${handlerName}`; + const name = isStreamedRequestHandlerSpan + ? spanAttributes[HTTP_ROUTE] || REQUEST_HANDLER_SPAN_NAME_FALLBACK + : op && typeof attributeHookName === 'string' + ? attributeHookName + : `${hookName} - ${handlerName}`; return startSpan( { diff --git a/packages/server-utils/src/integrations/hapi/hapi-utils.ts b/packages/server-utils/src/integrations/hapi/hapi-utils.ts index 6f418327d613..21c27d91eec5 100644 --- a/packages/server-utils/src/integrations/hapi/hapi-utils.ts +++ b/packages/server-utils/src/integrations/hapi/hapi-utils.ts @@ -14,6 +14,7 @@ import { getClient, hasSpanStreamingEnabled, isObjectLike, + REQUEST_HANDLER_SPAN_NAME_FALLBACK, ROUTER_SPAN_NAME_FALLBACK, SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, startSpan, @@ -122,15 +123,14 @@ export const getRouteMetadata = (route: ServerRoute, pluginName?: string): SpanM } const client = getClient(); - // With span streaming, span names have to be low cardinality, so router spans are named after their - // route alone, without the method prefix. - const isStreamedRouterSpan = !pluginName && !!client && hasSpanStreamingEnabled(client); + // With span streaming, span names have to be low cardinality, so router and request + // handler spans are named after their route alone, without the method prefix. + const isStreamedSpan = !!client && hasSpanStreamingEnabled(client); + const fallbackName = pluginName ? REQUEST_HANDLER_SPAN_NAME_FALLBACK : ROUTER_SPAN_NAME_FALLBACK; return { attributes, - name: isStreamedRouterSpan - ? route.path || ROUTER_SPAN_NAME_FALLBACK - : `${route.method.toUpperCase()} ${route.path}`, + name: isStreamedSpan ? route.path || fallbackName : `${route.method.toUpperCase()} ${route.path}`, }; }; diff --git a/packages/server-utils/test/integrations/hapi-utils.test.ts b/packages/server-utils/test/integrations/hapi-utils.test.ts index 9eec9f183c46..a0a7a43971fa 100644 --- a/packages/server-utils/test/integrations/hapi-utils.test.ts +++ b/packages/server-utils/test/integrations/hapi-utils.test.ts @@ -40,11 +40,21 @@ describe('getRouteMetadata', () => { expect(getRouteMetadata(route).name).toBe('/users/{id}'); }); - it('keeps the plugin span name when span streaming is enabled', () => { + it('drops the method from the plugin span name when span streaming is enabled', () => { const client = new TestClient(getDefaultTestClientOptions({ traceLifecycle: 'stream' })); setCurrentClient(client); - expect(getRouteMetadata(route, 'my-plugin').name).toBe('GET /users/{id}'); + expect(getRouteMetadata(route, 'my-plugin').name).toBe('/users/{id}'); + }); + + it('falls back to a static span name when the route has no path', () => { + const client = new TestClient(getDefaultTestClientOptions({ traceLifecycle: 'stream' })); + setCurrentClient(client); + + const pathlessRoute = { path: '', method: 'get' } as any; + + expect(getRouteMetadata(pathlessRoute).name).toBe('Router'); + expect(getRouteMetadata(pathlessRoute, 'my-plugin').name).toBe('Request handler'); }); });