removing hook for express v5 router

Author: crysmags

packages/datadog-instrumentations/src/express.js

@@ -60,23 +60,12 @@ function wrapResponseRender (render) {
 addHook({ name: 'express', versions: ['>=4'] }, express => {
   shimmer.wrap(express.application, 'handle', wrapHandle)
 
-  shimmer.wrap(express.response, 'json', wrapResponseJson)
-  shimmer.wrap(express.response, 'jsonp', wrapResponseJson)
-  shimmer.wrap(express.response, 'render', wrapResponseRender)
-
-  return express
-})
-
-addHook({ name: 'express', versions: ['4'] }, express => {
   shimmer.wrap(express.Router, 'use', wrapRouterMethod)
   shimmer.wrap(express.Router, 'route', wrapRouterMethod)
 
-  return express
-})
-
-addHook({ name: 'express', versions: ['>=5.0.0'] }, express => {
-  shimmer.wrap(express.Router.prototype, 'use', wrapRouterMethod)
-  shimmer.wrap(express.Router.prototype, 'route', wrapRouterMethod)
+  shimmer.wrap(express.response, 'json', wrapResponseJson)
+  shimmer.wrap(express.response, 'jsonp', wrapResponseJson)
+  shimmer.wrap(express.response, 'render', wrapResponseRender)
 
   return express
 })

packages/datadog-plugin-express/test/index.spec.js

@@ -27,7 +27,7 @@ describe('Plugin', () => {
 
       describe('without http', () => {
         before(() => {
-          return agent.load('express', { client: false })
+          return agent.load(['express', 'router'], [{ client: false }, {}])
         })
 
         after(() => {
@@ -81,7 +81,7 @@ describe('Plugin', () => {
 
       describe('without configuration', () => {
         before(() => {
-          return agent.load(['express', 'http'], [{}, { client: false }])
+          return agent.load(['express', 'http', 'router'], [{}, { client: false }, {}])
         })
 
         after(() => {
@@ -217,6 +217,9 @@ describe('Plugin', () => {
                 const spans = sort(traces[0])
                 const isExpress4 = semver.intersects(version, '<5.0.0')
                 let index = 0
+                const whichMiddleware = semver.intersects(version, '<5.0.0')
+                  ? 'express'
+                  : 'router'
 
                 const rootSpan = spans[index++]
                 expect(rootSpan).to.have.property('resource', 'GET /app/user/:id')
@@ -238,31 +241,31 @@ describe('Plugin', () => {
                 }
 
                 expect(spans[index]).to.have.property('resource', 'named')
-                expect(spans[index]).to.have.property('name', 'express.middleware')
+                expect(spans[index]).to.have.property('name', `${whichMiddleware}.middleware`)
                 expect(spans[index].parent_id.toString()).to.equal(rootSpan.span_id.toString())
-                expect(spans[index].meta).to.have.property('component', 'express')
+                expect(spans[index].meta).to.have.property('component', whichMiddleware)
                 index++
 
                 expect(spans[index]).to.have.property('resource', 'router')
-                expect(spans[index]).to.have.property('name', 'express.middleware')
+                expect(spans[index]).to.have.property('name', `${whichMiddleware}.middleware`)
                 expect(spans[index].parent_id.toString()).to.equal(rootSpan.span_id.toString())
-                expect(spans[index].meta).to.have.property('component', 'express')
+                expect(spans[index].meta).to.have.property('component', whichMiddleware)
                 index++
 
                 if (isExpress4) {
                   expect(spans[index].resource).to.match(/^bound\s.*$/)
                 } else {
                   expect(spans[index]).to.have.property('resource', 'handle')
                 }
-                expect(spans[index]).to.have.property('name', 'express.middleware')
+                expect(spans[index]).to.have.property('name', `${whichMiddleware}.middleware`)
                 expect(spans[index].parent_id.toString()).to.equal(spans[index - 1].span_id.toString())
-                expect(spans[index].meta).to.have.property('component', 'express')
+                expect(spans[index].meta).to.have.property('component', whichMiddleware)
                 index++
 
                 expect(spans[index]).to.have.property('resource', '<anonymous>')
-                expect(spans[index]).to.have.property('name', 'express.middleware')
+                expect(spans[index]).to.have.property('name', `${whichMiddleware}.middleware`)
                 expect(spans[index].parent_id.toString()).to.equal(spans[index - 1].span_id.toString())
-                expect(spans[index].meta).to.have.property('component', 'express')
+                expect(spans[index].meta).to.have.property('component', whichMiddleware)
 
                 expect(index).to.equal(spans.length - 1)
               })
@@ -301,13 +304,16 @@ describe('Plugin', () => {
                 const spans = sort(traces[0])
 
                 const breakingSpanIndex = semver.intersects(version, '<5.0.0') ? 3 : 1
+                const whichMiddleware = semver.intersects(version, '<5.0.0')
+                  ? 'express'
+                  : 'router'
 
                 expect(spans[0]).to.have.property('resource', 'GET /user/:id')
                 expect(spans[0]).to.have.property('name', 'express.request')
                 expect(spans[0].meta).to.have.property('component', 'express')
                 expect(spans[breakingSpanIndex]).to.have.property('resource', 'breaking')
-                expect(spans[breakingSpanIndex]).to.have.property('name', 'express.middleware')
-                expect(spans[breakingSpanIndex].meta).to.have.property('component', 'express')
+                expect(spans[breakingSpanIndex]).to.have.property('name', `${whichMiddleware}.middleware`)
+                expect(spans[breakingSpanIndex].meta).to.have.property('component', whichMiddleware)
               })
               .then(done)
               .catch(done)
@@ -346,12 +352,15 @@ describe('Plugin', () => {
               .assertSomeTraces(traces => {
                 const spans = sort(traces[0])
                 const errorSpanIndex = semver.intersects(version, '<5.0.0') ? 4 : 2
+                const whichMiddleware = semver.intersects(version, '<5.0.0')
+                  ? 'express'
+                  : 'router'
 
                 expect(spans[0]).to.have.property('name', 'express.request')
-                expect(spans[errorSpanIndex]).to.have.property('name', 'express.middleware')
+                expect(spans[errorSpanIndex]).to.have.property('name', `${whichMiddleware}.middleware`)
                 expect(spans[errorSpanIndex].meta).to.have.property(ERROR_TYPE, error.name)
                 expect(spans[0].meta).to.have.property('component', 'express')
-                expect(spans[errorSpanIndex].meta).to.have.property('component', 'express')
+                expect(spans[errorSpanIndex].meta).to.have.property('component', whichMiddleware)
               })
               .then(done)
               .catch(done)
@@ -1163,6 +1172,9 @@ describe('Plugin', () => {
               .assertSomeTraces(traces => {
                 const spans = sort(traces[0])
                 const secondErrorIndex = spans.length - 2
+                const whichMiddleware = semver.intersects(version, '<5.0.0')
+                  ? 'express'
+                  : 'router'
 
                 expect(spans[0]).to.have.property('error', 1)
                 expect(spans[0].meta).to.have.property(ERROR_TYPE, error.name)
@@ -1173,7 +1185,7 @@ describe('Plugin', () => {
                 expect(spans[secondErrorIndex].meta).to.have.property(ERROR_TYPE, error.name)
                 expect(spans[secondErrorIndex].meta).to.have.property(ERROR_MESSAGE, error.message)
                 expect(spans[secondErrorIndex].meta).to.have.property(ERROR_STACK, error.stack)
-                expect(spans[secondErrorIndex].meta).to.have.property('component', 'express')
+                expect(spans[secondErrorIndex].meta).to.have.property('component', whichMiddleware)
               })
               .then(done)
               .catch(done)
@@ -1435,12 +1447,12 @@ describe('Plugin', () => {
 
       describe('with configuration', () => {
         before(() => {
-          return agent.load(['express', 'http'], [{
+          return agent.load(['express', 'http', 'router'], [{
             service: 'custom',
             validateStatus: code => code < 400,
             headers: ['User-Agent'],
             blocklist: ['/health']
-          }, { client: false }])
+          }, { client: false }, {}])
         })
 
         after(() => {
@@ -1563,9 +1575,9 @@ describe('Plugin', () => {
 
       describe('with configuration for middleware disabled', () => {
         before(() => {
-          return agent.load(['express', 'http'], [{
+          return agent.load(['express', 'http', 'router'], [{
             middleware: false
-          }, { client: false }])
+          }, { client: false }, { middleware: false }])
         })
 
         after(() => {
@@ -1581,8 +1593,8 @@ describe('Plugin', () => {
 
           let span
 
-          app.use((req, res, next) => {
-            span = tracer.scope().active()
+          app.use(async (req, res, next) => {
+            span = await tracer.scope().active()
             next()
           })
 

packages/datadog-plugin-express/test/integration-test/client.spec.js

@@ -38,7 +38,10 @@ describe('esm', () => {
     describe('with DD_TRACE_MIDDLEWARE_TRACING_ENABLED unset', () => {
       it('is instrumented', async () => {
         proc = await spawnPluginIntegrationTestProc(sandbox.folder, 'server.mjs', agent.port)
-        const numberOfSpans = semver.intersects(version, '<5.0.0') ? 4 : 3
+        const numberOfSpans = semver.intersects(version, '<5.0.0') ? 4 : 2
+        const whichMiddleware = semver.intersects(version, '<5.0.0')
+          ? 'express'
+          : 'router'
 
         return curlAndAssertMessage(agent, proc, ({ headers, payload }) => {
           assert.propertyVal(headers, 'host', `127.0.0.1:${agent.port}`)
@@ -47,7 +50,7 @@ describe('esm', () => {
           assert.isArray(payload[0])
           assert.strictEqual(payload[0].length, numberOfSpans)
           assert.propertyVal(payload[0][0], 'name', 'express.request')
-          assert.propertyVal(payload[0][1], 'name', 'express.middleware')
+          assert.propertyVal(payload[0][1], 'name', `${whichMiddleware}.middleware`)
         })
       }).timeout(50000)
     })