Skip to content

Commit

Permalink
feat(node): Add tracePropagationTargets option (#5521)
Browse files Browse the repository at this point in the history
Co-authored-by: Lukas Stracke <[email protected]>
  • Loading branch information
lforst and Lms24 authored Aug 4, 2022
1 parent 78395c6 commit f240077
Show file tree
Hide file tree
Showing 9 changed files with 278 additions and 29 deletions.
18 changes: 13 additions & 5 deletions packages/nextjs/test/integration/test/utils/server.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
// @ts-check
const { get } = require('http');
const nock = require('nock');
const nodeSDK = require('@sentry/node');
Expand Down Expand Up @@ -132,14 +133,16 @@ function ensureWrappedGet(importedGet, url) {
// As of Next 12.1, creating a `NextServer` instance (which we do immediately upon starting this test runner) loads
// `_app`, which has the effect of initializing the SDK. So, unless something's gone wrong, we should always be able
// to find the integration
let httpIntegration;
try {
httpIntegration = nodeSDK.getCurrentHub().getClient().getIntegration(nodeSDK.Integrations.Http);
} catch (err) {
const hub = nodeSDK.getCurrentHub();
const client = hub.getClient();

if (!client) {
console.warn(`Warning: Sentry SDK not set up at \`NextServer\` initialization. Request URL: ${url}`);
return importedGet;
}

const httpIntegration = client.getIntegration(nodeSDK.Integrations.Http);

// This rewraps `http.get` and `http.request`, which, at this point, look like `nockWrapper(sentryWrapper(get))` and
// `nockWrapper(sentryWrapper(request))`. By the time we're done with this function, they'll look like
// `sentryWrapper(nockWrapper(sentryWrapper(get)))` and `sentryWrapper(nockWrapper(sentryWrapper(request)))`,
Expand All @@ -160,7 +163,12 @@ function ensureWrappedGet(importedGet, url) {
//
// TODO: add in a "don't do this twice" check (in `fill`, maybe moved from `wrap`), so that we don't wrap the outer
// wrapper with a third wrapper
httpIntegration.setupOnce();
if (httpIntegration) {
httpIntegration.setupOnce(
() => undefined,
() => hub,
);
}

// now that we've rewrapped it, grab the correct version of `get` for use in our tests
const httpModule = require('http');
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
// eslint-disable-next-line @typescript-eslint/no-unused-vars
import '@sentry/tracing';

import * as Sentry from '@sentry/node';
import * as http from 'http';

Sentry.init({
dsn: 'https://[email protected]/1337',
release: '1.0',
tracesSampleRate: 1.0,
tracePropagationTargets: [/\/v0/, 'v1'],
integrations: [new Sentry.Integrations.Http({ tracing: true })],
});

const transaction = Sentry.startTransaction({ name: 'test_transaction' });

Sentry.configureScope(scope => {
scope.setSpan(transaction);
});

http.get('http://match-this-url.com/api/v0');
http.get('http://match-this-url.com/api/v1');
http.get('http://dont-match-this-url.com/api/v2');
http.get('http://dont-match-this-url.com/api/v3');

transaction.finish();
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
import nock from 'nock';

import { runScenario, runServer } from '../../../utils';

test('HttpIntegration should instrument correct requests when tracePropagationTargets option is provided', async () => {
const match1 = nock('http://match-this-url.com')
.get('/api/v0')
.matchHeader('baggage', val => typeof val === 'string')
.matchHeader('sentry-trace', val => typeof val === 'string')
.reply(200);

const match2 = nock('http://match-this-url.com')
.get('/api/v1')
.matchHeader('baggage', val => typeof val === 'string')
.matchHeader('sentry-trace', val => typeof val === 'string')
.reply(200);

const match3 = nock('http://dont-match-this-url.com')
.get('/api/v2')
.matchHeader('baggage', val => val === undefined)
.matchHeader('sentry-trace', val => val === undefined)
.reply(200);

const match4 = nock('http://dont-match-this-url.com')
.get('/api/v3')
.matchHeader('baggage', val => val === undefined)
.matchHeader('sentry-trace', val => val === undefined)
.reply(200);

const serverUrl = await runServer(__dirname);
await runScenario(serverUrl);

expect(match1.isDone()).toBe(true);
expect(match2.isDone()).toBe(true);
expect(match3.isDone()).toBe(true);
expect(match4.isDone()).toBe(true);
});
21 changes: 18 additions & 3 deletions packages/node-integration-tests/utils/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -152,12 +152,16 @@ export async function runServer(testDir: string, serverPath?: string, scenarioPa
const url = `http://localhost:${port}/test`;
const defaultServerPath = path.resolve(process.cwd(), 'utils', 'defaults', 'server');

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

app.get('/test', async () => {
require(scenarioPath || `${testDir}/scenario`);
app.get('/test', (_req, res) => {
try {
require(scenarioPath || `${testDir}/scenario`);
} finally {
res.status(200).end();
}
});

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

return url;
}

export async function runScenario(serverUrl: string): Promise<void> {
return new Promise<void>(resolve => {
http
.get(serverUrl, res => {
res.on('data', () => undefined);
res.on('end', resolve);
})
.end();
});
}
81 changes: 61 additions & 20 deletions packages/node/src/integrations/http.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
import { getCurrentHub } from '@sentry/core';
import { Integration, Span } from '@sentry/types';
import { fill, logger, mergeAndSerializeBaggage, parseSemver } from '@sentry/utils';
import { getCurrentHub, Hub } from '@sentry/core';
import { EventProcessor, Integration, Span, TracePropagationTargets } from '@sentry/types';
import { fill, isMatchingPattern, logger, mergeAndSerializeBaggage, parseSemver } from '@sentry/utils';
import * as http from 'http';
import * as https from 'https';

import { NodeClientOptions } from '../types';
import {
cleanSpanDescription,
extractUrl,
Expand All @@ -15,7 +16,10 @@ import {

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

/** http module integration */
/**
* The http module integration instruments Node's internal http module. It creates breadcrumbs, transactions for outgoing
* http requests and attaches trace data when tracing is enabled via its `tracing` option.
*/
export class Http implements Integration {
/**
* @inheritDoc
Expand Down Expand Up @@ -48,13 +52,22 @@ export class Http implements Integration {
/**
* @inheritDoc
*/
public setupOnce(): void {
public setupOnce(
_addGlobalEventProcessor: (callback: EventProcessor) => void,
setupOnceGetCurrentHub: () => Hub,
): void {
// No need to instrument if we don't want to track anything
if (!this._breadcrumbs && !this._tracing) {
return;
}

const wrappedHandlerMaker = _createWrappedRequestMethodFactory(this._breadcrumbs, this._tracing);
const clientOptions = setupOnceGetCurrentHub().getClient()?.getOptions() as NodeClientOptions | undefined;

const wrappedHandlerMaker = _createWrappedRequestMethodFactory(
this._breadcrumbs,
this._tracing,
clientOptions?.tracePropagationTargets,
);

// eslint-disable-next-line @typescript-eslint/no-var-requires
const httpModule = require('http');
Expand Down Expand Up @@ -90,7 +103,26 @@ type WrappedRequestMethodFactory = (original: OriginalRequestMethod) => WrappedR
function _createWrappedRequestMethodFactory(
breadcrumbsEnabled: boolean,
tracingEnabled: boolean,
tracePropagationTargets: TracePropagationTargets | undefined,
): WrappedRequestMethodFactory {
// We're caching results so we dont have to recompute regexp everytime we create a request.
const urlMap: Record<string, boolean> = {};
const shouldAttachTraceData = (url: string): boolean => {
if (tracePropagationTargets === undefined) {
return true;
}

if (urlMap[url]) {
return urlMap[url];
}

urlMap[url] = tracePropagationTargets.some(tracePropagationTarget =>
isMatchingPattern(url, tracePropagationTarget),
);

return urlMap[url];
};

return function wrappedRequestMethodFactory(originalRequestMethod: OriginalRequestMethod): WrappedRequestMethod {
return function wrappedMethod(this: typeof http | typeof https, ...args: RequestMethodArgs): http.ClientRequest {
// eslint-disable-next-line @typescript-eslint/no-this-alias
Expand All @@ -109,28 +141,37 @@ function _createWrappedRequestMethodFactory(
let parentSpan: Span | undefined;

const scope = getCurrentHub().getScope();

if (scope && tracingEnabled) {
parentSpan = scope.getSpan();

if (parentSpan) {
span = parentSpan.startChild({
description: `${requestOptions.method || 'GET'} ${requestUrl}`,
op: 'http.client',
});

const sentryTraceHeader = span.toTraceparent();
__DEBUG_BUILD__ &&
logger.log(
`[Tracing] Adding sentry-trace header ${sentryTraceHeader} to outgoing request to ${requestUrl}: `,
);

const baggage = parentSpan.transaction && parentSpan.transaction.getBaggage();
const headerBaggageString = requestOptions.headers && requestOptions.headers.baggage;

requestOptions.headers = {
...requestOptions.headers,
'sentry-trace': sentryTraceHeader,
baggage: mergeAndSerializeBaggage(baggage, headerBaggageString),
};
if (shouldAttachTraceData(requestUrl)) {
const sentryTraceHeader = span.toTraceparent();
__DEBUG_BUILD__ &&
logger.log(
`[Tracing] Adding sentry-trace header ${sentryTraceHeader} to outgoing request to "${requestUrl}": `,
);

const baggage = parentSpan.transaction && parentSpan.transaction.getBaggage();
const headerBaggageString = requestOptions.headers && requestOptions.headers.baggage;

requestOptions.headers = {
...requestOptions.headers,
'sentry-trace': sentryTraceHeader,
baggage: mergeAndSerializeBaggage(baggage, headerBaggageString),
};
} else {
__DEBUG_BUILD__ &&
logger.log(
`[Tracing] Not adding sentry-trace header to outgoing request (${requestUrl}) due to mismatching tracePropagationTargets option.`,
);
}
}
}

Expand Down
15 changes: 14 additions & 1 deletion packages/node/src/types.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,24 @@
import { ClientOptions, Options } from '@sentry/types';
import { ClientOptions, Options, TracePropagationTargets } from '@sentry/types';

import { NodeTransportOptions } from './transports';

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

// We have this option separately in both, the node options and the browser options, so that we can have different JSDoc
// comments, since this has different behaviour in the Browser and Node SDKs.
/**
* List of strings/regex controlling to which outgoing requests
* the SDK will attach tracing headers.
*
* By default the SDK will attach those headers to all outgoing
* requests. If this option is provided, the SDK will match the
* request URL of outgoing requests against the items in this
* array, and only attach tracing headers if a match was found.
*/
tracePropagationTargets?: TracePropagationTargets;

/** Callback that is executed when a fatal global error occurs. */
onFatalError?(error: Error): void;
}
Expand Down
Loading

0 comments on commit f240077

Please sign in to comment.