feat(basic-tracer): configure logger (#183)

open-telemetry · Aug 14, 2019 · b498ba2 · b498ba2
1 parent 38a1b0f
commit b498ba2
Show file tree

Hide file tree

Showing 4 changed files with 63 additions and 14 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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();
   });
 });