Skip to content

Commit 3e7001c

Browse files
isaacsisaacs
committed
feat(http): portable node:http client instrumentation (#20393)
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 10e81cb commit 3e7001c

File tree

38 files changed

+1227
-1154
lines changed

38 files changed

+1227
-1154
lines changed

‎.size-limit.js‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -221,7 +221,7 @@ module.exports = [
221221
name: 'CDN Bundle (incl. Tracing, Replay, Logs, Metrics)',
222222
path: createCDNPath('bundle.tracing.replay.logs.metrics.min.js'),
223223
gzip: true,
224-
limit: '83 KB',
224+
limit: '84 KB',
225225
},
226226
{
227227
name: 'CDN Bundle (incl. Tracing, Replay, Feedback)',
@@ -283,7 +283,7 @@ module.exports = [
283283
path: createCDNPath('bundle.tracing.replay.logs.metrics.min.js'),
284284
gzip: false,
285285
brotli: false,
286-
limit: '255 KB',
286+
limit: '256 KB',
287287
},
288288
{
289289
name: 'CDN Bundle (incl. Tracing, Replay, Feedback) - uncompressed',
@@ -297,7 +297,7 @@ module.exports = [
297297
path: createCDNPath('bundle.tracing.replay.feedback.logs.metrics.min.js'),
298298
gzip: false,
299299
brotli: false,
300-
limit: '268 KB',
300+
limit: '269 KB',
301301
},
302302
// Next.js SDK (ESM)
303303
{

‎dev-packages/e2e-tests/test-applications/aws-serverless/tests/layer.test.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ test.describe('Lambda layer', () => {
4545
expect.objectContaining({
4646
data: expect.objectContaining({
4747
'sentry.op': 'http.client',
48-
'sentry.origin': 'auto.http.otel.http',
48+
'sentry.origin': 'auto.http.client',
4949
url: 'http://example.com/',
5050
}),
5151
description: 'GET http://example.com/',
@@ -113,7 +113,7 @@ test.describe('Lambda layer', () => {
113113
expect.objectContaining({
114114
data: expect.objectContaining({
115115
'sentry.op': 'http.client',
116-
'sentry.origin': 'auto.http.otel.http',
116+
'sentry.origin': 'auto.http.client',
117117
url: 'http://example.com/',
118118
}),
119119
description: 'GET http://example.com/',

‎dev-packages/e2e-tests/test-applications/aws-serverless/tests/npm.test.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ test.describe('NPM package', () => {
4545
expect.objectContaining({
4646
data: expect.objectContaining({
4747
'sentry.op': 'http.client',
48-
'sentry.origin': 'auto.http.otel.http',
48+
'sentry.origin': 'auto.http.client',
4949
url: 'http://example.com/',
5050
}),
5151
description: 'GET http://example.com/',
@@ -113,7 +113,7 @@ test.describe('NPM package', () => {
113113
expect.objectContaining({
114114
data: expect.objectContaining({
115115
'sentry.op': 'http.client',
116-
'sentry.origin': 'auto.http.otel.http',
116+
'sentry.origin': 'auto.http.client',
117117
url: 'http://example.com/',
118118
}),
119119
description: 'GET http://example.com/',

‎dev-packages/e2e-tests/test-applications/nextjs-14/tests/request-instrumentation.test.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ test('Should send a transaction with a fetch span', async ({ page }) => {
2828
data: expect.objectContaining({
2929
'http.method': 'GET',
3030
'sentry.op': 'http.client',
31-
'sentry.origin': 'auto.http.otel.http',
31+
'sentry.origin': 'auto.http.client',
3232
}),
3333
description: 'GET https://github.com/',
3434
}),

‎dev-packages/e2e-tests/test-applications/nextjs-app-dir/tests/request-instrumentation.test.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ test.skip('Should send a transaction with a http span', async ({ request }) => {
1616
data: expect.objectContaining({
1717
'http.method': 'GET',
1818
'sentry.op': 'http.client',
19-
'sentry.origin': 'auto.http.otel.http',
19+
'sentry.origin': 'auto.http.client',
2020
}),
2121
description: 'GET https://example.com/',
2222
}),

‎dev-packages/e2e-tests/test-applications/nextjs-pages-dir/tests/request-instrumentation.test.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ test.skip('Should send a transaction with a http span', async ({ request }) => {
1616
data: expect.objectContaining({
1717
'http.method': 'GET',
1818
'sentry.op': 'http.client',
19-
'sentry.origin': 'auto.http.otel.http',
19+
'sentry.origin': 'auto.http.client',
2020
}),
2121
description: 'GET https://example.com/',
2222
}),

‎dev-packages/e2e-tests/test-applications/node-express-esm-preload/tests/server.test.ts‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -133,7 +133,7 @@ test('Should record spans from http instrumentation', async ({ request }) => {
133133
trace_id: expect.stringMatching(/[a-f0-9]{32}/),
134134
data: expect.objectContaining({
135135
'http.flavor': '1.1',
136-
'http.host': 'example.com:80',
136+
'http.host': 'example.com',
137137
'http.method': 'GET',
138138
'http.response.status_code': 200,
139139
'http.status_code': 200,
@@ -146,7 +146,7 @@ test('Should record spans from http instrumentation', async ({ request }) => {
146146
'net.transport': 'ip_tcp',
147147
'otel.kind': 'CLIENT',
148148
'sentry.op': 'http.client',
149-
'sentry.origin': 'auto.http.otel.http',
149+
'sentry.origin': 'auto.http.client',
150150
url: 'http://example.com/',
151151
}),
152152
description: 'GET http://example.com/',
@@ -155,6 +155,6 @@ test('Should record spans from http instrumentation', async ({ request }) => {
155155
timestamp: expect.any(Number),
156156
status: 'ok',
157157
op: 'http.client',
158-
origin: 'auto.http.otel.http',
158+
origin: 'auto.http.client',
159159
});
160160
});

‎dev-packages/node-core-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
});

0 commit comments

Comments
 (0)