From a2b91e76b8c939e2be82fbc515600ab50c4e4792 Mon Sep 17 00:00:00 2001 From: Mayur Kale Date: Mon, 21 Oct 2019 15:17:30 -0700 Subject: [PATCH] fix: allow recording links only at Span creation time --- .../src/shim.ts | 12 ++--- .../opentelemetry-tracing/src/BasicTracer.ts | 1 + packages/opentelemetry-tracing/src/Span.ts | 13 +----- .../opentelemetry-tracing/test/Span.test.ts | 45 ++++++++----------- .../src/trace/SpanOptions.ts | 4 ++ .../opentelemetry-types/src/trace/span.ts | 9 ---- 6 files changed, 30 insertions(+), 54 deletions(-) diff --git a/packages/opentelemetry-shim-opentracing/src/shim.ts b/packages/opentelemetry-shim-opentracing/src/shim.ts index 57f16d883b..ccf74b9889 100644 --- 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 index a9936df1a8..2e4b9b072d 100644 --- 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 index c9e9152654..0fe7cfdbd2 100644 --- 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 index 11ef1c76ea..c571a4790c 100644 --- 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 index c396d2df46..94039f62d4 100644 --- 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 index 0a88a97895..2a725afdc2 100644 --- 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}.