Skip to content

Commit 4e33c46

Browse files
author
Luca Forstner
committed
feat(node): Add tracePropagationTargets option
1 parent 6ea53ed commit 4e33c46

File tree

6 files changed

+262
-23
lines changed

6 files changed

+262
-23
lines changed
Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
// eslint-disable-next-line @typescript-eslint/no-unused-vars
2+
import '@sentry/tracing';
3+
4+
import * as Sentry from '@sentry/node';
5+
import * as http from 'http';
6+
7+
Sentry.init({
8+
dsn: 'https://[email protected]/1337',
9+
release: '1.0',
10+
tracesSampleRate: 1.0,
11+
tracePropagationTargets: [/\/v0/, 'v1'],
12+
integrations: [new Sentry.Integrations.Http({ tracing: true })],
13+
});
14+
15+
const transaction = Sentry.startTransaction({ name: 'test_transaction' });
16+
17+
Sentry.configureScope(scope => {
18+
scope.setSpan(transaction);
19+
});
20+
21+
http.get('http://match-this-url.com/api/v0');
22+
http.get('http://match-this-url.com/api/v1');
23+
http.get('http://dont-match-this-url.com/api/v2');
24+
http.get('http://dont-match-this-url.com/api/v3');
25+
26+
transaction.finish();
Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
import nock from 'nock';
2+
3+
import { runScenario, runServer } from '../../../utils';
4+
5+
test('HttpIntegration should instrument correct requests when tracePropagationTargets option is provided', async () => {
6+
const match1 = nock('http://match-this-url.com')
7+
.get('/api/v0')
8+
.matchHeader('baggage', val => typeof val === 'string')
9+
.matchHeader('sentry-trace', val => typeof val === 'string')
10+
.reply(200);
11+
12+
const match2 = nock('http://match-this-url.com')
13+
.get('/api/v1')
14+
.matchHeader('baggage', val => typeof val === 'string')
15+
.matchHeader('sentry-trace', val => typeof val === 'string')
16+
.reply(200);
17+
18+
const match3 = nock('http://dont-match-this-url.com')
19+
.get('/api/v2')
20+
.matchHeader('baggage', val => val === undefined)
21+
.matchHeader('sentry-trace', val => val === undefined)
22+
.reply(200);
23+
24+
const match4 = nock('http://dont-match-this-url.com')
25+
.get('/api/v3')
26+
.matchHeader('baggage', val => val === undefined)
27+
.matchHeader('sentry-trace', val => val === undefined)
28+
.reply(200);
29+
30+
const serverUrl = await runServer(__dirname);
31+
await runScenario(serverUrl);
32+
33+
expect(match1.isDone()).toBe(true);
34+
expect(match2.isDone()).toBe(true);
35+
expect(match3.isDone()).toBe(true);
36+
expect(match4.isDone()).toBe(true);
37+
});

packages/node-integration-tests/utils/index.ts

Lines changed: 18 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -152,12 +152,16 @@ export async function runServer(testDir: string, serverPath?: string, scenarioPa
152152
const url = `http://localhost:${port}/test`;
153153
const defaultServerPath = path.resolve(process.cwd(), 'utils', 'defaults', 'server');
154154

155-
await new Promise(resolve => {
155+
await new Promise<void>(resolve => {
156156
// eslint-disable-next-line @typescript-eslint/no-var-requires, @typescript-eslint/no-unsafe-member-access
157157
const app = require(serverPath || defaultServerPath).default as Express;
158158

159-
app.get('/test', async () => {
160-
require(scenarioPath || `${testDir}/scenario`);
159+
app.get('/test', (_req, res) => {
160+
try {
161+
require(scenarioPath || `${testDir}/scenario`);
162+
} finally {
163+
res.status(200).end();
164+
}
161165
});
162166

163167
const server = app.listen(port, () => {
@@ -170,3 +174,14 @@ export async function runServer(testDir: string, serverPath?: string, scenarioPa
170174

171175
return url;
172176
}
177+
178+
export async function runScenario(serverUrl: string): Promise<void> {
179+
return new Promise<void>(resolve => {
180+
http
181+
.get(serverUrl, res => {
182+
res.on('data', () => undefined);
183+
res.on('end', resolve);
184+
})
185+
.end();
186+
});
187+
}

packages/node/src/integrations/http.ts

Lines changed: 61 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,10 @@
1-
import { getCurrentHub } from '@sentry/core';
2-
import { Integration, Span } from '@sentry/types';
3-
import { fill, logger, mergeAndSerializeBaggage, parseSemver } from '@sentry/utils';
1+
import { getCurrentHub, Hub } from '@sentry/core';
2+
import { EventProcessor, Integration, Span } from '@sentry/types';
3+
import { fill, isMatchingPattern, logger, mergeAndSerializeBaggage, parseSemver } from '@sentry/utils';
44
import * as http from 'http';
55
import * as https from 'https';
66

7+
import { NodeClientOptions } from '../types';
78
import {
89
cleanSpanDescription,
910
extractUrl,
@@ -15,7 +16,10 @@ import {
1516

1617
const NODE_VERSION = parseSemver(process.versions.node);
1718

18-
/** http module integration */
19+
/**
20+
* The http module integration instruments nodes internal http module. It creates breadcrumbs, transactions for outgoing
21+
* http requests and attaches trace data when tracing is enabled via its `tracing` option.
22+
*/
1923
export class Http implements Integration {
2024
/**
2125
* @inheritDoc
@@ -48,13 +52,22 @@ export class Http implements Integration {
4852
/**
4953
* @inheritDoc
5054
*/
51-
public setupOnce(): void {
55+
public setupOnce(
56+
_addGlobalEventProcessor: (callback: EventProcessor) => void,
57+
setupOnceGetCurrentHub: () => Hub,
58+
): void {
5259
// No need to instrument if we don't want to track anything
5360
if (!this._breadcrumbs && !this._tracing) {
5461
return;
5562
}
5663

57-
const wrappedHandlerMaker = _createWrappedRequestMethodFactory(this._breadcrumbs, this._tracing);
64+
const clientOptions = setupOnceGetCurrentHub().getClient()?.getOptions() as NodeClientOptions | undefined;
65+
66+
const wrappedHandlerMaker = _createWrappedRequestMethodFactory(
67+
this._breadcrumbs,
68+
this._tracing,
69+
clientOptions?.tracePropagationTargets,
70+
);
5871

5972
// eslint-disable-next-line @typescript-eslint/no-var-requires
6073
const httpModule = require('http');
@@ -90,7 +103,26 @@ type WrappedRequestMethodFactory = (original: OriginalRequestMethod) => WrappedR
90103
function _createWrappedRequestMethodFactory(
91104
breadcrumbsEnabled: boolean,
92105
tracingEnabled: boolean,
106+
tracePropagationTargets: (string | RegExp)[] | undefined,
93107
): WrappedRequestMethodFactory {
108+
// We're caching results so we dont have to recompute regexp everytime we create a request.
109+
const urlMap: Record<string, boolean> = {};
110+
const shouldAttachTraceData = (url: string): boolean => {
111+
if (tracePropagationTargets === undefined) {
112+
return true;
113+
}
114+
115+
if (urlMap[url]) {
116+
return urlMap[url];
117+
}
118+
119+
urlMap[url] = tracePropagationTargets.some(tracePropagationTarget =>
120+
isMatchingPattern(url, tracePropagationTarget),
121+
);
122+
123+
return urlMap[url];
124+
};
125+
94126
return function wrappedRequestMethodFactory(originalRequestMethod: OriginalRequestMethod): WrappedRequestMethod {
95127
return function wrappedMethod(this: typeof http | typeof https, ...args: RequestMethodArgs): http.ClientRequest {
96128
// eslint-disable-next-line @typescript-eslint/no-this-alias
@@ -109,28 +141,37 @@ function _createWrappedRequestMethodFactory(
109141
let parentSpan: Span | undefined;
110142

111143
const scope = getCurrentHub().getScope();
144+
112145
if (scope && tracingEnabled) {
113146
parentSpan = scope.getSpan();
147+
114148
if (parentSpan) {
115149
span = parentSpan.startChild({
116150
description: `${requestOptions.method || 'GET'} ${requestUrl}`,
117151
op: 'http.client',
118152
});
119153

120-
const sentryTraceHeader = span.toTraceparent();
121-
__DEBUG_BUILD__ &&
122-
logger.log(
123-
`[Tracing] Adding sentry-trace header ${sentryTraceHeader} to outgoing request to ${requestUrl}: `,
124-
);
125-
126-
const baggage = parentSpan.transaction && parentSpan.transaction.getBaggage();
127-
const headerBaggageString = requestOptions.headers && requestOptions.headers.baggage;
128-
129-
requestOptions.headers = {
130-
...requestOptions.headers,
131-
'sentry-trace': sentryTraceHeader,
132-
baggage: mergeAndSerializeBaggage(baggage, headerBaggageString),
133-
};
154+
if (shouldAttachTraceData(requestUrl)) {
155+
const sentryTraceHeader = span.toTraceparent();
156+
__DEBUG_BUILD__ &&
157+
logger.log(
158+
`[Tracing] Adding sentry-trace header ${sentryTraceHeader} to outgoing request to "${requestUrl}": `,
159+
);
160+
161+
const baggage = parentSpan.transaction && parentSpan.transaction.getBaggage();
162+
const headerBaggageString = requestOptions.headers && requestOptions.headers.baggage;
163+
164+
requestOptions.headers = {
165+
...requestOptions.headers,
166+
'sentry-trace': sentryTraceHeader,
167+
baggage: mergeAndSerializeBaggage(baggage, headerBaggageString),
168+
};
169+
} else {
170+
__DEBUG_BUILD__ &&
171+
logger.log(
172+
`[Tracing] Not adding sentry-trace header to outgoing request (${requestUrl}) due to mismatching tracePropagationTargets option.`,
173+
);
174+
}
134175
}
135176
}
136177

packages/node/src/types.ts

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,19 @@ export interface BaseNodeOptions {
66
/** Sets an optional server name (device name) */
77
serverName?: string;
88

9+
// We have this option seperately in both the node options and the browser options, so that we can have different JSDoc
10+
// comments, since this has different behaviour in the Browser and Node SDKs.
11+
/**
12+
* List of strings/regex controlling to which outgoing requests
13+
* the SDK will attach tracing headers.
14+
*
15+
* By default the SDK will attach those headers to all outgoing
16+
* requests. If this option is provided, the SDK will match the
17+
* request URL of outgoing requests against the items in this
18+
* array, and only attach tracing headers if a match was found.
19+
*/
20+
tracePropagationTargets?: (string | RegExp)[];
21+
922
/** Callback that is executed when a fatal global error occurs. */
1023
onFatalError?(error: Error): void;
1124
}

packages/node/test/integrations/http.test.ts

Lines changed: 107 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,9 @@ import { getDefaultNodeClientOptions } from '../helper/node-client-options';
1717

1818
const NODE_VERSION = parseSemver(process.versions.node);
1919

20+
const originalHttpGet = http.get;
21+
const originalHttpRequest = http.request;
22+
2023
describe('tracing', () => {
2124
function createTransactionOnScope(
2225
customOptions: Partial<NodeClientOptions> = {},
@@ -168,6 +171,110 @@ describe('tracing', () => {
168171

169172
expect(baggage).not.toBeDefined();
170173
});
174+
175+
describe('tracePropagationTargets option', () => {
176+
beforeEach(() => {
177+
// hacky way of restoring monkey patched functions
178+
// @ts-ignore TS doesn't let us assign to this but we want to
179+
http.get = originalHttpGet;
180+
// @ts-ignore TS doesn't let us assign to this but we want to
181+
http.request = originalHttpRequest;
182+
});
183+
184+
function createHub(customOptions: Partial<NodeClientOptions> = {}) {
185+
const options = getDefaultNodeClientOptions({
186+
dsn: 'https://[email protected]/12312012',
187+
tracesSampleRate: 1.0,
188+
release: '1.0.0',
189+
environment: 'production',
190+
...customOptions,
191+
});
192+
193+
const hub = new Hub();
194+
195+
// we need to mock both of these because the tracing handler relies on `@sentry/core` while the sampler relies on
196+
// `@sentry/hub`, and mocking breaks the link between the two
197+
jest.spyOn(sentryCore, 'getCurrentHub').mockImplementation(() => hub);
198+
jest.spyOn(hubModule, 'getCurrentHub').mockImplementation(() => hub);
199+
200+
const client = new NodeClient(options);
201+
jest.spyOn(hub, 'getClient').mockImplementation(() => client);
202+
hub.bindClient(client);
203+
204+
return hub;
205+
}
206+
207+
function createTransactionAndPutOnScope(hub: Hub) {
208+
addExtensionMethods();
209+
const transaction = hub.startTransaction({ name: 'dogpark' });
210+
hub.getScope()?.setSpan(transaction);
211+
}
212+
213+
it.each([
214+
['http://dogs.are.great/api/v1/index/', [/.*/]],
215+
['http://dogs.are.great/api/v1/index/', [/\/api/]],
216+
['http://dogs.are.great/api/v1/index/', [/\/(v1|v2)/]],
217+
['http://dogs.are.great/api/v1/index/', [/dogs\.are\.great/, /dogs\.are\.not\.great/]],
218+
['http://dogs.are.great/api/v1/index/', [/http:/]],
219+
['http://dogs.are.great/api/v1/index/', ['dogs.are.great']],
220+
['http://dogs.are.great/api/v1/index/', ['/api/v1']],
221+
['http://dogs.are.great/api/v1/index/', ['http://']],
222+
['http://dogs.are.great/api/v1/index/', ['']],
223+
])(
224+
'attaches trace inforation to header of outgoing requests when url matches tracePropagationTargets (url="%s", tracePropagationTargets=%p)',
225+
(url, tracePropagationTargets) => {
226+
nock(url).get(/.*/).reply(200);
227+
228+
const httpIntegration = new HttpIntegration({ tracing: true });
229+
230+
const hub = createHub({ tracePropagationTargets });
231+
232+
httpIntegration.setupOnce(
233+
() => undefined,
234+
() => hub,
235+
);
236+
237+
createTransactionAndPutOnScope(hub);
238+
239+
const request = http.get(url);
240+
241+
expect(request.getHeader('sentry-trace')).toBeDefined();
242+
expect(request.getHeader('baggage')).toBeDefined();
243+
},
244+
);
245+
246+
it.each([
247+
['http://dogs.are.great/api/v1/index/', []],
248+
['http://cats.are.great/api/v1/index/', [/\/v2/]],
249+
['http://cats.are.great/api/v1/index/', [/\/(v2|v3)/]],
250+
['http://cats.are.great/api/v1/index/', [/dogs\.are\.great/, /dogs\.are\.not\.great/]],
251+
['http://cats.are.great/api/v1/index/', [/https:/]],
252+
['http://cats.are.great/api/v1/index/', ['dogs.are.great']],
253+
['http://cats.are.great/api/v1/index/', ['/api/v2']],
254+
['http://cats.are.great/api/v1/index/', ['https://']],
255+
])(
256+
'doesn\'t attach trace inforation to header of outgoing requests when url doesn\'t match tracePropagationTargets (url="%s", tracePropagationTargets=%p)',
257+
(url, tracePropagationTargets) => {
258+
nock(url).get(/.*/).reply(200);
259+
260+
const httpIntegration = new HttpIntegration({ tracing: true });
261+
262+
const hub = createHub({ tracePropagationTargets });
263+
264+
httpIntegration.setupOnce(
265+
() => undefined,
266+
() => hub,
267+
);
268+
269+
createTransactionAndPutOnScope(hub);
270+
271+
const request = http.get(url);
272+
273+
expect(request.getHeader('sentry-trace')).not.toBeDefined();
274+
expect(request.getHeader('baggage')).not.toBeDefined();
275+
},
276+
);
277+
});
171278
});
172279

173280
describe('default protocols', () => {

0 commit comments

Comments
 (0)