feat(node): Support Express v5 (#15380)

The goal is for Express to eventually support publishing events to
`diagnostics_channel` (#15107) so that Open Telemetry can instrument it
without monkey-patching internal code. However, this might take a while
and it would be great to support Express v5 now. This PR is a stop-gap
solution until that work is complete and published.

This PR vendors the code added in my otel PR: 
- open-telemetry/opentelemetry-js-contrib#2437 
- Adds a new instrumentation specifically for hooking express v5
- Copies the Express v4 integration tests to test v5
- The only changes in the tests is the removal of a couple of complex
regex tests where the regexes are no longer supported by Express.
- Modifies the NestJs v11 tests which now support full Express spans

Author: timfish

dev-packages/e2e-tests/test-applications/nestjs-11/tests/errors.test.ts

@@ -50,13 +50,11 @@ test('Does not send HttpExceptions to Sentry', async ({ baseURL }) => {
   });
 
   const transactionEventPromise400 = waitForTransaction('nestjs-11', transactionEvent => {
-    // todo(express-5): parametrize /test-expected-400-exception/:id
-    return transactionEvent?.transaction === 'GET /test-expected-400-exception/123';
+    return transactionEvent?.transaction === 'GET /test-expected-400-exception/:id';
   });
 
   const transactionEventPromise500 = waitForTransaction('nestjs-11', transactionEvent => {
-    // todo(express-5): parametrize /test-expected-500-exception/:id
-    return transactionEvent?.transaction === 'GET /test-expected-500-exception/123';
+    return transactionEvent?.transaction === 'GET /test-expected-500-exception/:id';
   });
 
   const response400 = await fetch(`${baseURL}/test-expected-400-exception/123`);
@@ -81,13 +79,11 @@ test('Does not send RpcExceptions to Sentry', async ({ baseURL }) => {
       errorEventOccurred = true;
     }
 
-    // todo(express-5): parametrize /test-expected-rpc-exception/:id
-    return event?.transaction === 'GET /test-expected-rpc-exception/123';
+    return event?.transaction === 'GET /test-expected-rpc-exception/:id';
   });
 
   const transactionEventPromise = waitForTransaction('nestjs-11', transactionEvent => {
-    // todo(express-5): parametrize /test-expected-rpc-exception/:id
-    return transactionEvent?.transaction === 'GET /test-expected-rpc-exception/123';
+    return transactionEvent?.transaction === 'GET /test-expected-rpc-exception/:id';
   });
 
   const response = await fetch(`${baseURL}/test-expected-rpc-exception/123`);

dev-packages/e2e-tests/test-applications/nestjs-11/tests/transactions.test.ts

@@ -15,7 +15,7 @@ test('Sends an API route transaction', async ({ baseURL }) => {
 
   expect(transactionEvent.contexts?.trace).toEqual({
     data: {
-      'sentry.source': 'url', // todo(express-5): 'route'
+      'sentry.source': 'route',
       'sentry.origin': 'auto.http.otel.http',
       'sentry.op': 'http.server',
       'sentry.sample_rate': 1,
@@ -37,7 +37,7 @@ test('Sends an API route transaction', async ({ baseURL }) => {
       'net.peer.port': expect.any(Number),
       'http.status_code': 200,
       'http.status_text': 'OK',
-      // 'http.route': '/test-transaction', // todo(express-5): add this line again
+      'http.route': '/test-transaction',
     },
     op: 'http.server',
     span_id: expect.stringMatching(/[a-f0-9]{16}/),
@@ -49,7 +49,6 @@ test('Sends an API route transaction', async ({ baseURL }) => {
   expect(transactionEvent).toEqual(
     expect.objectContaining({
       spans: expect.arrayContaining([
-        /* todo(express-5): add this part again
         {
           data: {
             'express.name': '/test-transaction',
@@ -67,7 +66,7 @@ test('Sends an API route transaction', async ({ baseURL }) => {
           timestamp: expect.any(Number),
           trace_id: expect.stringMatching(/[a-f0-9]{32}/),
           origin: 'auto.http.otel.express',
-        }, */
+        },
         {
           data: {
             'sentry.origin': 'manual',
@@ -117,7 +116,7 @@ test('Sends an API route transaction', async ({ baseURL }) => {
       transaction: 'GET /test-transaction',
       type: 'transaction',
       transaction_info: {
-        source: 'url', // todo(express-5): 'route'
+        source: 'route',
       },
     }),
   );
@@ -272,8 +271,7 @@ test('API route transaction includes nest pipe span for valid request', async ({
   const transactionEventPromise = waitForTransaction('nestjs-11', transactionEvent => {
     return (
       transactionEvent?.contexts?.trace?.op === 'http.server' &&
-      // todo(express-5): parametrize test-pipe-instrumentation/:id
-      transactionEvent?.transaction === 'GET /test-pipe-instrumentation/123' &&
+      transactionEvent?.transaction === 'GET /test-pipe-instrumentation/:id' &&
       transactionEvent?.request?.url?.includes('/test-pipe-instrumentation/123')
     );
   });
@@ -310,8 +308,7 @@ test('API route transaction includes nest pipe span for invalid request', async
   const transactionEventPromise = waitForTransaction('nestjs-11', transactionEvent => {
     return (
       transactionEvent?.contexts?.trace?.op === 'http.server' &&
-      // todo(express-5): parametrize test-pipe-instrumentation/:id
-      transactionEvent?.transaction === 'GET /test-pipe-instrumentation/abc' &&
+      transactionEvent?.transaction === 'GET /test-pipe-instrumentation/:id' &&
       transactionEvent?.request?.url?.includes('/test-pipe-instrumentation/abc')
     );
   });

dev-packages/node-integration-tests/package.json

@@ -16,9 +16,11 @@
     "build:types": "tsc -p tsconfig.types.json",
     "clean": "rimraf -g **/node_modules && run-p clean:script",
     "clean:script": "node scripts/clean.js",
+    "express-v5-install": "cd suites/express-v5 && yarn --no-lockfile",
     "lint": "eslint . --format stylish",
     "fix": "eslint . --format stylish --fix",
     "type-check": "tsc",
+    "pretest": "yarn express-v5-install",
     "test": "jest --config ./jest.config.js",
     "test:no-prisma": "jest --config ./jest.config.js",
     "test:watch": "yarn test --watch"

dev-packages/node-integration-tests/suites/express-v5/handle-error-scope-data-loss/server.ts

@@ -0,0 +1,31 @@
+import { loggingTransport } from '@sentry-internal/node-integration-tests';
+import * as Sentry from '@sentry/node';
+
+Sentry.init({
+  dsn: 'https://[email protected]/1337',
+  release: '1.0',
+  transport: loggingTransport,
+});
+
+import { startExpressServerAndSendPortToRunner } from '@sentry-internal/node-integration-tests';
+import express from 'express';
+
+const app = express();
+
+Sentry.setTag('global', 'tag');
+
+app.get('/test/withScope', () => {
+  Sentry.withScope(scope => {
+    scope.setTag('local', 'tag');
+    throw new Error('test_error');
+  });
+});
+
+app.get('/test/isolationScope', () => {
+  Sentry.getIsolationScope().setTag('isolation-scope', 'tag');
+  throw new Error('isolation_test_error');
+});
+
+Sentry.setupExpressErrorHandler(app);
+
+startExpressServerAndSendPortToRunner(app);

dev-packages/node-integration-tests/suites/express-v5/handle-error-scope-data-loss/test.ts

@@ -0,0 +1,85 @@
+import { cleanupChildProcesses, createRunner } from '../../../utils/runner';
+
+afterAll(() => {
+  cleanupChildProcesses();
+});
+
+/**
+ * Why does this test exist?
+ *
+ * We recently discovered that errors caught by global handlers will potentially loose scope data from the active scope
+ * where the error was originally thrown in. The simple example in this test (see subject.ts) demonstrates this behavior
+ * (in a Node environment but the same behavior applies to the browser; see the test there).
+ *
+ * This test nevertheless covers the behavior so that we're aware.
+ */
+test('withScope scope is NOT applied to thrown error caught by global handler', done => {
+  createRunner(__dirname, 'server.ts')
+    .expect({
+      event: {
+        exception: {
+          values: [
+            {
+              mechanism: {
+                type: 'middleware',
+                handled: false,
+              },
+              type: 'Error',
+              value: 'test_error',
+              stacktrace: {
+                frames: expect.arrayContaining([
+                  expect.objectContaining({
+                    function: expect.any(String),
+                    lineno: expect.any(Number),
+                    colno: expect.any(Number),
+                  }),
+                ]),
+              },
+            },
+          ],
+        },
+        // 'local' tag is not applied to the event
+        tags: expect.not.objectContaining({ local: expect.anything() }),
+      },
+    })
+    .start(done)
+    .makeRequest('get', '/test/withScope', { expectError: true });
+});
+
+/**
+ * This test shows that the isolation scope set tags are applied correctly to the error.
+ */
+test('isolation scope is applied to thrown error caught by global handler', done => {
+  createRunner(__dirname, 'server.ts')
+    .expect({
+      event: {
+        exception: {
+          values: [
+            {
+              mechanism: {
+                type: 'middleware',
+                handled: false,
+              },
+              type: 'Error',
+              value: 'isolation_test_error',
+              stacktrace: {
+                frames: expect.arrayContaining([
+                  expect.objectContaining({
+                    function: expect.any(String),
+                    lineno: expect.any(Number),
+                    colno: expect.any(Number),
+                  }),
+                ]),
+              },
+            },
+          ],
+        },
+        tags: {
+          global: 'tag',
+          'isolation-scope': 'tag',
+        },
+      },
+    })
+    .start(done)
+    .makeRequest('get', '/test/isolationScope', { expectError: true });
+});

dev-packages/node-integration-tests/suites/express-v5/handle-error-tracesSampleRate-0/server.ts

@@ -0,0 +1,22 @@
+import { loggingTransport } from '@sentry-internal/node-integration-tests';
+import * as Sentry from '@sentry/node';
+
+Sentry.init({
+  dsn: 'https://[email protected]/1337',
+  release: '1.0',
+  transport: loggingTransport,
+  tracesSampleRate: 1,
+});
+
+import { startExpressServerAndSendPortToRunner } from '@sentry-internal/node-integration-tests';
+import express from 'express';
+
+const app = express();
+
+app.get('/test/express/:id', req => {
+  throw new Error(`test_error with id ${req.params.id}`);
+});
+
+Sentry.setupExpressErrorHandler(app);
+
+startExpressServerAndSendPortToRunner(app);

dev-packages/node-integration-tests/suites/express-v5/handle-error-tracesSampleRate-0/test.ts

@@ -0,0 +1,38 @@
+import { cleanupChildProcesses, createRunner } from '../../../utils/runner';
+
+afterAll(() => {
+  cleanupChildProcesses();
+});
+
+test('should capture and send Express controller error with txn name if tracesSampleRate is 0', done => {
+  createRunner(__dirname, 'server.ts')
+    .ignore('transaction')
+    .expect({
+      event: {
+        exception: {
+          values: [
+            {
+              mechanism: {
+                type: 'middleware',
+                handled: false,
+              },
+              type: 'Error',
+              value: 'test_error with id 123',
+              stacktrace: {
+                frames: expect.arrayContaining([
+                  expect.objectContaining({
+                    function: expect.any(String),
+                    lineno: expect.any(Number),
+                    colno: expect.any(Number),
+                  }),
+                ]),
+              },
+            },
+          ],
+        },
+        transaction: 'GET /test/express/:id',
+      },
+    })
+    .start(done)
+    .makeRequest('get', '/test/express/123', { expectError: true });
+});

dev-packages/node-integration-tests/suites/express-v5/handle-error-tracesSampleRate-unset/server.ts

@@ -0,0 +1,21 @@
+import { loggingTransport } from '@sentry-internal/node-integration-tests';
+import * as Sentry from '@sentry/node';
+
+Sentry.init({
+  dsn: 'https://[email protected]/1337',
+  release: '1.0',
+  transport: loggingTransport,
+});
+
+import { startExpressServerAndSendPortToRunner } from '@sentry-internal/node-integration-tests';
+import express from 'express';
+
+const app = express();
+
+app.get('/test/express/:id', req => {
+  throw new Error(`test_error with id ${req.params.id}`);
+});
+
+Sentry.setupExpressErrorHandler(app);
+
+startExpressServerAndSendPortToRunner(app);

dev-packages/node-integration-tests/suites/express-v5/handle-error-tracesSampleRate-unset/test.ts

@@ -0,0 +1,37 @@
+import { cleanupChildProcesses, createRunner } from '../../../utils/runner';
+
+afterAll(() => {
+  cleanupChildProcesses();
+});
+
+test('should capture and send Express controller error if tracesSampleRate is not set.', done => {
+  createRunner(__dirname, 'server.ts')
+    .ignore('transaction')
+    .expect({
+      event: {
+        exception: {
+          values: [
+            {
+              mechanism: {
+                type: 'middleware',
+                handled: false,
+              },
+              type: 'Error',
+              value: 'test_error with id 123',
+              stacktrace: {
+                frames: expect.arrayContaining([
+                  expect.objectContaining({
+                    function: expect.any(String),
+                    lineno: expect.any(Number),
+                    colno: expect.any(Number),
+                  }),
+                ]),
+              },
+            },
+          ],
+        },
+      },
+    })
+    .start(done)
+    .makeRequest('get', '/test/express/123', { expectError: true });
+});