From 28bbad2a0a6861817b2b0da9f17f969897fd7a47 Mon Sep 17 00:00:00 2001 From: Gerhard Stoebich <18708370+Flarna@users.noreply.github.com> Date: Mon, 16 Dec 2019 12:50:39 +0100 Subject: [PATCH 1/4] feat: introduce ended property on Span Add a getter to Span and ReadableSpan to allow checking if the span is ended without using a private field or rely on internals like span.duration[0] < 0. Refs: https://github.com/open-telemetry/opentelemetry-java/pull/693 Refs: https://github.com/open-telemetry/opentelemetry-java/pull/697 --- packages/opentelemetry-api/src/trace/NoopSpan.ts | 10 +++++++++- packages/opentelemetry-api/src/trace/span.ts | 7 +++++++ .../opentelemetry-exporter-collector/test/helper.ts | 1 + .../opentelemetry-exporter-jaeger/test/jaeger.test.ts | 1 + .../test/transform.test.ts | 3 +++ .../test/exporter.test.ts | 4 ++++ .../test/transform.test.ts | 1 + .../opentelemetry-exporter-zipkin/test/zipkin.test.ts | 3 +++ packages/opentelemetry-tracing/src/Span.ts | 8 ++++++++ .../opentelemetry-tracing/src/export/ReadableSpan.ts | 1 + packages/opentelemetry-tracing/test/Span.test.ts | 7 +++++++ 11 files changed, 45 insertions(+), 1 deletion(-) diff --git a/packages/opentelemetry-api/src/trace/NoopSpan.ts b/packages/opentelemetry-api/src/trace/NoopSpan.ts index cfa3c9c4c0..00a97066af 100644 --- a/packages/opentelemetry-api/src/trace/NoopSpan.ts +++ b/packages/opentelemetry-api/src/trace/NoopSpan.ts @@ -75,12 +75,20 @@ export class NoopSpan implements Span { } // By default does nothing - end(endTime?: TimeInput): void {} + end(endTime?: TimeInput): void { + this._ended = true; + } // isRecording always returns false for noopSpan. isRecording(): boolean { return false; } + + isEnded(): boolean { + return this._ended; + } + + private _ended: boolean = false; } export const NOOP_SPAN = new NoopSpan(); diff --git a/packages/opentelemetry-api/src/trace/span.ts b/packages/opentelemetry-api/src/trace/span.ts index 2a725afdc2..5182e56221 100644 --- a/packages/opentelemetry-api/src/trace/span.ts +++ b/packages/opentelemetry-api/src/trace/span.ts @@ -100,4 +100,11 @@ export interface Span { * with the AddEvent operation and attributes using setAttributes. */ isRecording(): boolean; + + /** + * Returns if the span has been ended. + * + * @returns true if the Span has been ended. + */ + isEnded(): boolean; } diff --git a/packages/opentelemetry-exporter-collector/test/helper.ts b/packages/opentelemetry-exporter-collector/test/helper.ts index 46071b7e7f..c77bdcd16f 100644 --- a/packages/opentelemetry-exporter-collector/test/helper.ts +++ b/packages/opentelemetry-exporter-collector/test/helper.ts @@ -32,6 +32,7 @@ export const mockedReadableSpan: ReadableSpan = { parentSpanId: '78a8915098864388', startTime: [1574120165, 429803070], endTime: [1574120165, 438688070], + ended: true, status: { code: 0 }, attributes: { component: 'document-load' }, links: [ diff --git a/packages/opentelemetry-exporter-jaeger/test/jaeger.test.ts b/packages/opentelemetry-exporter-jaeger/test/jaeger.test.ts index 20ef8467c8..c487a138c2 100644 --- a/packages/opentelemetry-exporter-jaeger/test/jaeger.test.ts +++ b/packages/opentelemetry-exporter-jaeger/test/jaeger.test.ts @@ -117,6 +117,7 @@ describe('JaegerExporter', () => { spanContext, startTime: [1566156729, 709], endTime: [1566156731, 709], + ended: true, status: { code: types.CanonicalCode.DATA_LOSS, }, diff --git a/packages/opentelemetry-exporter-jaeger/test/transform.test.ts b/packages/opentelemetry-exporter-jaeger/test/transform.test.ts index 4e81dd724b..0f558a5e51 100644 --- a/packages/opentelemetry-exporter-jaeger/test/transform.test.ts +++ b/packages/opentelemetry-exporter-jaeger/test/transform.test.ts @@ -35,6 +35,7 @@ describe('transform', () => { spanContext, startTime: [1566156729, 709], endTime: [1566156731, 709], + ended: true, status: { code: types.CanonicalCode.OK, }, @@ -131,6 +132,7 @@ describe('transform', () => { spanContext, startTime: [1566156729, 709], endTime: [1566156731, 709], + ended: true, status: { code: types.CanonicalCode.DATA_LOSS, message: 'data loss', @@ -187,6 +189,7 @@ describe('transform', () => { spanContext, startTime: [1566156729, 709], endTime: [1566156731, 709], + ended: true, status: { code: types.CanonicalCode.OK, }, diff --git a/packages/opentelemetry-exporter-stackdriver-trace/test/exporter.test.ts b/packages/opentelemetry-exporter-stackdriver-trace/test/exporter.test.ts index 68e60baf23..45ea920873 100644 --- a/packages/opentelemetry-exporter-stackdriver-trace/test/exporter.test.ts +++ b/packages/opentelemetry-exporter-stackdriver-trace/test/exporter.test.ts @@ -108,6 +108,7 @@ describe('Stackdriver Trace Exporter', () => { duration: [32, 800000000], startTime: [1566156729, 709], endTime: [1566156731, 709], + ended: true, events: [], kind: types.SpanKind.CLIENT, links: [], @@ -140,6 +141,7 @@ describe('Stackdriver Trace Exporter', () => { duration: [32, 800000000], startTime: [1566156729, 709], endTime: [1566156731, 709], + ended: true, events: [], kind: types.SpanKind.CLIENT, links: [], @@ -171,6 +173,7 @@ describe('Stackdriver Trace Exporter', () => { duration: [32, 800000000], startTime: [1566156729, 709], endTime: [1566156731, 709], + ended: true, events: [], kind: types.SpanKind.CLIENT, links: [], @@ -200,6 +203,7 @@ describe('Stackdriver Trace Exporter', () => { duration: [32, 800000000], startTime: [1566156729, 709], endTime: [1566156731, 709], + ended: true, events: [], kind: types.SpanKind.CLIENT, links: [], diff --git a/packages/opentelemetry-exporter-stackdriver-trace/test/transform.test.ts b/packages/opentelemetry-exporter-stackdriver-trace/test/transform.test.ts index 6d9ddf7969..5e098358e0 100644 --- a/packages/opentelemetry-exporter-stackdriver-trace/test/transform.test.ts +++ b/packages/opentelemetry-exporter-stackdriver-trace/test/transform.test.ts @@ -41,6 +41,7 @@ describe('transform', () => { duration: [32, 800000000], startTime: [1566156729, 709], endTime: [1566156731, 709], + ended: true, events: [], kind: types.SpanKind.CLIENT, links: [], diff --git a/packages/opentelemetry-exporter-zipkin/test/zipkin.test.ts b/packages/opentelemetry-exporter-zipkin/test/zipkin.test.ts index b4b0a5177a..5c3010a4e3 100644 --- a/packages/opentelemetry-exporter-zipkin/test/zipkin.test.ts +++ b/packages/opentelemetry-exporter-zipkin/test/zipkin.test.ts @@ -38,6 +38,7 @@ function getReadableSpan() { }, startTime: [startTime, 0], endTime: [startTime + duration, 0], + ended: true, duration: [duration, 0], status: { code: types.CanonicalCode.OK, @@ -141,6 +142,7 @@ describe('ZipkinExporter', () => { }, startTime: [startTime, 0], endTime: [startTime + duration, 0], + ended: true, duration: [duration, 0], status: { code: types.CanonicalCode.OK, @@ -167,6 +169,7 @@ describe('ZipkinExporter', () => { }, startTime: [startTime, 0], endTime: [startTime + duration, 0], + ended: true, duration: [duration, 0], status: { code: types.CanonicalCode.OK, diff --git a/packages/opentelemetry-tracing/src/Span.ts b/packages/opentelemetry-tracing/src/Span.ts index 0ae73d8f93..902fd8f247 100644 --- a/packages/opentelemetry-tracing/src/Span.ts +++ b/packages/opentelemetry-tracing/src/Span.ts @@ -172,6 +172,10 @@ export class Span implements types.Span, ReadableSpan { return true; } + isEnded(): boolean { + return this._ended; + } + toReadableSpan(): ReadableSpan { return this; } @@ -180,6 +184,10 @@ export class Span implements types.Span, ReadableSpan { return this._duration; } + get ended(): boolean { + return this._ended; + } + private _isSpanEnded(): boolean { if (this._ended) { this._logger.warn( diff --git a/packages/opentelemetry-tracing/src/export/ReadableSpan.ts b/packages/opentelemetry-tracing/src/export/ReadableSpan.ts index 039752268d..3d461d285d 100644 --- a/packages/opentelemetry-tracing/src/export/ReadableSpan.ts +++ b/packages/opentelemetry-tracing/src/export/ReadableSpan.ts @@ -36,4 +36,5 @@ export interface ReadableSpan { readonly links: Link[]; readonly events: TimedEvent[]; readonly duration: HrTime; + readonly ended: boolean; } diff --git a/packages/opentelemetry-tracing/test/Span.test.ts b/packages/opentelemetry-tracing/test/Span.test.ts index 7c56dc5e71..47391598e1 100644 --- a/packages/opentelemetry-tracing/test/Span.test.ts +++ b/packages/opentelemetry-tracing/test/Span.test.ts @@ -349,4 +349,11 @@ describe('Span', () => { span.updateName('bar-span'); assert.strictEqual(span.name, 'foo-span'); }); + + it('should have ended', () => { + const span = new Span(tracer, name, spanContext, SpanKind.SERVER); + assert.strictEqual(span.ended, false); + span.end(); + assert.strictEqual(span.ended, true); + }); }); From 118a66dc57e0b383e6b6d69db3eeaf8424189d95 Mon Sep 17 00:00:00 2001 From: Gerhard Stoebich <18708370+Flarna@users.noreply.github.com> Date: Mon, 24 Feb 2020 12:51:54 +0100 Subject: [PATCH 2/4] chore: adapt after merge from master --- packages/opentelemetry-exporter-jaeger/test/transform.test.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/opentelemetry-exporter-jaeger/test/transform.test.ts b/packages/opentelemetry-exporter-jaeger/test/transform.test.ts index 3d302b7a1a..f8512aa286 100644 --- a/packages/opentelemetry-exporter-jaeger/test/transform.test.ts +++ b/packages/opentelemetry-exporter-jaeger/test/transform.test.ts @@ -233,6 +233,7 @@ describe('transform', () => { }, startTime: [1566156729, 709], endTime: [1566156731, 709], + ended: true, status: { code: types.CanonicalCode.DATA_LOSS, message: 'data loss', From 2c2b53207d28ee61ef90ec354d46e497dc48c5c1 Mon Sep 17 00:00:00 2001 From: Gerhard Stoebich <18708370+Flarna@users.noreply.github.com> Date: Thu, 5 Mar 2020 21:03:50 +0100 Subject: [PATCH 3/4] chore: revert changes in API --- packages/opentelemetry-api/src/trace/NoopSpan.ts | 10 +--------- packages/opentelemetry-api/src/trace/span.ts | 7 ------- 2 files changed, 1 insertion(+), 16 deletions(-) diff --git a/packages/opentelemetry-api/src/trace/NoopSpan.ts b/packages/opentelemetry-api/src/trace/NoopSpan.ts index 00a97066af..cfa3c9c4c0 100644 --- a/packages/opentelemetry-api/src/trace/NoopSpan.ts +++ b/packages/opentelemetry-api/src/trace/NoopSpan.ts @@ -75,20 +75,12 @@ export class NoopSpan implements Span { } // By default does nothing - end(endTime?: TimeInput): void { - this._ended = true; - } + end(endTime?: TimeInput): void {} // isRecording always returns false for noopSpan. isRecording(): boolean { return false; } - - isEnded(): boolean { - return this._ended; - } - - private _ended: boolean = false; } export const NOOP_SPAN = new NoopSpan(); diff --git a/packages/opentelemetry-api/src/trace/span.ts b/packages/opentelemetry-api/src/trace/span.ts index 5182e56221..2a725afdc2 100644 --- a/packages/opentelemetry-api/src/trace/span.ts +++ b/packages/opentelemetry-api/src/trace/span.ts @@ -100,11 +100,4 @@ export interface Span { * with the AddEvent operation and attributes using setAttributes. */ isRecording(): boolean; - - /** - * Returns if the span has been ended. - * - * @returns true if the Span has been ended. - */ - isEnded(): boolean; } From 4762b8ff2bd97b37032276dc1c0dc4563b93f26a Mon Sep 17 00:00:00 2001 From: Gerhard Stoebich <18708370+Flarna@users.noreply.github.com> Date: Tue, 10 Mar 2020 20:14:47 +0100 Subject: [PATCH 4/4] chore: remove unneeded isEnded --- packages/opentelemetry-tracing/src/Span.ts | 4 ---- 1 file changed, 4 deletions(-) diff --git a/packages/opentelemetry-tracing/src/Span.ts b/packages/opentelemetry-tracing/src/Span.ts index 902fd8f247..7292cb43a6 100644 --- a/packages/opentelemetry-tracing/src/Span.ts +++ b/packages/opentelemetry-tracing/src/Span.ts @@ -172,10 +172,6 @@ export class Span implements types.Span, ReadableSpan { return true; } - isEnded(): boolean { - return this._ended; - } - toReadableSpan(): ReadableSpan { return this; }