feat(node): Add request parameter to httpIntegration ignore callb…

…acks
getsentry · Jul 16, 2024 · 4c5288e · 4c5288e
1 parent 7cbfd34
commit 4c5288e
Show file tree

Hide file tree

Showing 4 changed files with 138 additions and 52 deletions.
diff --git a/...es/node-integration-tests/suites/tracing/httpIntegration/server-ignoreIncomingRequests.js b/...es/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/...es/node-integration-tests/suites/tracing/httpIntegration/server-ignoreOutgoingRequests.js b/...es/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
@@ -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
@@ -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';
@@ -35,13 +35,13 @@ interface HttpOptions {
    * Do not capture spans or breadcrumbs for outgoing HTTP requests to URLs where the given callback returns `true`.
    * This controls both span & breadcrumb creation - spans will be non recording if tracing is disabled.
    */
-  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`.
    * This controls both span & breadcrumb creation - spans will be non recording if tracing is disabled.
    */
-  ignoreIncomingRequests?: (url: string) => boolean;
+  ignoreIncomingRequests?: (url: string, request: IncomingMessage) => boolean;
 
   /**
    * Additional instrumentation options that are passed to the underlying HttpInstrumentation.
@@ -95,7 +95,7 @@ export const instrumentHttp = Object.assign(
         }
 
         const _ignoreOutgoingRequests = _httpOptions.ignoreOutgoingRequests;
-        if (_ignoreOutgoingRequests && _ignoreOutgoingRequests(url)) {
+        if (_ignoreOutgoingRequests && _ignoreOutgoingRequests(url, request)) {
           return true;
         }
 
@@ -112,7 +112,7 @@ export const instrumentHttp = Object.assign(
         }
 
         const _ignoreIncomingRequests = _httpOptions.ignoreIncomingRequests;
-        if (url && _ignoreIncomingRequests && _ignoreIncomingRequests(url)) {
+        if (url && _ignoreIncomingRequests && _ignoreIncomingRequests(url, request)) {
           return true;
         }