test(collector): add tests (#79)

Author: Shadowbeetle

lib/agent/tracer/collector.js

@@ -129,8 +129,7 @@ Collector.prototype.networkError = function (briefcase, error) {
   var self = this
 
   var root = !communicationId
-
-  var cacheId = root ? briefcase.csCtx.communicationId : communicationId
+  var cacheId = root ? 'root-' + briefcase.csCtx.communicationId : communicationId
 
   this._cache(cacheId, {
     t: EVENT_TYPE.ERROR,
@@ -193,7 +192,7 @@ Collector.prototype.clientSend = function (payload, briefcase, options) {
     transactionId: transactionId
   }
 
-  var cacheId = root ? communication.id : communication.parentId
+  var cacheId = root ? 'root-' + communication.id : communication.parentId
 
   this._cache(cacheId, {
     t: EVENT_TYPE.CLIENT_SEND,
@@ -319,7 +318,7 @@ Collector.prototype.clientRecv = function (payload, duffelBag, briefcase) {
 
   var root = !communication.parentId
 
-  var cacheId = root ? communication.id : communication.parentId
+  var cacheId = root ? 'root-' + communication.id : communication.parentId
 
   this._cache(cacheId, {
     t: EVENT_TYPE.CLIENT_RECV,

lib/agent/tracer/collector.spec.js

@@ -426,6 +426,16 @@ describe('Collector', function () {
       tracer.networkError(cs.briefcase, new Error())
       expect(tracer.collect().length).to.eql(2)
     })
+
+    it('is collected when not in a transaction and error sets severity', function () {
+      var tracer = new Collector(options)
+      var payload = { }
+      var briefcase = { }
+      var cs = tracer.clientSend(payload, briefcase)
+
+      tracer.networkError(cs.briefcase, new Error())
+      expect(tracer.collect().length).to.eql(2)
+    })
   })
 
   describe('CR', function () {
@@ -537,6 +547,35 @@ describe('Collector', function () {
       expect(tracer.collect().length).to.eql(2)
     })
   })
+
+  describe('SR - CS - NE - SS', function () {
+    it('is collected', function () {
+      var tracer = new Collector(options)
+      var sr = {
+        payload: {},
+        duffelBag: {
+          communicationId: 'communicationId',
+          transactionId: 'transactionId',
+          parentServiceKey: 8,
+          timestamp: 1
+        }
+      }
+      var cs = {
+        payload: {}
+      }
+      var ss = {
+        payload: {}
+      }
+
+      var srResult = tracer.serverRecv(sr.payload, sr.duffelBag)
+      var csResult = tracer.clientSend(cs.payload, srResult.briefcase)
+      tracer.networkError(csResult.briefcase, new Error())
+      tracer.serverSend(ss.payload, srResult.briefcase)
+      tracer.end(srResult.briefcase)
+      expect(tracer.collect().length).to.eql(4)
+    })
+  })
+
   describe('User sent error', function () {
     it('is collected', function () {
       var error = new Error('yikes!')

lib/instrumentations/core/http/request.spec.js

@@ -49,7 +49,8 @@ describe('The http.request wrapper module', function () {
           mustCollectSeverity: 2,
           defaultSeverity: 3,
           clientSend: this.sandbox.stub().returns(clientSendResult),
-          clientRecv: this.sandbox.stub()
+          clientRecv: this.sandbox.stub(),
+          networkError: this.sandbox.stub()
         }
       },
       externalEdgeMetrics: {
@@ -79,7 +80,7 @@ describe('The http.request wrapper module', function () {
     })
   })
 
-  describe('on non-whitelited request', function () {
+  describe('on non-whitelisted request', function () {
     it('calls agent.tracer.collector.clientSend with expected arguments', function () {
       var r = request(original, agent)
 
@@ -365,6 +366,28 @@ describe('The http.request wrapper module', function () {
 
       expect(agent.externalEdgeMetrics.report).not.to.be.called
     })
+
+    it('calls agent.tracer.networkError with expected arguments', function () {
+      var r = request(original, agent)
+      var error = new Error()
+      appliedOriginal.on = this.sandbox.spy(function (name, cb) {
+        if (name === 'error') {
+          cb(error)
+        }
+      })
+
+      r({
+        host: 'localhost',
+        path: '/',
+        headers: {},
+        method: 'GET'
+      })
+
+      expect(appliedOriginal.on).to.have.been.called
+      expect(agent.tracer.collector.clientSend).to.have.been.called
+      expect(agent.tracer.collector.networkError)
+        .to.be.calledWith(clientSendResult.briefcase, error)
+    })
   })
 
   it('does not report external edge metrics when statusCode >= 500', function () {

test/e2e/clientUnreachable.spec.js

@@ -0,0 +1,82 @@
+'use strict'
+
+var express = require('express')
+var request = require('superagent')
+var test = require('./utils/test')
+var serviceMocks = require('./utils/serviceMocks')
+var zlib = require('zlib')
+
+var TRACE_COLLECTOR_API_URL = 'https://trace-collector-api.risingstack.com'
+var TRACE_API_KEY = 'headers.payload.signature'
+var TRACE_SERVICE_NAME = 'service-name'
+var TEST_TRACE_SERVICE_KEY = 42
+var TEST_TIMEOUT = 3000
+var TEST_WEB_SERVER_PORT = process.env.TEST_WEBSERVER_PORT || 44332
+
+var cpOpts = {
+  env: {
+    TRACE_API_KEY: TRACE_API_KEY,
+    TRACE_SERVICE_NAME: TRACE_SERVICE_NAME,
+    TRACE_COLLECT_INTERVAL: 1000,
+    TRACE_UPDATE_INTERVAL: 3000,
+    TRACE_IGNORE_HEADERS: JSON.stringify({
+      'ignore-me': '1'
+    })
+  }
+}
+
+
+test('client Unreachable',
+  {
+    // FIXME: I patched nock, so it can gunzip requests on appropriate
+    // content-encoding headers, however it uses the sync API due to
+    // design limitations in nock.
+    skip: !zlib.gunzipSync,
+    isolate: 'child-process',
+    childProcessOpts: cpOpts
+  }, function (t) {
+    var timesCalled = 0
+    serviceMocks.mockServiceKeyRequest({
+      url: TRACE_COLLECTOR_API_URL,
+      apiKey: TRACE_API_KEY,
+      callback: function () {
+        return [200, { key: TEST_TRACE_SERVICE_KEY }]
+      }
+    })
+    serviceMocks.mockTraceRequest({
+      url: TRACE_COLLECTOR_API_URL,
+      apiKey: TRACE_API_KEY,
+      serviceKey: TEST_TRACE_SERVICE_KEY,
+      callback: function (uri, requestBody) {
+        t.pass() // TODO check events
+        t.end()
+        process.exit(0)
+      }
+    })
+    require('@risingstack/trace')
+    t.pass('Trace loaded into server')
+    // http server
+    var app = express()
+    app.get('/test', function (req, res) {
+      request
+        .get('127.0.0.1:64231')
+        .end(function (err) {
+          t.ok(err, 'error arrives')
+          res.send('test')
+        })
+    })
+    app.listen(TEST_WEB_SERVER_PORT, function (err) {
+      t.error(err, 'server starts listening at ' + TEST_WEB_SERVER_PORT)
+      // http client request
+      request
+        .get('127.0.0.1:' + TEST_WEB_SERVER_PORT + '/test')
+        // .set('x-must-collect', '1')
+        .end(function (err) {
+          t.error(err, 'client sends request to /test')
+        })
+    })
+    setTimeout(function () {
+      t.fail('test timed out without completing')
+      process.exit(1)
+    }, TEST_TIMEOUT)
+  })