fix(tracer): report every span to exporters (#493)

* fix(tracer): report every span to exporters * test(exporter-zipkin): separate root and child span translation tests * refactor(exporters): address PR comments * test(exporter-mongodb): fix * test(redis): fix exporter tests
census-instrumentation · May 8, 2019 · d4c3b19 · d4c3b19
1 parent 96a6ab7
commit d4c3b19
Show file tree

Hide file tree

Showing 22 changed files with 309 additions and 301 deletions.
diff --git a/packages/opencensus-core/src/exporters/console-exporter.ts b/packages/opencensus-core/src/exporters/console-exporter.ts
@@ -48,14 +48,14 @@ export class ConsoleExporter implements Exporter {
     this.logger = config.logger;
   }
 
-  onStartSpan(root: modelTypes.Span) {}
+  onStartSpan(span: modelTypes.Span) {}
 
   /**
    * Event called when a span is ended.
-   * @param root Ended span.
+   * @param span Ended span.
    */
-  onEndSpan(root: modelTypes.Span) {
-    this.buffer.addToBuffer(root);
+  onEndSpan(span: modelTypes.Span) {
+    this.buffer.addToBuffer(span);
   }
 
   /**

diff --git a/packages/opencensus-core/src/exporters/exporter-buffer.ts b/packages/opencensus-core/src/exporters/exporter-buffer.ts
@@ -75,12 +75,12 @@ export class ExporterBuffer {
   }
 
   /**
-   * Add a rootSpan in the buffer.
-   * @param root RootSpan to be added in the buffer.
+   * Add a span in the buffer.
+   * @param span Span to be added in the buffer.
    */
-  addToBuffer(root: modelTypes.Span) {
-    this.queue.push(root);
-    this.logger.debug('ExporterBuffer: added new rootspan');
+  addToBuffer(span: modelTypes.Span) {
+    this.queue.push(span);
+    this.logger.debug('ExporterBuffer: added new span');
 
     if (this.queue.length > this.bufferSize) {
       this.flush();

diff --git a/packages/opencensus-core/src/trace/model/root-span.ts b/packages/opencensus-core/src/trace/model/root-span.ts
@@ -20,8 +20,6 @@ import * as types from './types';
 
 /** Defines a root span */
 export class RootSpan extends Span {
-  /** A tracer object */
-  private tracer: types.TracerBase;
   /** Its trace ID. */
   private traceIdLocal: string;
   /** Its trace state. */
@@ -45,8 +43,7 @@ export class RootSpan extends Span {
   constructor(
       tracer: types.TracerBase, name: string, kind: types.SpanKind,
       traceId: string, parentSpanId: string, traceState?: types.TraceState) {
-    super();
-    this.tracer = tracer;
+    super(tracer);
     this.traceIdLocal = traceId;
     this.name = name;
     this.kind = kind;
@@ -71,17 +68,4 @@ export class RootSpan extends Span {
   get parentSpanId(): string {
     return this.parentSpanIdLocal;
   }
-
-  /** Starts a rootspan instance. */
-  start() {
-    super.start();
-
-    this.tracer.onStartSpan(this);
-  }
-
-  /** Ends a rootspan instance. */
-  end() {
-    super.end();
-    this.tracer.onEndSpan(this);
-  }
 }
diff --git a/packages/opencensus-core/src/trace/model/span.ts b/packages/opencensus-core/src/trace/model/span.ts
@@ -29,6 +29,8 @@ const STATUS_OK = {
 /** Defines a base model for spans. */
 export class Span implements types.Span {
   protected className: string;
+  /** A tracer object */
+  private tracer: types.TracerBase;
   /** The clock used to mesure the beginning and ending of a span */
   private clock!: Clock;
   /** Indicates if this span was started */
@@ -77,7 +79,8 @@ export class Span implements types.Span {
   droppedMessageEventsCount = 0;
 
   /** Constructs a new Span instance. */
-  constructor(parent?: Span) {
+  constructor(tracer: types.TracerBase, parent?: Span) {
+    this.tracer = tracer;
     this.className = this.constructor.name;
     this.id = randomSpanId();
     this.spansLocal = [];
@@ -310,6 +313,8 @@ export class Span implements types.Span {
       parentSpanId: this.parentSpanId,
       traceState: this.traceState
     });
+
+    this.tracer.onStartSpan(this);
   }
 
   /** Ends the span and all of its children, recursively. */
@@ -335,6 +340,8 @@ export class Span implements types.Span {
         span.truncate();
       }
     }
+
+    this.tracer.onEndSpan(this);
   }
 
   /** Forces the span to end. */
@@ -366,7 +373,7 @@ export class Span implements types.Span {
       return new NoRecordSpan();
     }
 
-    const child = new Span(this);
+    const child = new Span(this.tracer, this);
     const spanName =
         typeof nameOrOptions === 'object' ? nameOrOptions.name : nameOrOptions;
     const spanKind =

diff --git a/packages/opencensus-core/src/trace/model/tracer-base.ts b/packages/opencensus-core/src/trace/model/tracer-base.ts
@@ -180,20 +180,20 @@ export class CoreTracerBase implements types.TracerBase {
     }
   }
 
-  private notifyStartSpan(root: types.Span) {
-    this.logger.debug('starting to notify listeners the start of rootspans');
+  private notifyStartSpan(span: types.Span) {
+    this.logger.debug('starting to notify listeners the start of spans');
     if (this.eventListenersLocal && this.eventListenersLocal.length > 0) {
       for (const listener of this.eventListenersLocal) {
-        listener.onStartSpan(root);
+        listener.onStartSpan(span);
       }
     }
   }
 
-  private notifyEndSpan(root: types.Span) {
-    this.logger.debug('starting to notify listeners the end of rootspans');
+  private notifyEndSpan(span: types.Span) {
+    this.logger.debug('starting to notify listeners the end of spans');
     if (this.eventListenersLocal && this.eventListenersLocal.length > 0) {
       for (const listener of this.eventListenersLocal) {
-        listener.onEndSpan(root);
+        listener.onEndSpan(span);
       }
     }
   }

diff --git a/packages/opencensus-core/test/test-root-span.ts b/packages/opencensus-core/test/test-root-span.ts
@@ -293,7 +293,7 @@ describe('RootSpan', () => {
       const rootSpan = new RootSpan(tracer, name, kind, traceId, parentSpanId);
       rootSpan.start();
 
-      const span = new Span(rootSpan);
+      const span = new Span(tracer, rootSpan);
       span.start();
 
       rootSpan.addLink(

diff --git a/packages/opencensus-core/test/test-span.ts b/packages/opencensus-core/test/test-span.ts
@@ -44,7 +44,7 @@ describe('Span', () => {
   describe('new Span()', () => {
     it('should create a Span instance', () => {
       const rootSpan = new RootSpan(tracer, name, kind, traceId, parentSpanId);
-      const span = new Span(rootSpan);
+      const span = new Span(tracer, rootSpan);
       assert.ok(span instanceof Span);
     });
   });
@@ -56,7 +56,7 @@ describe('Span', () => {
     it('should return the trace id', () => {
       const rootSpan = new RootSpan(tracer, name, kind, traceId, parentSpanId);
       rootSpan.start();
-      const span = new Span(rootSpan);
+      const span = new Span(tracer, rootSpan);
       assert.equal(span.traceId, rootSpan.traceId);
     });
   });
@@ -69,7 +69,7 @@ describe('Span', () => {
       const rootSpan = new RootSpan(tracer, name, kind, traceId, parentSpanId);
       rootSpan.start();
 
-      const span = new Span(rootSpan);
+      const span = new Span(tracer, rootSpan);
       const context = span.spanContext;
 
       assert.equal(context.traceId, rootSpan.traceId);
@@ -87,7 +87,7 @@ describe('Span', () => {
     before(() => {
       const rootSpan = new RootSpan(tracer, name, kind, traceId, parentSpanId);
       rootSpan.start();
-      span = new Span(rootSpan);
+      span = new Span(tracer, rootSpan);
     });
     it('should get startTime()', () => {
       assert.ok(span.startTime);
@@ -108,7 +108,7 @@ describe('Span', () => {
       const rootSpan = new RootSpan(tracer, name, kind, traceId, parentSpanId);
       rootSpan.start();
 
-      const span = new Span(rootSpan);
+      const span = new Span(tracer, rootSpan);
       span.start();
 
       assert.ok(span.started);
@@ -122,7 +122,7 @@ describe('Span', () => {
     it('should not change the initial startTime', () => {
       const rootSpan = new RootSpan(tracer, name, kind, traceId, parentSpanId);
       rootSpan.start();
-      const span = new Span(rootSpan);
+      const span = new Span(tracer, rootSpan);
       span.start();
       const initialStartTime = span.startTime;
       span.start();
@@ -139,7 +139,7 @@ describe('Span', () => {
       const rootSpan = new RootSpan(tracer, name, kind, traceId, parentSpanId);
       rootSpan.start();
 
-      const span = new Span(rootSpan);
+      const span = new Span(tracer, rootSpan);
       span.start();
       span.end();
 
@@ -155,7 +155,7 @@ describe('Span', () => {
       const rootSpan = new RootSpan(tracer, name, kind, traceId, parentSpanId);
       rootSpan.start();
 
-      const span = new Span(rootSpan);
+      const span = new Span(tracer, rootSpan);
       span.end();
 
       assert.ok(!span.ended);
@@ -169,7 +169,7 @@ describe('Span', () => {
     it('should not change the endTime', () => {
       const rootSpan = new RootSpan(tracer, name, kind, traceId, parentSpanId);
       rootSpan.start();
-      const span = new Span(rootSpan);
+      const span = new Span(tracer, rootSpan);
       span.start();
       span.end();
       const initialEndTime = span.endTime;
@@ -187,7 +187,7 @@ describe('Span', () => {
       const rootSpan = new RootSpan(tracer, name, kind, traceId, parentSpanId);
       rootSpan.start();
 
-      const span = new Span(rootSpan);
+      const span = new Span(tracer, rootSpan);
       span.start();
 
       ['String', 'Number', 'Boolean'].map(attType => {
@@ -205,7 +205,7 @@ describe('Span', () => {
       const rootSpan = new RootSpan(tracer, name, kind, traceId, parentSpanId);
       rootSpan.start();
 
-      const span = new Span(rootSpan);
+      const span = new Span(tracer, rootSpan);
       span.start();
       for (let i = 0; i < 40; i++) {
         span.addAttribute('attr' + i, 100);
@@ -230,7 +230,7 @@ describe('Span', () => {
       const rootSpan = new RootSpan(tracer, name, kind, traceId, parentSpanId);
       rootSpan.start();
 
-      const span = new Span(rootSpan);
+      const span = new Span(tracer, rootSpan);
       span.start();
 
       span.addAnnotation('description test', {} as Attributes, Date.now());
@@ -244,7 +244,7 @@ describe('Span', () => {
       const rootSpan = new RootSpan(tracer, name, kind, traceId, parentSpanId);
       rootSpan.start();
 
-      const span = new Span(rootSpan);
+      const span = new Span(tracer, rootSpan);
       span.start();
       for (let i = 0; i < 40; i++) {
         span.addAnnotation('description test', {} as Attributes, Date.now());
@@ -268,7 +268,7 @@ describe('Span', () => {
       const rootSpan = new RootSpan(tracer, name, kind, traceId, parentSpanId);
       rootSpan.start();
 
-      const span = new Span(rootSpan);
+      const span = new Span(tracer, rootSpan);
       span.start();
 
       span.addLink(
@@ -282,7 +282,7 @@ describe('Span', () => {
     it('should drop extra links', () => {
       const rootSpan = new RootSpan(tracer, name, kind, traceId, parentSpanId);
       rootSpan.start();
-      const span = new Span(rootSpan);
+      const span = new Span(tracer, rootSpan);
       span.start();
 
       for (let i = 0; i < 35; i++) {
@@ -308,7 +308,7 @@ describe('Span', () => {
       const rootSpan = new RootSpan(tracer, name, kind, traceId, parentSpanId);
       rootSpan.start();
 
-      const span = new Span(rootSpan);
+      const span = new Span(tracer, rootSpan);
       span.start();
 
       span.addMessageEvent(
@@ -332,7 +332,7 @@ describe('Span', () => {
       const rootSpan = new RootSpan(tracer, name, kind, traceId, parentSpanId);
       rootSpan.start();
 
-      const span = new Span(rootSpan);
+      const span = new Span(tracer, rootSpan);
       span.start();
       for (let i = 0; i < 35; i++) {
         span.addMessageEvent(types.MessageEventType.UNSPECIFIED, 1);
@@ -348,7 +348,7 @@ describe('Span', () => {
       const rootSpan = new RootSpan(tracer, name, kind, traceId, parentSpanId);
       rootSpan.start();
 
-      const span = new Span(rootSpan);
+      const span = new Span(tracer, rootSpan);
       span.start();
 
       assert.equal(rootSpan.status.code, 0);
@@ -360,7 +360,7 @@ describe('Span', () => {
     it('should set an error status', () => {
       const rootSpan = new RootSpan(tracer, name, kind, traceId, parentSpanId);
       rootSpan.start();
-      const span = new Span(rootSpan);
+      const span = new Span(tracer, rootSpan);
       span.start();
       span.setStatus(types.CanonicalCode.PERMISSION_DENIED, 'This is an error');
 

diff --git a/packages/opencensus-exporter-instana/src/instana.ts b/packages/opencensus-exporter-instana/src/instana.ts
@@ -60,10 +60,10 @@ export class InstanaTraceExporter implements Exporter {
     this.exporterBuffer = new ExporterBuffer(this, options);
   }
 
-  onStartSpan(root: Span) {}
+  onStartSpan(span: Span) {}
 
-  onEndSpan(root: Span) {
-    this.exporterBuffer.addToBuffer(root);
+  onEndSpan(span: Span) {
+    this.exporterBuffer.addToBuffer(span);
   }
 
   /**

diff --git a/packages/opencensus-exporter-jaeger/src/jaeger.ts b/packages/opencensus-exporter-jaeger/src/jaeger.ts
@@ -98,19 +98,24 @@ export class JaegerTraceExporter implements Exporter {
 
   /**
    * Is called whenever a span is ended.
-   * @param root the ended span
+   * @param span the ended span
    */
-  onEndSpan(root: Span) {
-    this.logger.debug('onEndSpan: adding rootSpan: %s', root.name);
+  onEndSpan(span: Span) {
+    // Add spans of a trace together when root is ended, skip non root spans.
+    if (span.constructor.name !== 'RootSpan') {
+      return;
+    }
+
+    this.logger.debug('onEndSpan: adding rootSpan: %s', span.name);
 
     // UDPSender buffer is limited by maxPacketSize
-    this.addSpanToSenderBuffer(root)
+    this.addSpanToSenderBuffer(span)
         .then(result => {
-          this.addToBuffer(root, result as number);
-          for (const span of root.spans) {
-            this.addSpanToSenderBuffer(span)
+          this.addToBuffer(span, result as number);
+          for (const localSpan of span.spans) {
+            this.addSpanToSenderBuffer(localSpan)
                 .then(result => {
-                  this.addToBuffer(span, result as number);
+                  this.addToBuffer(localSpan, result as number);
                 })
                 .catch(err => {
                   return;

diff --git a/packages/opencensus-exporter-ocagent/src/ocagent.ts b/packages/opencensus-exporter-ocagent/src/ocagent.ts
@@ -204,10 +204,15 @@ export class OCAgentExporter implements Exporter {
     }
   }
 
-  onStartSpan(root: Span) {}
+  onStartSpan(span: Span) {}
 
-  onEndSpan(root: Span) {
-    this.buffer.addToBuffer(root);
+  onEndSpan(span: Span) {
+    // Add spans of a trace together when root is ended, skip non root spans.
+    // adaptRootSpan() will extract child spans from root.
+    if (span.constructor.name !== 'RootSpan') {
+      return;
+    }
+    this.buffer.addToBuffer(span);
   }
 
   publish(rootSpans: Span[]): Promise<number|string|void> {