Skip to content

Commit f240077

Browse files
Luca ForstnerLms24
Luca Forstner
andauthored
feat(node): Add tracePropagationTargets option (#5521)
Co-authored-by: Lukas Stracke <[email protected]>
1 parent 78395c6 commit f240077

File tree

9 files changed

+278
-29
lines changed

9 files changed

+278
-29
lines changed

packages/nextjs/test/integration/test/utils/server.js

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
// @ts-check
12
const { get } = require('http');
23
const nock = require('nock');
34
const nodeSDK = require('@sentry/node');
@@ -132,14 +133,16 @@ function ensureWrappedGet(importedGet, url) {
132133
// As of Next 12.1, creating a `NextServer` instance (which we do immediately upon starting this test runner) loads
133134
// `_app`, which has the effect of initializing the SDK. So, unless something's gone wrong, we should always be able
134135
// to find the integration
135-
let httpIntegration;
136-
try {
137-
httpIntegration = nodeSDK.getCurrentHub().getClient().getIntegration(nodeSDK.Integrations.Http);
138-
} catch (err) {
136+
const hub = nodeSDK.getCurrentHub();
137+
const client = hub.getClient();
138+
139+
if (!client) {
139140
console.warn(`Warning: Sentry SDK not set up at \`NextServer\` initialization. Request URL: ${url}`);
140141
return importedGet;
141142
}
142143

144+
const httpIntegration = client.getIntegration(nodeSDK.Integrations.Http);
145+
143146
// This rewraps `http.get` and `http.request`, which, at this point, look like `nockWrapper(sentryWrapper(get))` and
144147
// `nockWrapper(sentryWrapper(request))`. By the time we're done with this function, they'll look like
145148
// `sentryWrapper(nockWrapper(sentryWrapper(get)))` and `sentryWrapper(nockWrapper(sentryWrapper(request)))`,
@@ -160,7 +163,12 @@ function ensureWrappedGet(importedGet, url) {
160163
//
161164
// TODO: add in a "don't do this twice" check (in `fill`, maybe moved from `wrap`), so that we don't wrap the outer
162165
// wrapper with a third wrapper
163-
httpIntegration.setupOnce();
166+
if (httpIntegration) {
167+
httpIntegration.setupOnce(
168+
() => undefined,
169+
() => hub,
170+
);
171+
}
164172

165173
// now that we've rewrapped it, grab the correct version of `get` for use in our tests
166174
const httpModule = require('http');
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, TracePropagationTargets } 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 Node's 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: TracePropagationTargets | 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: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,24 @@
1-
import { ClientOptions, Options } from '@sentry/types';
1+
import { ClientOptions, Options, TracePropagationTargets } from '@sentry/types';
22

33
import { NodeTransportOptions } from './transports';
44

55
export interface BaseNodeOptions {
66
/** Sets an optional server name (device name) */
77
serverName?: string;
88

9+
// We have this option separately 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?: TracePropagationTargets;
21+
922
/** Callback that is executed when a fatal global error occurs. */
1023
onFatalError?(error: Error): void;
1124
}

0 commit comments

Comments
 (0)