feat: introduce "exit spans" and mark S3 spans as exit spans

Per https://github.com/elastic/apm/blob/main/specs/agents/tracing-spans.md#exit-spans
the result is that HTTP child spans of S3 spans are no longer captured.

Closes: #2125

CHANGELOG.asciidoc

@@ -49,6 +49,11 @@ Notes:
 * Add `transaction.name` to captured APM errors. This will allow the Kibana APM
   app to correlate error groups and transaction groups. ({issues}2456[#2456])
 
+* Mark S3 spans (from 'aws-sdk' instrumentation) as exit spans (per
+  https://github.com/elastic/apm/blob/main/specs/agents/tracing-spans.md#exit-spans).
+  The result is that HTTP child spans of S3 spans are no longer captured.
+  ({issues}2125[#2125])
+
 [float]
 ===== Bug fixes
 

docs/agent-api.asciidoc

@@ -643,6 +643,12 @@ Sub-millisecond precision can be achieved using decimals.
 If not provided,
 the current time will be used
 
+** `exitSpan` +{type-boolean}+ Make an "exit span".
+Exit spans represent outgoing communication. They are used to create a node
+in the {kibana-ref}/service-maps.html[Service Map] and a downstream service
+in the {kibana-ref}/dependencies.html[Dependencies Table]. The provided subtype
+will be used as the downstream service name.
+
 Start and return a new custom span associated with the current active transaction.
 This is the same as getting the current transaction with `apm.currentTransaction` and,
 if a transaction was found,

docs/transaction-api.asciidoc

@@ -113,6 +113,12 @@ Sub-millisecond precision can be achieved using decimals.
 If not provided,
 the current time will be used
 
+** `exitSpan` +{type-boolean}+ Make an "exit span".
+Exit spans represent outgoing communication. They are used to create a node
+in the {kibana-ref}/service-maps.html[Service Map] and a downstream service
+in the {kibana-ref}/dependencies.html[Dependencies Table]. The provided subtype
+will be used as the downstream service name.
+
 Start and return a new custom span associated with this transaction.
 When a span is started it will measure the time until <<span-end,`span.end()`>> is called.
 

index.d.ts

@@ -347,6 +347,7 @@ declare namespace apm {
   export interface SpanOptions {
     startTime?: number;
     childOf?: Transaction | Span | string;
+    exitSpan?: boolean;
   }
 
   type CaptureBody = 'off' | 'errors' | 'transactions' | 'all';

lib/agent.js

@@ -171,7 +171,7 @@ Agent.prototype.setTransactionOutcome = function (outcome) {
   return this._instrumentation.setTransactionOutcome.apply(this._instrumentation, arguments)
 }
 
-Agent.prototype.startSpan = function (name, type, subtype, action, { childOf } = {}) {
+Agent.prototype.startSpan = function (name, type, subtype, action, { startTime, childOf, exitSpan } = {}) {
   return this._instrumentation.startSpan.apply(this._instrumentation, arguments)
 }
 

lib/instrumentation/http-shared.js

@@ -146,12 +146,12 @@ exports.traceOutgoingRequest = function (agent, moduleName, method) {
 
       if (!options.headers) options.headers = {}
 
-      // Attempt to use the span context as a traceparent header.
-      // If the transaction is unsampled the span will not exist,
-      // however a traceparent header must still be propagated
-      // to indicate requested services should not be sampled.
-      // Use the transaction context as the parent, in this case.
-      var parent = span || ins.currTransaction()
+      // W3C trace-context propagation.
+      // There are a number of reasons why `span` might be null: child of an
+      // exit span, `transactionMaxSpans` was hit, unsampled transaction, etc.
+      // If so, then fallback to the current run context's span or transaction,
+      // if any.
+      var parent = span || parentRunContext.currSpan() || parentRunContext.currTransaction()
       if (parent && parent._context) {
         const headerValue = parent._context.toTraceParentString()
         const traceStateValue = parent._context.toTraceStateString()

lib/instrumentation/index.js

@@ -83,7 +83,7 @@ Instrumentation.prototype.currTransaction = function () {
   if (!this._started) {
     return null
   }
-  return this._runCtxMgr.active().currTransaction() || null
+  return this._runCtxMgr.active().currTransaction()
 }
 
 Instrumentation.prototype.currSpan = function () {

lib/instrumentation/modules/aws-sdk/s3.js

@@ -54,7 +54,7 @@ function instrumentationS3 (orig, origArguments, request, AWS, agent, { version,
 
   const ins = agent._instrumentation
 
-  const span = ins.createSpan(name, TYPE, SUBTYPE, opName)
+  const span = ins.createSpan(name, TYPE, SUBTYPE, opName, { exitSpan: true })
   if (!span) {
     return orig.apply(request, origArguments)
   }

lib/instrumentation/span.js

@@ -17,18 +17,21 @@ module.exports = Span
 util.inherits(Span, GenericSpan)
 
 function Span (transaction, name, ...args) {
-  const defaultChildOf = transaction._agent._instrumentation.currSpan() || transaction
   const opts = typeof args[args.length - 1] === 'object'
     ? (args.pop() || {})
     : {}
 
   if (!opts.childOf) {
+    const defaultChildOf = transaction._agent._instrumentation.currSpan() || transaction
     opts.childOf = defaultChildOf
     opts.timer = defaultChildOf._timer
   } else if (opts.childOf._timer) {
     opts.timer = opts.childOf._timer
   }
 
+  this._exitSpan = !!opts.exitSpan
+  delete opts.exitSpan
+
   GenericSpan.call(this, transaction._agent, ...args, opts)
 
   this._db = null

lib/instrumentation/transaction.js

@@ -116,8 +116,8 @@ Transaction.prototype.setCloudContext = function (cloudContext) {
 }
 
 // Create a span on this transaction and make it the current span.
-Transaction.prototype.startSpan = function (...spanArgs) {
-  const span = this.createSpan(...spanArgs)
+Transaction.prototype.startSpan = function (...args) {
+  const span = this.createSpan(...args)
   if (span) {
     this._agent._instrumentation.supersedeWithSpanRunContext(span)
   }
@@ -129,7 +129,7 @@ Transaction.prototype.startSpan = function (...spanArgs) {
 // This does *not* replace the current run context to make this span the
 // "current" one. This allows instrumentations to avoid impacting the run
 // context of the calling code. Compare to `startSpan`.
-Transaction.prototype.createSpan = function (...spanArgs) {
+Transaction.prototype.createSpan = function (...args) {
   if (!this.sampled) {
     return null
   }
@@ -142,8 +142,23 @@ Transaction.prototype.createSpan = function (...spanArgs) {
     return null
   }
 
+  // Exit spans must not have child spans. The spec allows child spans "that
+  // have the same type and subtype", but this agent currently has no use for
+  // this case.
+  // https://github.com/elastic/apm/blob/master/specs/agents/tracing-spans.md#child-spans-of-exit-spans
+  const opts = typeof args[args.length - 1] === 'object'
+    ? (args.pop() || {})
+    : {}
+  opts.childOf = opts.childOf || this._agent._instrumentation.currSpan() || this
+  const childOf = opts.childOf
+  if (childOf instanceof Span && childOf._exitSpan) {
+    this._agent.logger.trace({ exitSpanId: childOf.id, newSpanArgs: args },
+      'createSpan: drop child span of exit span')
+    return null
+  }
+
   this._builtSpans++
-  return new Span(this, ...spanArgs)
+  return new Span(this, ...args, opts)
 }
 
 // Note that this only returns a complete result when called *during* the call

test/instrumentation/modules/aws-sdk/s3.test.js

@@ -61,14 +61,9 @@ tape.test('simple S3 usage scenario', function (t) {
         const tx = events.shift().transaction
         const errors = events.filter(e => e.error).map(e => e.error)
 
-        // Currently HTTP spans under each S3 span are included. Eventually
-        // those will be excluded. Filter those out for now.
-        // https://github.com/elastic/apm-agent-nodejs/issues/2125
+        // Compare some common fields across all spans.
         const spans = events.filter(e => e.span)
           .map(e => e.span)
-          .filter(e => e.subtype !== 'http')
-
-        // Compare some common fields across all spans.
         t.equal(spans.filter(s => s.trace_id === tx.trace_id).length,
           spans.length, 'all spans have the same trace_id')
         t.equal(spans.filter(s => s.transaction_id === tx.id).length,

test/types/index.ts

@@ -106,6 +106,7 @@ apm.startSpan('foo', 'type', { childOf: 'baz' })
 apm.startSpan('foo', 'type', 'subtype', { childOf: 'baz' })
 apm.startSpan('foo', 'type', 'subtype', 'action', { childOf: 'baz' })
 apm.startSpan('foo', 'type', 'subtype', 'action', { startTime: 42 })
+apm.startSpan('foo', 'type', 'subtype', { exitSpan: true })
 
 apm.setLabel('foo', 'bar')
 apm.setLabel('foo', 1)
@@ -180,10 +181,13 @@ apm.logger.fatal('')
     trans.startSpan('foo', 'type', { childOf: 'baz' })
     trans.startSpan('foo', 'type', 'subtype', { childOf: 'baz' })
     trans.startSpan('foo', 'type', 'subtype', 'action', { childOf: 'baz' })
+    trans.startSpan('foo', 'type', 'subtype', { exitSpan: true })
 
     function ensureParentId (id: string) {}
     ensureParentId(trans.ensureParentId())
 
+    trans.setOutcome('failure')
+
     trans.end()
     trans.end('foo')
     trans.end('foo', 42)
@@ -205,6 +209,8 @@ apm.logger.fatal('')
 
       span.addLabels({ s: 'bar', n: 42, b: false })
 
+      span.setOutcome('failure')
+
       span.end()
       span.end(42)
     }