feat: span statistics -- ensure dropped span objects are still created

lib/instrumentation/index.js

@@ -385,6 +385,10 @@ Instrumentation.prototype.addEndedSpan = function (span) {
 }
 
 Instrumentation.prototype._encodeAndSendSpan = function (span) {
+  if (!span.isRecorded()) {
+    return
+  }
+
   const agent = this._agent
   // Note this error as an "inflight" event. See Agent#flush().
   const inflightEvents = agent._inflightEvents

lib/instrumentation/span.js

@@ -301,6 +301,14 @@ Span.prototype.tryToCompress = function (spanToCompress) {
   return this._compression.tryToCompress(this, spanToCompress)
 }
 
+Span.prototype.isRecorded = function () {
+  return this._context.isRecorded()
+}
+
+Span.prototype.setRecorded = function (value) {
+  return this._context.setRecorded(value)
+}
+
 function filterCallSite (callsite) {
   var filename = callsite.getFileName()
   return filename ? filename.indexOf('/node_modules/elastic-apm-node/') === -1 : true

lib/instrumentation/transaction.js

@@ -133,10 +133,6 @@ Transaction.prototype.createSpan = function (...args) {
   if (!this.sampled) {
     return null
   }
-  if (this._builtSpans >= this._agent._conf.transactionMaxSpans) {
-    this._droppedSpans++
-    return null
-  }
 
   // Exit spans must not have child spans (unless of the same type and subtype).
   // https://github.com/elastic/apm/blob/master/specs/agents/tracing-spans.md#child-spans-of-exit-spans
@@ -153,8 +149,15 @@ Transaction.prototype.createSpan = function (...args) {
     return null
   }
 
+  const span = new Span(this, ...args, opts)
+
+  if (this._builtSpans >= this._agent._conf.transactionMaxSpans) {
+    this._droppedSpans++
+    span.setRecorded(false)
+  }
+
   this._builtSpans++
-  return new Span(this, ...args, opts)
+  return span
 }
 
 // Note that this only returns a complete result when called *during* the call
@@ -172,7 +175,7 @@ Transaction.prototype.toJSON = function () {
     sampled: this.sampled,
     context: undefined,
     span_count: {
-      started: this._builtSpans
+      started: this._builtSpans - this._droppedSpans
     },
     outcome: this.outcome,
     faas: this._faas
@@ -191,7 +194,6 @@ Transaction.prototype.toJSON = function () {
       cloud: this._cloud || {},
       message: this._message || {}
     }
-
     // Only include dropped count when spans have been dropped.
     if (this._droppedSpans > 0) {
       payload.span_count.dropped = this._droppedSpans

lib/opentelemetry-bridge/OTelTracer.js

@@ -74,9 +74,9 @@ class OTelTracer {
       }
       newTransOrSpan = trans.createSpan(name, spanOpts)
 
-      // There might be no span, e.g. if transactionMaxSpans was hit. We have
-      // to return some OTelSpan, and we also want to propagate the parent's
-      // trace-context, if any.
+      // There might be no span, e.g. if the span is a child of an exit span. We
+      // have to return some OTelSpan, and we also want to propagate the
+      // parent's trace-context, if any.
       if (!newTransOrSpan) {
         return otel.trace.wrapSpanContext(otelSpanContextFromTraceContext(parentGenericSpan._context))
       }

lib/tracecontext/index.js

@@ -86,6 +86,10 @@ class TraceContext {
   setRecorded () {
     return this.traceparent.setRecorded()
   }
+
+  isRecorded () {
+    return this.traceparent.recorded
+  }
 }
 
 TraceContext.FLAGS = TraceParent.FLAGS

test/instrumentation/transaction.test.js

@@ -449,7 +449,7 @@ test('#_encode() - dropped spans', function (t) {
   var span0 = trans.startSpan('s0', 'type0')
   trans.startSpan('s1', 'type1')
   var span2 = trans.startSpan()
-  if (span2) {
+  if (span2.isRecorded()) {
     t.fail('should have dropped the span')
   }
   span0.end()

test/opentelemetry-bridge/fixtures.test.js

@@ -162,15 +162,11 @@ const cases = [
   {
     // Expected trace:
     //   trace $traceId
-    //   `- transaction $myTransId "myTrans"
-    //     `- span "s0"
+    // `- transaction $myTransId "myTrans"
+    //   `- span "s0"
     //       `- span "GET localhost:$port" (http)
-    //         `- transaction "GET unknown route"
-    //     `- span "s1"                           // This is the 3rd (max) span.
-    //       `- transaction "GET unknown route"
-    //     `- transaction "GET unknown route"
-    //     `- transaction "GET unknown route"
-    //     `- transaction "GET unknown route"
+    //           `- transaction  "GET unknown route"
+    //   `- span "s1" // This is the 3rd (max) span.
     script: 'hit-transaction-max-spans.js',
     testOpts: {
       // - This fixture fails with node.js [10.0, 10.4) due to an async context
@@ -184,7 +180,7 @@ const cases = [
       ELASTIC_APM_TRANSACTION_MAX_SPANS: '3'
     },
     check: (t, events) => {
-      t.equal(events.length, 10, 'exactly 10 events')
+      t.equal(events.length, 6, 'exactly 6 events')
       t.ok(events[0].metadata, 'APM server got event metadata object')
 
       function assertIncomingHttpTrans (trans, parentId) {
@@ -207,7 +203,7 @@ const cases = [
       //   `- transaction $myTransId "myTrans"
       const myTrans = tas[0].transaction
       t.equal(myTrans.name, 'myTrans', 'myTrans.name')
-      t.deepEqual(myTrans.span_count, { started: 3, dropped: 4 }, 'myTrans.span_count')
+      t.deepEqual(myTrans.span_count, { started: 3, dropped: 7 }, 'myTrans.span_count')
       //     `- span "s0"
       const s0 = tas[1].span
       t.equal(s0.name, 's0', 's0')
@@ -221,14 +217,6 @@ const cases = [
       const s1 = tas[4].span
       t.equal(s1.name, 's1', 's1')
       t.equal(s1.parent_id, myTrans.id, 's1.parent_id')
-      //       `- transaction "GET unknown route"
-      assertIncomingHttpTrans(tas[5].transaction, tas[4].span.id)
-      //     `- transaction "GET unknown route"
-      //     `- transaction "GET unknown route"
-      //     `- transaction "GET unknown route"
-      for (let i = 6; i < 9; i++) {
-        assertIncomingHttpTrans(tas[i].transaction, myTrans.id)
-      }
     }
   },