fix(core, node): support loading Express options lazily (#20211)

Update the Express integration to accept the module export and a
configuration function, rather than a configuration object. This is
needed to support lazily calling Sentry.init *after* the module has been
instrumented, without re-wrapping the methods to get the new config.

via: @mydea in #20188

Author: isaacs

dev-packages/node-integration-tests/suites/express/late-init/instrument.mjs

@@ -0,0 +1,19 @@
+import * as Sentry from '@sentry/node';
+import { loggingTransport } from '@sentry-internal/node-integration-tests';
+
+// First: preload the express instrumentation without calling Sentry.init().
+// registers OTel module hook, patches the Express module with no config.
+Sentry.preloadOpenTelemetry({ integrations: ['Express'] });
+
+// call Sentry.init() with express integration config.
+// instrumentExpress is already registered, so this calls setConfig() on the
+// existing instrumentation to update its options. The lazy getOptions()
+// in patchLayer ensures the updated options are read at request time.
+Sentry.init({
+  dsn: 'https://public@dsn.ingest.sentry.io/1337',
+  release: '1.0',
+  tracesSampleRate: 1.0,
+  transport: loggingTransport,
+  // suppress the middleware layer that the cors module generates
+  integrations: [Sentry.expressIntegration({ ignoreLayersType: ['middleware'] })],
+});

dev-packages/node-integration-tests/suites/express/late-init/scenario.mjs

@@ -0,0 +1,18 @@
+import cors from 'cors';
+import express from 'express';
+import * as Sentry from '@sentry/node';
+import { startExpressServerAndSendPortToRunner } from '@sentry-internal/node-integration-tests';
+
+const app = express();
+
+// cors() would normally create a 'middleware' type span, but the
+// ignoreLayersType: ['middleware'] option set via Sentry.init() suppresses it.
+app.use(cors());
+
+app.get('/test/express', (_req, res) => {
+  res.send({ response: 'response 1' });
+});
+
+Sentry.setupExpressErrorHandler(app);
+
+startExpressServerAndSendPortToRunner(app);

dev-packages/node-integration-tests/suites/express/late-init/test.ts

@@ -0,0 +1,49 @@
+import { afterAll, describe, expect } from 'vitest';
+import { assertSentryTransaction } from '../../../utils/assertions';
+import { cleanupChildProcesses, createEsmAndCjsTests } from '../../../utils/runner';
+
+describe('express late init', () => {
+  afterAll(() => {
+    cleanupChildProcesses();
+  });
+
+  createEsmAndCjsTests(__dirname, 'scenario.mjs', 'instrument.mjs', (createRunner, test) => {
+    test('applies expressIntegration config set via Sentry.init() called after instrumentExpress()', async () => {
+      const runner = createRunner()
+        .expect({
+          transaction: transaction => {
+            assertSentryTransaction(transaction, {
+              transaction: 'GET /test/express',
+              contexts: {
+                trace: {
+                  op: 'http.server',
+                  status: 'ok',
+                },
+              },
+            });
+            // request_handler span IS present
+            // confirms the express patch was applied.
+            expect(transaction.spans).toContainEqual(
+              expect.objectContaining({
+                data: expect.objectContaining({
+                  'express.type': 'request_handler',
+                }),
+              }),
+            );
+            // Middleware spans NOT present, ignoreLayersType: ['middleware']
+            // configured via the Sentry.init() AFTER instrumentExpress().
+            expect(transaction.spans).not.toContainEqual(
+              expect.objectContaining({
+                data: expect.objectContaining({
+                  'express.type': 'middleware',
+                }),
+              }),
+            );
+          },
+        })
+        .start();
+      runner.makeRequest('get', '/test/express');
+      await runner.completed();
+    });
+  });
+});

packages/core/src/integrations/express/index.ts

@@ -60,6 +60,24 @@ import { setSDKProcessingMetadata } from './set-sdk-processing-metadata';
 const getExpressExport = (express: ExpressModuleExport): ExpressExport =>
   hasDefaultProp(express) ? express.default : (express as ExpressExport);
 
+function isLegacyOptions(
+  options: ExpressModuleExport | (ExpressIntegrationOptions & { express: ExpressModuleExport }),
+): options is ExpressIntegrationOptions & { express: ExpressModuleExport } {
+  return !!(options as { express: ExpressModuleExport }).express;
+}
+
+// TODO: remove this deprecation handling in v11
+let didLegacyDeprecationWarning = false;
+function deprecationWarning() {
+  if (!didLegacyDeprecationWarning) {
+    didLegacyDeprecationWarning = true;
+    DEBUG_BUILD &&
+      debug.warn(
+        '[Express] `patchExpressModule(options)` is deprecated. Use `patchExpressModule(moduleExports, getOptions)` instead.',
+      );
+  }
+}
+
 /**
  * This is a portable instrumentatiton function that works in any environment
  * where Express can be loaded, without depending on OpenTelemetry.
@@ -69,11 +87,39 @@ const getExpressExport = (express: ExpressModuleExport): ExpressExport =>
  * import express from 'express';
  * import * as Sentry from '@sentry/deno'; // or any SDK that extends core
  *
- * Sentry.patchExpressModule({ express })
+ * Sentry.patchExpressModule(express, () => ({}));
+ * ```
  */
-export const patchExpressModule = (options: ExpressIntegrationOptions) => {
+export function patchExpressModule(
+  moduleExports: ExpressModuleExport,
+  getOptions: () => ExpressIntegrationOptions,
+): ExpressModuleExport;
+/**
+ * @deprecated Pass the Express module export as the first argument and options getter as the second argument.
+ */
+export function patchExpressModule(
+  options: ExpressIntegrationOptions & { express: ExpressModuleExport },
+): ExpressModuleExport;
+export function patchExpressModule(
+  optionsOrExports: ExpressModuleExport | (ExpressIntegrationOptions & { express: ExpressModuleExport }),
+  maybeGetOptions?: () => ExpressIntegrationOptions,
+): ExpressModuleExport {
+  let getOptions: () => ExpressIntegrationOptions;
+  let moduleExports: ExpressModuleExport;
+  if (!maybeGetOptions && isLegacyOptions(optionsOrExports)) {
+    const { express, ...options } = optionsOrExports;
+    moduleExports = express;
+    getOptions = () => options;
+    deprecationWarning();
+  } else if (typeof maybeGetOptions !== 'function') {
+    throw new TypeError('`patchExpressModule(moduleExports, getOptions)` requires a `getOptions` callback');
+  } else {
+    getOptions = maybeGetOptions;
+    moduleExports = optionsOrExports as ExpressModuleExport;
+  }
+
   // pass in the require() or import() result of express
-  const express = getExpressExport(options.express);
+  const express = getExpressExport(moduleExports);
   const routerProto: ExpressRouterv4 | ExpressRouterv5 | undefined = isExpressWithRouterPrototype(express)
     ? express.Router.prototype // Express v5
     : isExpressWithoutRouterPrototype(express)
@@ -93,7 +139,7 @@ export const patchExpressModule = (options: ExpressIntegrationOptions) => {
       function routeTrace(this: ExpressRouter, ...args: Parameters<typeof originalRouteMethod>[]) {
         const route = originalRouteMethod.apply(this, args);
         const layer = this.stack[this.stack.length - 1] as ExpressLayer;
-        patchLayer(options, layer, getLayerPath(args));
+        patchLayer(getOptions, layer, getLayerPath(args));
         return route;
       },
     );
@@ -113,7 +159,7 @@ export const patchExpressModule = (options: ExpressIntegrationOptions) => {
         if (!layer) {
           return route;
         }
-        patchLayer(options, layer, getLayerPath(args));
+        patchLayer(getOptions, layer, getLayerPath(args));
         return route;
       },
     );
@@ -141,7 +187,7 @@ export const patchExpressModule = (options: ExpressIntegrationOptions) => {
         if (router) {
           const layer = router.stack[router.stack.length - 1];
           if (layer) {
-            patchLayer(options, layer, getLayerPath(args));
+            patchLayer(getOptions, layer, getLayerPath(args));
           }
         }
         return route;
@@ -152,7 +198,7 @@ export const patchExpressModule = (options: ExpressIntegrationOptions) => {
   }
 
   return express;
-};
+}
 
 /**
  * An Express-compatible error handler, used by setupExpressErrorHandler

packages/core/src/integrations/express/patch-layer.ts

@@ -61,7 +61,11 @@ export type ExpressPatchLayerOptions = Pick<
   'onRouteResolved' | 'ignoreLayers' | 'ignoreLayersType'
 >;
 
-export function patchLayer(options: ExpressPatchLayerOptions, maybeLayer?: ExpressLayer, layerPath?: string): void {
+export function patchLayer(
+  getOptions: () => ExpressPatchLayerOptions,
+  maybeLayer?: ExpressLayer,
+  layerPath?: string,
+): void {
   if (!maybeLayer?.handle) {
     return;
   }
@@ -86,6 +90,8 @@ export function patchLayer(options: ExpressPatchLayerOptions, maybeLayer?: Expre
     //oxlint-disable-next-line no-explicit-any
     ...otherArgs: any[]
   ) {
+    const options = getOptions();
+
     // Set normalizedRequest here because expressRequestHandler middleware
     // (registered via setupExpressErrorHandler) is added after routes and
     // therefore never runs for successful requests — route handlers typically

packages/core/src/integrations/express/types.ts

@@ -136,7 +136,12 @@ export type ExpressRouter = {
 export type IgnoreMatcher = string | RegExp | ((name: string) => boolean);
 
 export type ExpressIntegrationOptions = {
-  express: ExpressModuleExport; //Express
+  /**
+   * @deprecated Pass the express module as the first argument, and an
+   * options getter as the second argument to patchExpressModule.
+   */
+  express?: ExpressModuleExport;
+
   /** Ignore specific based on their name */
   ignoreLayers?: IgnoreMatcher[];
   /** Ignore specific layers based on their type */