From 3099f6e974a1a70a087b40e7e80d853cbe49bb4c Mon Sep 17 00:00:00 2001 From: isaacs Date: Tue, 20 Oct 2026 13:54:27 -0700 Subject: [PATCH] feat(node): Add `onSpanEnd` to `httpIntegration` `httpIntegration` takes `onSpanCreated` for incoming requests, but not `onSpanEnd`. A user could not change the server span based on the response, for example to rename it from data that is only known when the response finishes. Core's `node:http` server code already supports `onSpanEnd`, and `denoHttpIntegration` and `bunHttpServerIntegration` pass it through. `httpServerSpansIntegration` now accepts `onSpanEnd` and calls it as core does: when the response finishes, after the SDK sets the response attributes and the span status, before the span ends. `httpIntegration` passes it through. The shared `ServerRequestOptions` check for Node now lists no unsupported options. Fixes #25572 Fixes JS-3970 Co-Authored-By: Claude Opus 5.5 (1M context) --- CHANGELOG.md | 4 + .../httpIntegration/instrument-options.mjs | 6 + .../instrument-throwing-callbacks.mjs | 3 + .../suites/tracing/httpIntegration/test.ts | 137 +++++++++++++++--- .../http/httpServerSpansIntegration.ts | 19 ++- packages/node/src/integrations/http/index.ts | 8 +- 6 files changed, 149 insertions(+), 28 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b9239d53f789..ad51d92b462e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -59,6 +59,10 @@ Work in this release was contributed by @adityareddy-dev, @jpdefo, @Jxxunnn, @ph `ignoreStaticAssets` defaults to `true`, so `Bun.serve` no longer creates spans for `GET` requests to common static assets such as `favicon.ico`, `robots.txt` or `*.js` files. To keep these spans, set `bunServerIntegration({ ignoreStaticAssets: false })`. `Bun.serve` spans also follow the client's tracing configuration by default, so the integration does not start spans when tracing is disabled. Errors thrown for `OPTIONS` and `HEAD` requests are now captured. +- **feat(node): Add `onSpanEnd` to `httpIntegration`** + + `httpIntegration` and `httpServerSpansIntegration` now accept `onSpanEnd`, a hook that runs when the response for an incoming request finishes, before its span ends. Use it to change the span based on the response. Deno and Bun already have this option. + ## 11.8.0 ### Important Changes diff --git a/dev-packages/node-integration-tests/suites/tracing/httpIntegration/instrument-options.mjs b/dev-packages/node-integration-tests/suites/tracing/httpIntegration/instrument-options.mjs index d283ccff7951..6a02d7d4a26c 100644 --- a/dev-packages/node-integration-tests/suites/tracing/httpIntegration/instrument-options.mjs +++ b/dev-packages/node-integration-tests/suites/tracing/httpIntegration/instrument-options.mjs @@ -18,6 +18,12 @@ Sentry.init({ 'onSpanCreated.resMethod': res.req.method, }); }, + onSpanEnd: (span, req, res) => { + span.setAttributes({ + 'onSpanEnd.reqUrl': req.url, + 'onSpanEnd.resStatusCode': res.statusCode, + }); + }, }), ], }); diff --git a/dev-packages/node-integration-tests/suites/tracing/httpIntegration/instrument-throwing-callbacks.mjs b/dev-packages/node-integration-tests/suites/tracing/httpIntegration/instrument-throwing-callbacks.mjs index c2d7c4763781..6672b0dc99eb 100644 --- a/dev-packages/node-integration-tests/suites/tracing/httpIntegration/instrument-throwing-callbacks.mjs +++ b/dev-packages/node-integration-tests/suites/tracing/httpIntegration/instrument-throwing-callbacks.mjs @@ -15,6 +15,9 @@ Sentry.init({ onSpanCreated: () => { throw new Error('onSpanCreated error'); }, + onSpanEnd: () => { + throw new Error('onSpanEnd error'); + }, }), ], }); diff --git a/dev-packages/node-integration-tests/suites/tracing/httpIntegration/test.ts b/dev-packages/node-integration-tests/suites/tracing/httpIntegration/test.ts index 28ae77d23617..70618d73671d 100644 --- a/dev-packages/node-integration-tests/suites/tracing/httpIntegration/test.ts +++ b/dev-packages/node-integration-tests/suites/tracing/httpIntegration/test.ts @@ -1,9 +1,62 @@ import { HTTP_CLIENT, HTTP_SERVER } from '@sentry/conventions/op'; import { createTestServer } from '@sentry-internal/test-utils'; -import { HTTP_RESPONSE_STATUS_CODE, SENTRY_OP, URL_FULL, URL_PATH } from '@sentry/conventions/attributes'; +import { + CLIENT_ADDRESS, + CLIENT_PORT, + HTTP_REQUEST_METHOD, + HTTP_RESPONSE_STATUS_CODE, + HTTP_RESPONSE_STATUS_TEXT, + HTTP_ROUTE, + NETWORK_LOCAL_ADDRESS, + NETWORK_LOCAL_PORT, + NETWORK_PEER_ADDRESS, + NETWORK_PEER_PORT, + NETWORK_PROTOCOL_NAME, + NETWORK_PROTOCOL_VERSION, + NETWORK_TRANSPORT, + SENTRY_ENVIRONMENT, + SENTRY_IS_LOCALHOST, + SENTRY_KIND, + SENTRY_OP, + SENTRY_ORIGIN, + SENTRY_RELEASE, + SENTRY_SDK_INTEGRATIONS, + SENTRY_SDK_NAME, + SENTRY_SDK_VERSION, + SENTRY_SEGMENT_ID, + SENTRY_SEGMENT_NAME, + SENTRY_SEGMENT_NAME_SOURCE, + SENTRY_TRACE_LIFECYCLE, + SERVER_ADDRESS, + SERVER_PORT, + URL_FULL, + URL_PATH, + URL_SCHEME, + USER_AGENT_ORIGINAL, + USER_IP_ADDRESS, +} from '@sentry/conventions/attributes'; +import { SDK_VERSION, SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE } from '@sentry/core'; import { afterAll, describe, expect, test } from 'vitest'; import { cleanupChildProcesses, createEsmAndCjsTests, createRunner } from '../../../utils/runner'; -import { supports } from '../../../utils'; +import { EXPECTED_SDK_NAME, supports } from '../../../utils'; + +const LOOPBACK_ADDRESS = /^(::1|127\.0\.0\.1|::ffff:127\.0\.0\.1)$/; + +// Attributes from the context of the host (app, device, OS, culture, runtime, cloud). Which of them +// the SDK sends depends on the platform, so tests only check that they are well formed. +const HOST_CONTEXT_ATTRIBUTE = /^(app|device|os|culture|process\.runtime|cloud)\./; + +function splitHostContextAttributes(attributes: Record): { + hostContext: Record; + rest: Record; +} { + const hostContext: Record = {}; + const rest: Record = {}; + for (const [key, value] of Object.entries(attributes)) { + (HOST_CONTEXT_ATTRIBUTE.test(key) ? hostContext : rest)[key] = value; + } + return { hostContext, rest }; +} function getCommonHttpRequestHeaders(): Record { return { @@ -22,30 +75,76 @@ describe('httpIntegration', () => { cleanupChildProcesses(); }); - describe('onSpanCreated option', () => { + describe('onSpanCreated and onSpanEnd options', () => { createEsmAndCjsTests(__dirname, 'server.mjs', 'instrument-options.mjs', (createRunner, test) => { - test('allows to configure onSpanCreated', async () => { + test('allows to configure onSpanCreated and onSpanEnd', async () => { const runner = createRunner() .expect({ - span: { - items: expect.arrayContaining([ + span: container => { + const segmentSpan = container.items.find(span => span.is_segment); + expect(segmentSpan).toEqual( expect.objectContaining({ - is_segment: true, span_id: expect.stringMatching(/[a-f\d]{16}/), trace_id: expect.stringMatching(/[a-f\d]{32}/), status: 'ok', - attributes: expect.objectContaining({ - [URL_FULL]: { type: 'string', value: expect.stringMatching(/\/test$/) }, - [HTTP_RESPONSE_STATUS_CODE]: { type: 'integer', value: 200 }, - [SENTRY_OP]: { type: 'string', value: HTTP_SERVER }, - onSpanCreated: { type: 'string', value: 'yes' }, - 'onSpanCreated.reqUrl': { type: 'string', value: expect.stringMatching(/\/test$/) }, - 'onSpanCreated.reqMethod': { type: 'string', value: 'GET' }, - 'onSpanCreated.resUrl': { type: 'string', value: expect.stringMatching(/\/test$/) }, - 'onSpanCreated.resMethod': { type: 'string', value: 'GET' }, - }), }), - ]), + ); + + // Every other attribute is listed, so the test fails when the span gets a new one. + const { hostContext, rest } = splitHostContextAttributes(segmentSpan!.attributes); + expect(rest).toEqual({ + onSpanCreated: { type: 'string', value: 'yes' }, + 'onSpanCreated.reqUrl': { type: 'string', value: '/test' }, + 'onSpanCreated.reqMethod': { type: 'string', value: 'GET' }, + 'onSpanCreated.resUrl': { type: 'string', value: '/test' }, + 'onSpanCreated.resMethod': { type: 'string', value: 'GET' }, + 'onSpanEnd.reqUrl': { type: 'string', value: '/test' }, + 'onSpanEnd.resStatusCode': { type: 'integer', value: 200 }, + [SENTRY_OP]: { type: 'string', value: HTTP_SERVER }, + [SENTRY_ORIGIN]: { type: 'string', value: 'auto.http.http_server' }, + [SENTRY_KIND]: { type: 'string', value: 'server' }, + [SENTRY_SEGMENT_ID]: { type: 'string', value: segmentSpan!.span_id }, + [SENTRY_SEGMENT_NAME]: { type: 'string', value: 'GET /test' }, + [SENTRY_SEGMENT_NAME_SOURCE]: { type: 'string', value: 'route' }, + [SENTRY_TRACE_LIFECYCLE]: { type: 'string', value: 'stream' }, + [SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE]: { type: 'integer', value: 1 }, + [SENTRY_IS_LOCALHOST]: { type: 'boolean', value: true }, + [SENTRY_ENVIRONMENT]: { type: 'string', value: 'production' }, + [SENTRY_RELEASE]: { type: 'string', value: '1.0' }, + [SENTRY_SDK_NAME]: { type: 'string', value: EXPECTED_SDK_NAME }, + [SENTRY_SDK_VERSION]: { type: 'string', value: SDK_VERSION }, + [SENTRY_SDK_INTEGRATIONS]: { type: 'array', value: expect.arrayContaining(['Http']) }, + [HTTP_REQUEST_METHOD]: { type: 'string', value: 'GET' }, + [HTTP_ROUTE]: { type: 'string', value: '/test' }, + [HTTP_RESPONSE_STATUS_CODE]: { type: 'integer', value: 200 }, + [HTTP_RESPONSE_STATUS_TEXT]: { type: 'string', value: 'OK' }, + [URL_FULL]: { type: 'string', value: expect.stringMatching(/^http:\/\/localhost:\d+\/test$/) }, + [URL_PATH]: { type: 'string', value: '/test' }, + [URL_SCHEME]: { type: 'string', value: 'http' }, + [SERVER_ADDRESS]: { type: 'string', value: 'localhost' }, + [SERVER_PORT]: { type: 'integer', value: expect.any(Number) }, + [CLIENT_ADDRESS]: { type: 'string', value: expect.stringMatching(LOOPBACK_ADDRESS) }, + [CLIENT_PORT]: { type: 'integer', value: expect.any(Number) }, + [USER_IP_ADDRESS]: { type: 'string', value: expect.stringMatching(LOOPBACK_ADDRESS) }, + [NETWORK_LOCAL_ADDRESS]: { type: 'string', value: expect.stringMatching(LOOPBACK_ADDRESS) }, + [NETWORK_LOCAL_PORT]: { type: 'integer', value: expect.any(Number) }, + [NETWORK_PEER_ADDRESS]: { type: 'string', value: expect.stringMatching(LOOPBACK_ADDRESS) }, + [NETWORK_PEER_PORT]: { type: 'integer', value: expect.any(Number) }, + [NETWORK_PROTOCOL_NAME]: { type: 'string', value: 'http' }, + [NETWORK_PROTOCOL_VERSION]: { type: 'string', value: '1.1' }, + [NETWORK_TRANSPORT]: { type: 'string', value: 'tcp' }, + [USER_AGENT_ORIGINAL]: { type: 'string', value: 'node' }, + 'http.request.header.accept': { type: 'array', value: ['*/*'] }, + 'http.request.header.accept-encoding': { type: 'array', value: ['gzip, deflate'] }, + 'http.request.header.accept-language': { type: 'array', value: ['*'] }, + 'http.request.header.connection': { type: 'array', value: ['keep-alive'] }, + 'http.request.header.host': { type: 'array', value: [expect.stringMatching(/^localhost:\d+$/)] }, + 'http.request.header.sec-fetch-mode': { type: 'array', value: ['cors'] }, + 'http.request.header.user-agent': { type: 'array', value: ['node'] }, + }); + for (const attribute of Object.values(hostContext)) { + expect(attribute).toEqual({ type: expect.any(String), value: expect.anything() }); + } }, }) .start(); @@ -57,7 +156,7 @@ describe('httpIntegration', () => { describe('user callbacks that throw', () => { createEsmAndCjsTests(__dirname, 'server.mjs', 'instrument-throwing-callbacks.mjs', (createRunner, test) => { - test('still handles requests and sends the span when `onSpanCreated` and `ignoreIncomingRequests` throw', async () => { + test('still handles requests and sends the span when `onSpanCreated`, `onSpanEnd` and `ignoreIncomingRequests` throw', async () => { const runner = createRunner() .expect({ span: { diff --git a/packages/node/src/integrations/http/httpServerSpansIntegration.ts b/packages/node/src/integrations/http/httpServerSpansIntegration.ts index 5765629bdee9..db9c231a810f 100644 --- a/packages/node/src/integrations/http/httpServerSpansIntegration.ts +++ b/packages/node/src/integrations/http/httpServerSpansIntegration.ts @@ -36,6 +36,7 @@ import { } from '@sentry/core/server'; import { debug, + safeCallback, getSpanStatusFromHttpCode, httpHeadersToSpanAttributes, getContentLengthFromHeaders, @@ -52,7 +53,6 @@ import { filterCollectedUrlQuery, hasSpanStreamingEnabled, HTTP_SPAN_NAME_FALLBACK, - safeCallback, } from '@sentry/core'; import { DEBUG_BUILD } from '../../debug-build'; import type { NodeClient } from '../../sdk/client'; @@ -105,6 +105,12 @@ export interface HttpServerSpansIntegrationOptions { * This is triggered after the span is created, but before it is recorded. */ onSpanCreated?: (span: Span, request: HttpIncomingMessage, response: HttpServerResponse) => void; + + /** + * A hook that can be used to mutate the span one last time when the response is finished. + * This is triggered after the response status is set on the span, before the span ends. + */ + onSpanEnd?: (span: Span, request: HttpIncomingMessage, response: HttpServerResponse) => void; } const _httpServerSpansIntegration = ((options: HttpServerSpansIntegrationOptions = {}) => { @@ -113,7 +119,7 @@ const _httpServerSpansIntegration = ((options: HttpServerSpansIntegrationOptions // oxlint-disable-next-line typescript/no-deprecated const ignoreStatusCodes = options.ignoreStatusCodes ?? DEFAULT_IGNORE_STATUS_CODES; - const { onSpanCreated } = options; + const { onSpanCreated, onSpanEnd } = options; return { name: INTEGRATION_NAME, @@ -214,6 +220,15 @@ const _httpServerSpansIntegration = ((options: HttpServerSpansIntegrationOptions ); span.setAttributes(newAttributes); span.setStatus(status); + if (onSpanEnd) { + // A hook that throws must not skip `span.end()` or crash the process from the + // response's event listener. + safeCallback( + DEBUG_BUILD ? 'The `onSpanEnd` callback threw an error:' : '', + () => onSpanEnd(span, request, response), + () => undefined, + ); + } span.end(); } diff --git a/packages/node/src/integrations/http/index.ts b/packages/node/src/integrations/http/index.ts index 7431a6a34537..40c07a09ad40 100644 --- a/packages/node/src/integrations/http/index.ts +++ b/packages/node/src/integrations/http/index.ts @@ -93,13 +93,7 @@ interface HttpOptions extends HttpServerIntegrationOptions, HttpServerSpansInteg // shared field fails to compile here, until this integration supports // it or lists it as unsupported. type _ServerRequestOptionsCheck = AssertTrue< - CheckServerRequestOptions< - HttpOptions, - HttpIncomingMessage, - HttpServerResponse, - // Not supported yet. - 'onSpanEnd' - > + CheckServerRequestOptions >; /**