Skip to content

Commit

Permalink
feat(node): Add shouldCreateSpanForRequest option (#6055)
Browse files Browse the repository at this point in the history
As per the summary here in #5285 (comment)) this PR adds support for an optional `shouldCreateSpanForRequest` function in the options. When it's defined and returns false, spans will not be attached.
  • Loading branch information
timfish committed Nov 4, 2022
1 parent fae7b24 commit 24e2a27
Show file tree
Hide file tree
Showing 3 changed files with 63 additions and 17 deletions.
46 changes: 29 additions & 17 deletions packages/node/src/integrations/http.ts
@@ -1,5 +1,5 @@
import { getCurrentHub, Hub } from '@sentry/core';
import { EventProcessor, Integration, Span, TracePropagationTargets } from '@sentry/types';
import { EventProcessor, Integration, Span } from '@sentry/types';
import {
dynamicSamplingContextToSentryBaggageHeader,
fill,
Expand All @@ -10,6 +10,7 @@ import {
import * as http from 'http';
import * as https from 'https';

import { NodeClient } from '../client';
import { NodeClientOptions } from '../types';
import {
cleanSpanDescription,
Expand Down Expand Up @@ -67,13 +68,8 @@ export class Http implements Integration {
return;
}

const clientOptions = setupOnceGetCurrentHub().getClient()?.getOptions() as NodeClientOptions | undefined;

const wrappedHandlerMaker = _createWrappedRequestMethodFactory(
this._breadcrumbs,
this._tracing,
clientOptions?.tracePropagationTargets,
);
const clientOptions = setupOnceGetCurrentHub().getClient<NodeClient>()?.getOptions();
const wrappedHandlerMaker = _createWrappedRequestMethodFactory(this._breadcrumbs, this._tracing, clientOptions);

// eslint-disable-next-line @typescript-eslint/no-var-requires
const httpModule = require('http');
Expand Down Expand Up @@ -109,24 +105,40 @@ type WrappedRequestMethodFactory = (original: OriginalRequestMethod) => WrappedR
function _createWrappedRequestMethodFactory(
breadcrumbsEnabled: boolean,
tracingEnabled: boolean,
tracePropagationTargets: TracePropagationTargets | undefined,
options: NodeClientOptions | undefined,
): WrappedRequestMethodFactory {
// We're caching results so we dont have to recompute regexp everytime we create a request.
const urlMap: Record<string, boolean> = {};
// We're caching results so we don't have to recompute regexp every time we create a request.
const createSpanUrlMap: Record<string, boolean> = {};
const headersUrlMap: Record<string, boolean> = {};

const shouldCreateSpan = (url: string): boolean => {
if (options?.shouldCreateSpanForRequest === undefined) {
return true;
}

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

createSpanUrlMap[url] = options.shouldCreateSpanForRequest(url);

return createSpanUrlMap[url];
};

const shouldAttachTraceData = (url: string): boolean => {
if (tracePropagationTargets === undefined) {
if (options?.tracePropagationTargets === undefined) {
return true;
}

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

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

return urlMap[url];
return headersUrlMap[url];
};

return function wrappedRequestMethodFactory(originalRequestMethod: OriginalRequestMethod): WrappedRequestMethod {
Expand All @@ -148,7 +160,7 @@ function _createWrappedRequestMethodFactory(

const scope = getCurrentHub().getScope();

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

if (parentSpan) {
Expand Down
6 changes: 6 additions & 0 deletions packages/node/src/types.ts
Expand Up @@ -19,6 +19,12 @@ export interface BaseNodeOptions {
*/
tracePropagationTargets?: TracePropagationTargets;

/**
* Function determining whether or not to create spans to track outgoing requests to the given URL.
* By default, spans will be created for all outgoing requests.
*/
shouldCreateSpanForRequest?(url: string): boolean;

/** Callback that is executed when a fatal global error occurs. */
onFatalError?(error: Error): void;
}
Expand Down
28 changes: 28 additions & 0 deletions packages/node/test/integrations/http.test.ts
Expand Up @@ -221,8 +221,36 @@ describe('tracing', () => {
addExtensionMethods();
const transaction = hub.startTransaction({ name: 'dogpark' });
hub.getScope()?.setSpan(transaction);
return transaction;
}

it("doesn't create span if shouldCreateSpanForRequest returns false", () => {
const url = 'http://dogs.are.great/api/v1/index/';
nock(url).get(/.*/).reply(200);

const httpIntegration = new HttpIntegration({ tracing: true });

const hub = createHub({ shouldCreateSpanForRequest: () => false });

httpIntegration.setupOnce(
() => undefined,
() => hub,
);

const transaction = createTransactionAndPutOnScope(hub);
const spans = (transaction as unknown as Span).spanRecorder?.spans as Span[];

const request = http.get(url);

// There should be no http spans
const httpSpans = spans.filter(span => span.op?.startsWith('http'));
expect(httpSpans.length).toBe(0);

// And headers are not attached without span creation
expect(request.getHeader('sentry-trace')).toBeUndefined();
expect(request.getHeader('baggage')).toBeUndefined();
});

it.each([
['http://dogs.are.great/api/v1/index/', [/.*/]],
['http://dogs.are.great/api/v1/index/', [/\/api/]],
Expand Down

0 comments on commit 24e2a27

Please sign in to comment.