fix express wrong resource name and missing error field (#166)

docs/API.md

@@ -175,6 +175,7 @@ This limitation doesn't apply to other commands. We are working on improving thi
 | Option           | Default                   | Description                            |
 |------------------|---------------------------|----------------------------------------|
 | service          | *Service name of the app* | The service name for this integration. |
+| validateStatus   | `code => code < 500`      | Callback function to determine if there was an error. It should take a status code as its only parameter and return `true` for success or `false` for errors. |
 
 <h3 id="graphql">graphql</h3>
 

src/plugins/express.js

@@ -9,6 +9,10 @@ const pathToRegExp = require('path-to-regexp')
 const OPERATION_NAME = 'express.request'
 
 function createWrapMethod (tracer, config) {
+  const validateStatus = typeof config.validateStatus === 'function'
+    ? config.validateStatus
+    : code => code < 500
+
   function middleware (req, res, next) {
     const url = `${req.protocol}://${req.get('host')}${req.originalUrl}`
     const childOf = tracer.extract(FORMAT_HTTP_HEADERS, req.headers)
@@ -29,12 +33,18 @@ function createWrapMethod (tracer, config) {
 
         if (paths) {
           span.setTag('resource.name', `${req.method} ${paths.join('')}`)
+        } else {
+          span.setTag('resource.name', req.method)
         }
 
         span.setTag('service.name', config.service || tracer._service)
         span.setTag('span.type', 'web')
         span.setTag(Tags.HTTP_STATUS_CODE, res.statusCode)
 
+        if (!validateStatus(res.statusCode)) {
+          span.setTag(Tags.ERROR, true)
+        }
+
         span.finish()
 
         return returned

test/plugins/express.spec.js

@@ -229,15 +229,15 @@ describe('Plugin', () => {
         })
       })
 
-      it('should fallback to the default resource name if a path pattern could not be found', done => {
+      it('should fallback to the the verb if a path pattern could not be found', done => {
         const app = express()
 
         app.use((req, res, next) => res.status(200).send())
 
         getPort().then(port => {
           agent
             .use(traces => {
-              expect(traces[0][0]).to.have.property('resource', 'express.request')
+              expect(traces[0][0]).to.have.property('resource', 'GET')
             })
             .then(done)
             .catch(done)
@@ -422,14 +422,45 @@ describe('Plugin', () => {
           })
         })
       })
+
+      it('should handle errors', done => {
+        const app = express()
+
+        app.use((req, res, next) => {
+          next()
+        })
+
+        app.get('/user', (req, res) => {
+          res.status(500).send()
+        })
+
+        getPort().then(port => {
+          agent.use(traces => {
+            expect(traces[0][0]).to.have.property('error', 1)
+            expect(traces[0][0]).to.have.property('resource', 'GET /user')
+            expect(traces[0][0].meta).to.have.property('http.status_code', '500')
+
+            done()
+          })
+
+          appListener = app.listen(port, 'localhost', () => {
+            axios
+              .get(`http://localhost:${port}/user`, {
+                validateStatus: status => status === 500
+              })
+              .catch(done)
+          })
+        })
+      })
     })
 
     describe('with configuration', () => {
       let config
 
       beforeEach(() => {
         config = {
-          service: 'custom'
+          service: 'custom',
+          validateStatus: code => code < 400
         }
 
         return agent.load(plugin, 'express', config)
@@ -438,7 +469,7 @@ describe('Plugin', () => {
           })
       })
 
-      it('should be configured with the correct values', done => {
+      it('should be configured with the correct service name', done => {
         const app = express()
 
         app.get('/user', (req, res) => {
@@ -460,6 +491,31 @@ describe('Plugin', () => {
           })
         })
       })
+
+      it('should be configured with the correct status code validator', done => {
+        const app = express()
+
+        app.get('/user', (req, res) => {
+          res.status(400).send()
+        })
+
+        getPort().then(port => {
+          agent
+            .use(traces => {
+              expect(traces[0][0]).to.have.property('error', 1)
+            })
+            .then(done)
+            .catch(done)
+
+          appListener = app.listen(port, 'localhost', () => {
+            axios
+              .get(`http://localhost:${port}/user`, {
+                validateStatus: status => status === 400
+              })
+              .catch(done)
+          })
+        })
+      })
     })
   })
 })