From ef2541ea42e78b37d8537c51084253d5338fa799 Mon Sep 17 00:00:00 2001 From: Julien FABRE Date: Thu, 3 Sep 2020 03:18:01 +0200 Subject: [PATCH 1/6] fix: add status on span during record exception --- .../src/platform/node/protos | 1 + packages/opentelemetry-tracing/src/Span.ts | 2 ++ packages/opentelemetry-tracing/test/Span.test.ts | 12 +++++++++--- 3 files changed, 12 insertions(+), 3 deletions(-) create mode 160000 packages/opentelemetry-exporter-collector/src/platform/node/protos diff --git a/packages/opentelemetry-exporter-collector/src/platform/node/protos b/packages/opentelemetry-exporter-collector/src/platform/node/protos new file mode 160000 index 00000000000..b5468856918 --- /dev/null +++ b/packages/opentelemetry-exporter-collector/src/platform/node/protos @@ -0,0 +1 @@ +Subproject commit b54688569186e0b862bf7462a983ccf2c50c0547 diff --git a/packages/opentelemetry-tracing/src/Span.ts b/packages/opentelemetry-tracing/src/Span.ts index b8117b0195e..c21833cb8cb 100644 --- a/packages/opentelemetry-tracing/src/Span.ts +++ b/packages/opentelemetry-tracing/src/Span.ts @@ -31,6 +31,7 @@ import { ReadableSpan } from './export/ReadableSpan'; import { Tracer } from './Tracer'; import { SpanProcessor } from './SpanProcessor'; import { TraceParams } from './types'; +import { CanonicalCode } from '../../opentelemetry-api/src/trace/status'; /** * This class represents a span. @@ -206,6 +207,7 @@ export class Span implements api.Span, ReadableSpan { attributes[ExceptionAttribute.MESSAGE] ) { this.addEvent(ExceptionEventName, attributes as api.Attributes, time); + this.setStatus({ code: CanonicalCode.INTERNAL }); } else { this._logger.warn(`Failed to record an exception ${exception}`); } diff --git a/packages/opentelemetry-tracing/test/Span.test.ts b/packages/opentelemetry-tracing/test/Span.test.ts index a86222e63b2..6d16f8df371 100644 --- a/packages/opentelemetry-tracing/test/Span.test.ts +++ b/packages/opentelemetry-tracing/test/Span.test.ts @@ -60,7 +60,7 @@ describe('Span', () => { const span = new Span(tracer, name, spanContext, SpanKind.SERVER); assert.ok( hrTimeToMilliseconds(span.startTime) > - hrTimeToMilliseconds(performanceTimeOrigin) + hrTimeToMilliseconds(performanceTimeOrigin) ); }); @@ -74,7 +74,7 @@ describe('Span', () => { assert.ok( hrTimeToMilliseconds(span.endTime) > - hrTimeToMilliseconds(performanceTimeOrigin), + hrTimeToMilliseconds(performanceTimeOrigin), 'end time must be bigger than time origin' ); }); @@ -90,7 +90,7 @@ describe('Span', () => { span.addEvent('my-event'); assert.ok( hrTimeToMilliseconds(span.events[0].time) > - hrTimeToMilliseconds(performanceTimeOrigin) + hrTimeToMilliseconds(performanceTimeOrigin) ); }); @@ -397,6 +397,9 @@ describe('Span', () => { 'exception.message': 'boom', }); assert.ok(event.time[0] > 0); + + const status = span.status; + assert.strictEqual(status.code, CanonicalCode.INTERNAL); }); }); @@ -432,6 +435,9 @@ describe('Span', () => { assert.strictEqual(type, 'Error'); assert.strictEqual(message, 'boom'); assert.strictEqual(stacktrace, 'bar'); + + const status = span.status; + assert.strictEqual(status.code, CanonicalCode.INTERNAL); }); }); }); From 5a83740a4c389d585bc94560ee3e3de99a726102 Mon Sep 17 00:00:00 2001 From: Julien FABRE Date: Thu, 3 Sep 2020 03:24:18 +0200 Subject: [PATCH 2/6] fix: canonical dep --- packages/opentelemetry-tracing/src/Span.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/packages/opentelemetry-tracing/src/Span.ts b/packages/opentelemetry-tracing/src/Span.ts index c21833cb8cb..b7893322940 100644 --- a/packages/opentelemetry-tracing/src/Span.ts +++ b/packages/opentelemetry-tracing/src/Span.ts @@ -31,7 +31,6 @@ import { ReadableSpan } from './export/ReadableSpan'; import { Tracer } from './Tracer'; import { SpanProcessor } from './SpanProcessor'; import { TraceParams } from './types'; -import { CanonicalCode } from '../../opentelemetry-api/src/trace/status'; /** * This class represents a span. @@ -207,7 +206,7 @@ export class Span implements api.Span, ReadableSpan { attributes[ExceptionAttribute.MESSAGE] ) { this.addEvent(ExceptionEventName, attributes as api.Attributes, time); - this.setStatus({ code: CanonicalCode.INTERNAL }); + this.setStatus({ code: api.CanonicalCode.INTERNAL }); } else { this._logger.warn(`Failed to record an exception ${exception}`); } From b470fccb2d38593fe3122df2da5c5d260cfa1e19 Mon Sep 17 00:00:00 2001 From: Julien FABRE Date: Thu, 3 Sep 2020 03:28:34 +0200 Subject: [PATCH 3/6] fix: proto? --- .../opentelemetry-exporter-collector/src/platform/node/protos | 1 - 1 file changed, 1 deletion(-) delete mode 160000 packages/opentelemetry-exporter-collector/src/platform/node/protos diff --git a/packages/opentelemetry-exporter-collector/src/platform/node/protos b/packages/opentelemetry-exporter-collector/src/platform/node/protos deleted file mode 160000 index b5468856918..00000000000 --- a/packages/opentelemetry-exporter-collector/src/platform/node/protos +++ /dev/null @@ -1 +0,0 @@ -Subproject commit b54688569186e0b862bf7462a983ccf2c50c0547 From d9d734f58662ac7a58f6b0bf56c04347305c1bda Mon Sep 17 00:00:00 2001 From: Julien FABRE Date: Thu, 3 Sep 2020 22:19:18 +0200 Subject: [PATCH 4/6] fix: lint --- packages/opentelemetry-api/src/metrics/NoopMeter.ts | 12 ++++-------- .../src/platform/browser/BasePlugin.ts | 3 +-- .../src/platform/node/BasePlugin.ts | 3 +-- packages/opentelemetry-tracing/test/Span.test.ts | 6 +++--- 4 files changed, 9 insertions(+), 15 deletions(-) diff --git a/packages/opentelemetry-api/src/metrics/NoopMeter.ts b/packages/opentelemetry-api/src/metrics/NoopMeter.ts index 765a0fd8ae3..f3c8feeddd1 100644 --- a/packages/opentelemetry-api/src/metrics/NoopMeter.ts +++ b/packages/opentelemetry-api/src/metrics/NoopMeter.ts @@ -131,24 +131,21 @@ export class NoopMetric implements UnboundMetric { } } -export class NoopCounterMetric - extends NoopMetric +export class NoopCounterMetric extends NoopMetric implements Counter { add(value: number, labels: Labels) { this.bind(labels).add(value); } } -export class NoopValueRecorderMetric - extends NoopMetric +export class NoopValueRecorderMetric extends NoopMetric implements ValueRecorder { record(value: number, labels: Labels) { this.bind(labels).record(value); } } -export class NoopBaseObserverMetric - extends NoopMetric +export class NoopBaseObserverMetric extends NoopMetric implements BaseObserver { observation() { return { @@ -158,8 +155,7 @@ export class NoopBaseObserverMetric } } -export class NoopBatchObserverMetric - extends NoopMetric +export class NoopBatchObserverMetric extends NoopMetric implements BatchObserver {} export class NoopBoundCounter implements BoundCounter { diff --git a/packages/opentelemetry-core/src/platform/browser/BasePlugin.ts b/packages/opentelemetry-core/src/platform/browser/BasePlugin.ts index bf6e227d8a0..4ace89ab48e 100644 --- a/packages/opentelemetry-core/src/platform/browser/BasePlugin.ts +++ b/packages/opentelemetry-core/src/platform/browser/BasePlugin.ts @@ -23,8 +23,7 @@ import { import { BaseAbstractPlugin } from '../BaseAbstractPlugin'; /** This class represent the base to patch plugin. */ -export abstract class BasePlugin - extends BaseAbstractPlugin +export abstract class BasePlugin extends BaseAbstractPlugin implements Plugin { enable( moduleExports: T, diff --git a/packages/opentelemetry-core/src/platform/node/BasePlugin.ts b/packages/opentelemetry-core/src/platform/node/BasePlugin.ts index 3581cee96ac..d1ebc622cf4 100644 --- a/packages/opentelemetry-core/src/platform/node/BasePlugin.ts +++ b/packages/opentelemetry-core/src/platform/node/BasePlugin.ts @@ -27,8 +27,7 @@ import * as path from 'path'; import { BaseAbstractPlugin } from '../BaseAbstractPlugin'; /** This class represent the base to patch plugin. */ -export abstract class BasePlugin - extends BaseAbstractPlugin +export abstract class BasePlugin extends BaseAbstractPlugin implements Plugin { enable( moduleExports: T, diff --git a/packages/opentelemetry-tracing/test/Span.test.ts b/packages/opentelemetry-tracing/test/Span.test.ts index 6d16f8df371..3c0531f8278 100644 --- a/packages/opentelemetry-tracing/test/Span.test.ts +++ b/packages/opentelemetry-tracing/test/Span.test.ts @@ -60,7 +60,7 @@ describe('Span', () => { const span = new Span(tracer, name, spanContext, SpanKind.SERVER); assert.ok( hrTimeToMilliseconds(span.startTime) > - hrTimeToMilliseconds(performanceTimeOrigin) + hrTimeToMilliseconds(performanceTimeOrigin) ); }); @@ -74,7 +74,7 @@ describe('Span', () => { assert.ok( hrTimeToMilliseconds(span.endTime) > - hrTimeToMilliseconds(performanceTimeOrigin), + hrTimeToMilliseconds(performanceTimeOrigin), 'end time must be bigger than time origin' ); }); @@ -90,7 +90,7 @@ describe('Span', () => { span.addEvent('my-event'); assert.ok( hrTimeToMilliseconds(span.events[0].time) > - hrTimeToMilliseconds(performanceTimeOrigin) + hrTimeToMilliseconds(performanceTimeOrigin) ); }); From 07f00fa6d9eccb3eb15f858e831de51dd51d0ab1 Mon Sep 17 00:00:00 2001 From: Julien FABRE Date: Thu, 3 Sep 2020 23:03:43 +0200 Subject: [PATCH 5/6] fix: lint? --- packages/opentelemetry-api/src/metrics/NoopMeter.ts | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/packages/opentelemetry-api/src/metrics/NoopMeter.ts b/packages/opentelemetry-api/src/metrics/NoopMeter.ts index f3c8feeddd1..765a0fd8ae3 100644 --- a/packages/opentelemetry-api/src/metrics/NoopMeter.ts +++ b/packages/opentelemetry-api/src/metrics/NoopMeter.ts @@ -131,21 +131,24 @@ export class NoopMetric implements UnboundMetric { } } -export class NoopCounterMetric extends NoopMetric +export class NoopCounterMetric + extends NoopMetric implements Counter { add(value: number, labels: Labels) { this.bind(labels).add(value); } } -export class NoopValueRecorderMetric extends NoopMetric +export class NoopValueRecorderMetric + extends NoopMetric implements ValueRecorder { record(value: number, labels: Labels) { this.bind(labels).record(value); } } -export class NoopBaseObserverMetric extends NoopMetric +export class NoopBaseObserverMetric + extends NoopMetric implements BaseObserver { observation() { return { @@ -155,7 +158,8 @@ export class NoopBaseObserverMetric extends NoopMetric } } -export class NoopBatchObserverMetric extends NoopMetric +export class NoopBatchObserverMetric + extends NoopMetric implements BatchObserver {} export class NoopBoundCounter implements BoundCounter { From 38ac9d26089a51f805b8830a6cb48cb2a922318d Mon Sep 17 00:00:00 2001 From: Julien FABRE Date: Fri, 4 Sep 2020 00:05:23 +0200 Subject: [PATCH 6/6] fix: lint again --- packages/opentelemetry-core/src/platform/browser/BasePlugin.ts | 3 ++- packages/opentelemetry-core/src/platform/node/BasePlugin.ts | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/packages/opentelemetry-core/src/platform/browser/BasePlugin.ts b/packages/opentelemetry-core/src/platform/browser/BasePlugin.ts index 4ace89ab48e..bf6e227d8a0 100644 --- a/packages/opentelemetry-core/src/platform/browser/BasePlugin.ts +++ b/packages/opentelemetry-core/src/platform/browser/BasePlugin.ts @@ -23,7 +23,8 @@ import { import { BaseAbstractPlugin } from '../BaseAbstractPlugin'; /** This class represent the base to patch plugin. */ -export abstract class BasePlugin extends BaseAbstractPlugin +export abstract class BasePlugin + extends BaseAbstractPlugin implements Plugin { enable( moduleExports: T, diff --git a/packages/opentelemetry-core/src/platform/node/BasePlugin.ts b/packages/opentelemetry-core/src/platform/node/BasePlugin.ts index d1ebc622cf4..3581cee96ac 100644 --- a/packages/opentelemetry-core/src/platform/node/BasePlugin.ts +++ b/packages/opentelemetry-core/src/platform/node/BasePlugin.ts @@ -27,7 +27,8 @@ import * as path from 'path'; import { BaseAbstractPlugin } from '../BaseAbstractPlugin'; /** This class represent the base to patch plugin. */ -export abstract class BasePlugin extends BaseAbstractPlugin +export abstract class BasePlugin + extends BaseAbstractPlugin implements Plugin { enable( moduleExports: T,