feat: Added instrumentation support for Express 5 beta (#2476)

This will be experimental until express@5.0.0 is generally available
newrelic · Aug 16, 2024 · 06a4c2f · 06a4c2f
1 parent c2b8879
commit 06a4c2f
Show file tree

Hide file tree

Showing 18 changed files with 1,425 additions and 1,520 deletions.
diff --git a/lib/instrumentation/express.js b/lib/instrumentation/express.js
@@ -6,7 +6,6 @@
 'use strict'
 
 const { MiddlewareSpec, MiddlewareMounterSpec, RenderSpec } = require('../../lib/shim/specs')
-const { MIDDLEWARE_TYPE_NAMES } = require('../../lib/shim/webframework-shim/common')
 
 /**
  * Express middleware generates traces where middleware are considered siblings
@@ -25,35 +24,30 @@ module.exports = function initialize(agent, express, moduleName, shim) {
     return err !== 'route' && err !== 'router'
   })
 
-  if (express.Router.use) {
-    wrapExpress4(shim, express)
-  } else {
-    wrapExpress3(shim, express)
-  }
-}
-
-function wrapExpress4(shim, express) {
-  // Wrap `use` and `route` which are hung off `Router` directly, not on a
-  // prototype.
   shim.wrapMiddlewareMounter(
-    express.Router,
+    express.application,
     'use',
     new MiddlewareMounterSpec({
       route: shim.FIRST,
       wrapper: wrapMiddleware
     })
   )
 
+  wrapExpressRouter(shim, express.Router.use ? express.Router : express.Router.prototype)
+  wrapResponse(shim, express.response)
+}
+
+function wrapExpressRouter(shim, router) {
   shim.wrapMiddlewareMounter(
-    express.application,
+    router,
     'use',
     new MiddlewareMounterSpec({
       route: shim.FIRST,
       wrapper: wrapMiddleware
     })
   )
 
-  shim.wrap(express.Router, 'route', function wrapRoute(shim, fn) {
+  shim.wrap(router, 'route', function wrapRoute(shim, fn) {
     if (!shim.isFunction(fn)) {
       return fn
     }
@@ -89,7 +83,7 @@ function wrapExpress4(shim, express) {
   })
 
   shim.wrapMiddlewareMounter(
-    express.Router,
+    router,
     'param',
     new MiddlewareMounterSpec({
       route: shim.FIRST,
@@ -105,56 +99,6 @@ function wrapExpress4(shim, express) {
       }
     })
   )
-
-  wrapResponse(shim, express.response)
-}
-
-function wrapExpress3(shim, express) {
-  // In Express 3 the app returned from `express()` is actually a `connect` app
-  // which we have no access to before creation. We can not easily wrap the app
-  // because there are a lot of methods dangling on it that act on the app itself.
-  // Really we just care about apps being used as `request` event listeners on
-  // `http.Server` instances so we'll wrap that instead.
-
-  shim.wrapMiddlewareMounter(
-    express.Router.prototype,
-    'param',
-    new MiddlewareMounterSpec({
-      route: shim.FIRST,
-      wrapper: function wrapParamware(shim, middleware, fnName, route) {
-        return shim.recordParamware(
-          middleware,
-          new MiddlewareSpec({
-            name: route,
-            req: shim.FIRST,
-            next: shim.THIRD,
-            type: MIDDLEWARE_TYPE_NAMES.PARAMWARE
-          })
-        )
-      }
-    })
-  )
-  shim.wrapMiddlewareMounter(
-    express.Router.prototype,
-    'use',
-    new MiddlewareMounterSpec({
-      route: shim.FIRST,
-      wrapper: wrapMiddleware
-    })
-  )
-  shim.wrapMiddlewareMounter(
-    express.application,
-    'use',
-    new MiddlewareMounterSpec({
-      route: shim.FIRST,
-      wrapper: wrapMiddleware
-    })
-  )
-
-  // NOTE: Do not wrap application route methods in Express 3, they all just
-  // forward their arguments to the router.
-  wrapRouteMethods(shim, express.Router.prototype, shim.FIRST)
-  wrapResponse(shim, express.response)
 }
 
 function wrapRouteMethods(shim, route, path) {

diff --git a/test/lib/metrics_helper.js b/test/lib/metrics_helper.js
@@ -48,7 +48,7 @@ function assertMetrics(metrics, expected, exclusive, assertValues) {
   for (let i = 0, len = expected.length; i < len; i++) {
     const expectedMetric = expected[i]
     const metric = metrics.getMetric(expectedMetric[0].name, expectedMetric[0].scope)
-    this.ok(metric)
+    this.ok(metric, `should find ${expectedMetric[0].name}`)
     if (assertValues) {
       this.same(metric.toJSON(), expectedMetric[1])
     }

diff --git a/test/versioned/express-esm/package.json b/test/versioned/express-esm/package.json
@@ -10,9 +10,22 @@
         "node": ">=18"
       },
       "dependencies": {
-        "express": ">=4.6.0",
-        "express-enrouten": "1.1",
-        "ejs": "2.5.9"
+        "express": {
+          "versions": ">=4.6.0",
+          "samples": 5
+        }
+      },
+      "files": [
+        "segments.tap.mjs",
+        "transaction-naming.tap.mjs"
+      ]
+    },
+    {
+      "engines": {
+        "node": ">=18"
+      },
+      "dependencies": {
+        "express": "5.0.0-beta.3"
       },
       "files": [
         "segments.tap.mjs",