Skip to content

Commit be699cb

Browse files
isaacsclaude
andcommitted
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) <noreply@anthropic.com>
1 parent a45bb60 commit be699cb

6 files changed

Lines changed: 86 additions & 10 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,10 @@ Work in this release was contributed by @adityareddy-dev, @jpdefo, @Jxxunnn, @ph
5959

6060
`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.
6161

62+
- **feat(node): Add `onSpanEnd` to `httpIntegration`**
63+
64+
`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.
65+
6266
## 11.8.0
6367

6468
### Important Changes
Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
import * as Sentry from '@sentry/node';
2+
import { loggingTransport } from '@sentry-internal/node-integration-tests';
3+
4+
Sentry.init({
5+
dsn: 'https://public@dsn.ingest.sentry.io/1337',
6+
release: '1.0',
7+
tracesSampleRate: 1.0,
8+
transport: loggingTransport,
9+
10+
integrations: [
11+
Sentry.httpIntegration({
12+
onSpanEnd: () => {
13+
throw new Error('onSpanEnd failed');
14+
},
15+
}),
16+
],
17+
});

‎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/test.ts‎

Lines changed: 41 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,9 +22,9 @@ describe('httpIntegration', () => {
2222
cleanupChildProcesses();
2323
});
2424

25-
describe('onSpanCreated option', () => {
25+
describe('onSpanCreated and onSpanEnd options', () => {
2626
createEsmAndCjsTests(__dirname, 'server.mjs', 'instrument-options.mjs', (createRunner, test) => {
27-
test('allows to configure onSpanCreated', async () => {
27+
test('allows to configure onSpanCreated and onSpanEnd', async () => {
2828
const runner = createRunner()
2929
.expect({
3030
span: {
@@ -43,6 +43,8 @@ describe('httpIntegration', () => {
4343
'onSpanCreated.reqMethod': { type: 'string', value: 'GET' },
4444
'onSpanCreated.resUrl': { type: 'string', value: expect.stringMatching(/\/test$/) },
4545
'onSpanCreated.resMethod': { type: 'string', value: 'GET' },
46+
'onSpanEnd.reqUrl': { type: 'string', value: expect.stringMatching(/\/test$/) },
47+
'onSpanEnd.resStatusCode': { type: 'integer', value: 200 },
4648
}),
4749
}),
4850
]),
@@ -55,6 +57,43 @@ describe('httpIntegration', () => {
5557
});
5658
});
5759

60+
describe('onSpanEnd that throws', () => {
61+
createEsmAndCjsTests(__dirname, 'server.mjs', 'instrument-onSpanEnd-throws.mjs', (createRunner, test) => {
62+
test('still ends and sends the span, and keeps the server running', async () => {
63+
const runner = createRunner()
64+
.expect({
65+
span: {
66+
items: expect.arrayContaining([
67+
expect.objectContaining({
68+
is_segment: true,
69+
attributes: expect.objectContaining({
70+
[URL_FULL]: { type: 'string', value: expect.stringMatching(/\/test$/) },
71+
[HTTP_RESPONSE_STATUS_CODE]: { type: 'integer', value: 200 },
72+
}),
73+
}),
74+
]),
75+
},
76+
})
77+
.expect({
78+
span: {
79+
items: expect.arrayContaining([
80+
expect.objectContaining({
81+
is_segment: true,
82+
attributes: expect.objectContaining({
83+
[URL_FULL]: { type: 'string', value: expect.stringMatching(/\/test$/) },
84+
}),
85+
}),
86+
]),
87+
},
88+
})
89+
.start();
90+
await runner.makeRequest('get', '/test');
91+
await runner.makeRequest('get', '/test');
92+
await runner.completed();
93+
});
94+
});
95+
});
96+
5897
describe('outgoing request span hooks', () => {
5998
test('runs outgoingRequestHook, outgoingResponseHook and outgoingRequestApplyCustomAttributes', async () => {
6099
const [SERVER_URL, closeTestServer] = await createTestServer()

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

Lines changed: 17 additions & 1 deletion
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,
@@ -104,6 +105,12 @@ export interface HttpServerSpansIntegrationOptions {
104105
* This is triggered after the span is created, but before it is recorded.
105106
*/
106107
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;
107114
}
108115

109116
const _httpServerSpansIntegration = ((options: HttpServerSpansIntegrationOptions = {}) => {
@@ -112,7 +119,7 @@ const _httpServerSpansIntegration = ((options: HttpServerSpansIntegrationOptions
112119
// oxlint-disable-next-line typescript/no-deprecated
113120
const ignoreStatusCodes = options.ignoreStatusCodes ?? DEFAULT_IGNORE_STATUS_CODES;
114121

115-
const { onSpanCreated } = options;
122+
const { onSpanCreated, onSpanEnd } = options;
116123

117124
return {
118125
name: INTEGRATION_NAME,
@@ -209,6 +216,15 @@ const _httpServerSpansIntegration = ((options: HttpServerSpansIntegrationOptions
209216
);
210217
span.setAttributes(newAttributes);
211218
span.setStatus(status);
219+
if (onSpanEnd) {
220+
// A hook that throws must not skip `span.end()` or crash the process from the
221+
// response's event listener.
222+
safeCallback(
223+
DEBUG_BUILD ? 'Error in `onSpanEnd`:' : '',
224+
() => onSpanEnd(span, request, response),
225+
() => undefined,
226+
);
227+
}
212228
span.end();
213229
}
214230

‎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)