Skip to content

Commit 5b2de99

Browse files
isaacsclaude
isaacs
and
claude
authored
feat(node): Add onSpanEnd to httpIntegration (#25581)
`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) <noreply@anthropic.com>
1 parent ad1bd23 commit 5b2de99

6 files changed

Lines changed: 149 additions & 28 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -203,6 +203,10 @@ Work in this release was contributed by @4ktLuffy, @adityareddy-dev, @dasilvalui
203203

204204
`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.
205205

206+
- **feat(node): Add `onSpanEnd` to `httpIntegration`**
207+
208+
`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.
209+
206210
## 11.8.0
207211

208212
### Important Changes

‎dev-packages/node-integration-tests/suites/tracing/httpIntegration/instrument-options.mjs‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,12 @@ Sentry.init({
1818
'onSpanCreated.resMethod': res.req.method,
1919
});
2020
},
21+
onSpanEnd: (span, req, res) => {
22+
span.setAttributes({
23+
'onSpanEnd.reqUrl': req.url,
24+
'onSpanEnd.resStatusCode': res.statusCode,
25+
});
26+
},
2127
}),
2228
],
2329
});

‎dev-packages/node-integration-tests/suites/tracing/httpIntegration/instrument-throwing-callbacks.mjs‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,9 @@ Sentry.init({
1515
onSpanCreated: () => {
1616
throw new Error('onSpanCreated error');
1717
},
18+
onSpanEnd: () => {
19+
throw new Error('onSpanEnd error');
20+
},
1821
}),
1922
],
2023
});

‎dev-packages/node-integration-tests/suites/tracing/httpIntegration/test.ts‎

Lines changed: 118 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,62 @@
11
import { HTTP_CLIENT, HTTP_SERVER } from '@sentry/conventions/op';
22
import { createTestServer } from '@sentry-internal/test-utils';
3-
import { HTTP_RESPONSE_STATUS_CODE, SENTRY_OP, URL_FULL, URL_PATH } from '@sentry/conventions/attributes';
3+
import {
4+
CLIENT_ADDRESS,
5+
CLIENT_PORT,
6+
HTTP_REQUEST_METHOD,
7+
HTTP_RESPONSE_STATUS_CODE,
8+
HTTP_RESPONSE_STATUS_TEXT,
9+
HTTP_ROUTE,
10+
NETWORK_LOCAL_ADDRESS,
11+
NETWORK_LOCAL_PORT,
12+
NETWORK_PEER_ADDRESS,
13+
NETWORK_PEER_PORT,
14+
NETWORK_PROTOCOL_NAME,
15+
NETWORK_PROTOCOL_VERSION,
16+
NETWORK_TRANSPORT,
17+
SENTRY_ENVIRONMENT,
18+
SENTRY_IS_LOCALHOST,
19+
SENTRY_KIND,
20+
SENTRY_OP,
21+
SENTRY_ORIGIN,
22+
SENTRY_RELEASE,
23+
SENTRY_SDK_INTEGRATIONS,
24+
SENTRY_SDK_NAME,
25+
SENTRY_SDK_VERSION,
26+
SENTRY_SEGMENT_ID,
27+
SENTRY_SEGMENT_NAME,
28+
SENTRY_SEGMENT_NAME_SOURCE,
29+
SENTRY_TRACE_LIFECYCLE,
30+
SERVER_ADDRESS,
31+
SERVER_PORT,
32+
URL_FULL,
33+
URL_PATH,
34+
URL_SCHEME,
35+
USER_AGENT_ORIGINAL,
36+
USER_IP_ADDRESS,
37+
} from '@sentry/conventions/attributes';
38+
import { SDK_VERSION, SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE } from '@sentry/core';
439
import { afterAll, describe, expect, test } from 'vitest';
540
import { cleanupChildProcesses, createEsmAndCjsTests, createRunner } from '../../../utils/runner';
6-
import { supports } from '../../../utils';
41+
import { EXPECTED_SDK_NAME, supports } from '../../../utils';
42+
43+
const LOOPBACK_ADDRESS = /^(::1|127\.0\.0\.1|::ffff:127\.0\.0\.1)$/;
44+
45+
// Attributes from the context of the host (app, device, OS, culture, runtime, cloud). Which of them
46+
// the SDK sends depends on the platform, so tests only check that they are well formed.
47+
const HOST_CONTEXT_ATTRIBUTE = /^(app|device|os|culture|process\.runtime|cloud)\./;
48+
49+
function splitHostContextAttributes(attributes: Record<string, unknown>): {
50+
hostContext: Record<string, unknown>;
51+
rest: Record<string, unknown>;
52+
} {
53+
const hostContext: Record<string, unknown> = {};
54+
const rest: Record<string, unknown> = {};
55+
for (const [key, value] of Object.entries(attributes)) {
56+
(HOST_CONTEXT_ATTRIBUTE.test(key) ? hostContext : rest)[key] = value;
57+
}
58+
return { hostContext, rest };
59+
}
760

861
function getCommonHttpRequestHeaders(): Record<string, unknown> {
962
return {
@@ -22,30 +75,76 @@ describe('httpIntegration', () => {
2275
cleanupChildProcesses();
2376
});
2477

25-
describe('onSpanCreated option', () => {
78+
describe('onSpanCreated and onSpanEnd options', () => {
2679
createEsmAndCjsTests(__dirname, 'server.mjs', 'instrument-options.mjs', (createRunner, test) => {
27-
test('allows to configure onSpanCreated', async () => {
80+
test('allows to configure onSpanCreated and onSpanEnd', async () => {
2881
const runner = createRunner()
2982
.expect({
30-
span: {
31-
items: expect.arrayContaining([
83+
span: container => {
84+
const segmentSpan = container.items.find(span => span.is_segment);
85+
expect(segmentSpan).toEqual(
3286
expect.objectContaining({
33-
is_segment: true,
3487
span_id: expect.stringMatching(/[a-f\d]{16}/),
3588
trace_id: expect.stringMatching(/[a-f\d]{32}/),
3689
status: 'ok',
37-
attributes: expect.objectContaining({
38-
[URL_FULL]: { type: 'string', value: expect.stringMatching(/\/test$/) },
39-
[HTTP_RESPONSE_STATUS_CODE]: { type: 'integer', value: 200 },
40-
[SENTRY_OP]: { type: 'string', value: HTTP_SERVER },
41-
onSpanCreated: { type: 'string', value: 'yes' },
42-
'onSpanCreated.reqUrl': { type: 'string', value: expect.stringMatching(/\/test$/) },
43-
'onSpanCreated.reqMethod': { type: 'string', value: 'GET' },
44-
'onSpanCreated.resUrl': { type: 'string', value: expect.stringMatching(/\/test$/) },
45-
'onSpanCreated.resMethod': { type: 'string', value: 'GET' },
46-
}),
4790
}),
48-
]),
91+
);
92+
93+
// Every other attribute is listed, so the test fails when the span gets a new one.
94+
const { hostContext, rest } = splitHostContextAttributes(segmentSpan!.attributes);
95+
expect(rest).toEqual({
96+
onSpanCreated: { type: 'string', value: 'yes' },
97+
'onSpanCreated.reqUrl': { type: 'string', value: '/test' },
98+
'onSpanCreated.reqMethod': { type: 'string', value: 'GET' },
99+
'onSpanCreated.resUrl': { type: 'string', value: '/test' },
100+
'onSpanCreated.resMethod': { type: 'string', value: 'GET' },
101+
'onSpanEnd.reqUrl': { type: 'string', value: '/test' },
102+
'onSpanEnd.resStatusCode': { type: 'integer', value: 200 },
103+
[SENTRY_OP]: { type: 'string', value: HTTP_SERVER },
104+
[SENTRY_ORIGIN]: { type: 'string', value: 'auto.http.http_server' },
105+
[SENTRY_KIND]: { type: 'string', value: 'server' },
106+
[SENTRY_SEGMENT_ID]: { type: 'string', value: segmentSpan!.span_id },
107+
[SENTRY_SEGMENT_NAME]: { type: 'string', value: 'GET /test' },
108+
[SENTRY_SEGMENT_NAME_SOURCE]: { type: 'string', value: 'route' },
109+
[SENTRY_TRACE_LIFECYCLE]: { type: 'string', value: 'stream' },
110+
[SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE]: { type: 'integer', value: 1 },
111+
[SENTRY_IS_LOCALHOST]: { type: 'boolean', value: true },
112+
[SENTRY_ENVIRONMENT]: { type: 'string', value: 'production' },
113+
[SENTRY_RELEASE]: { type: 'string', value: '1.0' },
114+
[SENTRY_SDK_NAME]: { type: 'string', value: EXPECTED_SDK_NAME },
115+
[SENTRY_SDK_VERSION]: { type: 'string', value: SDK_VERSION },
116+
[SENTRY_SDK_INTEGRATIONS]: { type: 'array', value: expect.arrayContaining(['Http']) },
117+
[HTTP_REQUEST_METHOD]: { type: 'string', value: 'GET' },
118+
[HTTP_ROUTE]: { type: 'string', value: '/test' },
119+
[HTTP_RESPONSE_STATUS_CODE]: { type: 'integer', value: 200 },
120+
[HTTP_RESPONSE_STATUS_TEXT]: { type: 'string', value: 'OK' },
121+
[URL_FULL]: { type: 'string', value: expect.stringMatching(/^http:\/\/localhost:\d+\/test$/) },
122+
[URL_PATH]: { type: 'string', value: '/test' },
123+
[URL_SCHEME]: { type: 'string', value: 'http' },
124+
[SERVER_ADDRESS]: { type: 'string', value: 'localhost' },
125+
[SERVER_PORT]: { type: 'integer', value: expect.any(Number) },
126+
[CLIENT_ADDRESS]: { type: 'string', value: expect.stringMatching(LOOPBACK_ADDRESS) },
127+
[CLIENT_PORT]: { type: 'integer', value: expect.any(Number) },
128+
[USER_IP_ADDRESS]: { type: 'string', value: expect.stringMatching(LOOPBACK_ADDRESS) },
129+
[NETWORK_LOCAL_ADDRESS]: { type: 'string', value: expect.stringMatching(LOOPBACK_ADDRESS) },
130+
[NETWORK_LOCAL_PORT]: { type: 'integer', value: expect.any(Number) },
131+
[NETWORK_PEER_ADDRESS]: { type: 'string', value: expect.stringMatching(LOOPBACK_ADDRESS) },
132+
[NETWORK_PEER_PORT]: { type: 'integer', value: expect.any(Number) },
133+
[NETWORK_PROTOCOL_NAME]: { type: 'string', value: 'http' },
134+
[NETWORK_PROTOCOL_VERSION]: { type: 'string', value: '1.1' },
135+
[NETWORK_TRANSPORT]: { type: 'string', value: 'tcp' },
136+
[USER_AGENT_ORIGINAL]: { type: 'string', value: 'node' },
137+
'http.request.header.accept': { type: 'array', value: ['*/*'] },
138+
'http.request.header.accept-encoding': { type: 'array', value: ['gzip, deflate'] },
139+
'http.request.header.accept-language': { type: 'array', value: ['*'] },
140+
'http.request.header.connection': { type: 'array', value: ['keep-alive'] },
141+
'http.request.header.host': { type: 'array', value: [expect.stringMatching(/^localhost:\d+$/)] },
142+
'http.request.header.sec-fetch-mode': { type: 'array', value: ['cors'] },
143+
'http.request.header.user-agent': { type: 'array', value: ['node'] },
144+
});
145+
for (const attribute of Object.values(hostContext)) {
146+
expect(attribute).toEqual({ type: expect.any(String), value: expect.anything() });
147+
}
49148
},
50149
})
51150
.start();
@@ -57,7 +156,7 @@ describe('httpIntegration', () => {
57156

58157
describe('user callbacks that throw', () => {
59158
createEsmAndCjsTests(__dirname, 'server.mjs', 'instrument-throwing-callbacks.mjs', (createRunner, test) => {
60-
test('still handles requests and sends the span when `onSpanCreated` and `ignoreIncomingRequests` throw', async () => {
159+
test('still handles requests and sends the span when `onSpanCreated`, `onSpanEnd` and `ignoreIncomingRequests` throw', async () => {
61160
const runner = createRunner()
62161
.expect({
63162
span: {

‎packages/node/src/integrations/http/httpServerSpansIntegration.ts‎

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@ import {
3636
} from '@sentry/core/server';
3737
import {
3838
debug,
39+
safeCallback,
3940
getSpanStatusFromHttpCode,
4041
httpHeadersToSpanAttributes,
4142
getContentLengthFromHeaders,
@@ -52,7 +53,6 @@ import {
5253
filterCollectedUrlQuery,
5354
hasSpanStreamingEnabled,
5455
HTTP_SPAN_NAME_FALLBACK,
55-
safeCallback,
5656
} from '@sentry/core';
5757
import { DEBUG_BUILD } from '../../debug-build';
5858
import type { NodeClient } from '../../sdk/client';
@@ -105,6 +105,12 @@ export interface HttpServerSpansIntegrationOptions {
105105
* This is triggered after the span is created, but before it is recorded.
106106
*/
107107
onSpanCreated?: (span: Span, request: HttpIncomingMessage, response: HttpServerResponse) => void;
108+
109+
/**
110+
* A hook that can be used to mutate the span one last time when the response is finished.
111+
* This is triggered after the response status is set on the span, before the span ends.
112+
*/
113+
onSpanEnd?: (span: Span, request: HttpIncomingMessage, response: HttpServerResponse) => void;
108114
}
109115

110116
const _httpServerSpansIntegration = ((options: HttpServerSpansIntegrationOptions = {}) => {
@@ -113,7 +119,7 @@ const _httpServerSpansIntegration = ((options: HttpServerSpansIntegrationOptions
113119
// oxlint-disable-next-line typescript/no-deprecated
114120
const ignoreStatusCodes = options.ignoreStatusCodes ?? DEFAULT_IGNORE_STATUS_CODES;
115121

116-
const { onSpanCreated } = options;
122+
const { onSpanCreated, onSpanEnd } = options;
117123

118124
return {
119125
name: INTEGRATION_NAME,
@@ -214,6 +220,15 @@ const _httpServerSpansIntegration = ((options: HttpServerSpansIntegrationOptions
214220
);
215221
span.setAttributes(newAttributes);
216222
span.setStatus(status);
223+
if (onSpanEnd) {
224+
// A hook that throws must not skip `span.end()` or crash the process from the
225+
// response's event listener.
226+
safeCallback(
227+
DEBUG_BUILD ? 'The `onSpanEnd` callback threw an error:' : '',
228+
() => onSpanEnd(span, request, response),
229+
() => undefined,
230+
);
231+
}
217232
span.end();
218233
}
219234

‎packages/node/src/integrations/http/index.ts‎

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -93,13 +93,7 @@ interface HttpOptions extends HttpServerIntegrationOptions, HttpServerSpansInteg
9393
// shared field fails to compile here, until this integration supports
9494
// it or lists it as unsupported.
9595
type _ServerRequestOptionsCheck = AssertTrue<
96-
CheckServerRequestOptions<
97-
HttpOptions,
98-
HttpIncomingMessage,
99-
HttpServerResponse,
100-
// Not supported yet.
101-
'onSpanEnd'
102-
>
96+
CheckServerRequestOptions<HttpOptions, HttpIncomingMessage, HttpServerResponse>
10397
>;
10498

10599
/**

0 commit comments

Comments
 (0)