diff --git a/package.json b/package.json index 54e8d564..9176bb9b 100644 --- a/package.json +++ b/package.json @@ -64,6 +64,7 @@ "@sentry/types": "7.60.0", "@sentry/utils": "7.60.0", "deepmerge": "4.3.0", + "lru_map": "^0.3.3", "tslib": "^2.5.0" }, "devDependencies": { diff --git a/src/main/integrations/net-breadcrumbs.ts b/src/main/integrations/net-breadcrumbs.ts index f7453e95..08ba85bc 100644 --- a/src/main/integrations/net-breadcrumbs.ts +++ b/src/main/integrations/net-breadcrumbs.ts @@ -1,12 +1,11 @@ /* eslint-disable deprecation/deprecation */ -import { getCurrentHub } from '@sentry/core'; -import { Integration, Span } from '@sentry/types'; -import { fill } from '@sentry/utils'; +import { getCurrentHub } from '@sentry/node'; +import { EventProcessor, Hub, Integration, Span, TracePropagationTargets } from '@sentry/types'; +import { fill, stringMatchesSomePattern } from '@sentry/utils'; import { ClientRequest, ClientRequestConstructorOptions, IncomingMessage, net } from 'electron'; +import { LRUMap } from 'lru_map'; import * as urlModule from 'url'; -import { OrBool, OrFalse } from '../../common/types'; - type ShouldTraceFn = (method: string, url: string) => boolean; interface NetOptions { @@ -15,37 +14,23 @@ interface NetOptions { * * Defaults to: true */ - breadcrumbs: boolean; + breadcrumbs?: boolean; /** * Whether to capture transaction spans for net requests * * true | false | (method: string, url: string) => boolean * Defaults to: true */ - tracing: ShouldTraceFn; + tracing?: ShouldTraceFn | boolean; + /** - * Whether to add 'sentry-trace' headers to outgoing requests + * @deprecated Use `tracePropagationTargets` client option instead. * - * true | false | (method: string, url: string) => boolean - * Defaults to: true + * Sentry.init({ + * tracePropagationTargets: ['api.site.com'], + * }) */ - tracingOrigins: ShouldTraceFn; -} - -const DEFAULT_OPTIONS: NetOptions = { - breadcrumbs: true, - tracing: (_method, _url) => true, - tracingOrigins: (_method, _url) => true, -}; - -/** Converts all user supplied options to T | false */ -export function normalizeOptions(options: Partial>): Partial> { - return (Object.keys(options) as (keyof NetOptions)[]).reduce((obj, k) => { - if (typeof options[k] === 'function' || options[k] === false) { - obj[k] = options[k] as boolean & (false | ShouldTraceFn); - } - return obj; - }, {} as Partial>); + tracingOrigins?: ShouldTraceFn | boolean; } /** http module integration */ @@ -56,22 +41,19 @@ export class Net implements Integration { /** @inheritDoc */ public name: string = Net.id; - private readonly _options: OrFalse; - /** @inheritDoc */ - public constructor(options: Partial> = {}) { - this._options = { - ...DEFAULT_OPTIONS, - ...normalizeOptions(options), - }; - } + public constructor(private readonly _options: NetOptions = {}) {} /** @inheritDoc */ - public setupOnce(): void { + public setupOnce(_addGlobalEventProcessor: (callback: EventProcessor) => void, getCurrentHub: () => Hub): void { + const clientOptions = getCurrentHub().getClient()?.getOptions(); + // No need to instrument if we don't want to track anything - if (this._options.breadcrumbs || this._options.tracing) { - fill(net, 'request', createWrappedRequestFactory(this._options)); + if (this._options.breadcrumbs === false && this._options.tracing === false) { + return; } + + fill(net, 'request', createWrappedRequestFactory(this._options, clientOptions?.tracePropagationTargets)); } } @@ -125,7 +107,73 @@ type RequestMethod = (opt: RequestOptions) => ClientRequest; type WrappedRequestMethodFactory = (original: RequestMethod) => RequestMethod; /** */ -function createWrappedRequestFactory(options: OrFalse): WrappedRequestMethodFactory { +function createWrappedRequestFactory( + options: NetOptions, + 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(100); + const headersUrlMap = new LRUMap(100); + + const shouldCreateSpan = (method: string, url: string): boolean => { + if (options.tracing === undefined) { + return true; + } + + if (options.tracing === false) { + return false; + } + + const key = `${method}:${url}`; + + const cachedDecision = createSpanUrlMap.get(key); + if (cachedDecision !== undefined) { + return cachedDecision; + } + + const decision = options.tracing === true || options.tracing(method, url); + createSpanUrlMap.set(key, decision); + return decision; + }; + + // This will be considerably simpler once `tracingOrigins` is removed in the next major release + const shouldAttachTraceData = (method: string, url: string): boolean => { + if (options.tracingOrigins === false) { + return false; + } + + // Neither integration nor client options are set or integration option is set to true + if ( + (options.tracingOrigins === undefined && tracePropagationTargets === undefined) || + options.tracingOrigins === true + ) { + return true; + } + + const key = `${method}:${url}`; + + const cachedDecision = headersUrlMap.get(key); + if (cachedDecision !== undefined) { + return cachedDecision; + } + + if (tracePropagationTargets) { + const decision = stringMatchesSomePattern(url, tracePropagationTargets); + headersUrlMap.set(key, decision); + return decision; + } + + if (options.tracingOrigins) { + const decision = options.tracingOrigins(method, url); + headersUrlMap.set(key, decision); + return decision; + } + + // We cannot reach here since either `tracePropagationTargets` or `tracingOrigins` will be defined but TypeScript + // cannot infer that + return true; + }; + return function wrappedRequestMethodFactory(originalRequestMethod: RequestMethod): RequestMethod { return function requestMethod(this: typeof net, reqOptions: RequestOptions): ClientRequest { // eslint-disable-next-line @typescript-eslint/no-this-alias @@ -140,8 +188,9 @@ function createWrappedRequestFactory(options: OrFalse): WrappedReque let span: Span | undefined; - const scope = getCurrentHub().getScope(); - if (scope && options.tracing && options.tracing(method, url)) { + const hub = getCurrentHub(); + const scope = hub.getScope(); + if (scope && shouldCreateSpan(method, url)) { const parentSpan = scope.getSpan(); if (parentSpan) { @@ -150,7 +199,7 @@ function createWrappedRequestFactory(options: OrFalse): WrappedReque op: 'http.client', }); - if (options.tracingOrigins && options.tracingOrigins(method, url)) { + if (shouldAttachTraceData(method, url)) { request.setHeader('sentry-trace', span.toTraceparent()); } } @@ -160,7 +209,7 @@ function createWrappedRequestFactory(options: OrFalse): WrappedReque .once('response', function (this: ClientRequest, res: IncomingMessage): void { // eslint-disable-next-line @typescript-eslint/no-this-alias const req = this; - if (options.breadcrumbs) { + if (options.breadcrumbs !== false) { addRequestBreadcrumb('response', method, url, req, res); } if (span) { @@ -174,7 +223,7 @@ function createWrappedRequestFactory(options: OrFalse): WrappedReque // eslint-disable-next-line @typescript-eslint/no-this-alias const req = this; - if (options.breadcrumbs) { + if (options.breadcrumbs !== false) { addRequestBreadcrumb('error', method, url, req, undefined); } if (span) { diff --git a/test/unit/net.test.ts b/test/unit/net.test.ts new file mode 100644 index 00000000..d921cb8f --- /dev/null +++ b/test/unit/net.test.ts @@ -0,0 +1,220 @@ +import { expect, should, use } from 'chai'; +import * as http from 'http'; +import chaiAsPromised = require('chai-as-promised'); +import { setAsyncContextStrategy, Span } from '@sentry/core'; +import { createTransport, Hub, NodeClient } from '@sentry/node'; +import { ClientOptions, Transaction, TransactionContext } from '@sentry/types'; +import { resolvedSyncPromise } from '@sentry/utils'; +import { net } from 'electron'; + +import { Net } from '../../src/main/integrations/net-breadcrumbs'; + +// eslint-disable-next-line @typescript-eslint/unbound-method +const originalRequest = net.request; + +should(); +use(chaiAsPromised); + +const TEST_SERVER_PORT = 8123; + +function startServer(): http.Server { + return http + .createServer(function (req, res) { + const headersJson = JSON.stringify(req.headers); + res.write(headersJson); + res.end(); + }) + .listen(TEST_SERVER_PORT); +} + +async function makeRequest(): Promise> { + return net.fetch(`http://localhost:${TEST_SERVER_PORT}`).then((res) => res.json()) as Promise>; +} + +function getDefaultNodeClientOptions(options: Partial = {}): ClientOptions { + return { + integrations: [], + transport: () => createTransport({ recordDroppedEvent: () => undefined }, (_) => resolvedSyncPromise({})), + stackParser: () => [], + instrumenter: 'sentry', + ...options, + }; +} + +function mockAsyncContextStrategy(getHub: () => Hub): void { + function getCurrentHub(): Hub | undefined { + return getHub(); + } + + function runWithAsyncContext(fn: (hub: Hub) => T): T { + return fn(getHub()); + } + + setAsyncContextStrategy({ getCurrentHub, runWithAsyncContext }); +} + +function createTransactionOnScope( + customOptions: Partial = {}, + customContext?: Partial, +): [Transaction, Hub] { + const hub = new Hub(); + mockAsyncContextStrategy(() => hub); + + const options = getDefaultNodeClientOptions({ + dsn: 'https://dogsarebadatkeepingsecrets@squirrelchasers.ingest.sentry.io/12312012', + debug: true, + tracesSampleRate: 1.0, + release: '1.0.0', + environment: 'production', + ...customOptions, + }); + + hub.bindClient(new NodeClient(options)); + + hub.configureScope((scope) => + scope.setUser({ + id: 'uid123', + segment: 'segmentA', + }), + ); + + const transaction = hub.startTransaction({ + name: 'dogpark', + traceId: '12312012123120121231201212312012', + ...customContext, + }); + + hub.getScope().setSpan(transaction); + + return [transaction, hub]; +} + +function getSpans(transaction: Transaction): Span[] { + return (transaction as unknown as Span).spanRecorder?.spans as Span[]; +} + +// eslint-disable-next-line jest/no-disabled-tests +describe.skip('net integration', () => { + let server: http.Server | undefined; + + beforeEach(() => { + net.request = originalRequest; + server = startServer(); + }); + + afterEach(() => { + if (server) { + server.close(); + } + }); + + it('creates spans and adds headers by default', async () => { + const [transaction] = createTransactionOnScope({ integrations: [new Net()] }); + const headers = await makeRequest(); + + const spans = getSpans(transaction); + expect(spans.length).to.equal(2); + + // our span is at index 1 because the transaction itself is at index 0 + expect(spans[1].description).to.equal(`GET http://localhost:${TEST_SERVER_PORT}/`); + expect(spans[1].op).to.equal('http.client'); + + expect(headers['sentry-trace']).not.to.be.empty; + }); + + describe('constructor options', () => { + it('tracing = false disables spans and headers', async () => { + const [transaction] = createTransactionOnScope({ + integrations: [new Net({ tracing: false })], + }); + const headers = await makeRequest(); + + const spans = getSpans(transaction); + + // We should have the original transaction span, but no request span + expect(spans.length).to.equal(1); + expect(headers['sentry-trace']).to.be.undefined; + }); + + it('tracing = fn can disable spans and headers', async () => { + const [transaction] = createTransactionOnScope({ + integrations: [new Net({ tracing: () => false })], + }); + const headers = await makeRequest(); + + const spans = getSpans(transaction); + + // We should have the original transaction span, but no request span + expect(spans.length).to.equal(1); + expect(headers['sentry-trace']).to.be.undefined; + }); + + it('tracing = fn can enable spans and headers', async () => { + const [transaction] = createTransactionOnScope({ + integrations: [new Net({ tracing: () => true })], + }); + const headers = await makeRequest(); + + const spans = getSpans(transaction); + expect(spans.length).to.equal(2); + + // our span is at index 1 because the transaction itself is at index 0 + expect(spans[1].description).to.equal(`GET http://localhost:${TEST_SERVER_PORT}/`); + expect(spans[1].op).to.equal('http.client'); + + expect(headers['sentry-trace']).not.to.be.empty; + }); + + it('tracingOrigins = fn can disable headers', async () => { + const [transaction] = createTransactionOnScope({ + integrations: [new Net({ tracing: () => true, tracingOrigins: () => false })], + }); + const headers = await makeRequest(); + + const spans = getSpans(transaction); + expect(spans.length).to.equal(2); + + // our span is at index 1 because the transaction itself is at index 0 + expect(spans[1].description).to.equal(`GET http://localhost:${TEST_SERVER_PORT}/`); + expect(spans[1].op).to.equal('http.client'); + + expect(headers['sentry-trace']).to.be.undefined; + }); + }); + + describe('client options', () => { + it('tracePropagationTargets can enable headers', async () => { + const [transaction] = createTransactionOnScope({ + tracePropagationTargets: ['localhost'], + integrations: [new Net()], + }); + const headers = await makeRequest(); + + const spans = getSpans(transaction); + expect(spans.length).to.equal(2); + + // our span is at index 1 because the transaction itself is at index 0 + expect(spans[1].description).to.equal(`GET http://localhost:${TEST_SERVER_PORT}/`); + expect(spans[1].op).to.equal('http.client'); + + expect(headers['sentry-trace']).not.to.be.empty; + }); + + it('tracePropagationTargets can disable headers', async () => { + const [transaction] = createTransactionOnScope({ + tracePropagationTargets: ['api.localhost'], + integrations: [new Net()], + }); + const headers = await makeRequest(); + + const spans = getSpans(transaction); + expect(spans.length).to.equal(2); + + // our span is at index 1 because the transaction itself is at index 0 + expect(spans[1].description).to.equal(`GET http://localhost:${TEST_SERVER_PORT}/`); + expect(spans[1].op).to.equal('http.client'); + + expect(headers['sentry-trace']).to.be.undefined; + }); + }); +});