feat: support OTel span.addLink, span.addLinks; add similar APIs to the APM agent API (#4078)

trentm · web-flow · commit c884fc9ca88c · 2024-06-13T08:00:13.000-07:00
This also updates a few places for the new otel/api v1.9.0. Refs: #4071 (the dependabot update doesn't get everything) Refs: #4070 Refs: #4069 Refs: #4077 (a separate issue for this other new feature in otel/api@1.9.0)
diff --git a/docs/span-api.asciidoc b/docs/span-api.asciidoc
@@ -242,3 +242,33 @@ If false-y values (e.g. `null`) are given for both `type` and `name`, then `serv
 If this method is not called, the service target values are inferred from other span fields (https://github.com/elastic/apm/blob/main/specs/agents/tracing-spans-service-target.md#field-values[spec]).
 
 `service.target.*` fields are ignored for APM Server before v8.3.
+
+[[span-addlink]]
+==== `span.addLink(link)`
+
+[small]#Added in: REPLACEME#
+
+* `link` +{type-link}+
+
+A span can refer to zero or more other transactions or spans (separate
+from its parent). Span links will be shown in the Kibana APM app trace view. The
+`link` argument is an object with a single "context" field that is a
+`Transaction`, `Span`, OpenTelemetry `SpanContext` object, or W3C trace-context
+'traceparent' string.
+For example: `span.addLink({ context: anotherSpan })`.
+
+[[span-addlinks]]
+==== `span.addLinks([links])`
+
+[small]#Added in: REPLACEME#
+
+* `links` +{type-array}+ Span links.
+
+Add span links to this span.
+
+A span can refer to zero or more other transactions or spans (separate
+from its parent). Span links will be shown in the Kibana APM app trace view. The
+`link` argument is an object with a single "context" field that is a
+`Transaction`, `Span`, OpenTelemetry `SpanContext` object, or W3C trace-context
+'traceparent' string.
+For example: `span.addLinks([{ context: anotherSpan }])`.
diff --git a/docs/supported-technologies.asciidoc b/docs/supported-technologies.asciidoc
@@ -84,7 +84,7 @@ Metrics API and Metrics SDK to allow
 [options="header"]
 |=======================================================================
 | Framework | Version
-| <<opentelemetry-bridge,@opentelemetry/api>> | >=1.0.0 <1.9.0
+| <<opentelemetry-bridge,@opentelemetry/api>> | >=1.0.0 <1.10.0
 | https://www.npmjs.com/package/@opentelemetry/sdk-metrics[@opentelemetry/sdk-metrics] | >=1.11.0 <2
 |=======================================================================
 
diff --git a/docs/transaction-api.asciidoc b/docs/transaction-api.asciidoc
@@ -308,3 +308,33 @@ Non-HTTP transactions will begin with an outcome of `unknown`.
 * `outcome` +{type-string}+
 
 The `setOutcome` method allows an end user to override the Node.js agent's default setting of a transaction's `outcome` property.  The `setOutcome` method accepts a string of either `success`, `failure`, or `unknown`, and will force the agent to report this value for a specific span.
+
+[[transaction-addlink]]
+==== `transaction.addLink(link)`
+
+[small]#Added in: REPLACEME#
+
+* `link` +{type-link}+
+
+A transaction can refer to zero or more other transactions or spans (separate
+from its parent). Span links will be shown in the Kibana APM app trace view. The
+`link` argument is an object with a single "context" field that is a
+`Transaction`, `Span`, OpenTelemetry `SpanContext` object, or W3C trace-context
+'traceparent' string.
+For example: `transaction.addLink({ context: anotherSpan })`.
+
+[[transaction-addlinks]]
+==== `transaction.addLinks([links])`
+
+[small]#Added in: REPLACEME#
+
+* `links` +{type-array}+ Span links.
+
+Add span links to this transaction.
+
+A transaction can refer to zero or more other transactions or spans (separate
+from its parent). Span links will be shown in the Kibana APM app trace view. The
+`link` argument is an object with a single "context" field that is a
+`Transaction`, `Span`, OpenTelemetry `SpanContext` object, or W3C trace-context
+'traceparent' string.
+For example: `transaction.addLinks([{ context: anotherSpan }])`.
diff --git a/index.d.ts b/index.d.ts
@@ -150,6 +150,8 @@ declare namespace apm {
     setLabel (name: string, value: LabelValue, stringify?: boolean): boolean;
     addLabels (labels: Labels, stringify?: boolean): boolean;
     setOutcome(outcome: Outcome): void;
+    addLink (link: Link): void;
+    addLinks (links: Link[]): void;
 
     startSpan(
       name?: string | null,
@@ -201,6 +203,8 @@ declare namespace apm {
     addLabels (labels: Labels, stringify?: boolean): boolean;
     setOutcome(outcome: Outcome): void;
     setServiceTarget(type?: string | null, name?: string | null): void;
+    addLink (link: Link): void;
+    addLinks (links: Link[]): void;
     end (endTime?: number): void;
   }
 
@@ -349,8 +353,8 @@ declare namespace apm {
   // equivalent APIs in "opentelemetry-js-api/src/trace/link.ts". Currently
   // span link attributes are not supported.
   export interface Link {
-    /** A W3C trace-context 'traceparent' string, Transaction, or Span. */
-    context: Transaction | Span | string; // This is a SpanContext in OTel.
+    /** A W3C trace-context 'traceparent' string, Transaction, Span, or OTel SpanContext. */
+    context: Transaction | Span | {traceId: string, spanId: string} | string;
   }
 
   export interface TransactionOptions {
diff --git a/lib/instrumentation/generic-span.js b/lib/instrumentation/generic-span.js
@@ -170,28 +170,31 @@ GenericSpan.prototype.addLabels = function (labels, stringify) {
   return true;
 };
 
-// This method is private because the APM agents spec says that (for OTel
-// compat), adding links after span creation should not be allowed.
-// https://github.com/elastic/apm/blob/main/specs/agents/span-links.md
-//
-// To support adding span links for SQS ReceiveMessage and equivalent, the
-// message data isn't known until the *response*, after the span has been
-// created.
+// Add span links.
 //
 // @param {Array} links - An array of objects with a `context` property that is
-//    a Transaction, Span, or TraceParent instance, or a W3C trace-context
-//    'traceparent' string.
-GenericSpan.prototype._addLinks = function (links) {
+//    a Transaction, Span, or TraceParent instance; an OTel SpanContext object;
+//    or a W3C trace-context 'traceparent' string.
+GenericSpan.prototype.addLinks = function (links) {
   if (links) {
     for (let i = 0; i < links.length; i++) {
-      const link = linkFromLinkArg(links[i]);
-      if (link) {
-        this._links.push(link);
-      }
+      this.addLink(links[i]);
     }
   }
 };
 
+// Add a span link.
+//
+// @param {Link} link - An object with a `context` property that is
+//    a Transaction, Span, or TraceParent instance; an OTel SpanContext object;
+//    or a W3C trace-context 'traceparent' string.
+GenericSpan.prototype.addLink = function (linkArg) {
+  const link = linkFromLinkArg(linkArg);
+  if (link) {
+    this._links.push(link);
+  }
+};
+
 GenericSpan.prototype._freezeOutcome = function () {
   this._isOutcomeFrozen = true;
 };
@@ -278,9 +281,9 @@ GenericSpan.prototype._serializeOTel = function (payload) {
 // span link as it will be serialized and sent to APM server. If the linkArg is
 // invalid, this will return null.
 //
-// @param {Object} linkArg - An object with a `context` property that is a
-//    Transaction, Span, or TraceParent instance, or a W3C trace-context
-//    'traceparent' string.
+// @param {Object} linkArg - An object with a `context` property that is
+//    a Transaction, Span, or TraceParent instance; an OTel SpanContext object;
+//    or a W3C trace-context 'traceparent' string.
 function linkFromLinkArg(linkArg) {
   if (!linkArg || !linkArg.context) {
     return null;
@@ -290,7 +293,13 @@ function linkFromLinkArg(linkArg) {
   let traceId;
   let spanId;
 
-  if (ctx._context instanceof TraceContext) {
+  if (ctx.traceId && ctx.spanId) {
+    // Duck-typing for an OTel SpanContext. APM intake v2 only supports the
+    // trace id and span id fields for span links, so we only need care about
+    // those attributes.
+    traceId = ctx.traceId;
+    spanId = ctx.spanId;
+  } else if (ctx._context instanceof TraceContext) {
     // Transaction or Span
     traceId = ctx._context.traceparent.traceId;
     spanId = ctx._context.traceparent.id;
diff --git a/lib/instrumentation/modules/@aws-sdk/client-sqs.js b/lib/instrumentation/modules/@aws-sdk/client-sqs.js
@@ -160,7 +160,7 @@ function sqsMiddlewareFactory(client, agent) {
             // Links
             const links = getSpanLinksFromResponseData(result && result.output);
             if (links) {
-              span._addLinks(links);
+              span.addLinks(links);
             }
 
             // Metrics
diff --git a/lib/instrumentation/modules/aws-sdk/sqs.js b/lib/instrumentation/modules/aws-sdk/sqs.js
@@ -320,7 +320,7 @@ function instrumentationSqs(
     if (receiveMsgData) {
       const links = getSpanLinksFromResponseData(receiveMsgData);
       if (links) {
-        span._addLinks(links);
+        span.addLinks(links);
       }
     }
 
diff --git a/lib/instrumentation/modules/kafkajs.js b/lib/instrumentation/modules/kafkajs.js
@@ -238,7 +238,7 @@ module.exports = function (mod, agent, { version, enabled }) {
             }
           }
         }
-        trans._addLinks(links);
+        trans.addLinks(links);
       }
 
       let result, err;
diff --git a/lib/lambda.js b/lib/lambda.js
@@ -383,7 +383,7 @@ function setSqsData(agent, trans, event, context, faasId, isColdStart) {
   trans.setCloudContext(cloudContext);
 
   const links = spanLinksFromSqsRecords(event.Records);
-  trans._addLinks(links);
+  trans.addLinks(links);
 }
 
 function setSnsData(agent, trans, event, context, faasId, isColdStart) {
@@ -424,7 +424,7 @@ function setSnsData(agent, trans, event, context, faasId, isColdStart) {
   trans.setCloudContext(cloudContext);
 
   const links = spanLinksFromSnsRecords(event.Records);
-  trans._addLinks(links);
+  trans.addLinks(links);
 }
 
 function setS3SingleData(trans, event, context, faasId, isColdStart) {
diff --git a/lib/opentelemetry-bridge/OTelBridgeNonRecordingSpan.js b/lib/opentelemetry-bridge/OTelBridgeNonRecordingSpan.js
@@ -127,6 +127,14 @@ class OTelBridgeNonRecordingSpan {
     return this;
   }
 
+  addLink(_link) {
+    return this;
+  }
+
+  addLinks(_links) {
+    return this;
+  }
+
   end(_endTime) {}
 
   // isRecording always returns false for NonRecordingSpan.
diff --git a/lib/opentelemetry-bridge/OTelSpan.js b/lib/opentelemetry-bridge/OTelSpan.js
@@ -167,6 +167,16 @@ class OTelSpan {
     return this;
   }
 
+  addLink(link) {
+    this._span.addLink(link);
+    return this;
+  }
+
+  addLinks(links) {
+    this._span.addLinks(links);
+    return this;
+  }
+
   end(otelEndTime) {
     oblog.apicall('%s.end(endTime=%s)', this, otelEndTime);
     const endTime =
diff --git a/test/instrumentation/span.test.js b/test/instrumentation/span.test.js
@@ -263,6 +263,38 @@ test('#addLabels', function (t) {
   t.end();
 });
 
+test('#addLink, #addLinks', function (t) {
+  var trans = new Transaction(agent);
+  var span = new Span(trans);
+
+  const theTraceId = '00000000000000000000000000000001';
+  span.addLink({
+    context: { traceId: theTraceId, spanId: '0000000000000002' },
+  });
+  t.deepEqual(span._links, [
+    {
+      trace_id: theTraceId,
+      span_id: '0000000000000002',
+    },
+  ]);
+
+  span.addLinks([
+    {
+      context: { traceId: theTraceId, spanId: '0000000000000003' },
+    },
+    {
+      context: { traceId: theTraceId, spanId: '0000000000000004' },
+    },
+  ]);
+  t.deepEqual(span._links, [
+    { trace_id: theTraceId, span_id: '0000000000000002' },
+    { trace_id: theTraceId, span_id: '0000000000000003' },
+    { trace_id: theTraceId, span_id: '0000000000000004' },
+  ]);
+
+  t.end();
+});
+
 test('span.sync', function (t) {
   var trans = agent.startTransaction();
 
@@ -528,6 +560,10 @@ test('Span API on ended span', function (t) {
   t.pass('span.addLabels(...) does not blow up');
   span.setOutcome('failure');
   t.pass('span.setOutcome(...) does not blow up');
+  span.addLink({ context: { traceId: '001', spanId: '002' } });
+  t.pass('span.addLink(...) does not blow up');
+  span.addLinks([{ context: { traceId: '001', spanId: '002' } }]);
+  t.pass('span.addLinks(...) does not blow up');
   span.end(42);
   t.pass('span.end(...) does not blow up');
 
diff --git a/test/instrumentation/transaction.test.js b/test/instrumentation/transaction.test.js
@@ -226,6 +226,37 @@ test('#addLabels', function (t) {
   t.end();
 });
 
+test('#addLink, #addLinks', function (t) {
+  var trans = new Transaction(agent);
+
+  const theTraceId = '00000000000000000000000000000001';
+  trans.addLink({
+    context: { traceId: theTraceId, spanId: '0000000000000002' },
+  });
+  t.deepEqual(trans._links, [
+    {
+      trace_id: theTraceId,
+      span_id: '0000000000000002',
+    },
+  ]);
+
+  trans.addLinks([
+    {
+      context: { traceId: theTraceId, spanId: '0000000000000003' },
+    },
+    {
+      context: { traceId: theTraceId, spanId: '0000000000000004' },
+    },
+  ]);
+  t.deepEqual(trans._links, [
+    { trace_id: theTraceId, span_id: '0000000000000002' },
+    { trace_id: theTraceId, span_id: '0000000000000003' },
+    { trace_id: theTraceId, span_id: '0000000000000004' },
+  ]);
+
+  t.end();
+});
+
 test('#startSpan()', function (t) {
   t.test('basic', function (t) {
     var trans = new Transaction(agent);
diff --git a/test/opentelemetry-bridge/.tav.yml b/test/opentelemetry-bridge/.tav.yml
@@ -1,5 +1,5 @@
 "@opentelemetry/api":
-  versions: '>=1.0.0 <1.9.0'
+  versions: '>=1.0.0 <1.10.0'
   node: '>=8.0.0'
   commands:
     - node OTelBridgeNonRecordingSpan.test.js
diff --git a/test/opentelemetry-bridge/OTelBridgeNonRecordingSpan.test.js b/test/opentelemetry-bridge/OTelBridgeNonRecordingSpan.test.js
@@ -78,6 +78,13 @@ tape.test('OTelBridgeNonRecordingSpan', (suite) => {
       'setStatus',
     );
     t.equal(nrsOTelSpan.updateName('anotherName'), nrsOTelSpan, 'updateName');
+    const linkContext = {
+      traceId: '8b46594050c89c3d87248476ed8e0c57',
+      spanId: 'ffe4cfa94865ee2a',
+      traceFlags: otel.TraceFlags.SAMPLED,
+    };
+    t.equal(nrsOTelSpan.addLink(linkContext), nrsOTelSpan, 'addLink');
+    t.equal(nrsOTelSpan.addLinks([linkContext]), nrsOTelSpan, 'addLinks');
     t.equal(nrsOTelSpan.end(), undefined, 'end');
     t.equal(nrsOTelSpan.isRecording(), false, 'isRecording');
     t.equal(
diff --git a/test/opentelemetry-bridge/fixtures.test.js b/test/opentelemetry-bridge/fixtures.test.js
@@ -376,6 +376,7 @@ const cases = [
         'sSetStatusChildERROR.outcome',
       );
 
+      // Span#updateName
       t.strictEqual(
         findObjInArray(
           events,
@@ -386,6 +387,23 @@ const cases = [
         'sUpdateName',
       );
 
+      // Span#addLink, Span#addLinks
+      t.deepEqual(
+        findObjInArray(events, 'transaction.name', 'sAddLinks').transaction
+          .links,
+        [
+          {
+            trace_id: '8b46594050c89c3d87248476ed8e0c57',
+            span_id: 'ffe4cfa94865ee2a',
+          },
+          {
+            trace_id: '8b46594050c89c3d87248476ed8e0c57',
+            span_id: 'ffe4cfa94865ee2a',
+          },
+        ],
+        'sAddLinks links',
+      );
+
       // Span#end
       function spanEndTimeIsApprox(transOrSpanName, t = Date.now()) {
         const foundTrans = findObjInArray(
diff --git a/test/opentelemetry-bridge/fixtures/interface-span.js b/test/opentelemetry-bridge/fixtures/interface-span.js
diff --git a/test/opentelemetry-metrics/fixtures/.tav.yml b/test/opentelemetry-metrics/fixtures/.tav.yml
diff --git a/test/opentelemetry-metrics/fixtures/package-lock.json b/test/opentelemetry-metrics/fixtures/package-lock.json
diff --git a/test/types/index.ts b/test/types/index.ts

Original file line number	Diff line number	Diff line change
`@@ -160,7 +160,7 @@ function sqsMiddlewareFactory(client, agent) {`
`160`	`160`	`// Links`
`161`	`161`	`const links = getSpanLinksFromResponseData(result && result.output);`
`162`	`162`	`if (links) {`
`163`		`- span._addLinks(links);`
	`163`	`+ span.addLinks(links);`
`164`	`164`	`}`
`165`	`165`
`166`	`166`	`// Metrics`
Original file line number	Diff line number	Diff line change
`@@ -320,7 +320,7 @@ function instrumentationSqs(`
`320`	`320`	`if (receiveMsgData) {`
`321`	`321`	`const links = getSpanLinksFromResponseData(receiveMsgData);`
`322`	`322`	`if (links) {`
`323`		`- span._addLinks(links);`
	`323`	`+ span.addLinks(links);`
`324`	`324`	`}`
`325`	`325`	`}`
`326`	`326`
Original file line number	Diff line number	Diff line change
`@@ -238,7 +238,7 @@ module.exports = function (mod, agent, { version, enabled }) {`
`238`	`238`	`}`
`239`	`239`	`}`
`240`	`240`	`}`
`241`		`- trans._addLinks(links);`
	`241`	`+ trans.addLinks(links);`
`242`	`242`	`}`
`243`	`243`
`244`	`244`	`let result, err;`
Original file line number	Diff line number	Diff line change
`@@ -383,7 +383,7 @@ function setSqsData(agent, trans, event, context, faasId, isColdStart) {`
`383`	`383`	`trans.setCloudContext(cloudContext);`
`384`	`384`
`385`	`385`	`const links = spanLinksFromSqsRecords(event.Records);`
`386`		`- trans._addLinks(links);`
	`386`	`+ trans.addLinks(links);`
`387`	`387`	`}`
`388`	`388`
`389`	`389`	`function setSnsData(agent, trans, event, context, faasId, isColdStart) {`
`@@ -424,7 +424,7 @@ function setSnsData(agent, trans, event, context, faasId, isColdStart) {`
`424`	`424`	`trans.setCloudContext(cloudContext);`
`425`	`425`
`426`	`426`	`const links = spanLinksFromSnsRecords(event.Records);`
`427`		`- trans._addLinks(links);`
	`427`	`+ trans.addLinks(links);`
`428`	`428`	`}`
`429`	`429`
`430`	`430`	`function setS3SingleData(trans, event, context, faasId, isColdStart) {`