diff --git a/dev-packages/e2e-tests/test-applications/nitro-3/tests/span-nesting.test.ts b/dev-packages/e2e-tests/test-applications/nitro-3/tests/span-nesting.test.ts index bfe35fd0b054..31b65ae4ef96 100644 --- a/dev-packages/e2e-tests/test-applications/nitro-3/tests/span-nesting.test.ts +++ b/dev-packages/e2e-tests/test-applications/nitro-3/tests/span-nesting.test.ts @@ -24,49 +24,56 @@ test('Span nesting: all spans share the same trace_id', async ({ request }) => { } }); -test('Span nesting: h3 middleware spans are children of the srvx request span', async ({ request }) => { +test('Span nesting: the segment is the only http.server span', async ({ request }) => { const spansPromise = collectNestingSpans(); await request.get('/api/test-nesting'); const spans = await spansPromise; - // The srvx request span nests below the http_server segment span of the request - const segmentSpan = spans.find(span => span.is_segment && span.name === 'GET /api/test-nesting'); - const srvxSpan = spans.find( - span => span.attributes['sentry.origin']?.value === 'auto.http.nitro.srvx' && getSpanOp(span) === 'http.server', - ); - expect(srvxSpan).toBeDefined(); - expect(srvxSpan?.parent_span_id).toBe(segmentSpan!.span_id); + const httpServerSpans = spans.filter(span => getSpanOp(span) === 'http.server'); + expect(httpServerSpans).toHaveLength(1); + expect(httpServerSpans[0]).toMatchObject({ + is_segment: true, + attributes: expect.objectContaining({ + 'sentry.origin': { type: 'string', value: 'auto.http.http_server' }, + }), + }); +}); + +test('Span nesting: h3 middleware spans are children of the segment span', async ({ request }) => { + const spansPromise = collectNestingSpans(); + + await request.get('/api/test-nesting'); - // All h3 middleware spans should be children of the srvx span + const spans = await spansPromise; + + const segmentSpan = spans.find(span => span.is_segment && span.name === 'GET /api/test-nesting'); const h3MiddlewareSpans = spans.filter( span => span.attributes['sentry.origin']?.value === 'auto.http.nitro.h3' && getSpanOp(span) === 'middleware', ); expect(h3MiddlewareSpans.length).toBeGreaterThanOrEqual(1); for (const span of h3MiddlewareSpans) { - expect(span.parent_span_id).toBe(srvxSpan!.span_id); + expect(span.parent_span_id).toBe(segmentSpan!.span_id); } }); -test('Span nesting: h3 route handler span is a child of the srvx request span', async ({ request }) => { +test('Span nesting: h3 route handler span is a child of the segment span', async ({ request }) => { const spansPromise = collectNestingSpans(); await request.get('/api/test-nesting'); const spans = await spansPromise; - const srvxSpan = spans.find( - span => span.attributes['sentry.origin']?.value === 'auto.http.nitro.srvx' && getSpanOp(span) === 'http.server', - ); - expect(srvxSpan).toBeDefined(); - + const segmentSpan = spans.find(span => span.is_segment && span.name === 'GET /api/test-nesting'); const h3HandlerSpan = spans.find( - span => span.attributes['sentry.origin']?.value === 'auto.http.nitro.h3' && getSpanOp(span) === 'http.server', + span => span.attributes['sentry.origin']?.value === 'auto.http.nitro.h3' && getSpanOp(span) === 'handler', ); - expect(h3HandlerSpan).toBeDefined(); - expect(h3HandlerSpan!.parent_span_id).toBe(srvxSpan!.span_id); + expect(h3HandlerSpan).toMatchObject({ + name: 'GET /api/test-nesting', + parent_span_id: segmentSpan!.span_id, + }); }); test('Span nesting: manual startSpan calls inside route handler are children of the h3 route handler span', async ({ @@ -80,7 +87,7 @@ test('Span nesting: manual startSpan calls inside route handler are children of // Find the h3 route handler span const h3HandlerSpan = spans.find( - span => span.attributes['sentry.origin']?.value === 'auto.http.nitro.h3' && getSpanOp(span) === 'http.server', + span => span.attributes['sentry.origin']?.value === 'auto.http.nitro.h3' && getSpanOp(span) === 'handler', ); expect(h3HandlerSpan).toBeDefined(); diff --git a/dev-packages/e2e-tests/test-applications/nitro-3/tests/transactions.test.ts b/dev-packages/e2e-tests/test-applications/nitro-3/tests/transactions.test.ts index 12fe83043014..e93073f97ba6 100644 --- a/dev-packages/e2e-tests/test-applications/nitro-3/tests/transactions.test.ts +++ b/dev-packages/e2e-tests/test-applications/nitro-3/tests/transactions.test.ts @@ -10,17 +10,19 @@ test('Sends a segment span for a successful route', async ({ request }) => { const spans = await spansPromise; - // The node http server integration creates the segment span; srvx and h3 spans nest below it + // The node http server integration creates the segment span, and the only http.server span const segmentSpan = spans.find(span => span.is_segment && span.name === 'GET /api/test-transaction'); expect(segmentSpan).toBeDefined(); expect(segmentSpan?.attributes['sentry.origin']?.value).toBe('auto.http.http_server'); expect(getSpanOp(segmentSpan!)).toBe('http.server'); + expect(spans.filter(span => getSpanOp(span) === 'http.server')).toHaveLength(1); + // h3 creates a child span for the route handler - const h3Spans = spans.filter( - span => span.trace_id === segmentSpan?.trace_id && span.attributes['sentry.origin']?.value === 'auto.http.nitro.h3', + const h3HandlerSpan = spans.find( + span => span.attributes['sentry.origin']?.value === 'auto.http.nitro.h3' && getSpanOp(span) === 'handler', ); - expect(h3Spans.length).toBeGreaterThanOrEqual(1); + expect(h3HandlerSpan?.parent_span_id).toBe(segmentSpan?.span_id); }); test('Sets correct HTTP status code on the segment span', async ({ request }) => { diff --git a/dev-packages/node-integration-tests/suites/tracing/nitro/test.ts b/dev-packages/node-integration-tests/suites/tracing/nitro/test.ts index 499815daca0d..bbc59e833ee8 100644 --- a/dev-packages/node-integration-tests/suites/tracing/nitro/test.ts +++ b/dev-packages/node-integration-tests/suites/tracing/nitro/test.ts @@ -9,41 +9,39 @@ describe.runIf(supports({ min: 20 }))('nitro integrations', () => { }); createEsmTests(__dirname, 'scenario.mjs', 'instrument.mjs', (createRunner, test) => { - test('creates h3 and srvx spans and sets Server-Timing headers', async () => { + test('creates a single http.server span with a request handler span and sets Server-Timing headers', async () => { let traceId: string | undefined; const runner = createRunner() .expect({ span: container => { const segment = container.items.find(span => span.is_segment); - const srvxSpan = container.items.find( - span => span.attributes['sentry.origin']?.value === 'auto.http.nitro.srvx', - ); - const h3Span = container.items.find( - span => span.attributes['sentry.origin']?.value === 'auto.http.nitro.h3', - ); + const children = container.items.filter(span => !span.is_segment); + const handlerSpan = children.find(span => span.attributes['sentry.origin']?.value === 'auto.http.nitro.h3'); traceId = segment?.trace_id; - expect(segment?.name).toBe('GET /hello/:name'); - expect(segment?.attributes['http.route']).toEqual({ type: 'string', value: '/hello/:name' }); - - // Bun publishes no `http.server.request.start`, so there the srvx span is the segment itself. - expect(srvxSpan).toMatchObject({ + // On Node the runtime's http instrumentation creates the segment. Bun publishes no + // `http.server.request.start`, so there srvx creates it instead. + expect(segment).toMatchObject({ + name: 'GET /hello/:name', status: 'ok', attributes: expect.objectContaining({ 'sentry.op': { type: 'string', value: 'http.server' }, + 'http.route': { type: 'string', value: '/hello/:name' }, 'http.response.status_code': { type: 'integer', value: 200 }, + 'url.path.parameter.name': { type: 'string', value: 'world' }, }), }); - expect(h3Span).toMatchObject({ + expect(children.filter(span => span.attributes['sentry.op']?.value === 'http.server')).toEqual([]); + + expect(handlerSpan).toMatchObject({ name: 'GET /hello/:name', - parent_span_id: srvxSpan?.span_id, + parent_span_id: segment?.span_id, status: 'ok', attributes: expect.objectContaining({ - 'sentry.op': { type: 'string', value: 'http.server' }, - 'http.route': { type: 'string', value: '/hello/:name' }, + 'sentry.op': { type: 'string', value: 'handler' }, }), }); }, diff --git a/packages/server-utils/src/integrations/nitro/captureTracingEvents.ts b/packages/server-utils/src/integrations/nitro/captureTracingEvents.ts index 2431d4115af8..42f02ed96152 100644 --- a/packages/server-utils/src/integrations/nitro/captureTracingEvents.ts +++ b/packages/server-utils/src/integrations/nitro/captureTracingEvents.ts @@ -1,5 +1,5 @@ import { SENTRY_OP, SENTRY_SEGMENT_NAME_SOURCE, SENTRY_ORIGIN } from '@sentry/conventions/attributes'; -import { HTTP_SERVER, MIDDLEWARE } from '@sentry/conventions/op'; +import { HANDLER, HTTP_SERVER, MIDDLEWARE } from '@sentry/conventions/op'; import { isObjectLike, getActiveSpan, @@ -13,6 +13,7 @@ import { parseStringToURLObject, setHttpStatus, type Span, + spanToJSON, startInactiveSpan, } from '@sentry/core'; import { bindTracingChannelToSpan, type TracingChannelPayloadWithSpan } from '../../tracing-channel'; @@ -60,6 +61,25 @@ function applyResponseStatus(span: Span, data: TracingChannelPayloadWithSpan<{ r } } +/** + * Whether the request already has an `http.server` root span, created by the runtime's own HTTP + * server instrumentation (Node `http`, `Bun.serve`, `Deno.serve`, Cloudflare's request wrapper). + */ +function hasHttpServerRootSpan(): boolean { + const activeSpan = getActiveSpan(); + return !!activeSpan && spanToJSON(getRootSpan(activeSpan)).attributes[SENTRY_OP] === HTTP_SERVER; +} + +/** + * The port the srvx server listens on. `options.port` is only the configured one, which is `0` for a + * random port. + */ +function getServerPort(server: SrvxRequestEvent['server']): number | undefined { + const url = server.url ? parseStringToURLObject(server.url) : undefined; + const port = url && 'port' in url ? Number(url.port) : undefined; + return port || server.options.port || undefined; +} + /** * Extracts the parameterized route pattern from the h3 event context. */ @@ -88,6 +108,25 @@ function setupH3TracingChannels(): void { bindTracingChannelToSpan( diagnosticsChannel.tracingChannel('h3.request'), data => { + const method = data.event.req.method?.toUpperCase() || HTTP_SPAN_NAME_FALLBACK; + + // The route lives on the request's `http.server` root span, so the route handler only gets a + // span that times the handler itself instead of yet another nested `http.server` span. + if (data.type === 'route' && hasHttpServerRootSpan()) { + const matchedRoute = data.event.context?.matchedRoute?.route; + const span = startInactiveSpan({ + name: matchedRoute ? `${method} ${matchedRoute}` : method, + attributes: { + [SENTRY_ORIGIN]: 'auto.http.nitro.h3', + [SENTRY_OP]: HANDLER, + }, + }); + + setParameterizedRouteAttributes(span, data.event); + + return span; + } + const parsedUrl = parseStringToURLObject(data.event.url.href); const routePattern = getParameterizedRoute(data.event); @@ -103,19 +142,17 @@ function setupH3TracingChannels(): void { // With span streaming, span names have to be low cardinality, so we can't fall back to the URL. // Only applies to the http.server span; middleware spans keep their own naming. const isUnparameterizedStreamedServerSpan = - data?.type !== 'middleware' && + data.type !== 'middleware' && urlAttributes[SENTRY_SEGMENT_NAME_SOURCE] !== 'route' && !!client && hasSpanStreamingEnabled(client); const span = startInactiveSpan({ - name: isUnparameterizedStreamedServerSpan - ? data.event.req.method?.toUpperCase() || HTTP_SPAN_NAME_FALLBACK - : spanName, + name: isUnparameterizedStreamedServerSpan ? method : spanName, attributes: { ...urlAttributes, [SENTRY_ORIGIN]: 'auto.http.nitro.h3', - [SENTRY_OP]: data?.type === 'middleware' ? MIDDLEWARE : HTTP_SERVER, + [SENTRY_OP]: data.type === 'middleware' ? MIDDLEWARE : HTTP_SERVER, }, }); @@ -127,8 +164,8 @@ function setupH3TracingChannels(): void { beforeSpanEnd(span, data) { applyResponseStatus(span, data); - // Update the root span (srvx transaction) with the parameterized route name. - // The srvx span is created before h3 resolves the route, so it initially has the raw URL. + // Update the root span with the parameterized route name. The root span is created before h3 + // resolves the route, so it initially has the raw URL. // Note: data.type is always 'middleware' here regardless of handler type, so we rely on // getParameterizedRoute() to filter out catch-all routes instead. const routePattern = getParameterizedRoute(data.event); @@ -152,6 +189,11 @@ function setupSrvxTracingChannels(): void { bindTracingChannelToSpan( diagnosticsChannel.tracingChannel('srvx.request'), data => { + // srvx only opens the request's `http.server` span when the runtime has not already created one. + if (hasHttpServerRootSpan()) { + return undefined; + } + const parsedUrl = data.request._url ? parseStringToURLObject(data.request._url.href) : undefined; const client = getClient(); const [spanName, urlAttributes] = getHttpSpanDetailsFromUrlObject( @@ -188,7 +230,7 @@ function setupSrvxTracingChannels(): void { ...headerAttributes, [SENTRY_ORIGIN]: 'auto.http.nitro.srvx', [SENTRY_OP]: data.middleware ? MIDDLEWARE : HTTP_SERVER, - 'server.port': data.server.options.port, + 'server.port': getServerPort(data.server), }, // Use the same parent span as middleware to make them siblings parentSpan: requestParentSpans.get(data.request) || undefined, diff --git a/packages/server-utils/src/integrations/nitro/nitroTypes.ts b/packages/server-utils/src/integrations/nitro/nitroTypes.ts index 4736a2dd1ec3..192ce3c34952 100644 --- a/packages/server-utils/src/integrations/nitro/nitroTypes.ts +++ b/packages/server-utils/src/integrations/nitro/nitroTypes.ts @@ -33,7 +33,7 @@ export interface H3TracingRequestEvent { /** Payload of the `srvx.request`/`srvx.middleware` tracing channels (`srvx/tracing`'s `RequestEvent`). */ export interface SrvxRequestEvent { - server: { options: { port?: number } }; + server: { url?: string; options: { port?: number } }; request: { _url?: { href: string; pathname: string }; method: string;