Skip to content

feat(node): Add tracePropagationTargets option #5521

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 6 commits into from
Aug 4, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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