feat: Drop unsampled transactions when connected to APM Server 8.0+

CHANGELOG.asciidoc

@@ -37,6 +37,8 @@ Notes:
 [float]
 ===== Features
 
+* Drop unsampled transactions when sending to APM Server v8.0+. ({issues}2455[#2455])
+
 * The default <<service-name, `serviceName`>> string (when it is not configured
   and cannot be inferred from a "package.json" file) has been changed from
   "nodejs_service" to "unknown-nodejs-service". This is a standardized pattern

lib/config.js

@@ -925,9 +925,10 @@ function getBaseClientConfig (conf, agent) {
     time: conf.apiRequestTime * 1000,
     maxQueueSize: conf.maxQueueSize,
 
-    // Debugging
+    // Debugging/testing options
     logger: clientLogger,
     payloadLogFile: conf.payloadLogFile,
+    apmServerVersion: conf.apmServerVersion,
 
     // Container conf
     containerId: conf.containerId,

lib/instrumentation/index.js

@@ -281,6 +281,11 @@ Instrumentation.prototype.addEndedTransaction = function (transaction) {
     return
   }
 
+  // https://github.com/elastic/apm/blob/main/specs/agents/tracing-sampling.md#non-sampled-transactions
+  if (!transaction.sampled && !agent._transport.supportsKeepingUnsampledTransaction()) {
+    return
+  }
+
   var payload = agent._transactionFilters.process(transaction._encode())
   if (!payload) {
     agent.logger.debug('transaction ignored by filter %o', { trans: transaction.id, trace: transaction.traceId })

lib/noop-transport.js

@@ -43,6 +43,10 @@ class NoopTransport {
     }
   }
 
+  supportsKeepingUnsampledTransaction () {
+    return true
+  }
+
   // Inherited from Writable, called in agent.js.
   destroy () {}
 }

package.json

@@ -86,7 +86,7 @@
     "basic-auth": "^2.0.1",
     "cookie": "^0.4.0",
     "core-util-is": "^1.0.2",
-    "elastic-apm-http-client": "^10.3.0",
+    "elastic-apm-http-client": "^10.4.0",
     "end-of-stream": "^1.4.4",
     "error-callsites": "^2.0.4",
     "error-stack-parser": "^2.0.6",

test/_capturing_transport.js

@@ -77,6 +77,10 @@ class CapturingTransport {
     }
   }
 
+  supportsKeepingUnsampledTransaction () {
+    return true
+  }
+
   // Inherited from Writable, called in agent.js.
   destroy () {}
 }

test/_mock_apm_server.js

@@ -1,7 +1,7 @@
 // A mock APM server to use in tests.
 //
 // Usage:
-//    const server = new MockAPMServer()
+//    const server = new MockAPMServer(opts)
 //    server.start(function (serverUrl) {
 //      // Test code using `serverUrl`...
 //      // - Events received on the intake API will be on `server.events`.
@@ -16,9 +16,14 @@ const { URL } = require('url')
 const zlib = require('zlib')
 
 class MockAPMServer {
-  constructor () {
+  // - @param {Object} opts
+  //    - {String} opts.apmServerVersion - The version to report in the
+  //      "GET /" response body. Defaults to "8.0.0".
+  constructor (opts) {
+    opts = opts || {}
     this.clear()
     this.serverUrl = null // set in .start()
+    this.apmServerVersion = opts.apmServerVersion || '8.0.0'
     this._http = http.createServer(this._onRequest.bind(this))
   }
 
@@ -43,7 +48,15 @@ class MockAPMServer {
 
     instream.on('end', () => {
       let resBody = ''
-      if (parsedUrl.pathname === '/config/v1/agents') {
+      if (req.method === 'GET' && parsedUrl.pathname === '/') {
+        // https://www.elastic.co/guide/en/apm/server/current/server-info.html#server-info-endpoint
+        res.writeHead(200)
+        resBody = JSON.stringify({
+          build_date: '2021-09-16T02:05:39Z',
+          build_sha: 'a183f675ecd03fca4a897cbe85fda3511bc3ca43',
+          version: this.apmServerVersion
+        })
+      } else if (parsedUrl.pathname === '/config/v1/agents') {
         // Central config mocking.
         res.writeHead(200)
         resBody = '{}'

test/_mock_http_client.js

@@ -54,6 +54,9 @@ module.exports = function (expected, done) {
     },
     flush (cb) {
       if (cb) process.nextTick(cb)
+    },
+    supportsKeepingUnsampledTransaction () {
+      return true
     }
   }
 

test/_mock_http_client_states.js

@@ -35,6 +35,9 @@ module.exports = function (expectations = [], done) {
     },
     flush (cb) {
       if (cb) process.nextTick(cb)
+    },
+    supportsKeepingUnsampledTransaction () {
+      return true
     }
   }
 }

test/agent.test.js

@@ -553,7 +553,12 @@ test('filters', function (t) {
       filterAgentOpts = Object.assign(
         {},
         agentOpts,
-        { serverUrl }
+        {
+          serverUrl,
+          // Ensure the APM client's `GET /` requests do not get in the way of
+          // the test asserts below.
+          apmServerVersion: '8.0.0'
+        }
       )
       t.end()
     })

test/central-config-enabled.test.js

@@ -46,6 +46,7 @@ const runTestsWithServer = (t, updates, expect) => {
       serviceName: 'test',
       logLevel: 'off', // silence for cleaner test output
       captureExceptions: false,
+      apmServerVersion: '8.0.0',
       metricsInterval: 0,
       centralConfig: true
     })
@@ -159,6 +160,7 @@ test('agent.logger updates for central config `log_level` change', { timeout: 10
       serverUrl: 'http://localhost:' + server.address().port,
       serviceName: 'test',
       captureExceptions: false,
+      apmServerVersion: '8.0.0',
       metricsInterval: 0,
       centralConfig: true,
       logLevel: 'warn'
@@ -207,6 +209,7 @@ test('central config change does not erroneously update cloudProvider', { timeou
       cloudProvider: 'aws',
       // These settings to reduce some agent activity:
       captureExceptions: false,
+      apmServerVersion: '8.0.0',
       metricsInterval: 0
     })
 

test/config.test.js

@@ -934,6 +934,10 @@ test('custom transport', function (t) {
     flush (cb) {
       if (cb) setImmediate(cb)
     }
+
+    supportsKeepingUnsampledTransaction () {
+      return true
+    }
   }
   const myTransport = new MyTransport()
 

test/fixtures/do-not-trace-self.js

@@ -4,6 +4,7 @@
 var apm = require('../../').start({ // elastic-apm-node
   serviceName: 'test-do-not-trace-self',
   metricsInterval: 0,
+  apmServerVersion: '8.0.0',
   cloudProvider: 'none',
   centralConfig: false
 })

test/instrumentation/index.test.js

@@ -15,9 +15,7 @@ var http = require('http')
 
 var test = require('tape')
 
-const logging = require('../../lib/logging')
 var mockClient = require('../_mock_http_client')
-var Instrumentation = require('../../lib/instrumentation')
 var findObjInArray = require('../_utils').findObjInArray
 
 var origCaptureError = agent.captureError
@@ -250,88 +248,9 @@ test('captureError should handle opts.captureAttributes', function (t) {
   agent.captureError(ex2, { captureAttributes: false })
 })
 
-test('sampling', function (t) {
-  function generateSamples (rate, count) {
-    count = count || 1000
-    var agent = {
-      _conf: {
-        transactionSampleRate: rate
-      },
-      logger: logging.createLogger('off')
-    }
-    var ins = new Instrumentation(agent)
-    agent._instrumentation = ins
-
-    var results = {
-      count: count,
-      sampled: 0,
-      unsampled: 0
-    }
-    for (var i = 0; i < count; i++) {
-      var trans = ins.startTransaction()
-      if (trans && trans.sampled) {
-        results.sampled++
-      } else {
-        results.unsampled++
-      }
-    }
-
-    return results
-  }
-
-  function toRatios (samples) {
-    return {
-      count: samples.count,
-      sampled: samples.sampled / samples.count,
-      unsampled: samples.unsampled / samples.count
-    }
-  }
-
-  var high = generateSamples(1.0)
-  t.ok(high.sampled > high.unsampled)
-
-  var low = generateSamples(0.1)
-  t.ok(low.sampled < low.unsampled)
-
-  var mid = toRatios(generateSamples(0.5))
-  t.ok(mid.sampled > 0.4 && mid.sampled < 0.6)
-  t.ok(mid.unsampled > 0.4 && mid.unsampled < 0.6)
-
-  t.end()
-})
-
-test('unsampled transactions do not include spans', function (t) {
-  resetAgent(1, function (data, cb) {
-    t.strictEqual(data.transactions.length, 1)
-
-    data.transactions.forEach(function (trans) {
-      t.ok(/^[\da-f]{16}$/.test(trans.id))
-      t.ok(/^[\da-f]{32}$/.test(trans.trace_id))
-      t.ok(trans.duration > 0, 'duration should be >0ms')
-      t.ok(trans.duration < 100, 'duration should be <100ms')
-      t.notOk(Number.isNaN((new Date(trans.timestamp)).getTime()))
-      t.strictEqual(trans.sampled, false)
-    })
-
-    t.end()
-  })
-
-  agent._conf.transactionSampleRate = 0.0
-  var ins = agent._instrumentation
-
-  var trans = ins.startTransaction()
-  var span = ins.startSpan('span 0', 'type')
-  process.nextTick(function () {
-    if (span) span.end()
-    span = ins.startSpan('span 1', 'type')
-    process.nextTick(function () {
-      if (span) span.end()
-      trans.end()
-    })
-  })
-})
-
 test('unsampled request transactions should have the correct result', function (t) {
+  // This test is relying on `resetAgent` creating an `agent._transaction`
+  // that returns true from `.supportsKeepingUnsampledTransaction()`.
   resetAgent(1, function (data) {
     t.strictEqual(data.transactions.length, 1)
 

test/integration/allow-invalid-cert.test.js

@@ -4,11 +4,12 @@ var getPort = require('get-port')
 
 getPort().then(function (port) {
   var agent = require('../../').start({
-    serviceName: 'test',
+    serviceName: 'test-allow-invalid-cert',
     serverUrl: 'https://localhost:' + port,
     captureExceptions: false,
     metricsInterval: 0,
     centralConfig: false,
+    apmServerVersion: '8.0.0',
     disableInstrumentations: ['https'], // avoid the agent instrumenting the mock APM Server
     verifyServerCert: false
   })
@@ -27,7 +28,7 @@ getPort().then(function (port) {
 
     server.listen(port, function () {
       agent.captureError(new Error('boom!'), function (err) {
-        t.error(err)
+        t.error(err, 'no error in captureError')
         t.pass('agent.captureError callback called')
         server.close()
         agent.destroy()

test/integration/api-schema/basic.test.js

@@ -244,6 +244,7 @@ function newAgent (server) {
     serverUrl: 'http://localhost:' + server.address().port,
     captureExceptions: false,
     disableInstrumentations: ['http'],
+    apmServerVersion: '8.0.0',
     metricsInterval: 0,
     centralConfig: false
   })

test/integration/api-schema/capture-error-log-stack-traces.test.js

@@ -73,6 +73,7 @@ function newAgent (server) {
     disableInstrumentations: ['http'],
     captureErrorLogStackTraces: true,
     metricsInterval: 0,
+    apmServerVersion: '8.0.0',
     centralConfig: false
   })
 }

test/integration/api-schema/capture-span-stack-traces.test.js

@@ -69,6 +69,7 @@ function newAgent (server) {
     captureExceptions: false,
     disableInstrumentations: ['http'],
     captureSpanStackTraces: false,
+    apmServerVersion: '8.0.0',
     metricsInterval: 0,
     centralConfig: false
   })

test/integration/api-schema/source-lines-error-frames.test.js

@@ -73,6 +73,7 @@ function newAgent (server) {
     disableInstrumentations: ['http'],
     sourceLinesErrorAppFrames: 0,
     sourceLinesErrorLibraryFrames: 0,
+    apmServerVersion: '8.0.0',
     metricsInterval: 0,
     centralConfig: false
   })

test/integration/api-schema/source-lines-span-frames.test.js

@@ -70,6 +70,7 @@ function newAgent (server) {
     disableInstrumentations: ['http'],
     sourceLinesSpanAppFrames: 5,
     sourceLinesSpanLibraryFrames: 5,
+    apmServerVersion: '8.0.0',
     metricsInterval: 0,
     centralConfig: false
   })

test/integration/no-sampling.test.js

@@ -5,11 +5,12 @@ var ndjson = require('ndjson')
 
 getPort().then(function (port) {
   var agent = require('../../').start({
-    serviceName: 'test',
+    serviceName: 'test-no-sampling',
     serverUrl: 'http://localhost:' + port,
     captureExceptions: false,
     metricsInterval: 0,
     centralConfig: false,
+    apmServerVersion: '8.0.0',
     disableInstrumentations: ['http'], // avoid the agent instrumenting the mock APM Server
     apiRequestTime: '1s'
   })

test/integration/server-url-path.test.js

@@ -9,6 +9,7 @@ getPort().then(function (port) {
     captureExceptions: false,
     metricsInterval: 0,
     centralConfig: false,
+    apmServerVersion: '8.0.0',
     disableInstrumentations: ['http'] // avoid the agent instrumenting the mock APM Server
   })
 

test/integration/skip-internal-http-spans.test.js

@@ -10,6 +10,7 @@ getPort().then(port => {
     serverUrl: 'http://localhost:' + port,
     captureExceptions: false,
     metricsInterval: 0,
+    apmServerVersion: '8.0.0',
     centralConfig: false
   })
 

test/integration/socket-close.test.js

@@ -9,6 +9,7 @@ getPort().then(function (port) {
     captureExceptions: false,
     metricsInterval: 0,
     centralConfig: false,
+    apmServerVersion: '8.0.0',
     disableInstrumentations: ['http'] // avoid the agent instrumenting the mock APM Server
   })
 

test/integration/verify-server-ca-cert.test.js

@@ -12,6 +12,7 @@ getPort().then(function (port) {
     captureExceptions: false,
     metricsInterval: 0,
     centralConfig: false,
+    apmServerVersion: '8.0.0',
     disableInstrumentations: ['https'], // avoid the agent instrumenting the mock APM Server
     serverCaCertFile: path.join(__dirname, 'cert.pem') // self-signed certificate
   })