From 67f4e321a45cd81c18f8966e44913da56be894d4 Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Tue, 16 Jul 2024 12:30:08 +0200 Subject: [PATCH] feat(node): Add `request` parameter to `httpIntegration` ignore callbacks --- .../server-ignoreIncomingRequests.js | 14 +- .../server-ignoreOutgoingRequests.js | 21 ++- .../suites/tracing/httpIntegration/test.ts | 145 ++++++++++++------ packages/node/src/integrations/http.ts | 16 +- 4 files changed, 144 insertions(+), 52 deletions(-) diff --git a/dev-packages/node-integration-tests/suites/tracing/httpIntegration/server-ignoreIncomingRequests.js b/dev-packages/node-integration-tests/suites/tracing/httpIntegration/server-ignoreIncomingRequests.js index f1e5d9870fcf..bafd8b8f2bc4 100644 --- a/dev-packages/node-integration-tests/suites/tracing/httpIntegration/server-ignoreIncomingRequests.js +++ b/dev-packages/node-integration-tests/suites/tracing/httpIntegration/server-ignoreIncomingRequests.js @@ -9,8 +9,14 @@ Sentry.init({ integrations: [ Sentry.httpIntegration({ - ignoreIncomingRequests: url => { - return url.includes('/liveness'); + ignoreIncomingRequests: (url, request) => { + if (url.includes('/liveness')) { + return true; + } + if (request.method === 'POST' && request.url.includes('readiness')) { + return true; + } + return false; }, }), ], @@ -33,6 +39,10 @@ app.get('/liveness', (_req, res) => { res.send({ response: 'liveness' }); }); +app.post('/readiness', (_req, res) => { + res.send({ response: 'readiness' }); +}); + Sentry.setupExpressErrorHandler(app); startExpressServerAndSendPortToRunner(app); diff --git a/dev-packages/node-integration-tests/suites/tracing/httpIntegration/server-ignoreOutgoingRequests.js b/dev-packages/node-integration-tests/suites/tracing/httpIntegration/server-ignoreOutgoingRequests.js index ce520c999259..9d7d2ed069d1 100644 --- a/dev-packages/node-integration-tests/suites/tracing/httpIntegration/server-ignoreOutgoingRequests.js +++ b/dev-packages/node-integration-tests/suites/tracing/httpIntegration/server-ignoreOutgoingRequests.js @@ -10,8 +10,14 @@ Sentry.init({ integrations: [ Sentry.httpIntegration({ - ignoreOutgoingRequests: url => { - return url.includes('example.com'); + ignoreOutgoingRequests: (url, request) => { + if (url.includes('example.com')) { + return true; + } + if (request.method === 'POST' && request.path === '/path') { + return true; + } + return false; }, }), ], @@ -37,6 +43,17 @@ app.get('/test', (_req, response) => { .end(); }); +app.post('/testPath', (_req, response) => { + http + .request('http://example.com/path', res => { + res.on('data', () => {}); + res.on('end', () => { + response.send({ response: 'done' }); + }); + }) + .end(); +}); + Sentry.setupExpressErrorHandler(app); startExpressServerAndSendPortToRunner(app); diff --git a/dev-packages/node-integration-tests/suites/tracing/httpIntegration/test.ts b/dev-packages/node-integration-tests/suites/tracing/httpIntegration/test.ts index aebe0dd676ba..972ba30eab43 100644 --- a/dev-packages/node-integration-tests/suites/tracing/httpIntegration/test.ts +++ b/dev-packages/node-integration-tests/suites/tracing/httpIntegration/test.ts @@ -76,58 +76,117 @@ describe('httpIntegration', () => { .makeRequest('get', '/test'); }); - test("doesn't create a root span for incoming requests ignored via `ignoreIncomingRequests`", done => { - const runner = createRunner(__dirname, 'server-ignoreIncomingRequests.js') - .expect({ - transaction: { - contexts: { - trace: { - span_id: expect.any(String), - trace_id: expect.any(String), - data: { - url: expect.stringMatching(/\/test$/), - 'http.response.status_code': 200, + describe("doesn't create a root span for incoming requests ignored via `ignoreIncomingRequests`", () => { + test('via the url param', done => { + const runner = createRunner(__dirname, 'server-ignoreIncomingRequests.js') + .expect({ + transaction: { + contexts: { + trace: { + span_id: expect.any(String), + trace_id: expect.any(String), + data: { + url: expect.stringMatching(/\/test$/), + 'http.response.status_code': 200, + }, + op: 'http.server', + status: 'ok', }, - op: 'http.server', - status: 'ok', }, + transaction: 'GET /test', }, - transaction: 'GET /test', - }, - }) - .start(done); + }) + .start(done); + + runner.makeRequest('get', '/liveness'); // should be ignored + runner.makeRequest('get', '/test'); + }); + + test('via the request param', done => { + const runner = createRunner(__dirname, 'server-ignoreIncomingRequests.js') + .expect({ + transaction: { + contexts: { + trace: { + span_id: expect.any(String), + trace_id: expect.any(String), + data: { + url: expect.stringMatching(/\/test$/), + 'http.response.status_code': 200, + }, + op: 'http.server', + status: 'ok', + }, + }, + transaction: 'GET /test', + }, + }) + .start(done); - runner.makeRequest('get', '/liveness'); // should be ignored - runner.makeRequest('get', '/test'); + runner.makeRequest('post', '/readiness'); // should be ignored + runner.makeRequest('get', '/test'); + }); }); - test("doesn't create child spans for outgoing requests ignored via `ignoreOutgoingRequests`", done => { - const runner = createRunner(__dirname, 'server-ignoreOutgoingRequests.js') - .expect({ - transaction: { - contexts: { - trace: { - span_id: expect.any(String), - trace_id: expect.any(String), - data: { - url: expect.stringMatching(/\/test$/), - 'http.response.status_code': 200, + describe("doesn't create child spans for outgoing requests ignored via `ignoreOutgoingRequests`", () => { + test('via the url param', done => { + const runner = createRunner(__dirname, 'server-ignoreOutgoingRequests.js') + .expect({ + transaction: { + contexts: { + trace: { + span_id: expect.any(String), + trace_id: expect.any(String), + data: { + url: expect.stringMatching(/\/test$/), + 'http.response.status_code': 200, + }, + op: 'http.server', + status: 'ok', }, - op: 'http.server', - status: 'ok', }, + transaction: 'GET /test', + spans: [ + expect.objectContaining({ op: 'middleware.express', description: 'query' }), + expect.objectContaining({ op: 'middleware.express', description: 'expressInit' }), + expect.objectContaining({ op: 'middleware.express', description: 'corsMiddleware' }), + expect.objectContaining({ op: 'request_handler.express', description: '/test' }), + ], }, - transaction: 'GET /test', - spans: [ - expect.objectContaining({ op: 'middleware.express', description: 'query' }), - expect.objectContaining({ op: 'middleware.express', description: 'expressInit' }), - expect.objectContaining({ op: 'middleware.express', description: 'corsMiddleware' }), - expect.objectContaining({ op: 'request_handler.express', description: '/test' }), - ], - }, - }) - .start(done); + }) + .start(done); + + runner.makeRequest('get', '/test'); + }); + + test('via the request param', done => { + const runner = createRunner(__dirname, 'server-ignoreOutgoingRequests.js') + .expect({ + transaction: { + contexts: { + trace: { + span_id: expect.any(String), + trace_id: expect.any(String), + data: { + url: expect.stringMatching(/\/testPath$/), + 'http.response.status_code': 200, + }, + op: 'http.server', + status: 'ok', + }, + }, + transaction: 'POST /testPath', + spans: [ + expect.objectContaining({ op: 'middleware.express', description: 'query' }), + expect.objectContaining({ op: 'middleware.express', description: 'expressInit' }), + expect.objectContaining({ op: 'middleware.express', description: 'corsMiddleware' }), + expect.objectContaining({ op: 'request_handler.express', description: '/testPath' }), + ], + }, + }) + .start(done); - runner.makeRequest('get', '/test'); + runner.makeRequest('post', '/testPath'); + }); }); }); diff --git a/packages/node/src/integrations/http.ts b/packages/node/src/integrations/http.ts index 632b6023e7a3..615506605c9b 100644 --- a/packages/node/src/integrations/http.ts +++ b/packages/node/src/integrations/http.ts @@ -1,4 +1,4 @@ -import type { ClientRequest, ServerResponse } from 'node:http'; +import type { ClientRequest, IncomingMessage, RequestOptions, ServerResponse } from 'node:http'; import type { Span } from '@opentelemetry/api'; import { HttpInstrumentation } from '@opentelemetry/instrumentation-http'; import { addOpenTelemetryInstrumentation } from '@sentry/opentelemetry'; @@ -37,8 +37,11 @@ interface HttpOptions { * * The `url` param contains the entire URL, including query string (if any), protocol, host, etc. of the outgoing request. * For example: `'https://someService.com/users/details?id=123'` + * + * The `request` param contains the original {@type RequestOptions} object used to make the outgoing request. + * You can use it to filter on additional properties like method, headers, etc. */ - ignoreOutgoingRequests?: (url: string) => boolean; + ignoreOutgoingRequests?: (url: string, request: RequestOptions) => boolean; /** * Do not capture spans or breadcrumbs for incoming HTTP requests to URLs where the given callback returns `true`. @@ -46,8 +49,11 @@ interface HttpOptions { * * The `urlPath` param consists of the URL path and query string (if any) of the incoming request. * For example: `'/users/details?id=123'` + * + * The `request` param contains the original {@type IncomingMessage} object of the incoming request. + * You can use it to filter on additional properties like method, headers, etc. */ - ignoreIncomingRequests?: (urlPath: string) => boolean; + ignoreIncomingRequests?: (urlPath: string, request: IncomingMessage) => boolean; /** * Additional instrumentation options that are passed to the underlying HttpInstrumentation. @@ -101,7 +107,7 @@ export const instrumentHttp = Object.assign( } const _ignoreOutgoingRequests = _httpOptions.ignoreOutgoingRequests; - if (_ignoreOutgoingRequests && _ignoreOutgoingRequests(url)) { + if (_ignoreOutgoingRequests && _ignoreOutgoingRequests(url, request)) { return true; } @@ -120,7 +126,7 @@ export const instrumentHttp = Object.assign( } const _ignoreIncomingRequests = _httpOptions.ignoreIncomingRequests; - if (urlPath && _ignoreIncomingRequests && _ignoreIncomingRequests(urlPath)) { + if (urlPath && _ignoreIncomingRequests && _ignoreIncomingRequests(urlPath, request)) { return true; }