Skip to content

Commit fd04ed5

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 fd04ed5

File tree

30 files changed

+1135
-960
lines changed

30 files changed

+1135
-960
lines changed

‎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/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';

‎packages/core/src/integrations/http/add-outgoing-request-breadcrumb.ts‎

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
import { addBreadcrumb } from '../../breadcrumbs';
2+
import { getBreadcrumbLogLevelFromHttpStatusCode } from '../../utils/breadcrumb-log-level';
3+
import { getSanitizedUrlString, parseUrl } from '../../utils/url';
4+
import { getRequestUrl } from './get-request-url';
5+
import type { HttpClientRequest, HttpIncomingMessage } from './types';
6+
7+
/**
8+
* Create a breadcrumb for a finished outgoing HTTP request.
9+
*/
10+
export function addOutgoingRequestBreadcrumb(
11+
request: HttpClientRequest,
12+
response: HttpIncomingMessage | undefined,
13+
): void {
14+
const url = getRequestUrl(request);
15+
const parsedUrl = parseUrl(url);
16+
17+
const statusCode = response?.statusCode;
18+
const level = getBreadcrumbLogLevelFromHttpStatusCode(statusCode);
19+
20+
addBreadcrumb(
21+
{
22+
category: 'http',
23+
data: {
24+
status_code: statusCode,
25+
url: getSanitizedUrlString(parsedUrl),
26+
'http.method': request.method || 'GET',
27+
...(parsedUrl.search ? { 'http.query': parsedUrl.search } : {}),
28+
...(parsedUrl.hash ? { 'http.fragment': parsedUrl.hash } : {}),
29+
},
30+
type: 'http',
31+
level,
32+
},
33+
{
34+
event: 'response',
35+
request,
36+
response,
37+
},
38+
);
39+
}

0 commit comments

Comments
 (0)