Skip to content

Commit

Permalink
refactor how we do integration options
Browse files Browse the repository at this point in the history
  • Loading branch information
AbhiPrasad committed Jul 11, 2023
1 parent 1e9f389 commit a38d24d
Show file tree
Hide file tree
Showing 4 changed files with 23 additions and 17 deletions.
35 changes: 21 additions & 14 deletions packages/node/src/integrations/http.ts
Original file line number Diff line number Diff line change
Expand Up @@ -108,13 +108,20 @@ export class Http implements Integration {
return;
}

// TODO (v8): `tracePropagationTargets` and `shouldCreateSpanForRequest` will be removed from clientOptions
// and we will no longer have to do this optional merge, we can just pass `this._tracing` directly.
const tracingOptions = this._tracing ? { ...clientOptions, ...this._tracing } : undefined;
const shouldCreateSpanForRequest =
// eslint-disable-next-line deprecation/deprecation
this._tracing?.shouldCreateSpanForRequest || clientOptions?.shouldCreateSpanForRequest;
// eslint-disable-next-line deprecation/deprecation
const tracePropagationTargets = clientOptions?.tracePropagationTargets || this._tracing?.tracePropagationTargets;

// eslint-disable-next-line @typescript-eslint/no-var-requires
const httpModule = require('http');
const wrappedHttpHandlerMaker = _createWrappedRequestMethodFactory(this._breadcrumbs, tracingOptions, httpModule);
const wrappedHttpHandlerMaker = _createWrappedRequestMethodFactory(
httpModule,
this._breadcrumbs,
shouldCreateSpanForRequest,
tracePropagationTargets,
);
fill(httpModule, 'get', wrappedHttpHandlerMaker);
fill(httpModule, 'request', wrappedHttpHandlerMaker);

Expand All @@ -125,9 +132,10 @@ export class Http implements Integration {
// eslint-disable-next-line @typescript-eslint/no-var-requires
const httpsModule = require('https');
const wrappedHttpsHandlerMaker = _createWrappedRequestMethodFactory(
this._breadcrumbs,
tracingOptions,
httpsModule,
this._breadcrumbs,
shouldCreateSpanForRequest,
tracePropagationTargets,
);
fill(httpsModule, 'get', wrappedHttpsHandlerMaker);
fill(httpsModule, 'request', wrappedHttpsHandlerMaker);
Expand All @@ -150,16 +158,17 @@ type WrappedRequestMethodFactory = (original: OriginalRequestMethod) => WrappedR
* @returns A function which accepts the exiting handler and returns a wrapped handler
*/
function _createWrappedRequestMethodFactory(
breadcrumbsEnabled: boolean,
tracingOptions: TracingOptions | undefined,
httpModule: typeof http | typeof https,
breadcrumbsEnabled: boolean,
shouldCreateSpanForRequest: ((url: string) => boolean) | undefined,
tracePropagationTargets: TracePropagationTargets | undefined,
): WrappedRequestMethodFactory {
// We're caching results so we don't have to recompute regexp every time we create a request.
const createSpanUrlMap = new LRUMap<string, boolean>(100);
const headersUrlMap = new LRUMap<string, boolean>(100);

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

Expand All @@ -168,14 +177,13 @@ function _createWrappedRequestMethodFactory(
return cachedDecision;
}

const decision = tracingOptions.shouldCreateSpanForRequest(url);
const decision = shouldCreateSpanForRequest(url);
createSpanUrlMap.set(url, decision);
return decision;
};

const shouldAttachTraceData = (url: string): boolean => {
// eslint-disable-next-line deprecation/deprecation
if (tracingOptions?.tracePropagationTargets === undefined) {
if (tracePropagationTargets === undefined) {
return true;
}

Expand All @@ -184,8 +192,7 @@ function _createWrappedRequestMethodFactory(
return cachedDecision;
}

// eslint-disable-next-line deprecation/deprecation
const decision = stringMatchesSomePattern(url, tracingOptions.tracePropagationTargets);
const decision = stringMatchesSomePattern(url, tracePropagationTargets);
headersUrlMap.set(url, decision);
return decision;
};
Expand Down
1 change: 0 additions & 1 deletion packages/node/test/integrations/http.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -330,7 +330,6 @@ describe('tracing', () => {
return transaction;
}

// TODO (v8): These can be removed once we remove these properties from client options
describe('as client options', () => {
it('creates span with propagation context if shouldCreateSpanForRequest returns false', () => {
const url = 'http://dogs.are.great/api/v1/index/';
Expand Down
2 changes: 1 addition & 1 deletion packages/remix/test/integration/app_v1/entry.server.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import * as Sentry from '@sentry/remix';
Sentry.init({
dsn: 'https://[email protected]/1337',
tracesSampleRate: 1,
tracePropagationTargets: [/^(?!.*^\/).*$/],
tracePropagationTargets: ['example.org'],
// Disabling to test series of envelopes deterministically.
autoSessionTracking: false,
});
Expand Down
2 changes: 1 addition & 1 deletion packages/remix/test/integration/app_v2/entry.server.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import * as Sentry from '@sentry/remix';
Sentry.init({
dsn: 'https://[email protected]/1337',
tracesSampleRate: 1,
tracePropagationTargets: [/^(?!.*^\/).*$/],
tracePropagationTargets: ['example.org'],
// Disabling to test series of envelopes deterministically.
autoSessionTracking: false,
});
Expand Down

0 comments on commit a38d24d

Please sign in to comment.