fix so that an ended span is never used as a parent

Author: trentm

lib/instrumentation/index.js

@@ -99,17 +99,8 @@ Instrumentation.prototype.currTx = function () {
   return this._runCtxMgr.active().tx || null
 }
 Instrumentation.prototype.currSpan = function () {
-  const spanStack = this._runCtxMgr.active().spans
-  if (spanStack.length === 0) {
-    return null
-  } else {
-    return spanStack[spanStack.length - 1]
-  }
+  return this._runCtxMgr.active().currSpan()
 }
-// XXX unneeded?
-// Instrumentation.prototype.currParent = function () {
-//   return this._runCtxMgr.active().topSpanOrTx()
-// }
 
 Object.defineProperty(Instrumentation.prototype, 'ids', {
   get () {
@@ -287,7 +278,7 @@ Instrumentation.prototype.addEndedSpan = function (span) {
   }
 
   const rc = this._runCtxMgr.active()
-  if (rc.topSpanOrTx() === span) {
+  if (rc.topSpan() === span) {
     // Replace the active run context with this span popped off the stack,
     // i.e. this span is no longer active.
     this._runCtxMgr.replaceActive(rc.exitSpan())
@@ -325,6 +316,7 @@ Instrumentation.prototype.startTransaction = function (name, ...args) {
   return tx
 }
 
+// XXX !
 Instrumentation.prototype.endTransaction = function (result, endTime) {
   if (!this.currentTransaction) {
     this._agent.logger.debug('cannot end transaction - no active transaction found')

lib/run-context/BasicRunContextManager.js

@@ -48,11 +48,25 @@ class RunContext {
     return !this.tx
   }
 
-  topSpanOrTx () {
+  // Returns the currently active span, if any.
+  //
+  // Because the `startSpan()/endSpan()` API allows (a) affecting the current
+  // run context and (b) out of order start/end, the "currently active span"
+  // must skip over ended spans.
+  currSpan () {
+    for (let i = this.spans.length - 1; i >= 0; i--) {
+      const span = this.spans[i]
+      if (!span.ended) {
+        return span
+      }
+    }
+    return null
+  }
+
+  // This returns the top span in the span stack (even if it is ended).
+  topSpan () {
     if (this.spans.length > 0) {
       return this.spans[this.spans.length - 1]
-    } else if (this.tx) {
-      return this.tx
     } else {
       return null
     }
@@ -62,6 +76,12 @@ class RunContext {
   // stack.
   enterSpan (span) {
     const newSpans = this.spans.slice()
+
+    // Any ended spans at the top of the stack are cruft -- remove them.
+    while (newSpans.length > 0 && newSpans[newSpans.length - 1].ended) {
+      newSpans.pop()
+    }
+
     newSpans.push(span)
     return new RunContext(this.tx, newSpans)
   }