Skip to content

Commit 5379392

Browse files
isaacsisaacs
committed
feat(http): refactor node:http client instrumentation for portability
Refactor the `node:http` outgoing request instrumentation so that it can be applied to non-Node.js environments by patching the http module. Also, refactor so that the diagnostics_channel and monkeypatching paths can share code, and so that light and normal node-core instrumentations can share more of the functionality as well. To facilitate this, some portable minimal types are vendored in from the `node:http` module.
1 parent 73b1f46 commit 5379392

File tree

32 files changed

+1140
-966
lines changed

32 files changed

+1140
-966
lines changed

‎dev-packages/cloudflare-integration-tests/suites/public-api/startSpan-streamed/test.ts‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -187,6 +187,10 @@ it('sends a streamed span envelope with correct spans for a manually started spa
187187
type: 'string',
188188
value: 'localhost',
189189
},
190+
'http.url': {
191+
type: 'string',
192+
value: expect.stringMatching(/^http:\/\/localhost:.+$/),
193+
},
190194
'url.full': {
191195
type: 'string',
192196
value: expect.stringMatching(/^http:\/\/localhost:.+$/),

‎dev-packages/e2e-tests/test-applications/nextjs-16/tests/middleware.test.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -113,6 +113,7 @@ test('Should trace outgoing fetch requests inside middleware and create breadcru
113113
'server.port': 3030,
114114
url: 'http://localhost:3030/',
115115
'url.full': 'http://localhost:3030/',
116+
'http.url': 'http://localhost:3030/',
116117
'url.path': '/',
117118
'url.query': '',
118119
'url.scheme': 'http',

‎dev-packages/node-integration-tests/suites/tracing/http-client-spans/fetch-strip-query/test.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ test('strips and handles query params in spans of outgoing fetch requests', asyn
2222
data: {
2323
url: `${SERVER_URL}/api/v0/users`,
2424
'url.full': `${SERVER_URL}/api/v0/users?id=1`,
25+
'http.url': `${SERVER_URL}/api/v0/users?id=1`,
2526
'url.path': '/api/v0/users',
2627
'url.query': '?id=1',
2728
'url.scheme': 'http',

‎dev-packages/node-integration-tests/suites/tracing/http-client-spans/http-basic/test.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -29,13 +29,13 @@ test('captures spans for outgoing http requests', async () => {
2929
expect.objectContaining({
3030
description: expect.stringMatching(/GET .*\/api\/v0/),
3131
op: 'http.client',
32-
origin: 'auto.http.otel.http',
32+
origin: 'auto.http.client',
3333
status: 'ok',
3434
}),
3535
expect.objectContaining({
3636
description: expect.stringMatching(/GET .*\/api\/v1/),
3737
op: 'http.client',
38-
origin: 'auto.http.otel.http',
38+
origin: 'auto.http.client',
3939
status: 'not_found',
4040
data: expect.objectContaining({
4141
'http.response.status_code': 404,

‎dev-packages/node-integration-tests/suites/tracing/http-client-spans/http-strip-query/test.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -37,11 +37,11 @@ test('strips and handles query params in spans of outgoing http requests', async
3737
'net.transport': 'ip_tcp',
3838
'otel.kind': 'CLIENT',
3939
'sentry.op': 'http.client',
40-
'sentry.origin': 'auto.http.otel.http',
40+
'sentry.origin': 'auto.http.client',
4141
},
4242
description: `GET ${SERVER_URL}/api/v0/users`,
4343
op: 'http.client',
44-
origin: 'auto.http.otel.http',
44+
origin: 'auto.http.client',
4545
status: 'ok',
4646
parent_span_id: txn.contexts?.trace?.span_id,
4747
span_id: expect.stringMatching(/[a-f\d]{16}/),

‎dev-packages/node-integration-tests/suites/tracing/requests/http-no-tracing-no-spans/test.ts‎

Lines changed: 80 additions & 181 deletions
Original file line numberDiff line numberDiff line change
@@ -1,202 +1,101 @@
11
import { createTestServer } from '@sentry-internal/test-utils';
22
import { describe, expect } from 'vitest';
3-
import { conditionalTest } from '../../../../utils';
43
import { createEsmAndCjsTests } from '../../../../utils/runner';
54

65
describe('outgoing http requests with tracing & spans disabled', () => {
76
createEsmAndCjsTests(__dirname, 'scenario.mjs', 'instrument.mjs', (createRunner, test) => {
8-
conditionalTest({ min: 22 })('node >=22', () => {
9-
test('outgoing http requests are correctly instrumented with tracing & spans disabled', async () => {
10-
expect.assertions(11);
7+
test('outgoing http requests are correctly instrumented with tracing & spans disabled', async () => {
8+
expect.assertions(11);
119

12-
const [SERVER_URL, closeTestServer] = await createTestServer()
13-
.get('/api/v0', headers => {
14-
expect(headers['sentry-trace']).toEqual(expect.stringMatching(/^([a-f\d]{32})-([a-f\d]{16})$/));
15-
expect(headers['sentry-trace']).not.toEqual('00000000000000000000000000000000-0000000000000000');
16-
expect(headers['baggage']).toEqual(expect.any(String));
17-
})
18-
.get('/api/v1', headers => {
19-
expect(headers['sentry-trace']).toEqual(expect.stringMatching(/^([a-f\d]{32})-([a-f\d]{16})$/));
20-
expect(headers['sentry-trace']).not.toEqual('00000000000000000000000000000000-0000000000000000');
21-
expect(headers['baggage']).toEqual(expect.any(String));
22-
})
23-
.get('/api/v2', headers => {
24-
expect(headers['baggage']).toBeUndefined();
25-
expect(headers['sentry-trace']).toBeUndefined();
26-
})
27-
.get('/api/v3', headers => {
28-
expect(headers['baggage']).toBeUndefined();
29-
expect(headers['sentry-trace']).toBeUndefined();
30-
})
31-
.start();
10+
const [SERVER_URL, closeTestServer] = await createTestServer()
11+
.get('/api/v0', headers => {
12+
expect(headers['sentry-trace']).toEqual(expect.stringMatching(/^([a-f\d]{32})-([a-f\d]{16})$/));
13+
expect(headers['sentry-trace']).not.toEqual('00000000000000000000000000000000-0000000000000000');
14+
expect(headers['baggage']).toEqual(expect.any(String));
15+
})
16+
.get('/api/v1', headers => {
17+
expect(headers['sentry-trace']).toEqual(expect.stringMatching(/^([a-f\d]{32})-([a-f\d]{16})$/));
18+
expect(headers['sentry-trace']).not.toEqual('00000000000000000000000000000000-0000000000000000');
19+
expect(headers['baggage']).toEqual(expect.any(String));
20+
})
21+
.get('/api/v2', headers => {
22+
expect(headers['baggage']).toBeUndefined();
23+
expect(headers['sentry-trace']).toBeUndefined();
24+
})
25+
.get('/api/v3', headers => {
26+
expect(headers['baggage']).toBeUndefined();
27+
expect(headers['sentry-trace']).toBeUndefined();
28+
})
29+
.start();
3230

33-
await createRunner()
34-
.withEnv({ SERVER_URL })
35-
.expect({
36-
event: {
37-
exception: {
38-
values: [
39-
{
40-
type: 'Error',
41-
value: 'foo',
42-
},
43-
],
44-
},
45-
breadcrumbs: [
46-
{
47-
message: 'manual breadcrumb',
48-
timestamp: expect.any(Number),
49-
},
50-
{
51-
category: 'http',
52-
data: {
53-
'http.method': 'GET',
54-
url: `${SERVER_URL}/api/v0`,
55-
status_code: 200,
56-
ADDED_PATH: '/api/v0',
57-
},
58-
timestamp: expect.any(Number),
59-
type: 'http',
60-
},
61-
{
62-
category: 'http',
63-
data: {
64-
'http.method': 'GET',
65-
url: `${SERVER_URL}/api/v1`,
66-
status_code: 200,
67-
ADDED_PATH: '/api/v1',
68-
},
69-
timestamp: expect.any(Number),
70-
type: 'http',
71-
},
31+
await createRunner()
32+
.withEnv({ SERVER_URL })
33+
.expect({
34+
event: {
35+
exception: {
36+
values: [
7237
{
73-
category: 'http',
74-
data: {
75-
'http.method': 'GET',
76-
url: `${SERVER_URL}/api/v2`,
77-
status_code: 200,
78-
ADDED_PATH: '/api/v2',
79-
},
80-
timestamp: expect.any(Number),
81-
type: 'http',
82-
},
83-
{
84-
category: 'http',
85-
data: {
86-
'http.method': 'GET',
87-
url: `${SERVER_URL}/api/v3`,
88-
status_code: 200,
89-
ADDED_PATH: '/api/v3',
90-
},
91-
timestamp: expect.any(Number),
92-
type: 'http',
38+
type: 'Error',
39+
value: 'foo',
9340
},
9441
],
9542
},
96-
})
97-
.start()
98-
.completed();
99-
100-
closeTestServer();
101-
});
102-
});
103-
104-
// On older node versions, outgoing requests do not get trace-headers injected, sadly
105-
// This is because the necessary diagnostics channel hook is not available yet
106-
conditionalTest({ max: 21 })('node <22', () => {
107-
test('outgoing http requests generate breadcrumbs correctly with tracing & spans disabled', async () => {
108-
expect.assertions(9);
109-
110-
const [SERVER_URL, closeTestServer] = await createTestServer()
111-
.get('/api/v0', headers => {
112-
// This is not instrumented, sadly
113-
expect(headers['baggage']).toBeUndefined();
114-
expect(headers['sentry-trace']).toBeUndefined();
115-
})
116-
.get('/api/v1', headers => {
117-
// This is not instrumented, sadly
118-
expect(headers['baggage']).toBeUndefined();
119-
expect(headers['sentry-trace']).toBeUndefined();
120-
})
121-
.get('/api/v2', headers => {
122-
expect(headers['baggage']).toBeUndefined();
123-
expect(headers['sentry-trace']).toBeUndefined();
124-
})
125-
.get('/api/v3', headers => {
126-
expect(headers['baggage']).toBeUndefined();
127-
expect(headers['sentry-trace']).toBeUndefined();
128-
})
129-
.start();
130-
131-
await createRunner()
132-
.withEnv({ SERVER_URL })
133-
.expect({
134-
event: {
135-
exception: {
136-
values: [
137-
{
138-
type: 'Error',
139-
value: 'foo',
140-
},
141-
],
43+
breadcrumbs: [
44+
{
45+
message: 'manual breadcrumb',
46+
timestamp: expect.any(Number),
14247
},
143-
breadcrumbs: [
144-
{
145-
message: 'manual breadcrumb',
146-
timestamp: expect.any(Number),
147-
},
148-
{
149-
category: 'http',
150-
data: {
151-
'http.method': 'GET',
152-
url: `${SERVER_URL}/api/v0`,
153-
status_code: 200,
154-
ADDED_PATH: '/api/v0',
155-
},
156-
timestamp: expect.any(Number),
157-
type: 'http',
48+
{
49+
category: 'http',
50+
data: {
51+
'http.method': 'GET',
52+
url: `${SERVER_URL}/api/v0`,
53+
status_code: 200,
54+
ADDED_PATH: '/api/v0',
15855
},
159-
{
160-
category: 'http',
161-
data: {
162-
'http.method': 'GET',
163-
url: `${SERVER_URL}/api/v1`,
164-
status_code: 200,
165-
ADDED_PATH: '/api/v1',
166-
},
167-
timestamp: expect.any(Number),
168-
type: 'http',
56+
timestamp: expect.any(Number),
57+
type: 'http',
58+
},
59+
{
60+
category: 'http',
61+
data: {
62+
'http.method': 'GET',
63+
url: `${SERVER_URL}/api/v1`,
64+
status_code: 200,
65+
ADDED_PATH: '/api/v1',
16966
},
170-
{
171-
category: 'http',
172-
data: {
173-
'http.method': 'GET',
174-
url: `${SERVER_URL}/api/v2`,
175-
status_code: 200,
176-
ADDED_PATH: '/api/v2',
177-
},
178-
timestamp: expect.any(Number),
179-
type: 'http',
67+
timestamp: expect.any(Number),
68+
type: 'http',
69+
},
70+
{
71+
category: 'http',
72+
data: {
73+
'http.method': 'GET',
74+
url: `${SERVER_URL}/api/v2`,
75+
status_code: 200,
76+
ADDED_PATH: '/api/v2',
18077
},
181-
{
182-
category: 'http',
183-
data: {
184-
'http.method': 'GET',
185-
url: `${SERVER_URL}/api/v3`,
186-
status_code: 200,
187-
ADDED_PATH: '/api/v3',
188-
},
189-
timestamp: expect.any(Number),
190-
type: 'http',
78+
timestamp: expect.any(Number),
79+
type: 'http',
80+
},
81+
{
82+
category: 'http',
83+
data: {
84+
'http.method': 'GET',
85+
url: `${SERVER_URL}/api/v3`,
86+
status_code: 200,
87+
ADDED_PATH: '/api/v3',
19188
},
192-
],
193-
},
194-
})
195-
.start()
196-
.completed();
89+
timestamp: expect.any(Number),
90+
type: 'http',
91+
},
92+
],
93+
},
94+
})
95+
.start()
96+
.completed();
19797

198-
closeTestServer();
199-
});
98+
closeTestServer();
20099
});
201100
});
202101
});

‎packages/cloudflare/test/request.test.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -350,6 +350,7 @@ describe('withSentry', () => {
350350
'sentry.source': 'route',
351351
'http.request.method': 'GET',
352352
'url.full': 'https://example.com/',
353+
'http.url': 'https://example.com/',
353354
'server.address': 'example.com',
354355
'network.protocol.name': 'HTTP/1.1',
355356
'url.scheme': 'https:',

‎packages/core/src/index.ts‎

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -141,9 +141,20 @@ export { instrumentPostgresJsSql } from './integrations/postgresjs';
141141
export { zodErrorsIntegration } from './integrations/zoderrors';
142142
export { thirdPartyErrorFilterIntegration } from './integrations/third-party-errors-filter';
143143
export { consoleIntegration } from './integrations/console';
144-
export { featureFlagsIntegration, type FeatureFlagsIntegration } from './integrations/featureFlags';
144+
export type { FeatureFlagsIntegration } from './integrations/featureFlags';
145+
export { featureFlagsIntegration } from './integrations/featureFlags';
145146
export { growthbookIntegration } from './integrations/featureFlags';
146147
export { conversationIdIntegration } from './integrations/conversationId';
148+
export { patchHttpModuleClient, patchHttpsModuleClient } from './integrations/http/client-patch';
149+
export { getHttpClientSubscriptions } from './integrations/http/client-subscriptions';
150+
export { HTTP_ON_CLIENT_REQUEST, HTTP_ON_SERVER_REQUEST } from './integrations/http/constants';
151+
export type {
152+
HttpInstrumentationOptions,
153+
HttpClientRequest,
154+
HttpIncomingMessage,
155+
HttpServerResponse,
156+
HttpModuleExport,
157+
} from './integrations/http/types';
147158

148159
export { profiler } from './profiling';
149160
// eslint thinks the entire function is deprecated (while only one overload is actually deprecated)
@@ -558,9 +569,9 @@ export type {
558569
UnstableRollupPluginOptions,
559570
UnstableWebpackPluginOptions,
560571
} from './build-time-plugins/buildTimeOptionsBase';
572+
export type { RandomSafeContextRunner as _INTERNAL_RandomSafeContextRunner } from './utils/randomSafeContext';
561573
export {
562574
withRandomSafeContext as _INTERNAL_withRandomSafeContext,
563-
type RandomSafeContextRunner as _INTERNAL_RandomSafeContextRunner,
564575
safeMathRandom as _INTERNAL_safeMathRandom,
565576
safeDateNow as _INTERNAL_safeDateNow,
566577
} from './utils/randomSafeContext';

0 commit comments

Comments
 (0)