use integration and avoid global module scope

Author: mydea

packages/node-core/src/integrations/http/httpServerIntegration.ts

@@ -31,7 +31,7 @@ interface ServerCallbackOptions {
   normalizedRequest: RequestEventData;
 }
 
-const clientToCallbackMap = new WeakMap<Client, ((fn: () => boolean, options: ServerCallbackOptions) => boolean)[]>();
+type ServerCallback = (fn: () => boolean, options: ServerCallbackOptions) => boolean;
 
 const clientToRequestSessionAggregatesMap = new Map<
   Client,
@@ -94,18 +94,22 @@ const _httpServerIntegration = ((options: HttpServerIntegrationOptions = {}) =>
     ignoreRequestBody: options.ignoreRequestBody,
   };
 
+  const serverCallbacks: ServerCallback[] = [];
+
   return {
     name: INTEGRATION_NAME,
     setupOnce() {
       const onHttpServerRequestStart = ((_data: unknown) => {
         const data = _data as { server: Server };
 
-        instrumentServer(data.server, _options);
+        instrumentServer(data.server, _options, serverCallbacks);
       }) satisfies ChannelListener;
 
       subscribe('http.server.request.start', onHttpServerRequestStart);
     },
-
+    addServerCallback(callback: ServerCallback) {
+      serverCallbacks.push(callback);
+    },
     afterAllSetup(client) {
       if (DEBUG_BUILD && client.getIntegrationByName('Http')) {
         debug.warn(
@@ -125,6 +129,7 @@ export const httpServerIntegration = _httpServerIntegration as (
 ) => Integration & {
   name: 'HttpServer';
   setupOnce: () => void;
+  addServerCallback: (callback: ServerCallback) => void;
 };
 
 /**
@@ -144,6 +149,7 @@ function instrumentServer(
     sessions: boolean;
     sessionFlushingDelayMS: number;
   },
+  serverCallbacks: ServerCallback[],
 ): void {
   // eslint-disable-next-line @typescript-eslint/unbound-method
   const originalEmit: ServerEmit = server.emit;
@@ -215,13 +221,11 @@ function instrumentServer(
           .setValue(HTTP_SERVER_INSTRUMENTED_KEY, true);
 
         return context.with(ctx, () => {
-          const callbacks = clientToCallbackMap.get(client);
-
-          if (callbacks?.length) {
+          if (serverCallbacks?.length) {
             return wrapInCallbacks(
               () => target.apply(thisArg, args),
               { request, response, normalizedRequest },
-              callbacks.slice(),
+              serverCallbacks.slice(),
             );
           } else {
             return target.apply(thisArg, args);
@@ -235,18 +239,6 @@ function instrumentServer(
   server.emit = newEmit;
 }
 
-/**
- * Register a client callback that will be called when a request is received.
- */
-export function registerServerCallback(
-  client: Client,
-  callback: (fn: () => boolean, options: ServerCallbackOptions) => boolean,
-): void {
-  const callbacks = clientToCallbackMap.get(client) || [];
-  callbacks.push(callback);
-  clientToCallbackMap.set(client, callbacks);
-}
-
 /**
  * Starts a session and tracks it in the context of a given isolation scope.
  * When the passed response is finished, the session is put into a task and is

packages/node-core/src/integrations/http/httpServerSpansIntegration.ts

@@ -33,7 +33,7 @@ import {
 } from '@sentry/core';
 import { DEBUG_BUILD } from '../../debug-build';
 import type { NodeClient } from '../../sdk/client';
-import { registerServerCallback } from './httpServerIntegration';
+import type { httpServerIntegration } from './httpServerIntegration';
 
 const INTEGRATION_NAME = 'Http.ServerSpans';
 
@@ -104,9 +104,36 @@ const _httpServerSpansIntegration = ((options: HttpServerSpansIntegrationOptions
   // eslint-disable-next-line deprecation/deprecation
   const { requestHook, responseHook, applyCustomAttributesOnSpan } = options.instrumentation ?? {};
 
+  // We track if setup() was called, which indicates that this integration was added directly, not called from httpIntegration
+  let _isSetupDirectly = false;
+
   return {
     name: INTEGRATION_NAME,
-    setup(client: NodeClient) {
+    setup() {
+      _isSetupDirectly = true;
+    },
+    processEvent(event) {
+      // Drop transaction if it has a status code that should be ignored
+      if (event.type === 'transaction') {
+        const statusCode = event.contexts?.trace?.data?.['http.response.status_code'];
+        if (typeof statusCode === 'number') {
+          const shouldDrop = shouldFilterStatusCode(statusCode, ignoreStatusCodes);
+          if (shouldDrop) {
+            DEBUG_BUILD && debug.log('Dropping transaction due to status code', statusCode);
+            return null;
+          }
+        }
+      }
+
+      return event;
+    },
+    afterAllSetup(client: NodeClient) {
+      if (DEBUG_BUILD && client.getIntegrationByName('Http') && _isSetupDirectly) {
+        debug.warn(
+          'It seems that you have manually added `httpServerSpansIntergation` while `httpIntegration` is also present. Make sure to remove `httpIntegration` when adding `httpServerSpansIntegration`.',
+        );
+      }
+
       // If no tracing, we can just skip everything here
       if (typeof __SENTRY_TRACING__ !== 'undefined' && !__SENTRY_TRACING__) {
         return;
@@ -222,28 +249,12 @@ const _httpServerSpansIntegration = ((options: HttpServerSpansIntegrationOptions
         });
       }
 
-      registerServerCallback(client, startSpan);
-    },
-    processEvent(event) {
-      // Drop transaction if it has a status code that should be ignored
-      if (event.type === 'transaction') {
-        const statusCode = event.contexts?.trace?.data?.['http.response.status_code'];
-        if (typeof statusCode === 'number') {
-          const shouldDrop = shouldFilterStatusCode(statusCode, ignoreStatusCodes);
-          if (shouldDrop) {
-            DEBUG_BUILD && debug.log('Dropping transaction due to status code', statusCode);
-            return null;
-          }
-        }
-      }
-
-      return event;
-    },
-    afterAllSetup(client) {
-      if (DEBUG_BUILD && client.getIntegrationByName('Http')) {
-        debug.warn(
-          'It seems that you have manually added `httpServerSpansIntergation` while `httpIntegration` is also present. Make sure to remove `httpIntegration` when adding `httpServerSpansIntegration`.',
-        );
+      const serverIntegration = client.getIntegrationByName<ReturnType<typeof httpServerIntegration>>('HttpServer');
+      if (serverIntegration) {
+        serverIntegration.addServerCallback(startSpan);
+      } else {
+        DEBUG_BUILD &&
+          debug.warn('httpServerSpansIntegration requires the httpServerIntegration to be present. Skipping...');
       }
     },
   };
@@ -257,7 +268,7 @@ export const httpServerSpansIntegration = _httpServerSpansIntegration as (
   options?: HttpServerSpansIntegrationOptions,
 ) => Integration & {
   name: 'HttpServerSpans';
-  setup: (client: NodeClient) => void;
+  afterAllSetup: (client: NodeClient) => void;
   processEvent: (event: Event) => Event | null;
 };
 

packages/node-core/src/integrations/http/index.ts

@@ -1,6 +1,5 @@
 import type { IncomingMessage, RequestOptions } from 'node:http';
-import { debug, defineIntegration } from '@sentry/core';
-import { DEBUG_BUILD } from '../../debug-build';
+import { defineIntegration } from '@sentry/core';
 import { generateInstrumentOnce } from '../../otel/instrument';
 import type { NodeClient } from '../../sdk/client';
 import type { HttpServerIntegrationOptions } from './httpServerIntegration';
@@ -157,18 +156,16 @@ export const httpIntegration = defineIntegration((options: HttpOptions = {}) =>
 
   return {
     name: INTEGRATION_NAME,
-
-    setup(client: NodeClient) {
-      if (enabledServerSpans) {
-        serverSpans.setup(client);
-      }
-    },
-
     setupOnce() {
       server.setupOnce();
 
       instrumentSentryHttp(httpInstrumentationOptions);
     },
+    afterAllSetup(client: NodeClient) {
+      if (enabledServerSpans) {
+        serverSpans.afterAllSetup(client);
+      }
+    },
     processEvent(event) {
       // Note: We always run this, even if spans are disabled
       // The reason being that e.g. the remix integration disables span creation here but still wants to use the ignore status codes option

packages/node/src/integrations/http.ts

@@ -226,15 +226,6 @@ export const httpIntegration = defineIntegration((options: HttpOptions = {}) =>
 
   return {
     name: INTEGRATION_NAME,
-
-    setup(client: NodeClient) {
-      const clientOptions = client.getOptions();
-
-      if (enableServerSpans && hasSpansEnabled(clientOptions)) {
-        serverSpans.setup(client);
-      }
-    },
-
     setupOnce() {
       const clientOptions = (getClient<NodeClient>()?.getOptions() || {}) satisfies Partial<NodeClientOptions>;
       const useOtelHttpInstrumentation = _shouldUseOtelHttpInstrumentation(options, clientOptions);
@@ -256,6 +247,13 @@ export const httpIntegration = defineIntegration((options: HttpOptions = {}) =>
         instrumentOtelHttp(instrumentationConfig);
       }
     },
+    afterAllSetup(client: NodeClient) {
+      const clientOptions = client.getOptions();
+
+      if (enableServerSpans && hasSpansEnabled(clientOptions)) {
+        serverSpans.afterAllSetup(client);
+      }
+    },
     processEvent(event) {
       // Note: We always run this, even if spans are disabled
       // The reason being that e.g. the remix integration disables span creation here but still wants to use the ignore status codes option