fix(mustCollect): use fix value for error

Author: gergelyke

lib/agent/index.js

@@ -6,6 +6,7 @@ var defaults = require('lodash.defaults')
 
 var CollectorApi = require('./api')
 var Metrics = require('./metrics')
+var consts = require('../consts')
 
 var REQUEST_ID = 'request-id'
 var SPAN_ID = 'span-id'
@@ -103,7 +104,7 @@ Agent.prototype.serverSend = function (data) {
   })
 
   span.isSampled = (1 / this.sampleRate) > Math.random()
-  span.isForceSampled = span.isForceSampled || !!data.mustCollect
+  span.isForceSampled = span.isForceSampled || data.mustCollect === consts.MUST_COLLECT.ERROR
 
   if (span.isForceSampled || span.isSampled) {
     this.spans.push(span)

lib/consts.js

@@ -0,0 +1,5 @@
+module.exports = {
+  MUST_COLLECT: {
+    ERROR: '1'
+  }
+}

lib/instrumentations/core/http/request.js

@@ -3,6 +3,7 @@ var url = require('url')
 var microtime = require('microtime')
 
 var util = require('./util')
+var consts = require('../../../consts')
 
 function wrapRequest (originalHttpRequest, agent, mustCollectStore) {
   var whiteListHosts = agent.getConfig().whiteListHosts
@@ -41,7 +42,7 @@ function wrapRequest (originalHttpRequest, agent, mustCollectStore) {
 
     if (mustCollectStore[requestId]) {
       debug('trace event (cs); reqId: %s, spanId: %s must collect', requestId, spanId)
-      requestParams.headers['x-must-collect'] = '1'
+      requestParams.headers['x-must-collect'] = consts.MUST_COLLECT.ERROR
     }
 
     if (typeof agent.getServiceKey() !== 'undefined') {
@@ -59,7 +60,7 @@ function wrapRequest (originalHttpRequest, agent, mustCollectStore) {
       url: util.formatDataUrl(requestParams.path),
       time: clientSendTime,
       method: requestParams.method,
-      mustCollect: !!mustCollectStore[requestId]
+      mustCollect: mustCollectStore[requestId]
     }
 
     // Collect request start
@@ -79,7 +80,7 @@ function wrapRequest (originalHttpRequest, agent, mustCollectStore) {
         spanId: spanId,
         host: requestParams.host,
         url: util.formatDataUrl(requestParams.path),
-        mustCollect: true,
+        mustCollect: consts.MUST_COLLECT.ERROR,
         err: {
           type: 'network-error',
           message: err.message,
@@ -97,8 +98,8 @@ function wrapRequest (originalHttpRequest, agent, mustCollectStore) {
 
     // returns with response
     returned.on('response', function (incomingMessage) {
-      mustCollectStore[requestId] = !!incomingMessage.headers['x-must-collect'] ||
-        !!mustCollectStore[requestId]
+      mustCollectStore[requestId] = incomingMessage.headers['x-must-collect'] ||
+        mustCollectStore[requestId]
 
       if (mustCollectStore[requestId]) {
         debug('trace event (cr) on response; reqId: %s, spanId: %s must collect', requestId, spanId)

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

@@ -93,15 +93,15 @@ describe('The http.request wrapper module', function () {
         id: transactionId,
         spanId: spanId,
         method: 'GET',
-        mustCollect: false,
+        mustCollect: undefined,
         time: 12345678,
         url: '/'
       })
 
       expect(agent.clientReceive).to.be.calledWith({
         host: 'localhost',
         id: transactionId,
-        mustCollect: false,
+        mustCollect: undefined,
         spanId: spanId,
         statusCode: 200,
         url: '/'

lib/instrumentations/core/http/server.js

@@ -5,6 +5,7 @@ var microtime = require('microtime')
 var debug = require('debug')('risingstack/trace')
 
 var util = require('./util')
+var consts = require('../../../consts')
 
 function wrapListener (listener, agent, mustCollectStore) {
   var ignoreHeaders = agent.getConfig().ignoreHeaders
@@ -36,7 +37,7 @@ function wrapListener (listener, agent, mustCollectStore) {
     // must be collected
     if (headers['x-must-collect']) {
       debug('trace event (sr); request: %s, set must collect store', requestId)
-      mustCollectStore[requestId] = true
+      mustCollectStore[requestId] = consts.MUST_COLLECT.ERROR
     }
 
     // setting the spanId in cls
@@ -91,7 +92,9 @@ function wrapListener (listener, agent, mustCollectStore) {
       serverSendTime = microtime.now()
 
       // collected because previous reason like (x-must-collect etc.) or wrong status code
-      mustCollectStore[requestId] = mustCollectStore[requestId] || response.statusCode > 399
+      if (response.statusCode > 399) {
+        mustCollectStore[requestId] = consts.MUST_COLLECT.ERROR
+      }
 
       /* Service name may be unavailable due to uninitialized reporter */
       var serviceKey = agent.getServiceKey()
@@ -110,7 +113,7 @@ function wrapListener (listener, agent, mustCollectStore) {
 
       if (mustCollectStore[requestId]) {
         debug('trace event (ss); request: %s x-must-collect header has been set', requestId)
-        response.setHeader('x-must-collect', 1)
+        response.setHeader('x-must-collect', consts.MUST_COLLECT.ERROR)
       }
 
       originalWriteHead.apply(response, arguments)

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

@@ -2,6 +2,7 @@ var expect = require('chai').expect
 var microtime = require('microtime')
 
 var server = require('./server')
+var consts = require('../../../consts')
 
 describe('The http.Server.prototype wrapper module', function () {
   var mustCollectStore
@@ -143,7 +144,7 @@ describe('The http.Server.prototype wrapper module', function () {
       expect(agent.serverSend).to.be.calledWith({
         host: 'localhost',
         id: transactionId,
-        mustCollect: false,
+        mustCollect: undefined,
         spanId: undefined,
         statusCode: statusCode,
         time: 12345678,
@@ -166,7 +167,7 @@ describe('The http.Server.prototype wrapper module', function () {
         headers: {
           host: 'localhost',
           'x-span-id': spanId,
-          'x-must-collect': 1
+          'x-must-collect': '1'
         },
         pathname: '/',
         url: '/?id=2',
@@ -207,7 +208,7 @@ describe('The http.Server.prototype wrapper module', function () {
         id: transactionId,
         spanId: spanId,
         statusCode: statusCode,
-        mustCollect: true,
+        mustCollect: consts.MUST_COLLECT.ERROR,
         time: 12345678,
         url: '/',
         responseTime: 0
@@ -217,7 +218,7 @@ describe('The http.Server.prototype wrapper module', function () {
       expect(setHeaderSpy).to.be.calledWith('x-client-send', 12345678)
       expect(setHeaderSpy).to.be.calledWith('x-parent', 0)
       expect(setHeaderSpy).to.be.calledWith('x-span-id', spanId)
-      expect(setHeaderSpy).to.be.calledWith('x-must-collect', 1)
+      expect(setHeaderSpy).to.be.calledWith('x-must-collect', '1')
       expect(writeHeadSpy).to.be.calledOnce
     })
   })