fix: allow recording links only at Span creation time

open-telemetry · Oct 23, 2019 · a2b91e7 · a2b91e7
1 parent b21ed97
commit a2b91e7
Show file tree

Hide file tree

Showing 6 changed files with 30 additions and 54 deletions.
diff --git a/packages/opentelemetry-shim-opentracing/src/shim.ts b/packages/opentelemetry-shim-opentracing/src/shim.ts
@@ -41,7 +41,10 @@ function translateSpanOptions(
     startTime: options.startTime,
   };
 
-  // because there's no `Links` in SpanOptions, we set them in `TracerShim.startSpan()`
+  if (options.references) {
+    opts.links = translateReferences(options.references);
+  }
+
   if (options.childOf) {
     if (options.childOf instanceof SpanShim) {
       opts.parent = (options.childOf as SpanShim).getSpan();
@@ -111,13 +114,6 @@ export class TracerShim extends opentracing.Tracer {
       span.setAttributes(options.tags);
     }
 
-    if (options.references) {
-      const links = translateReferences(options.references);
-      for (const link of links) {
-        span.addLink(link.spanContext, link.attributes);
-      }
-    }
-
     return new SpanShim(this, span);
   }
 

diff --git a/packages/opentelemetry-tracing/src/BasicTracer.ts b/packages/opentelemetry-tracing/src/BasicTracer.ts
@@ -100,6 +100,7 @@ export class BasicTracer implements types.Tracer {
       spanContext,
       options.kind || types.SpanKind.INTERNAL,
       parentContext ? parentContext.spanId : undefined,
+      options.links || [],
       options.startTime
     );
     // Set default attributes

diff --git a/packages/opentelemetry-tracing/src/Span.ts b/packages/opentelemetry-tracing/src/Span.ts
@@ -57,12 +57,14 @@ export class Span implements types.Span, ReadableSpan {
     spanContext: types.SpanContext,
     kind: types.SpanKind,
     parentSpanId?: string,
+    links: types.Link[] = [],
     startTime: types.TimeInput = hrTime()
   ) {
     this.name = spanName;
     this.spanContext = spanContext;
     this.parentSpanId = parentSpanId;
     this.kind = kind;
+    this.links = links;
     this.startTime = timeInputToHrTime(startTime);
     this._logger = parentTracer.logger;
     this._traceParams = parentTracer.getActiveTraceParams();
@@ -134,17 +136,6 @@ export class Span implements types.Span, ReadableSpan {
     return this;
   }
 
-  addLink(spanContext: types.SpanContext, attributes?: types.Attributes): this {
-    if (this._isSpanEnded()) return this;
-
-    if (this.links.length >= this._traceParams.numberOfLinksPerSpan!) {
-      this._logger.warn('Dropping extra links.');
-      this.links.shift();
-    }
-    this.links.push({ spanContext, attributes });
-    return this;
-  }
-
   setStatus(status: types.Status): this {
     if (this._isSpanEnded()) return this;
     this.status = status;

diff --git a/packages/opentelemetry-tracing/test/Span.test.ts b/packages/opentelemetry-tracing/test/Span.test.ts
@@ -167,22 +167,22 @@ describe('Span', () => {
       spanId: '5e0c63257de34c92',
       traceFlags: TraceFlags.SAMPLED,
     };
-    const span = new Span(tracer, name, spanContext, SpanKind.CLIENT);
-    span.addLink(spanContext);
-    span.addLink(spanContext, { attr1: 'value', attr2: 123, attr3: true });
+    const attributes = { attr1: 'value', attr2: 123, attr3: true };
+    const span = new Span(tracer, name, spanContext, SpanKind.CLIENT, '12345', [
+      { spanContext },
+      { spanContext, attributes },
+    ]);
     span.end();
   });
 
   it('should drop extra links, attributes and events', () => {
     const span = new Span(tracer, name, spanContext, SpanKind.CLIENT);
     for (let i = 0; i < 150; i++) {
-      span.addLink(spanContext);
       span.setAttribute('foo' + i, 'bar' + i);
       span.addEvent('sent' + i);
     }
     span.end();
 
-    assert.strictEqual(span.links.length, 32);
     assert.strictEqual(span.events.length, 128);
     assert.strictEqual(Object.keys(span.attributes).length, 32);
     assert.strictEqual(span.events[span.events.length - 1].name, 'sent149');
@@ -246,27 +246,24 @@ describe('Span', () => {
   });
 
   it('should return ReadableSpan with links', () => {
-    const span = new Span(tracer, 'my-span', spanContext, SpanKind.CLIENT);
-    span.addLink(spanContext);
-    let readableSpan = span.toReadableSpan();
-    assert.strictEqual(readableSpan.links.length, 1);
-    assert.deepStrictEqual(readableSpan.links, [
-      {
-        attributes: undefined,
-        spanContext: {
-          spanId: '6e0c63257de34c92',
-          traceId: 'd4cda95b652f4a1592b449d5929fda1b',
-          traceFlags: 1,
+    const span = new Span(
+      tracer,
+      'my-span',
+      spanContext,
+      SpanKind.CLIENT,
+      undefined,
+      [
+        { spanContext },
+        {
+          spanContext,
+          attributes: { attr1: 'value', attr2: 123, attr3: true },
         },
-      },
-    ]);
-
-    span.addLink(spanContext, { attr1: 'value', attr2: 123, attr3: true });
-    readableSpan = span.toReadableSpan();
+      ]
+    );
+    const readableSpan = span.toReadableSpan();
     assert.strictEqual(readableSpan.links.length, 2);
     assert.deepStrictEqual(readableSpan.links, [
       {
-        attributes: undefined,
         spanContext,
       },
       {
@@ -276,10 +273,6 @@ describe('Span', () => {
     ]);
 
     span.end();
-    // shouldn't add new link
-    span.addLink(spanContext);
-    readableSpan = span.toReadableSpan();
-    assert.strictEqual(readableSpan.links.length, 2);
   });
 
   it('should return ReadableSpan with events', () => {

diff --git a/packages/opentelemetry-types/src/trace/SpanOptions.ts b/packages/opentelemetry-types/src/trace/SpanOptions.ts
@@ -18,6 +18,7 @@ import { Span } from './span';
 import { Attributes } from './attributes';
 import { SpanKind } from './span_kind';
 import { SpanContext } from './span_context';
+import { Link } from './link';
 
 /**
  * Options needed for span creation
@@ -32,6 +33,9 @@ export interface SpanOptions {
   /** Indicates that if this Span is active and recording information like events with the `AddEvent` operation and attributes using `setAttributes`. */
   isRecording?: boolean;
 
+  /** A spans links */
+  links?: Link[];
+
   /**
    * A parent SpanContext (or Span, for convenience) that the newly-started
    * span will be the child of.

diff --git a/packages/opentelemetry-types/src/trace/span.ts b/packages/opentelemetry-types/src/trace/span.ts
@@ -64,15 +64,6 @@ export interface Span {
     startTime?: TimeInput
   ): this;
 
-  /**
-   * Adds a link to the Span.
-   *
-   * @param spanContext the context of the linked span.
-   * @param [attributes] the attributes that will be added; these are
-   *     associated with this link.
-   */
-  addLink(spanContext: SpanContext, attributes?: Attributes): this;
-
   /**
    * Sets a status to the span. If used, this will override the default Span
    * status. Default is {@link CanonicalCode.OK}.