Add validateStatus option to HTTP plugin

docs/API.md

@@ -214,8 +214,9 @@ query HelloWorld {
 
 | Option           | Default          | Description                            |
 |------------------|------------------|----------------------------------------|
-| service          | http-client      | The service name for this integration. |
-| splitByDomain    | false            | Use the remote endpoint host as the service name instead of the default. |
+| service          | http-client                           | The service name for this integration. |
+| splitByDomain    | false                                 | Use the remote endpoint host as the service name instead of the default. |
+| validateStatus   | `code => code < 400 || code >= 500`   | Callback function to determine if an HTTP response should be recorded as an error. It should take a status code as its only parameter and return `true` for success or `false` for errors.
 
 <h3 id="ioredis">ioredis</h3>
 

src/plugins/http.js

@@ -3,11 +3,13 @@
 const url = require('url')
 const opentracing = require('opentracing')
 const semver = require('semver')
+const log = require('../log')
 
 const Tags = opentracing.Tags
 const FORMAT_HTTP_HEADERS = opentracing.FORMAT_HTTP_HEADERS
 
 function patch (http, methodName, tracer, config) {
+  config = normalizeConfig(config)
   this.wrap(http, methodName, fn => makeRequestTrace(fn))
 
   function makeRequestTrace (request) {
@@ -53,6 +55,10 @@ function patch (http, methodName, tracer, config) {
       req.on('response', res => {
         span.setTag(Tags.HTTP_STATUS_CODE, res.statusCode)
 
+        if (!config.validateStatus(res.statusCode)) {
+          span.setTag('error', 1)
+        }
+
         res.on('end', () => span.finish())
       })
 
@@ -148,6 +154,23 @@ function unpatch (http) {
   this.unwrap(http, 'get')
 }
 
+function getStatusValidator (config) {
+  if (typeof config.validateStatus === 'function') {
+    return config.validateStatus
+  } else if (config.hasOwnProperty('validateStatus')) {
+    log.error('Expected `validateStatus` to be a function.')
+  }
+  return code => code < 400 || code >= 500
+}
+
+function normalizeConfig (config) {
+  const validateStatus = getStatusValidator(config)
+
+  return Object.assign({}, config, {
+    validateStatus
+  })
+}
+
 module.exports = [
   {
     name: 'http',

test/plugins/http.spec.js

@@ -454,7 +454,7 @@ describe('Plugin', () => {
           })
         })
 
-        it('should handle errors', done => {
+        it('should handle connection errors', done => {
           getPort().then(port => {
             agent
               .use(traces => {
@@ -471,6 +471,56 @@ describe('Plugin', () => {
           })
         })
 
+        it('should not record HTTP 5XX responses as errors by default', done => {
+          const app = express()
+
+          app.get('/user', (req, res) => {
+            res.status(500).send()
+          })
+
+          getPort().then(port => {
+            agent
+              .use(traces => {
+                expect(traces[0][0]).to.have.property('error', 0)
+              })
+              .then(done)
+              .catch(done)
+
+            appListener = server(app, port, () => {
+              const req = http.request(`${protocol}://localhost:${port}/user`, res => {
+                res.on('data', () => { })
+              })
+
+              req.end()
+            })
+          })
+        })
+
+        it('should record HTTP 4XX responses as errors by default', 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 = server(app, port, () => {
+              const req = http.request(`${protocol}://localhost:${port}/user`, res => {
+                res.on('data', () => { })
+              })
+
+              req.end()
+            })
+          })
+        })
+
         it('should only record a request once', done => {
           // Make sure both plugins are loaded, which could cause double-counting.
           require('http')
@@ -555,6 +605,47 @@ describe('Plugin', () => {
         })
       })
 
+      describe('with validateStatus configuration', () => {
+        let config
+
+        beforeEach(() => {
+          config = {
+            validateStatus: status => status < 500
+          }
+
+          return agent.load(plugin, 'http', config)
+            .then(() => {
+              http = require(protocol)
+              express = require('express')
+            })
+        })
+
+        it('should use the supplied function to decide if a response is an error', done => {
+          const app = express()
+
+          app.get('/user', (req, res) => {
+            res.status(500).send()
+          })
+
+          getPort().then(port => {
+            agent
+              .use(traces => {
+                expect(traces[0][0]).to.have.property('error', 1)
+              })
+              .then(done)
+              .catch(done)
+
+            appListener = server(app, port, () => {
+              const req = http.request(`${protocol}://localhost:${port}/user`, res => {
+                res.on('data', () => { })
+              })
+
+              req.end()
+            })
+          })
+        })
+      })
+
       describe('with splitByDomain configuration', () => {
         let config