From 46f6503da9adf52faf497d45fac71c857653e8c1 Mon Sep 17 00:00:00 2001 From: Pagan Gazzard Date: Wed, 2 Apr 2025 16:26:22 +0100 Subject: [PATCH 1/2] feat(node): Add `ignoreIncomingRequestBody` to avoid capturing request bodies This allows ignoring the request body for certain requests, which is very useful in situations of long-running requests with large/streaming request bodies which can result in a lot of memory usage --- .../suites/express/tracing/server.js | 14 +++++++++ .../suites/express/tracing/test.ts | 29 +++++++++++++++++++ .../http/SentryHttpInstrumentation.ts | 17 +++++++++-- 3 files changed, 58 insertions(+), 2 deletions(-) diff --git a/dev-packages/node-integration-tests/suites/express/tracing/server.js b/dev-packages/node-integration-tests/suites/express/tracing/server.js index f9b4ae24b339..0291ee656995 100644 --- a/dev-packages/node-integration-tests/suites/express/tracing/server.js +++ b/dev-packages/node-integration-tests/suites/express/tracing/server.js @@ -8,6 +8,16 @@ Sentry.init({ tracePropagationTargets: [/^(?!.*test).*$/], tracesSampleRate: 1.0, transport: loggingTransport, + integrations: [ + Sentry.httpIntegration({ + ignoreIncomingRequestBody: (url) => { + if (url.includes('/test-post-ignore-body')) { + return true; + } + return false; + }, + }), + ], }); // express must be required after Sentry is initialized @@ -43,6 +53,10 @@ app.post('/test-post', function (req, res) { res.send({ status: 'ok', body: req.body }); }); +app.post('/test-post-ignore-body', function (req, res) { + res.send({ status: 'ok', body: req.body }); +}); + Sentry.setupExpressErrorHandler(app); startExpressServerAndSendPortToRunner(app); diff --git a/dev-packages/node-integration-tests/suites/express/tracing/test.ts b/dev-packages/node-integration-tests/suites/express/tracing/test.ts index ebf9977e722b..4ecb0b13995b 100644 --- a/dev-packages/node-integration-tests/suites/express/tracing/test.ts +++ b/dev-packages/node-integration-tests/suites/express/tracing/test.ts @@ -1,5 +1,6 @@ import { afterAll, describe, expect, test } from 'vitest'; import { cleanupChildProcesses, createRunner } from '../../../utils/runner'; +import { assertSentryTransaction } from '../../../utils/assertions'; describe('express tracing', () => { afterAll(() => { @@ -244,6 +245,34 @@ describe('express tracing', () => { }); await runner.completed(); }); + + test('correctly ignores request data', async () => { + const runner = createRunner(__dirname, 'server.js') + .expect({ + transaction: (e) => { + assertSentryTransaction(e, { + transaction: 'POST /test-post-ignore-body', + request: { + url: expect.stringMatching(/^http:\/\/localhost:(\d+)\/test-post-ignore-body$/), + method: 'POST', + headers: { + 'user-agent': expect.stringContaining(''), + 'content-type': 'application/octet-stream', + }, + }, + }); + // Ensure the request body has been ignored + expect(e).have.property('request').that.does.not.have.property('data'); + }, + }) + .start(); + + runner.makeRequest('post', '/test-post-ignore-body', { + headers: { 'Content-Type': 'application/octet-stream' }, + data: Buffer.from('some plain text in buffer'), + }); + await runner.completed(); + }); }); }); }); diff --git a/packages/node/src/integrations/http/SentryHttpInstrumentation.ts b/packages/node/src/integrations/http/SentryHttpInstrumentation.ts index aa1f0157f2cf..0e0502b6fd1f 100644 --- a/packages/node/src/integrations/http/SentryHttpInstrumentation.ts +++ b/packages/node/src/integrations/http/SentryHttpInstrumentation.ts @@ -58,6 +58,15 @@ export type SentryHttpInstrumentationOptions = InstrumentationConfig & { */ ignoreOutgoingRequests?: (url: string, request: RequestOptions) => boolean; + /** + * Do not capture the request body for incoming HTTP requests to URLs where the given callback returns `true`. + * This can be useful for long running requests where the body is not needed and we want to avoid capturing it. + * + * @param url Contains the entire URL, including query string (if any), protocol, host, etc. of the outgoing request. + * @param request Contains the {@type RequestOptions} object used to make the outgoing request. + */ + ignoreIncomingRequestBody?: (url: string, request: RequestOptions) => boolean; + /** * Whether the integration should create [Sessions](https://docs.sentry.io/product/releases/health/#sessions) for incoming requests to track the health and crash-free rate of your releases in Sentry. * Read more about Release Health: https://docs.sentry.io/product/releases/health/ @@ -150,6 +159,7 @@ export class SentryHttpInstrumentation extends InstrumentationBase (this: unknown, event: string, ...args: unknown[]) => boolean { // eslint-disable-next-line @typescript-eslint/no-this-alias const instrumentation = this; + const { ignoreIncomingRequestBody } = instrumentation.getConfig(); return ( original: (event: string, ...args: unknown[]) => boolean, @@ -171,7 +181,10 @@ export class SentryHttpInstrumentation extends InstrumentationBase Date: Fri, 4 Apr 2025 10:26:06 +0200 Subject: [PATCH 2/2] add integration-level option --- .../suites/express/tracing/test.ts | 32 +++++++++---------- packages/node/src/integrations/http/index.ts | 9 ++++++ 2 files changed, 25 insertions(+), 16 deletions(-) diff --git a/dev-packages/node-integration-tests/suites/express/tracing/test.ts b/dev-packages/node-integration-tests/suites/express/tracing/test.ts index 4ecb0b13995b..ebe710bdc0d0 100644 --- a/dev-packages/node-integration-tests/suites/express/tracing/test.ts +++ b/dev-packages/node-integration-tests/suites/express/tracing/test.ts @@ -248,23 +248,23 @@ describe('express tracing', () => { test('correctly ignores request data', async () => { const runner = createRunner(__dirname, 'server.js') - .expect({ - transaction: (e) => { - assertSentryTransaction(e, { - transaction: 'POST /test-post-ignore-body', - request: { - url: expect.stringMatching(/^http:\/\/localhost:(\d+)\/test-post-ignore-body$/), - method: 'POST', - headers: { - 'user-agent': expect.stringContaining(''), - 'content-type': 'application/octet-stream', + .expect({ + transaction: e => { + assertSentryTransaction(e, { + transaction: 'POST /test-post-ignore-body', + request: { + url: expect.stringMatching(/^http:\/\/localhost:(\d+)\/test-post-ignore-body$/), + method: 'POST', + headers: { + 'user-agent': expect.stringContaining(''), + 'content-type': 'application/octet-stream', + }, }, - }, - }); - // Ensure the request body has been ignored - expect(e).have.property('request').that.does.not.have.property('data'); - }, - }) + }); + // Ensure the request body has been ignored + expect(e).have.property('request').that.does.not.have.property('data'); + }, + }) .start(); runner.makeRequest('post', '/test-post-ignore-body', { diff --git a/packages/node/src/integrations/http/index.ts b/packages/node/src/integrations/http/index.ts index c8ba11655dcb..f46724aa9b72 100644 --- a/packages/node/src/integrations/http/index.ts +++ b/packages/node/src/integrations/http/index.ts @@ -73,6 +73,15 @@ interface HttpOptions { */ ignoreIncomingRequests?: (urlPath: string, request: IncomingMessage) => boolean; + /** + * Do not capture the request body for incoming HTTP requests to URLs where the given callback returns `true`. + * This can be useful for long running requests where the body is not needed and we want to avoid capturing it. + * + * @param url Contains the entire URL, including query string (if any), protocol, host, etc. of the outgoing request. + * @param request Contains the {@type RequestOptions} object used to make the outgoing request. + */ + ignoreIncomingRequestBody?: (url: string, request: RequestOptions) => boolean; + /** * If true, do not generate spans for incoming requests at all. * This is used by Remix to avoid generating spans for incoming requests, as it generates its own spans.