From b498ba2cfc3662f659d9ac8cc92040279fff35f4 Mon Sep 17 00:00:00 2001 From: Mayur Kale Date: Wed, 14 Aug 2019 08:27:57 -0700 Subject: [PATCH] feat(basic-tracer): configure logger (#183) --- .../src/BasicTracer.ts | 6 +++ .../opentelemetry-basic-tracer/src/Span.ts | 14 ++++- .../test/BasicTracer.test.ts | 5 ++ .../test/Span.test.ts | 52 ++++++++++++++----- 4 files changed, 63 insertions(+), 14 deletions(-) diff --git a/packages/opentelemetry-basic-tracer/src/BasicTracer.ts b/packages/opentelemetry-basic-tracer/src/BasicTracer.ts index 0edb602e6d5..de25e265d7e 100644 --- a/packages/opentelemetry-basic-tracer/src/BasicTracer.ts +++ b/packages/opentelemetry-basic-tracer/src/BasicTracer.ts @@ -23,11 +23,13 @@ import { isValid, randomSpanId, NoRecordingSpan, + NoopLogger, } from '@opentelemetry/core'; import { BinaryFormat, HttpTextFormat, TraceOptions, + Logger, } from '@opentelemetry/types'; import { BasicTracerConfig } from '../src/types'; import { ScopeManager } from '@opentelemetry/scope-base'; @@ -42,6 +44,7 @@ export class BasicTracer implements types.Tracer { private readonly _httpTextFormat: types.HttpTextFormat; private readonly _sampler: types.Sampler; private readonly _scopeManager: ScopeManager; + private readonly _logger: Logger; /** * Constructs a new Tracer instance. @@ -52,6 +55,7 @@ export class BasicTracer implements types.Tracer { this._httpTextFormat = config.httpTextFormat || new HttpTraceContext(); this._sampler = config.sampler || ALWAYS_SAMPLER; this._scopeManager = config.scopeManager; + this._logger = config.logger || new NoopLogger(); } /** @@ -79,11 +83,13 @@ export class BasicTracer implements types.Tracer { const spanContext = { traceId, spanId, traceOptions, traceState }; const recordEvents = options.isRecordingEvents || false; if (!recordEvents && !samplingDecision) { + this._logger.debug('Sampling is off, starting no recording span'); return new NoRecordingSpan(spanContext); } const span = new Span( this, + this._logger, name, spanContext, options.kind || types.SpanKind.INTERNAL, diff --git a/packages/opentelemetry-basic-tracer/src/Span.ts b/packages/opentelemetry-basic-tracer/src/Span.ts index c57d7e6c946..401f25e95a9 100644 --- a/packages/opentelemetry-basic-tracer/src/Span.ts +++ b/packages/opentelemetry-basic-tracer/src/Span.ts @@ -38,10 +38,12 @@ export class Span implements types.Span, ReadableSpan { }; endTime = 0; private _ended = false; + private readonly _logger: types.Logger; /** Constructs a new Span instance. */ constructor( parentTracer: types.Tracer, + logger: types.Logger, spanName: string, spanContext: types.SpanContext, kind: types.SpanKind, @@ -54,6 +56,7 @@ export class Span implements types.Span, ReadableSpan { this.parentSpanId = parentSpanId; this.kind = kind; this.startTime = startTime || performance.now(); + this._logger = logger; } tracer(): types.Tracer { @@ -102,7 +105,10 @@ export class Span implements types.Span, ReadableSpan { } end(endTime?: number): void { - if (this._isSpanEnded()) return; + if (this._isSpanEnded()) { + this._logger.error('You can only call end() on a span once.'); + return; + } this._ended = true; this.endTime = endTime || performance.now(); // @todo: record or export the span @@ -118,7 +124,11 @@ export class Span implements types.Span, ReadableSpan { private _isSpanEnded(): boolean { if (this._ended) { - // @todo: log a warning + this._logger.warn( + 'Can not execute the operation on ended Span {traceId: %s, spanId: %s}', + this.spanContext.traceId, + this.spanContext.spanId + ); } return this._ended; } diff --git a/packages/opentelemetry-basic-tracer/test/BasicTracer.test.ts b/packages/opentelemetry-basic-tracer/test/BasicTracer.test.ts index 91eae9fe958..f4c5d84967e 100644 --- a/packages/opentelemetry-basic-tracer/test/BasicTracer.test.ts +++ b/packages/opentelemetry-basic-tracer/test/BasicTracer.test.ts @@ -104,6 +104,7 @@ describe('BasicTracer', () => { assert.ok(context.spanId.match(/[a-f0-9]{16}/)); assert.strictEqual(context.traceOptions, TraceOptions.SAMPLED); assert.deepStrictEqual(context.traceState, undefined); + span.end(); }); it('should start a span with name and parent spancontext', () => { @@ -123,6 +124,7 @@ describe('BasicTracer', () => { assert.strictEqual(context.traceId, 'd4cda95b652f4a1592b449d5929fda1b'); assert.strictEqual(context.traceOptions, TraceOptions.SAMPLED); assert.deepStrictEqual(context.traceState, state); + span.end(); }); it('should start a span with name and parent span', () => { @@ -136,6 +138,8 @@ describe('BasicTracer', () => { const context = childSpan.context(); assert.strictEqual(context.traceId, span.context().traceId); assert.strictEqual(context.traceOptions, TraceOptions.SAMPLED); + span.end(); + childSpan.end(); }); it('should start a span with name and with invalid spancontext', () => { @@ -165,6 +169,7 @@ describe('BasicTracer', () => { assert.ok(context.spanId.match(/[a-f0-9]{16}/)); assert.strictEqual(context.traceOptions, TraceOptions.UNSAMPLED); assert.deepStrictEqual(context.traceState, undefined); + span.end(); }); it('Should create real span when not sampled but recording events true', () => { diff --git a/packages/opentelemetry-basic-tracer/test/Span.test.ts b/packages/opentelemetry-basic-tracer/test/Span.test.ts index 134d075b9da..f196c1ced9b 100644 --- a/packages/opentelemetry-basic-tracer/test/Span.test.ts +++ b/packages/opentelemetry-basic-tracer/test/Span.test.ts @@ -22,10 +22,11 @@ import { TraceOptions, SpanContext, } from '@opentelemetry/types'; -import { NoopTracer } from '@opentelemetry/core'; +import { NoopTracer, NoopLogger } from '@opentelemetry/core'; describe('Span', () => { const tracer = new NoopTracer(); + const logger = new NoopLogger(); const name = 'span1'; const spanContext: SpanContext = { traceId: 'd4cda95b652f4a1592b449d5929fda1b', @@ -34,39 +35,44 @@ describe('Span', () => { }; it('should create a Span instance', () => { - const span = new Span(tracer, name, spanContext, SpanKind.SERVER); + const span = new Span(tracer, logger, name, spanContext, SpanKind.SERVER); assert.ok(span instanceof Span); assert.strictEqual(span.tracer(), tracer); + span.end(); }); it('should get the span context of span', () => { - const span = new Span(tracer, name, spanContext, SpanKind.CLIENT); + const span = new Span(tracer, logger, name, spanContext, SpanKind.CLIENT); const context = span.context(); assert.strictEqual(context.traceId, spanContext.traceId); assert.strictEqual(context.traceOptions, TraceOptions.SAMPLED); assert.strictEqual(context.traceState, undefined); assert.ok(context.spanId.match(/[a-f0-9]{16}/)); assert.ok(span.isRecordingEvents()); + span.end(); }); it('should return true when isRecordingEvents:true', () => { - const span = new Span(tracer, name, spanContext, SpanKind.CLIENT); + const span = new Span(tracer, logger, name, spanContext, SpanKind.CLIENT); assert.ok(span.isRecordingEvents()); + span.end(); }); it('should set an attribute', () => { - const span = new Span(tracer, name, spanContext, SpanKind.CLIENT); + const span = new Span(tracer, logger, name, spanContext, SpanKind.CLIENT); ['String', 'Number', 'Boolean'].map(attType => { span.setAttribute('testKey' + attType, 'testValue' + attType); }); span.setAttribute('object', { foo: 'bar' }); + span.end(); }); it('should set an event', () => { - const span = new Span(tracer, name, spanContext, SpanKind.CLIENT); + const span = new Span(tracer, logger, name, spanContext, SpanKind.CLIENT); span.addEvent('sent'); span.addEvent('rev', { attr1: 'value', attr2: 123, attr3: true }); + span.end(); }); it('should set a link', () => { @@ -75,23 +81,26 @@ describe('Span', () => { spanId: '5e0c63257de34c92', traceOptions: TraceOptions.SAMPLED, }; - const span = new Span(tracer, name, spanContext, SpanKind.CLIENT); + const span = new Span(tracer, logger, name, spanContext, SpanKind.CLIENT); span.addLink(spanContext); span.addLink(spanContext, { attr1: 'value', attr2: 123, attr3: true }); + span.end(); }); it('should set an error status', () => { - const span = new Span(tracer, name, spanContext, SpanKind.CLIENT); + const span = new Span(tracer, logger, name, spanContext, SpanKind.CLIENT); span.setStatus({ code: CanonicalCode.PERMISSION_DENIED, message: 'This is an error', }); + span.end(); }); it('should return ReadableSpan', () => { const parentId = '5c1c63257de34c67'; const span = new Span( tracer, + logger, 'my-span', spanContext, SpanKind.INTERNAL, @@ -112,7 +121,13 @@ describe('Span', () => { }); it('should return ReadableSpan with attributes', () => { - const span = new Span(tracer, 'my-span', spanContext, SpanKind.CLIENT); + const span = new Span( + tracer, + logger, + 'my-span', + spanContext, + SpanKind.CLIENT + ); span.setAttribute('attr1', 'value1'); let readableSpan = span.toReadableSpan(); assert.deepStrictEqual(readableSpan.attributes, { attr1: 'value1' }); @@ -135,7 +150,13 @@ describe('Span', () => { }); it('should return ReadableSpan with links', () => { - const span = new Span(tracer, 'my-span', spanContext, SpanKind.CLIENT); + const span = new Span( + tracer, + logger, + 'my-span', + spanContext, + SpanKind.CLIENT + ); span.addLink(spanContext); let readableSpan = span.toReadableSpan(); assert.strictEqual(readableSpan.links.length, 1); @@ -172,7 +193,13 @@ describe('Span', () => { }); it('should return ReadableSpan with events', () => { - const span = new Span(tracer, 'my-span', spanContext, SpanKind.CLIENT); + const span = new Span( + tracer, + logger, + 'my-span', + spanContext, + SpanKind.CLIENT + ); span.addEvent('sent'); let readableSpan = span.toReadableSpan(); assert.strictEqual(readableSpan.events.length, 1); @@ -210,7 +237,7 @@ describe('Span', () => { }); it('should return ReadableSpan with new status', () => { - const span = new Span(tracer, name, spanContext, SpanKind.CLIENT); + const span = new Span(tracer, logger, name, spanContext, SpanKind.CLIENT); span.setStatus({ code: CanonicalCode.PERMISSION_DENIED, message: 'This is an error', @@ -221,5 +248,6 @@ describe('Span', () => { CanonicalCode.PERMISSION_DENIED ); assert.strictEqual(readableSpan.status.message, 'This is an error'); + span.end(); }); });