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

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

@@ -69,11 +69,12 @@ 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 const patchExpressModule = (moduleExports: ExpressModuleExport, getOptions: () => ExpressIntegrationOptions) => {
   // 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 +94,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 +114,7 @@ export const patchExpressModule = (options: ExpressIntegrationOptions) => {
         if (!layer) {
           return route;
         }
-        patchLayer(options, layer, getLayerPath(args));
+        patchLayer(getOptions, layer, getLayerPath(args));
         return route;
       },
     );
@@ -141,7 +142,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;

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,6 @@ export type ExpressRouter = {
 export type IgnoreMatcher = string | RegExp | ((name: string) => boolean);
 
 export type ExpressIntegrationOptions = {
-  express: ExpressModuleExport; //Express
   /** Ignore specific based on their name */
   ignoreLayers?: IgnoreMatcher[];
   /** Ignore specific layers based on their type */

packages/core/test/lib/integrations/express/index.test.ts

@@ -61,12 +61,12 @@ vi.mock('../../../../src/utils/debug-logger', () => ({
 }));
 
 beforeEach(() => (patchLayerCalls.length = 0));
-const patchLayerCalls: [options: ExpressIntegrationOptions, layer: ExpressLayer, layerPath?: string][] = [];
+const patchLayerCalls: [getOptions: () => ExpressIntegrationOptions, layer: ExpressLayer, layerPath?: string][] = [];
 
 vi.mock('../../../../src/integrations/express/patch-layer', () => ({
-  patchLayer: (options: ExpressIntegrationOptions, layer?: ExpressLayer, layerPath?: string) => {
+  patchLayer: (getOptions: () => ExpressIntegrationOptions, layer?: ExpressLayer, layerPath?: string) => {
     if (layer) {
-      patchLayerCalls.push([options, layer, layerPath]);
+      patchLayerCalls.push([getOptions, layer, layerPath]);
     }
   },
 }));
@@ -131,24 +131,22 @@ function getExpress5(): ExpressExportv5 & { spies: ExpressSpies } {
 describe('patchExpressModule', () => {
   it('throws trying to patch/unpatch the wrong thing', () => {
     expect(() => {
-      patchExpressModule({
-        express: {} as unknown as ExpressModuleExport,
-      } as unknown as ExpressIntegrationOptions);
+      patchExpressModule({} as unknown as ExpressModuleExport, () => ({}));
     }).toThrowError('no valid Express route function to instrument');
   });
 
   it('can patch and restore expressv4 style module', () => {
     for (const useDefault of [false, true]) {
       const express = getExpress4();
-      const module = useDefault ? { default: express } : express;
+      const moduleExports = useDefault ? { default: express } : express;
       const r = express.Router as ExpressRouterv4;
       const a = express.application;
-      const options = { express: module } as unknown as ExpressIntegrationOptions;
+      const getOptions = () => ({});
       expect((r.use as WrappedFunction).__sentry_original__).toBe(undefined);
       expect((r.route as WrappedFunction).__sentry_original__).toBe(undefined);
       expect((a.use as WrappedFunction).__sentry_original__).toBe(undefined);
 
-      patchExpressModule(options);
+      patchExpressModule(moduleExports, getOptions);
 
       expect(typeof (r.use as WrappedFunction).__sentry_original__).toBe('function');
       expect(typeof (r.route as WrappedFunction).__sentry_original__).toBe('function');
@@ -161,13 +159,13 @@ describe('patchExpressModule', () => {
       const express = getExpress5();
       const r = express.Router as ExpressRouterv5;
       const a = express.application;
-      const module = useDefault ? { default: express } : express;
-      const options = { express: module } as unknown as ExpressIntegrationOptions;
+      const moduleExports = useDefault ? { default: express } : express;
+      const getOptions = () => ({});
       expect((r.prototype.use as WrappedFunction).__sentry_original__).toBe(undefined);
       expect((r.prototype.route as WrappedFunction).__sentry_original__).toBe(undefined);
       expect((a.use as WrappedFunction).__sentry_original__).toBe(undefined);
 
-      patchExpressModule(options);
+      patchExpressModule(moduleExports, getOptions);
 
       expect(typeof (r.prototype.use as WrappedFunction).__sentry_original__).toBe('function');
       expect(typeof (r.prototype.route as WrappedFunction).__sentry_original__).toBe('function');
@@ -178,27 +176,27 @@ describe('patchExpressModule', () => {
   it('calls patched and original Router.route', () => {
     const expressv4 = getExpress4();
     const { spies } = expressv4;
-    const options = { express: expressv4 };
-    patchExpressModule(options);
+    const getOptions = () => ({});
+    patchExpressModule(expressv4, getOptions);
     expressv4.Router.route('a');
     expect(spies.routerRoute).toHaveBeenCalledExactlyOnceWith('a');
   });
 
   it('calls patched and original Router.use', () => {
     const expressv4 = getExpress4();
     const { spies } = expressv4;
-    const options = { express: expressv4 };
-    patchExpressModule(options);
+    const getOptions = () => ({});
+    patchExpressModule(expressv4, getOptions);
     expressv4.Router.use('a');
-    expect(patchLayerCalls).toStrictEqual([[options, { name: 'layerFinal' }, 'a']]);
+    expect(patchLayerCalls).toStrictEqual([[getOptions, { name: 'layerFinal' }, 'a']]);
     expect(spies.routerUse).toHaveBeenCalledExactlyOnceWith('a');
   });
 
   it('skips patchLayer call in Router.use if no layer in the stack', () => {
     const expressv4 = getExpress4();
     const { spies } = expressv4;
-    const options = { express: expressv4 };
-    patchExpressModule(options);
+    const getOptions = () => ({});
+    patchExpressModule(expressv4, getOptions);
     const { stack } = expressv4.Router;
     expressv4.Router.stack = [];
     expressv4.Router.use('a');
@@ -210,28 +208,28 @@ describe('patchExpressModule', () => {
   it('calls patched and original application.use', () => {
     const expressv4 = getExpress4();
     const { spies } = expressv4;
-    const options = { express: expressv4 };
-    patchExpressModule(options);
+    const getOptions = () => ({});
+    patchExpressModule(expressv4, getOptions);
     expressv4.application.use('a');
-    expect(patchLayerCalls).toStrictEqual([[options, { name: 'layerFinal' }, 'a']]);
+    expect(patchLayerCalls).toStrictEqual([[getOptions, { name: 'layerFinal' }, 'a']]);
     expect(spies.appUse).toHaveBeenCalledExactlyOnceWith('a');
   });
 
   it('calls patched and original application.use on express v5', () => {
     const expressv5 = getExpress5();
     const { spies } = expressv5;
-    const options = { express: expressv5 };
-    patchExpressModule(options);
+    const getOptions = () => ({});
+    patchExpressModule(expressv5, getOptions);
     expressv5.application.use('a');
-    expect(patchLayerCalls).toStrictEqual([[options, { name: 'layerFinal' }, 'a']]);
+    expect(patchLayerCalls).toStrictEqual([[getOptions, { name: 'layerFinal' }, 'a']]);
     expect(spies.appUse).toHaveBeenCalledExactlyOnceWith('a');
   });
 
   it('skips patchLayer on application.use if no router found', () => {
     const expressv4 = getExpress4();
     const { spies } = expressv4;
-    const options = { express: expressv4 };
-    patchExpressModule(options);
+    const getOptions = () => ({});
+    patchExpressModule(expressv4, getOptions);
     const app = expressv4.application as {
       _router?: ExpressRoute;
     };
@@ -246,8 +244,9 @@ describe('patchExpressModule', () => {
 
   it('debug error when patching fails', () => {
     const expressv5 = getExpress5();
-    patchExpressModule({ express: expressv5 });
-    patchExpressModule({ express: expressv5 });
+    const getOptions = () => ({});
+    patchExpressModule(expressv5, getOptions);
+    patchExpressModule(expressv5, getOptions);
     expect(debugErrors).toStrictEqual([
       ['Failed to patch express route method:', new Error('Attempting to wrap method route multiple times')],
       ['Failed to patch express use method:', new Error('Attempting to wrap method use multiple times')],