Add validateStatus option to HTTP plugin

The [OpenTracing spec][1] states that a span should be tagged as
an error "if and only if the application considers the operation
represented by the Span to have failed".

The `dd-trace-js` HTTP plugin allows you to automatically
instrument outgoing HTTP requests.

Currently, it only tags a span as an error when the request
emits an `error` event, which only happens in the case of "hard
failures" like a timeout or connection problem.

https://github.com/DataDog/dd-trace-js/blob/34c168555f44a961a61ede95a39341d7c527701a/src/plugins/http.js#L59-L67

This means that requests that return responses never are
considered errors, even if a 4XX or 5XX response is received.

Having spans tagged as errors is useful, because it means they
are flagged as such in the Datadog APM interface, useful for
reporting.

This commit adds support for a new `validateStatus` option for
the HTTP plugin which allows the user to supply a
`validateStatus` function which, when a response is received, is
passed the response status and expected to return `false` if the
response should be considered an error, or `true` if not. If it
returns `false`, the span will be marked as an error.

By default, we will continue not to consider any requests that
receive a response to be errors, but this configuration option
allows the user to "opt in" if they want to see these spans as
errors.

Fixes #297.

[1]: https://github.com/opentracing/specification/blob/master/semantic_conventions.md

docs/API.md

@@ -216,6 +216,7 @@ query HelloWorld {
 |------------------|------------------|----------------------------------------|
 | 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 => true`   | 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. Requests where no response is received (e.g. connection failures, timeouts) will always be considered to be errors.
 
 <h3 id="ioredis">ioredis</h3>
 

src/plugins/http.js

@@ -53,6 +53,12 @@ function patch (http, methodName, tracer, config) {
       req.on('response', res => {
         span.setTag(Tags.HTTP_STATUS_CODE, res.statusCode)
 
+        if (config.validateStatus && !config.validateStatus(res.statusCode)) {
+          span.addTags({
+            'error': 1
+          })
+        }
+
         res.on('end', () => span.finish())
       })
 

test/plugins/http.spec.js

@@ -471,6 +471,31 @@ describe('Plugin', () => {
           })
         })
 
+        it('should not record HTTP error 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 only record a request once', done => {
           // Make sure both plugins are loaded, which could cause double-counting.
           require('http')
@@ -555,6 +580,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