feat(node): Update httpIntegration handling of incoming requests

Author: mydea

CHANGELOG.md

@@ -4,6 +4,15 @@
 
 - "You miss 100 percent of the chances you don't take. — Wayne Gretzky" — Michael Scott
 
+### Important Changes
+
+- **feat(node): Update `httpIntegration` handling of incoming requests
+
+This version updates the handling of the Node SDK of incoming requests. Instead of relying on @opentelemetry/instrumentation-http for this, we now handle this internally, ensuring that we can optimize performance as much as possible and avoid interop problems.
+
+This change should not affect users, unless they are relying on very in-depth implementation details. Importantly, this also drops the `_experimentalConfig` option of the integration - this will no longer do anything.
+Finally, you can still pass `instrumentation.{requestHook,responseHook,applyCustomAttributesOnSpan}` options, but they are deprecated and will be removed in v11. Instead, you can use the new `incomingRequestSpanHook` configuration option if you want to adjust the incoming request span.
+
 ## 10.3.0
 
 - feat(core): MCP Server - Capture prompt results from prompt function calls (#17284)

dev-packages/node-integration-tests/suites/express/tracing/updateName/test.ts

@@ -29,11 +29,11 @@ describe('express tracing', () => {
 
     // Also calling `updateName` AND setting a source doesn't change anything - Otel has no concept of source, this is sentry-internal.
     // Therefore, only the source is updated but the name is still overwritten by Otel.
-    test("calling `span.updateName` and setting attribute source doesn't update the final name in express but it updates the source", async () => {
+    test('calling `span.updateName` and setting attribute source updates the final name in express', async () => {
       const runner = createRunner(__dirname, 'server.js')
         .expect({
           transaction: {
-            transaction: 'GET /test/:id/span-updateName-source',
+            transaction: 'new-name',
             transaction_info: {
               source: 'custom',
             },

dev-packages/node-integration-tests/suites/tracing/httpIntegration/instrument.mjs

@@ -11,6 +11,15 @@ Sentry.init({
 
   integrations: [
     Sentry.httpIntegration({
+      incomingRequestSpanHook: (span, req, res) => {
+        span.setAttribute('incomingRequestSpanHook', 'yes');
+        Sentry.setExtra('incomingRequestSpanHookCalled', {
+          reqUrl: req.url,
+          reqMethod: req.method,
+          resUrl: res.req.url,
+          resMethod: res.req.method,
+        });
+      },
       instrumentation: {
         requestHook: (span, req) => {
           span.setAttribute('attr1', 'yes');

dev-packages/node-integration-tests/suites/tracing/httpIntegration/server-experimental.js

@@ -49,30 +49,38 @@ describe('httpIntegration', () => {
       runner.makeRequest('get', '/test');
       await runner.completed();
     });
-  });
 
-  test('allows to pass experimental config through to integration', async () => {
-    const runner = createRunner(__dirname, 'server-experimental.js')
-      .expect({
-        transaction: {
-          contexts: {
-            trace: {
-              span_id: expect.stringMatching(/[a-f0-9]{16}/),
-              trace_id: expect.stringMatching(/[a-f0-9]{32}/),
-              data: {
-                url: expect.stringMatching(/\/test$/),
-                'http.response.status_code': 200,
-                'http.server_name': 'sentry-test-server-name',
+    test('allows to configure incomingRequestSpanHook', async () => {
+      const runner = createRunner()
+        .expect({
+          transaction: {
+            contexts: {
+              trace: {
+                span_id: expect.stringMatching(/[a-f0-9]{16}/),
+                trace_id: expect.stringMatching(/[a-f0-9]{32}/),
+                data: {
+                  url: expect.stringMatching(/\/test$/),
+                  'http.response.status_code': 200,
+                  incomingRequestSpanHook: 'yes',
+                },
+                op: 'http.server',
+                status: 'ok',
               },
-              op: 'http.server',
-              status: 'ok',
             },
+            extra: expect.objectContaining({
+              incomingRequestSpanHookCalled: {
+                reqUrl: expect.stringMatching(/\/test$/),
+                reqMethod: 'GET',
+                resUrl: expect.stringMatching(/\/test$/),
+                resMethod: 'GET',
+              },
+            }),
           },
-        },
-      })
-      .start();
-    runner.makeRequest('get', '/test');
-    await runner.completed();
+        })
+        .start();
+      runner.makeRequest('get', '/test');
+      await runner.completed();
+    });
   });
 
   describe("doesn't create a root span for incoming requests ignored via `ignoreIncomingRequests`", () => {

dev-packages/node-integration-tests/suites/tracing/httpIntegration/test.ts

@@ -2,6 +2,7 @@ import type { ChannelListener } from 'node:diagnostics_channel';
 import { subscribe, unsubscribe } from 'node:diagnostics_channel';
 import type * as http from 'node:http';
 import type * as https from 'node:https';
+import type { Span } from '@opentelemetry/api';
 import { context } from '@opentelemetry/api';
 import { isTracingSuppressed } from '@opentelemetry/core';
 import type { InstrumentationConfig } from '@opentelemetry/instrumentation';
@@ -28,12 +29,22 @@ export type SentryHttpInstrumentationOptions = InstrumentationConfig & {
    */
   breadcrumbs?: boolean;
 
+  /**
+   * Whether to create spans for requests or not.
+   * As of now, creates spans for incoming requests, but not outgoing requests.
+   *
+   * @default `true`
+   */
+  spans?: boolean;
+
   /**
    * Whether to extract the trace ID from the `sentry-trace` header for incoming requests.
    * By default this is done by the HttpInstrumentation, but if that is not added (e.g. because tracing is disabled, ...)
    * then this instrumentation can take over.
    *
-   * @default `false`
+   * @deprecated This is always true and the option will be removed in the future.
+   *
+   * @default `true`
    */
   extractIncomingTraceFromHeader?: boolean;
 
@@ -57,6 +68,14 @@ export type SentryHttpInstrumentationOptions = InstrumentationConfig & {
    */
   ignoreOutgoingRequests?: (url: string, request: http.RequestOptions) => boolean;
 
+  /**
+   * Do not capture spans for incoming HTTP requests to URLs where the given callback returns `true`.
+   *
+   * @param urlPath Contains the URL path and query string (if any) of the incoming request.
+   * @param request Contains the {@type IncomingMessage} object of the incoming request.
+   */
+  ignoreSpansForIncomingRequests?: (urlPath: string, request: http.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.
@@ -66,6 +85,12 @@ export type SentryHttpInstrumentationOptions = InstrumentationConfig & {
    */
   ignoreIncomingRequestBody?: (url: string, request: http.RequestOptions) => boolean;
 
+  /**
+   * A hook that can be used to mutate the span for incoming requests.
+   * This is triggered after the span is created, but before it is recorded.
+   */
+  incomingRequestSpanHook?: (span: Span, request: http.IncomingMessage, response: http.ServerResponse) => void;
+
   /**
    * Controls the maximum size of incoming HTTP request bodies attached to events.
    *
@@ -90,6 +115,19 @@ export type SentryHttpInstrumentationOptions = InstrumentationConfig & {
    */
   trackIncomingRequestsAsSessions?: boolean;
 
+  /**
+   * @deprecated This is deprecated in favor of `incomingRequestSpanHook`.
+   */
+  instrumentation?: {
+    requestHook?: (span: Span, req: http.ClientRequest | http.IncomingMessage) => void;
+    responseHook?: (span: Span, response: http.IncomingMessage | http.ServerResponse) => void;
+    applyCustomAttributesOnSpan?: (
+      span: Span,
+      request: http.ClientRequest | http.IncomingMessage,
+      response: http.IncomingMessage | http.ServerResponse,
+    ) => void;
+  };
+
   /**
    * Number of milliseconds until sessions tracked with `trackIncomingRequestsAsSessions` will be flushed as a session aggregate.
    *
@@ -131,10 +169,15 @@ export class SentryHttpInstrumentation extends InstrumentationBase<SentryHttpIns
     const onHttpServerRequestStart = ((_data: unknown) => {
       const data = _data as { server: http.Server };
       instrumentServer(data.server, {
+        // eslint-disable-next-line deprecation/deprecation
+        instrumentation: this.getConfig().instrumentation,
         ignoreIncomingRequestBody: this.getConfig().ignoreIncomingRequestBody,
+        ignoreSpansForIncomingRequests: this.getConfig().ignoreSpansForIncomingRequests,
+        incomingRequestSpanHook: this.getConfig().incomingRequestSpanHook,
         maxIncomingRequestBodySize: this.getConfig().maxIncomingRequestBodySize,
         trackIncomingRequestsAsSessions: this.getConfig().trackIncomingRequestsAsSessions,
         sessionFlushingDelayMS: this.getConfig().sessionFlushingDelayMS ?? 60_000,
+        spans: this.getConfig().spans ?? true,
       });
     }) satisfies ChannelListener;