fix(collector): sample according to TRN-41

Author: dszakallas

lib/agent/tracer/collector.js

@@ -27,20 +27,6 @@ describe('Collector', function () {
       parentServiceKey: 8,
       timestamp: 1
     }
-    it('should set default severity', function () {
-      var incompleteDuffelBag = { }
-
-      var tracer = new Collector(options)
-
-      var id = 'uuid'
-
-      this.sandbox.stub(uuid, 'v4', function () {
-        return id
-      })
-
-      var result = tracer.serverRecv(payload, incompleteDuffelBag)
-      expect(result.briefcase.severity).to.eql(tracer.defaultSeverity)
-    })
 
     it('should fill missing communication info', function () {
       var incompleteDuffelBag = { }
@@ -76,19 +62,6 @@ describe('Collector', function () {
       })
     })
 
-    it('should set severity', function () {
-      var tracer = new Collector(options)
-
-      var id = 'uuid'
-
-      this.sandbox.stub(uuid, 'v4', function () {
-        return id
-      })
-
-      var result = tracer.serverRecv(payload, duffelBag)
-      expect(result.briefcase.severity).to.eql(duffelBag.severity)
-    })
-
     it('should not be collected', function () {
       var tracer = new Collector(options)
 
@@ -238,14 +211,14 @@ describe('Collector', function () {
           status: 'ok',
           data: {
             statusCode: 200
-          }
+          },
+          severity: tracer.mustCollectSeverity
         },
         briefcase: {
           communication: {
             id: 'communicationId',
             transactionId: 'transactionId'
-          },
-          severity: tracer.mustCollectSeverity
+          }
         }
       }
       tracer.serverSend(ss.payload, ss.briefcase)
@@ -289,13 +262,14 @@ describe('Collector', function () {
         }
       }
       var ss = {
-        payload: { },
+        payload: {
+          severity: tracer.mustCollectSeverity
+        },
         briefcase: {
           communication: {
             id: sr.duffelBag.communicationId,
             transactionId: sr.duffelBag.transactionId
-          },
-          severity: tracer.mustCollectSeverity
+          }
         }
       }
 
@@ -375,14 +349,14 @@ describe('Collector', function () {
         action: 'action',
         resource: 'resource',
         host: 'host',
-        data: { }
+        data: { },
+        severity: tracer.mustCollectSeverity
       }
       var briefcase = {
         communication: {
           id: 'communicationId',
           transactionId: 'transactionId'
-        },
-        severity: tracer.mustCollectSeverity
+        }
       }
       var id = 'uuid'
 
@@ -421,18 +395,16 @@ describe('Collector', function () {
   describe('CS - CR', function () {
     it('is collected when not in a transaction', function () {
       var tracer = new Collector(options)
-      var payload = { }
-      var briefcase = {
-        severity: tracer.mustCollectSeverity
-      }
+      var payload = { severity: tracer.mustCollectSeverity }
+      var briefcase = { }
       var cs = tracer.clientSend(payload, briefcase)
 
       tracer.clientRecv({}, {}, cs.briefcase)
       expect(tracer.collect().length).to.eql(2)
     })
     it('two different ones do not affect each other', function () {
       var tracer = new Collector(options)
-      var cs1 = tracer.clientSend({ }, { severity: tracer.mustCollectSeverity })
+      var cs1 = tracer.clientSend({ severity: tracer.mustCollectSeverity }, { })
       tracer.clientRecv({}, {}, cs1.briefcase)
 
       var cs2 = tracer.clientSend({ }, { })
@@ -478,10 +450,10 @@ describe('Collector', function () {
         }
       }
       var duffelBag = {
+        severity: tracer.mustCollectSeverity,
         timestamp: 1
       }
       var briefcase = {
-        severity: tracer.mustCollectSeverity,
         communication: {
           id: 'id',
           transactionId: 'id'
@@ -648,7 +620,7 @@ describe('Collector', function () {
   })
 
   describe('Sampling', function () {
-    it('happens when there is a big amount of data', function () {
+    it('happens when there is a big amount of data #1', function () {
       this.sandbox.stub(microtime, 'now', function () {
         return 2
       })
@@ -659,25 +631,49 @@ describe('Collector', function () {
           duffelBag: {
             communicationId: 'commId',
             transactionId: 'trId',
-            severity: tracer.mustCollectSeverity + 1
+            severity: tracer.mustCollectSeverity
           }
         }
         var ss = {
-          payload: { },
+          payload: { severity: tracer.mustCollectSeverity },
           briefcase: {
             communication: {
               id: sr.duffelBag.communicationId,
               transactionId: sr.duffelBag.transactionId
-            },
-            severity: tracer.mustCollectSeverity
+            }
           }
         }
 
         tracer.serverRecv(sr.payload, sr.duffelBag)
         tracer.serverSend(ss.payload, ss.briefcase)
       }
-      for (var i = 0; i < 100; ++i) { report() }
-      expect(tracer.collect().length).to.be.eql(20)
+      for (var i = 0; i < 50; ++i) { report() }
+      var length = tracer.collect().length
+      expect(length).to.be.below(50)
+      expect(length).to.be.above(0)
+    })
+
+    it('happens when there is a big amount of data #2', function () {
+      this.sandbox.stub(microtime, 'now', function () {
+        return 2
+      })
+      var tracer = new Collector(assign({ samplerLimit: 10 }, options))
+      function report () {
+        var cs = {
+          payload: { severity: tracer.mustCollectSeverity },
+          briefcase: { }
+        }
+        var csResult = tracer.clientSend(cs.payload, cs.briefcase)
+
+        var cr = {
+          payload: { }
+        }
+        tracer.clientRecv(cr.payload, {}, csResult.briefcase)
+      }
+      for (var i = 0; i < 50; ++i) { report() }
+      var length = tracer.collect().length
+      expect(length).to.be.below(50)
+      expect(length).to.be.above(0)
     })
   })
 })

lib/agent/tracer/collector.spec.js

@@ -13,7 +13,6 @@ function httpClient (originalHttpRequest, agent) {
     var externalEdgeMetrics = agent.externalEdgeMetrics
     var briefcase = agent.storage.get('tracer.briefcase')
     var communication = briefcase && briefcase.communication
-    var severity = briefcase && briefcase.severity
 
     // parse request
     if (typeof requestParams === 'string') {
@@ -36,10 +35,10 @@ function httpClient (originalHttpRequest, agent) {
       protocol: 'http',
       host: requestParams.host,
       resource: resource,
-      action: requestParams.method
+      action: requestParams.method,
+      severity: collector.defaultSeverity
     }, {
-      communication: communication,
-      severity: severity
+      communication: communication
     })
 
     var csCtx = cs.briefcase.csCtx

lib/instrumentations/core/http/request.js

@@ -94,8 +94,9 @@ describe('The http.request wrapper module', function () {
         host: 'localhost',
         action: 'GET',
         protocol: 'http',
-        resource: '/'
-      }, { communication: {}, severity: undefined })
+        resource: '/',
+        severity: agent.tracer.collector.defaultSeverity
+      }, { communication: {} })
     })
 
     it('sets headers, does not set mustCollect header', function () {

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

@@ -77,10 +77,10 @@ function httpServer (listener, agent) {
     var resource = util.formatDataUrl(requestUrl)
 
     var payload = {
+      protocol: 'http',
       host: headers.host,
       action: request.method,
-      resource: resource,
-      protocol: 'http'
+      resource: resource
     }
 
     var sr = collector.serverRecv(payload, duffelBag)
@@ -121,7 +121,10 @@ function httpServer (listener, agent) {
             status: status,
             data: {
               statusCode: statusCode
-            }
+            },
+            severity: status === 'bad'
+              ? collector.mustCollectSeverity
+              : collector.defaultSeverity
           }, tracerBriefcase, { skip: skipCollecting })
 
           var mustCollect = collector.LEVELS.gte(ss.duffelBag.severity, collector.mustCollectSeverity)

lib/instrumentations/core/http/server.js

@@ -247,7 +247,8 @@ describe('The http.Server.prototype wrapper module', function () {
     expect(agent.tracer.collector.serverSend.args[0][0]).to.eql({
       data: { statusCode: 401 },
       protocol: 'http',
-      status: 'bad'
+      status: 'bad',
+      severity: agent.tracer.collector.mustCollectSeverity
     })
   })
 

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

@@ -21,10 +21,10 @@ function wrapQuery (original, args, agent, params) {
         action: _params.method,
         resource: _params.url,
         host: _params.host,
-        data: _params.protocolSpecific
-      }, {
-        communication: communication,
+        data: _params.protocolSpecific,
         severity: severity
+      }, {
+        communication: communication
       })
 
       debug('#wrapQuery', format('CS(%s) [%s %s %s %s]',

lib/instrumentations/utils/wrapQuery.js

@@ -57,10 +57,10 @@ describe('wrapQuery', function () {
       data: undefined,
       host: 'fakeHost',
       protocol: 'protocol',
-      resource: 'fakeUrl'
+      resource: 'fakeUrl',
+      severity: agent.tracer.collector.defaultSeverity
     }, {
-      communication: {},
-      severity: undefined
+      communication: {}
     })
   })
   it('should call the original', function () {

lib/instrumentations/utils/wrapQuery.spec.js

@@ -57,6 +57,8 @@
     "lodash.get": "4.4.2",
     "lodash.isnumber": "3.0.3",
     "lodash.uniq": "4.5.0",
+    "lodash.remove": "4.7.0",
+    "lodash.some": "^4.6.0",
     "qs": "6.3.0",
     "semver": "5.3.0",
     "sync-request": "3.0.1",