feat(node): Allow to pass instrumentation config to httpIntegration

getsentry · Jul 5, 2024 · 28a397f · 28a397f
1 parent 0bad43b
commit 28a397f
Show file tree

Hide file tree

Showing 7 changed files with 201 additions and 29 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
@@ -1,6 +1,6 @@
 import { cleanupChildProcesses, createRunner } from '../../../utils/runner';
 
-describe('express tracing experimental', () => {
+describe('express tracing', () => {
   afterAll(() => {
     cleanupChildProcesses();
   });

diff --git a/dev-packages/node-integration-tests/suites/tracing/httpIntegration/server-experimental.js b/dev-packages/node-integration-tests/suites/tracing/httpIntegration/server-experimental.js
@@ -0,0 +1,38 @@
+const { loggingTransport } = require('@sentry-internal/node-integration-tests');
+const Sentry = require('@sentry/node');
+
+Sentry.init({
+  dsn: 'https://public@dsn.ingest.sentry.io/1337',
+  release: '1.0',
+  // disable attaching headers to /test/* endpoints
+  tracePropagationTargets: [/^(?!.*test).*$/],
+  tracesSampleRate: 1.0,
+  transport: loggingTransport,
+
+  integrations: [
+    Sentry.httpIntegration({
+      instrumentation: {
+        _experimentalConfig: {
+          serverName: 'sentry-test-server-name',
+        },
+      },
+    }),
+  ],
+});
+
+// express must be required after Sentry is initialized
+const express = require('express');
+const cors = require('cors');
+const { startExpressServerAndSendPortToRunner } = require('@sentry-internal/node-integration-tests');
+
+const app = express();
+
+app.use(cors());
+
+app.get('/test', (_req, res) => {
+  res.send({ response: 'response 1' });
+});
+
+Sentry.setupExpressErrorHandler(app);
+
+startExpressServerAndSendPortToRunner(app);
diff --git a/dev-packages/node-integration-tests/suites/tracing/httpIntegration/server.js b/dev-packages/node-integration-tests/suites/tracing/httpIntegration/server.js
@@ -0,0 +1,58 @@
+const { loggingTransport } = require('@sentry-internal/node-integration-tests');
+const Sentry = require('@sentry/node');
+
+Sentry.init({
+  dsn: 'https://public@dsn.ingest.sentry.io/1337',
+  release: '1.0',
+  // disable attaching headers to /test/* endpoints
+  tracePropagationTargets: [/^(?!.*test).*$/],
+  tracesSampleRate: 1.0,
+  transport: loggingTransport,
+
+  integrations: [
+    Sentry.httpIntegration({
+      instrumentation: {
+        requestHook: (span, req) => {
+          span.setAttribute('attr1', 'yes');
+          Sentry.setExtra('requestHookCalled', {
+            url: req.url,
+            method: req.method,
+          });
+        },
+        responseHook: (span, res) => {
+          span.setAttribute('attr2', 'yes');
+          Sentry.setExtra('responseHookCalled', {
+            url: res.req.url,
+            method: res.req.method,
+          });
+        },
+        applyCustomAttributesOnSpan: (span, req, res) => {
+          span.setAttribute('attr3', 'yes');
+          Sentry.setExtra('applyCustomAttributesOnSpanCalled', {
+            reqUrl: req.url,
+            reqMethod: req.method,
+            resUrl: res.req.url,
+            resMethod: res.req.method,
+          });
+        },
+      },
+    }),
+  ],
+});
+
+// express must be required after Sentry is initialized
+const express = require('express');
+const cors = require('cors');
+const { startExpressServerAndSendPortToRunner } = require('@sentry-internal/node-integration-tests');
+
+const app = express();
+
+app.use(cors());
+
+app.get('/test', (_req, res) => {
+  res.send({ response: 'response 1' });
+});
+
+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
@@ -0,0 +1,73 @@
+import { cleanupChildProcesses, createRunner } from '../../../utils/runner';
+
+describe('httpIntegration', () => {
+  afterAll(() => {
+    cleanupChildProcesses();
+  });
+
+  test('allows to pass instrumentation options to integration', done => {
+    createRunner(__dirname, 'server.js')
+      .ignore('session', 'sessions')
+      .expect({
+        transaction: {
+          contexts: {
+            trace: {
+              span_id: expect.any(String),
+              trace_id: expect.any(String),
+              data: {
+                url: expect.stringMatching(/\/test$/),
+                'http.response.status_code': 200,
+                attr1: 'yes',
+                attr2: 'yes',
+                attr3: 'yes',
+              },
+              op: 'http.server',
+              status: 'ok',
+            },
+          },
+          extra: {
+            requestHookCalled: {
+              url: expect.stringMatching(/\/test$/),
+              method: 'GET',
+            },
+            responseHookCalled: {
+              url: expect.stringMatching(/\/test$/),
+              method: 'GET',
+            },
+            applyCustomAttributesOnSpanCalled: {
+              reqUrl: expect.stringMatching(/\/test$/),
+              reqMethod: 'GET',
+              resUrl: expect.stringMatching(/\/test$/),
+              resMethod: 'GET',
+            },
+          },
+        },
+      })
+      .start(done)
+      .makeRequest('get', '/test');
+  });
+
+  test('allows to pass experimental config through to integration', done => {
+    createRunner(__dirname, 'server-experimental.js')
+      .ignore('session', 'sessions')
+      .expect({
+        transaction: {
+          contexts: {
+            trace: {
+              span_id: expect.any(String),
+              trace_id: expect.any(String),
+              data: {
+                url: expect.stringMatching(/\/test$/),
+                'http.response.status_code': 200,
+                'http.server_name': 'sentry-test-server-name',
+              },
+              op: 'http.server',
+              status: 'ok',
+            },
+          },
+        },
+      })
+      .start(done)
+      .makeRequest('get', '/test');
+  });
+});
diff --git a/packages/nextjs/src/server/httpIntegration.ts b/packages/nextjs/src/server/httpIntegration.ts
@@ -22,19 +22,7 @@ class CustomNextjsHttpIntegration extends HttpInstrumentation {
   }
 }
 
-interface HttpOptions {
-  /**
-   * Whether breadcrumbs should be recorded for requests.
-   * Defaults to true
-   */
-  breadcrumbs?: boolean;
-
-  /**
-   * 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;
-}
+type HttpOptions = Parameters<typeof originalHttpIntegration>[0];
 
 /**
  * The http integration instruments Node's internal http and https modules.

diff --git a/packages/node/src/integrations/http.ts b/packages/node/src/integrations/http.ts
@@ -43,6 +43,25 @@ interface HttpOptions {
    */
   ignoreIncomingRequests?: (url: string) => boolean;
 
+  /**
+   * Additional instrumentation options that are passed to the underlying HttpInstrumentation.
+   */
+  instrumentation?: {
+    requestHook?: (span: Span, req: ClientRequest | HTTPModuleRequestIncomingMessage) => void;
+    responseHook?: (span: Span, response: HTTPModuleRequestIncomingMessage | ServerResponse) => void;
+    applyCustomAttributesOnSpan?: (
+      span: Span,
+      request: ClientRequest | HTTPModuleRequestIncomingMessage,
+      response: HTTPModuleRequestIncomingMessage | ServerResponse,
+    ) => void;
+
+    /**
+     * You can pass any configuration through to the underlying instrumention.
+     * Note that there are no semver guarantees for this!
+     */
+    _experimentalConfig?: ConstructorParameters<typeof HttpInstrumentation>[0];
+  };
+
   /** Allows to pass a custom version of HttpInstrumentation. We use this for Next.js. */
   _instrumentation?: typeof HttpInstrumentation;
 }
@@ -63,6 +82,7 @@ export const instrumentHttp = Object.assign(
     const _InstrumentationClass = _httpOptions._instrumentation || HttpInstrumentation;
 
     _httpInstrumentation = new _InstrumentationClass({
+      ..._httpOptions.instrumentation?._experimentalConfig,
       ignoreOutgoingRequestHook: request => {
         const url = getRequestUrl(request);
 
@@ -107,6 +127,7 @@ export const instrumentHttp = Object.assign(
         // both, incoming requests and "client" requests made within the app trigger the requestHook
         // we only want to isolate and further annotate incoming requests (IncomingMessage)
         if (_isClientRequest(req)) {
+          _httpOptions.instrumentation?.requestHook?.(span, req);
           return;
         }
 
@@ -134,24 +155,30 @@ export const instrumentHttp = Object.assign(
         const bestEffortTransactionName = `${httpMethod} ${httpTarget}`;
 
         isolationScope.setTransactionName(bestEffortTransactionName);
+
+        _httpOptions.instrumentation?.requestHook?.(span, req);
       },
-      responseHook: () => {
+      responseHook: (span, res) => {
         const client = getClient<NodeClient>();
         if (client && client.getOptions().autoSessionTracking) {
           setImmediate(() => {
             client['_captureRequestSession']();
           });
         }
+
+        _httpOptions.instrumentation?.responseHook?.(span, res);
       },
       applyCustomAttributesOnSpan: (
-        _span: Span,
+        span: Span,
         request: ClientRequest | HTTPModuleRequestIncomingMessage,
         response: HTTPModuleRequestIncomingMessage | ServerResponse,
       ) => {
         const _breadcrumbs = typeof _httpOptions.breadcrumbs === 'undefined' ? true : _httpOptions.breadcrumbs;
         if (_breadcrumbs) {
           _addRequestBreadcrumb(request, response);
         }
+
+        _httpOptions.instrumentation?.applyCustomAttributesOnSpan?.(span, request, response);
       },
     });
 

diff --git a/packages/remix/src/utils/integrations/http.ts b/packages/remix/src/utils/integrations/http.ts
@@ -16,19 +16,7 @@ class RemixHttpIntegration extends HttpInstrumentation {
   }
 }
 
-interface HttpOptions {
-  /**
-   * Whether breadcrumbs should be recorded for requests.
-   * Defaults to true
-   */
-  breadcrumbs?: boolean;
-
-  /**
-   * 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;
-}
+type HttpOptions = Parameters<typeof originalHttpIntegration>[0];
 
 /**
  * The http integration instruments Node's internal http and https modules.