Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Sign up
Appearance settings

Commit c120396

Browse files
committed
feat(node): Update httpIntegration handling of incoming requests
1 parent 6518d12 commit c120396

File tree

12 files changed

+420
-191
lines changed

12 files changed

+420
-191
lines changed

‎.size-limit.js

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -224,7 +224,7 @@ module.exports = [
224224
import: createImport('init'),
225225
ignore: [...builtinModules, ...nodePrefixedBuiltinModules],
226226
gzip: true,
227-
limit: '116 KB',
227+
limit: '51 KB',
228228
},
229229
// Node SDK (ESM)
230230
{
@@ -233,14 +233,14 @@ module.exports = [
233233
import: createImport('init'),
234234
ignore: [...builtinModules, ...nodePrefixedBuiltinModules],
235235
gzip: true,
236-
limit: '147 KB',
236+
limit: '150 KB',
237237
},
238238
{
239239
name: '@sentry/node - without tracing',
240240
path: 'packages/node/build/esm/index.js',
241241
import: createImport('initWithoutDefaultIntegrations', 'getDefaultIntegrationsWithoutPerformance'),
242242
gzip: true,
243-
limit: '110 KB',
243+
limit: '95 KB',
244244
ignore: [...builtinModules, ...nodePrefixedBuiltinModules],
245245
modifyWebpackConfig: function (config) {
246246
const webpack = require('webpack');
@@ -263,7 +263,7 @@ module.exports = [
263263
import: createImport('init'),
264264
ignore: [...builtinModules, ...nodePrefixedBuiltinModules],
265265
gzip: true,
266-
limit: '135 KB',
266+
limit: '105 KB',
267267
},
268268
];
269269

‎CHANGELOG.md

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,6 @@
44

55
- "You miss 100 percent of the chances you don't take. — Wayne Gretzky" — Michael Scott
66

7-
## Unreleased
8-
97
### Important Changes
108

119
- **fix(browser): Ensure IP address is only inferred by Relay if `sendDefaultPii` is `true`**
@@ -22,6 +20,13 @@ user IP addresses, if you set `sendDefaultPii: true` in your `Sentry.init` optio
2220

2321
We apologize for any inconvenience caused!
2422

23+
- **feat(node)**: Update `httpIntegration` handling of incoming requests
24+
25+
This version updates the handling of the Node SDK of incoming requests. Instead of relying on @opentelemetry/instrumentation-http for this, we now handle this internally, ensuring that we can optimize performance as much as possible and avoid interop problems.
26+
27+
This change should not affect users, unless they are relying on very in-depth implementation details. Importantly, this also drops the `_experimentalConfig` option of the integration - this will no longer do anything.
28+
Finally, you can still pass `instrumentation.{requestHook,responseHook,applyCustomAttributesOnSpan}` options, but they are deprecated and will be removed in v11. Instead, you can use the new `incomingRequestSpanHook` configuration option if you want to adjust the incoming request span.
29+
2530
## 10.3.0
2631

2732
- feat(core): MCP Server - Capture prompt results from prompt function calls (#17284)

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -29,11 +29,11 @@ describe('express tracing', () => {
2929

3030
// Also calling `updateName` AND setting a source doesn't change anything - Otel has no concept of source, this is sentry-internal.
3131
// Therefore, only the source is updated but the name is still overwritten by Otel.
32-
test("calling `span.updateName` and setting attribute source doesn't update the final name in express but it updates the source", async () => {
32+
test('calling `span.updateName` and setting attribute source updates the final name in express', async () => {
3333
const runner = createRunner(__dirname, 'server.js')
3434
.expect({
3535
transaction: {
36-
transaction: 'GET /test/:id/span-updateName-source',
36+
transaction: 'new-name',
3737
transaction_info: {
3838
source: 'custom',
3939
},

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

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,13 +4,20 @@ import { loggingTransport } from '@sentry-internal/node-integration-tests';
44
Sentry.init({
55
dsn: 'https://public@dsn.ingest.sentry.io/1337',
66
release: '1.0',
7-
// disable attaching headers to /test/* endpoints
8-
tracePropagationTargets: [/^(?!.*test).*$/],
97
tracesSampleRate: 1.0,
108
transport: loggingTransport,
119

1210
integrations: [
1311
Sentry.httpIntegration({
12+
incomingRequestSpanHook: (span, req, res) => {
13+
span.setAttribute('incomingRequestSpanHook', 'yes');
14+
Sentry.setExtra('incomingRequestSpanHookCalled', {
15+
reqUrl: req.url,
16+
reqMethod: req.method,
17+
resUrl: res.req.url,
18+
resMethod: res.req.method,
19+
});
20+
},
1421
instrumentation: {
1522
requestHook: (span, req) => {
1623
span.setAttribute('attr1', 'yes');

‎dev-packages/node-integration-tests/suites/tracing/httpIntegration/server-experimental.js

Lines changed: 0 additions & 38 deletions
This file was deleted.

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

Lines changed: 32 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,38 @@ describe('httpIntegration', () => {
5050
runner.makeRequest('get', '/test');
5151
await runner.completed();
5252
});
53+
54+
test('allows to configure incomingRequestSpanHook', async () => {
55+
const runner = createRunner()
56+
.expect({
57+
transaction: {
58+
contexts: {
59+
trace: {
60+
span_id: expect.stringMatching(/[a-f0-9]{16}/),
61+
trace_id: expect.stringMatching(/[a-f0-9]{32}/),
62+
data: {
63+
url: expect.stringMatching(/\/test$/),
64+
'http.response.status_code': 200,
65+
incomingRequestSpanHook: 'yes',
66+
},
67+
op: 'http.server',
68+
status: 'ok',
69+
},
70+
},
71+
extra: expect.objectContaining({
72+
incomingRequestSpanHookCalled: {
73+
reqUrl: expect.stringMatching(/\/test$/),
74+
reqMethod: 'GET',
75+
resUrl: expect.stringMatching(/\/test$/),
76+
resMethod: 'GET',
77+
},
78+
}),
79+
},
80+
})
81+
.start();
82+
runner.makeRequest('get', '/test');
83+
await runner.completed();
84+
});
5385
});
5486
});
5587

@@ -138,30 +170,6 @@ describe('httpIntegration', () => {
138170
});
139171
});
140172

141-
test('allows to pass experimental config through to integration', async () => {
142-
const runner = createRunner(__dirname, 'server-experimental.js')
143-
.expect({
144-
transaction: {
145-
contexts: {
146-
trace: {
147-
span_id: expect.stringMatching(/[a-f0-9]{16}/),
148-
trace_id: expect.stringMatching(/[a-f0-9]{32}/),
149-
data: {
150-
url: expect.stringMatching(/\/test$/),
151-
'http.response.status_code': 200,
152-
'http.server_name': 'sentry-test-server-name',
153-
},
154-
op: 'http.server',
155-
status: 'ok',
156-
},
157-
},
158-
},
159-
})
160-
.start();
161-
runner.makeRequest('get', '/test');
162-
await runner.completed();
163-
});
164-
165173
describe("doesn't create a root span for incoming requests ignored via `ignoreIncomingRequests`", () => {
166174
test('via the url param', async () => {
167175
const runner = createRunner(__dirname, 'server-ignoreIncomingRequests.js')

‎packages/node-core/src/integrations/http/SentryHttpInstrumentation.ts

Lines changed: 61 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import type { ChannelListener } from 'node:diagnostics_channel';
22
import { subscribe, unsubscribe } from 'node:diagnostics_channel';
33
import type * as http from 'node:http';
44
import type * as https from 'node:https';
5+
import type { Span } from '@opentelemetry/api';
56
import { context } from '@opentelemetry/api';
67
import { isTracingSuppressed } from '@opentelemetry/core';
78
import type { InstrumentationConfig } from '@opentelemetry/instrumentation';
@@ -28,12 +29,22 @@ export type SentryHttpInstrumentationOptions = InstrumentationConfig & {
2829
*/
2930
breadcrumbs?: boolean;
3031

32+
/**
33+
* Whether to create spans for requests or not.
34+
* As of now, creates spans for incoming requests, but not outgoing requests.
35+
*
36+
* @default `true`
37+
*/
38+
spans?: boolean;
39+
3140
/**
3241
* Whether to extract the trace ID from the `sentry-trace` header for incoming requests.
3342
* By default this is done by the HttpInstrumentation, but if that is not added (e.g. because tracing is disabled, ...)
3443
* then this instrumentation can take over.
3544
*
36-
* @default `false`
45+
* @deprecated This is always true and the option will be removed in the future.
46+
*
47+
* @default `true`
3748
*/
3849
extractIncomingTraceFromHeader?: boolean;
3950

@@ -46,6 +57,20 @@ export type SentryHttpInstrumentationOptions = InstrumentationConfig & {
4657
*/
4758
propagateTraceInOutgoingRequests?: boolean;
4859

60+
/**
61+
* Whether to automatically ignore common static asset requests like favicon.ico, robots.txt, etc.
62+
* This helps reduce noise in your transactions.
63+
*
64+
* @default `true`
65+
*/
66+
ignoreStaticAssets?: boolean;
67+
68+
/**
69+
* If true, do not generate spans for incoming requests at all.
70+
* This is used by Remix to avoid generating spans for incoming requests, as it generates its own spans.
71+
*/
72+
disableIncomingRequestSpans?: boolean;
73+
4974
/**
5075
* Do not capture breadcrumbs for outgoing HTTP requests to URLs where the given callback returns `true`.
5176
* For the scope of this instrumentation, this callback only controls breadcrumb creation.
@@ -57,6 +82,14 @@ export type SentryHttpInstrumentationOptions = InstrumentationConfig & {
5782
*/
5883
ignoreOutgoingRequests?: (url: string, request: http.RequestOptions) => boolean;
5984

85+
/**
86+
* Do not capture spans for incoming HTTP requests to URLs where the given callback returns `true`.
87+
*
88+
* @param urlPath Contains the URL path and query string (if any) of the incoming request.
89+
* @param request Contains the {@type IncomingMessage} object of the incoming request.
90+
*/
91+
ignoreSpansForIncomingRequests?: (urlPath: string, request: http.IncomingMessage) => boolean;
92+
6093
/**
6194
* Do not capture the request body for incoming HTTP requests to URLs where the given callback returns `true`.
6295
* This can be useful for long running requests where the body is not needed and we want to avoid capturing it.
@@ -66,6 +99,12 @@ export type SentryHttpInstrumentationOptions = InstrumentationConfig & {
6699
*/
67100
ignoreIncomingRequestBody?: (url: string, request: http.RequestOptions) => boolean;
68101

102+
/**
103+
* A hook that can be used to mutate the span for incoming requests.
104+
* This is triggered after the span is created, but before it is recorded.
105+
*/
106+
incomingRequestSpanHook?: (span: Span, request: http.IncomingMessage, response: http.ServerResponse) => void;
107+
69108
/**
70109
* Controls the maximum size of incoming HTTP request bodies attached to events.
71110
*
@@ -90,6 +129,19 @@ export type SentryHttpInstrumentationOptions = InstrumentationConfig & {
90129
*/
91130
trackIncomingRequestsAsSessions?: boolean;
92131

132+
/**
133+
* @deprecated This is deprecated in favor of `incomingRequestSpanHook`.
134+
*/
135+
instrumentation?: {
136+
requestHook?: (span: Span, req: http.ClientRequest | http.IncomingMessage) => void;
137+
responseHook?: (span: Span, response: http.IncomingMessage | http.ServerResponse) => void;
138+
applyCustomAttributesOnSpan?: (
139+
span: Span,
140+
request: http.ClientRequest | http.IncomingMessage,
141+
response: http.IncomingMessage | http.ServerResponse,
142+
) => void;
143+
};
144+
93145
/**
94146
* Number of milliseconds until sessions tracked with `trackIncomingRequestsAsSessions` will be flushed as a session aggregate.
95147
*
@@ -128,13 +180,21 @@ export class SentryHttpInstrumentation extends InstrumentationBase<SentryHttpIns
128180
// but we only want to register them once, whichever is loaded first
129181
let hasRegisteredHandlers = false;
130182

183+
const spansEnabled = this.getConfig().spans ?? true;
184+
131185
const onHttpServerRequestStart = ((_data: unknown) => {
132186
const data = _data as { server: http.Server };
133187
instrumentServer(data.server, {
188+
// eslint-disable-next-line deprecation/deprecation
189+
instrumentation: this.getConfig().instrumentation,
134190
ignoreIncomingRequestBody: this.getConfig().ignoreIncomingRequestBody,
191+
ignoreSpansForIncomingRequests: this.getConfig().ignoreSpansForIncomingRequests,
192+
incomingRequestSpanHook: this.getConfig().incomingRequestSpanHook,
135193
maxIncomingRequestBodySize: this.getConfig().maxIncomingRequestBodySize,
136194
trackIncomingRequestsAsSessions: this.getConfig().trackIncomingRequestsAsSessions,
137195
sessionFlushingDelayMS: this.getConfig().sessionFlushingDelayMS ?? 60_000,
196+
ignoreStaticAssets: this.getConfig().ignoreStaticAssets,
197+
spans: spansEnabled && !this.getConfig().disableIncomingRequestSpans,
138198
});
139199
}) satisfies ChannelListener;
140200

0 commit comments

Comments
(0)

AltStyle によって変換されたページ (->オリジナル) /