Fix duplicate spans issue and clean up (#545)

* Clean up and ignore non root spans

* fix build

* Add isRootSpan on the span

Author: mayurkale22

packages/opencensus-core/src/exporters/console-exporter.ts

@@ -24,9 +24,9 @@ import {Exporter, ExporterConfig, StatsEventListener} from './types';
 /** Do not send span data */
 export class NoopExporter implements Exporter {
   logger?: loggerTypes.Logger;
-  onStartSpan(root: modelTypes.Span) {}
-  onEndSpan(root: modelTypes.Span) {}
-  publish(rootSpans: modelTypes.Span[]) {
+  onStartSpan(span: modelTypes.Span) {}
+  onEndSpan(span: modelTypes.Span) {}
+  publish(spans: modelTypes.Span[]) {
     return Promise.resolve();
   }
 }
@@ -39,7 +39,7 @@ export class ConsoleExporter implements Exporter {
   private buffer: ExporterBuffer;
 
   /**
-   * Constructs a new ConsoleLogExporter instance.
+   * Constructs a new ConsoleExporter instance.
    * @param config Exporter configuration object to create a console log
    *     exporter.
    */
@@ -55,19 +55,22 @@ export class ConsoleExporter implements Exporter {
    * @param span Ended span.
    */
   onEndSpan(span: modelTypes.Span) {
+    // Add spans of a trace together when root is ended, skip non root spans.
+    // publish function will extract child spans from root.
+    if (!span.isRootSpan()) return;
     this.buffer.addToBuffer(span);
   }
 
   /**
    * Sends the spans information to the console.
-   * @param rootSpans A list of root spans to publish.
+   * @param spans A list of spans to publish.
    */
-  publish(rootSpans: modelTypes.Span[]) {
-    rootSpans.map((root) => {
-      const ROOT_STR = `RootSpan: {traceId: ${root.traceId}, spanId: ${
-          root.id}, name: ${root.name} }`;
-      const SPANS_STR: string[] = root.spans.map(
-          (span) => [`\t\t{spanId: ${span.id}, name: ${span.name}}`].join(
+  publish(spans: modelTypes.Span[]) {
+    spans.map((span) => {
+      const ROOT_STR = `RootSpan: {traceId: ${span.traceId}, spanId: ${
+          span.id}, name: ${span.name} }`;
+      const SPANS_STR: string[] = span.spans.map(
+          (child) => [`\t\t{spanId: ${child.id}, name: ${child.name}}`].join(
               '\n'));
 
       const result: string[] = [];
@@ -85,7 +88,6 @@ export class ConsoleStatsExporter implements StatsEventListener {
   /**
    * Event called when a view is registered
    * @param view registered view
-   * @param measure registered measure
    */
   onRegisterView(view: View) {
     console.log(`View registered: ${view.name}, Measure registered: ${

packages/opencensus-core/src/exporters/types.ts

@@ -22,10 +22,10 @@ import * as modelTypes from '../trace/model/types';
 /** Defines a trace exporter interface. */
 export interface Exporter extends modelTypes.SpanEventListener {
   /**
-   * Sends a list of root spans to the service.
-   * @param rootSpans A list of root spans to publish.
+   * Sends a list of spans to the service.
+   * @param spans A list of spans to publish.
    */
-  publish(rootSpans: modelTypes.Span[]): Promise<number|string|void>;
+  publish(spans: modelTypes.Span[]): Promise<number|string|void>;
 }
 
 /**

packages/opencensus-core/src/trace/model/no-record/no-record-root-span.ts

@@ -57,6 +57,11 @@ export class NoRecordRootSpan extends NoRecordSpan {
     this.logger = this.tracer.logger || logger.logger();
   }
 
+  /** Returns whether a span is root or not. */
+  isRootSpan(): boolean {
+    return true;
+  }
+
   /** No-op implementation of this method. */
   get traceId(): string {
     return this.traceIdLocal;

packages/opencensus-core/src/trace/model/no-record/no-record-span.ts

@@ -80,6 +80,11 @@ export class NoRecordSpan implements types.Span {
     this.logger = (this.root && this.root.logger) || this.logger;
   }
 
+  /** Returns whether a span is root or not. */
+  isRootSpan(): boolean {
+    return false;
+  }
+
   /** Gets trace id of no-record span. */
   get traceId(): string {
     return '';

packages/opencensus-core/src/trace/model/root-span.ts

@@ -55,6 +55,11 @@ export class RootSpan extends Span {
     this.activeTraceParams = tracer.activeTraceParams;
   }
 
+  /** Returns whether a span is root or not. */
+  isRootSpan(): boolean {
+    return true;
+  }
+
   /** Gets trace id from rootspan instance. */
   get traceId(): string {
     return this.traceIdLocal;

packages/opencensus-core/src/trace/model/span.ts

@@ -94,6 +94,11 @@ export class Span implements types.Span {
     this.logger = (this.root && this.root.logger) || this.logger;
   }
 
+  /** Returns whether a span is root or not. */
+  isRootSpan(): boolean {
+    return false;
+  }
+
   /** Gets the trace ID. */
   get traceId(): string {
     return this.root.traceId;
@@ -330,6 +335,8 @@ export class Span implements types.Span {
     this.endedLocal = true;
     this.clock.end();
 
+    // TODO: Should ending a span force its children to end by default?
+    // Issue: https://github.com/open-telemetry/opentelemetry-node/issues/4
     for (const span of this.spansLocal) {
       if (!span.ended && span.started) {
         span.truncate();

packages/opencensus-core/src/trace/model/tracer-base.ts

@@ -152,22 +152,15 @@ export class CoreTracerBase implements types.TracerBase {
   }
 
   /** Notifies listeners of the span start. */
-  onStartSpan(root: types.Span): void {
+  onStartSpan(span: types.Span): void {
     if (!this.active) return;
-    if (!root) {
-      return this.logger.debug('cannot start trace - no active trace found');
-    }
-    this.notifyStartSpan(root);
+    this.notifyStartSpan(span);
   }
 
   /** Notifies listeners of the span end. */
-  onEndSpan(root: types.Span): void {
+  onEndSpan(span: types.Span): void {
     if (!this.active) return;
-    if (!root) {
-      this.logger.debug('cannot end trace - no active trace found');
-      return;
-    }
-    this.notifyEndSpan(root);
+    this.notifyEndSpan(span);
   }
 
   /**
@@ -191,19 +184,15 @@ export class CoreTracerBase implements types.TracerBase {
 
   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(span);
-      }
+    for (const listener of this.eventListenersLocal) {
+      listener.onStartSpan(span);
     }
   }
 
   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(span);
-      }
+    for (const listener of this.eventListenersLocal) {
+      listener.onEndSpan(span);
     }
   }
 

packages/opencensus-core/src/trace/model/tracer.ts

@@ -15,7 +15,6 @@
  */
 
 import * as cls from '../../internal/cls';
-
 import {NoRecordSpan} from './no-record/no-record-span';
 import {CoreTracerBase} from './tracer-base';
 import * as types from './types';
@@ -73,32 +72,25 @@ export class CoreTracer extends CoreTracerBase implements types.Tracer {
   }
 
   /** Notifies listeners of the span start. */
-  onStartSpan(root: types.Span): void {
+  onStartSpan(span: types.Span): void {
     if (!this.active) return;
-    if (!root) {
-      return this.logger.debug('cannot start trace - no active trace found');
-    }
     if (!this.currentRootSpan ||
-        this.currentRootSpan.traceId !== root.traceId) {
+        this.currentRootSpan.traceId !== span.traceId) {
       this.logger.debug(
           'currentRootSpan != root on notifyStart. Need more investigation.');
     }
-    return super.onStartSpan(root);
+    return super.onStartSpan(span);
   }
 
   /** Notifies listeners of the span end. */
-  onEndSpan(root: types.Span): void {
+  onEndSpan(span: types.Span): void {
     if (!this.active) return;
-    if (!root) {
-      this.logger.debug('cannot end trace - no active trace found');
-      return;
-    }
     if (!this.currentRootSpan ||
-        this.currentRootSpan.traceId !== root.traceId) {
+        this.currentRootSpan.traceId !== span.traceId) {
       this.logger.debug(
           'currentRootSpan != root on notifyEnd. Need more investigation.');
     }
-    super.onEndSpan(root);
+    super.onEndSpan(span);
   }
 
   /** Clears the current root span. */

packages/opencensus-core/src/trace/model/types.ts

@@ -460,6 +460,9 @@ export interface Span {
 
   /** Starts a new Span instance as a child of this instance */
   startChildSpan(options?: SpanOptions): Span;
+
+  /** Returns whether a span is root or not. */
+  isRootSpan(): boolean;
 }
 
 /** Interface for TracerBase */

packages/opencensus-exporter-instana/src/instana.ts

@@ -63,6 +63,9 @@ export class InstanaTraceExporter implements Exporter {
   onStartSpan(span: Span) {}
 
   onEndSpan(span: Span) {
+    // Add spans of a trace together when root is ended, skip non root spans.
+    // translateRootSpans function will extract child spans from root.
+    if (!span.isRootSpan()) return;
     this.exporterBuffer.addToBuffer(span);
   }
 
@@ -76,13 +79,13 @@ export class InstanaTraceExporter implements Exporter {
    *
    * This Promise is meant as a problem indicator for tests only.
    *
-   * @param rootSpans The spans to transmit to Instana
+   * @param spans The list of spans to transmit to Instana
    * @returns An indicator whether publishing was successful.
    */
-  publish(rootSpans: Span[]): Promise<void> {
+  publish(spans: Span[]): Promise<void> {
     try {
       return this
-          .transmit(this.translateRootSpans(rootSpans))
+          .transmit(this.translateRootSpans(spans))
 
           .catch(e => e);
     } catch (e) {
@@ -92,11 +95,11 @@ export class InstanaTraceExporter implements Exporter {
     }
   }
 
-  private translateRootSpans(rootSpans: Span[]): InstanaSpan[] {
+  private translateRootSpans(spans: Span[]): InstanaSpan[] {
     const result: InstanaSpan[] = [];
-    return rootSpans.reduce((agg, rootSpan) => {
-      agg.push(this.translateSpan(rootSpan));
-      return agg.concat(rootSpan.spans.map(this.translateSpan));
+    return spans.reduce((agg, span) => {
+      agg.push(this.translateSpan(span));
+      return agg.concat(span.spans.map(this.translateSpan));
     }, result);
   }