feat(node): Update httpIntegration handling of incoming requests

Author: mydea

.size-limit.js

@@ -224,7 +224,7 @@ module.exports = [
     import: createImport('init'),
     ignore: [...builtinModules, ...nodePrefixedBuiltinModules],
     gzip: true,
-    limit: '116 KB',
+    limit: '51 KB',
   },
   // Node SDK (ESM)
   {
@@ -233,14 +233,14 @@ module.exports = [
     import: createImport('init'),
     ignore: [...builtinModules, ...nodePrefixedBuiltinModules],
     gzip: true,
-    limit: '147 KB',
+    limit: '150 KB',
   },
   {
     name: '@sentry/node - without tracing',
     path: 'packages/node/build/esm/index.js',
     import: createImport('initWithoutDefaultIntegrations', 'getDefaultIntegrationsWithoutPerformance'),
     gzip: true,
-    limit: '110 KB',
+    limit: '95 KB',
     ignore: [...builtinModules, ...nodePrefixedBuiltinModules],
     modifyWebpackConfig: function (config) {
       const webpack = require('webpack');
@@ -263,7 +263,7 @@ module.exports = [
     import: createImport('init'),
     ignore: [...builtinModules, ...nodePrefixedBuiltinModules],
     gzip: true,
-    limit: '135 KB',
+    limit: '105 KB',
   },
 ];
 

CHANGELOG.md

@@ -4,8 +4,6 @@
 
 - "You miss 100 percent of the chances you don't take. — Wayne Gretzky" — Michael Scott
 
-## Unreleased
-
 ### Important Changes
 
 - **fix(browser): Ensure IP address is only inferred by Relay if `sendDefaultPii` is `true`**
@@ -22,6 +20,13 @@ user IP addresses, if you set `sendDefaultPii: true` in your `Sentry.init` optio
 
 We apologize for any inconvenience caused!
 
+- **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-options.mjs

@@ -4,13 +4,20 @@ import { loggingTransport } from '@sentry-internal/node-integration-tests';
 Sentry.init({
   dsn: 'https://[email protected]/1337',
   release: '1.0',
-  // disable attaching headers to /test/* endpoints
-  tracePropagationTargets: [/^(?!.*test).*$/],
   tracesSampleRate: 1.0,
   transport: loggingTransport,
 
   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

@@ -50,6 +50,38 @@ describe('httpIntegration', () => {
         runner.makeRequest('get', '/test');
         await runner.completed();
       });
+
+      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',
+                },
+              },
+              extra: expect.objectContaining({
+                incomingRequestSpanHookCalled: {
+                  reqUrl: expect.stringMatching(/\/test$/),
+                  reqMethod: 'GET',
+                  resUrl: expect.stringMatching(/\/test$/),
+                  resMethod: 'GET',
+                },
+              }),
+            },
+          })
+          .start();
+        runner.makeRequest('get', '/test');
+        await runner.completed();
+      });
     });
   });
 
@@ -138,30 +170,6 @@ describe('httpIntegration', () => {
     });
   });
 
-  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',
-              },
-              op: 'http.server',
-              status: 'ok',
-            },
-          },
-        },
-      })
-      .start();
-    runner.makeRequest('get', '/test');
-    await runner.completed();
-  });
-
   describe("doesn't create a root span for incoming requests ignored via `ignoreIncomingRequests`", () => {
     test('via the url param', async () => {
       const runner = createRunner(__dirname, 'server-ignoreIncomingRequests.js')

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;
 
@@ -46,6 +57,20 @@ export type SentryHttpInstrumentationOptions = InstrumentationConfig & {
    */
   propagateTraceInOutgoingRequests?: boolean;
 
+  /**
+   * Whether to automatically ignore common static asset requests like favicon.ico, robots.txt, etc.
+   * This helps reduce noise in your transactions.
+   *
+   * @default `true`
+   */
+  ignoreStaticAssets?: 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.
+   */
+  disableIncomingRequestSpans?: boolean;
+
   /**
    * Do not capture breadcrumbs for outgoing HTTP requests to URLs where the given callback returns `true`.
    * For the scope of this instrumentation, this callback only controls breadcrumb creation.
@@ -57,6 +82,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 +99,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 +129,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.
    *
@@ -128,13 +180,21 @@ export class SentryHttpInstrumentation extends InstrumentationBase<SentryHttpIns
     // but we only want to register them once, whichever is loaded first
     let hasRegisteredHandlers = false;
 
+    const spansEnabled = this.getConfig().spans ?? true;
+
     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,
+        ignoreStaticAssets: this.getConfig().ignoreStaticAssets,
+        spans: spansEnabled && !this.getConfig().disableIncomingRequestSpans,
       });
     }) satisfies ChannelListener;