fix(tracing): span processor should receive a readable span as parame…

…ters
open-telemetry · May 8, 2020 · 112dad4 · 112dad4
1 parent 1dd3804
commit 112dad4
Show file tree

Hide file tree

Showing 9 changed files with 42 additions and 57 deletions.
diff --git a/packages/opentelemetry-exporter-zipkin/test/transform.test.ts b/packages/opentelemetry-exporter-zipkin/test/transform.test.ts
@@ -68,7 +68,7 @@ describe('transform', () => {
       span.end();
 
       const zipkinSpan = toZipkinSpan(
-        span.toReadableSpan(),
+        span,
         'my-service',
         statusCodeTagName,
         statusDescriptionTagName
@@ -112,7 +112,7 @@ describe('transform', () => {
       span.end();
 
       const zipkinSpan = toZipkinSpan(
-        span.toReadableSpan(),
+        span,
         'my-service',
         statusCodeTagName,
         statusDescriptionTagName
@@ -154,7 +154,7 @@ describe('transform', () => {
         span.end();
 
         const zipkinSpan = toZipkinSpan(
-          span.toReadableSpan(),
+          span,
           'my-service',
           statusCodeTagName,
           statusDescriptionTagName

diff --git a/packages/opentelemetry-plugin-http/test/functionals/utils.test.ts b/packages/opentelemetry-plugin-http/test/functionals/utils.test.ts
@@ -261,7 +261,7 @@ describe('Utility', () => {
         );
         /* tslint:disable-next-line:no-any */
         utils.setSpanWithError(span, new Error(errorMessage), obj as any);
-        const attributes = span.toReadableSpan().attributes;
+        const attributes = span.attributes;
         assert.strictEqual(
           attributes[AttributeNames.HTTP_ERROR_MESSAGE],
           errorMessage

diff --git a/packages/opentelemetry-tracing/src/MultiSpanProcessor.ts b/packages/opentelemetry-tracing/src/MultiSpanProcessor.ts
@@ -14,8 +14,8 @@
  * limitations under the License.
  */
 
-import { Span } from '@opentelemetry/api';
 import { SpanProcessor } from './SpanProcessor';
+import { ReadableSpan } from './export/ReadableSpan';
 
 /**
  * Implementation of the {@link SpanProcessor} that simply forwards all
@@ -30,13 +30,13 @@ export class MultiSpanProcessor implements SpanProcessor {
     }
   }
 
-  onStart(span: Span): void {
+  onStart(span: ReadableSpan): void {
     for (const spanProcessor of this._spanProcessors) {
       spanProcessor.onStart(span);
     }
   }
 
-  onEnd(span: Span): void {
+  onEnd(span: ReadableSpan): void {
     for (const spanProcessor of this._spanProcessors) {
       spanProcessor.onEnd(span);
     }

diff --git a/packages/opentelemetry-tracing/src/NoopSpanProcessor.ts b/packages/opentelemetry-tracing/src/NoopSpanProcessor.ts
@@ -14,13 +14,13 @@
  * limitations under the License.
  */
 
-import { Span } from '@opentelemetry/api';
 import { SpanProcessor } from './SpanProcessor';
+import { ReadableSpan } from './export/ReadableSpan';
 
 /** No-op implementation of SpanProcessor */
 export class NoopSpanProcessor implements SpanProcessor {
-  onStart(span: Span): void {}
-  onEnd(span: Span): void {}
+  onStart(span: ReadableSpan): void {}
+  onEnd(span: ReadableSpan): void {}
   shutdown(): void {}
   forceFlush(): void {}
 }
diff --git a/packages/opentelemetry-tracing/src/Span.ts b/packages/opentelemetry-tracing/src/Span.ts
@@ -175,10 +175,6 @@ export class Span implements api.Span, ReadableSpan {
     return true;
   }
 
-  toReadableSpan(): ReadableSpan {
-    return this;
-  }
-
   get duration(): api.HrTime {
     return this._duration;
   }

diff --git a/packages/opentelemetry-tracing/src/SpanProcessor.ts b/packages/opentelemetry-tracing/src/SpanProcessor.ts
@@ -14,7 +14,7 @@
  * limitations under the License.
  */
 
-import { Span } from '@opentelemetry/api';
+import { ReadableSpan } from './export/ReadableSpan';
 
 /**
  * SpanProcessor is the interface Tracer SDK uses to allow synchronous hooks
@@ -27,18 +27,18 @@ export interface SpanProcessor {
   forceFlush(): void;
 
   /**
-   * Called when a {@link Span} is started, if the `span.isRecording()`
+   * Called when a {@link ReadableSpan} is started, if the `span.isRecording()`
    * returns true.
    * @param span the Span that just started.
    */
-  onStart(span: Span): void;
+  onStart(span: ReadableSpan): void;
 
   /**
-   * Called when a {@link Span} is ended, if the `span.isRecording()`
+   * Called when a {@link ReadableSpan} is ended, if the `span.isRecording()`
    * returns true.
    * @param span the Span that just ended.
    */
-  onEnd(span: Span): void;
+  onEnd(span: ReadableSpan): void;
 
   /**
    * Shuts down the processor. Called when SDK is shut down. This is an

diff --git a/packages/opentelemetry-tracing/src/export/BatchSpanProcessor.ts b/packages/opentelemetry-tracing/src/export/BatchSpanProcessor.ts
@@ -15,7 +15,6 @@
  */
 
 import { unrefTimer } from '@opentelemetry/core';
-import { Span } from '../Span';
 import { SpanProcessor } from '../SpanProcessor';
 import { BufferConfig } from '../types';
 import { ReadableSpan } from './ReadableSpan';
@@ -53,13 +52,13 @@ export class BatchSpanProcessor implements SpanProcessor {
   }
 
   // does nothing.
-  onStart(span: Span): void {}
+  onStart(span: ReadableSpan): void {}
 
-  onEnd(span: Span): void {
+  onEnd(span: ReadableSpan): void {
     if (this._isShutdown) {
       return;
     }
-    this._addToBuffer(span.toReadableSpan());
+    this._addToBuffer(span);
   }
 
   shutdown(): void {

diff --git a/packages/opentelemetry-tracing/src/export/SimpleSpanProcessor.ts b/packages/opentelemetry-tracing/src/export/SimpleSpanProcessor.ts
@@ -14,9 +14,9 @@
  * limitations under the License.
  */
 
-import { Span } from '../Span';
 import { SpanProcessor } from '../SpanProcessor';
 import { SpanExporter } from './SpanExporter';
+import { ReadableSpan } from './ReadableSpan';
 
 /**
  * An implementation of the {@link SpanProcessor} that converts the {@link Span}
@@ -33,13 +33,13 @@ export class SimpleSpanProcessor implements SpanProcessor {
   }
 
   // does nothing.
-  onStart(span: Span): void {}
+  onStart(span: ReadableSpan): void {}
 
-  onEnd(span: Span): void {
+  onEnd(span: ReadableSpan): void {
     if (this._isShutdown) {
       return;
     }
-    this._exporter.export([span.toReadableSpan()], () => {});
+    this._exporter.export([span], () => {});
   }
 
   shutdown(): void {

diff --git a/packages/opentelemetry-tracing/test/Span.test.ts b/packages/opentelemetry-tracing/test/Span.test.ts
@@ -220,37 +220,33 @@ describe('Span', () => {
       parentId
     );
 
-    const readableSpan = span.toReadableSpan();
-    assert.strictEqual(readableSpan.name, 'my-span');
-    assert.strictEqual(readableSpan.kind, SpanKind.INTERNAL);
-    assert.strictEqual(readableSpan.parentSpanId, parentId);
-    assert.strictEqual(readableSpan.spanContext.traceId, spanContext.traceId);
-    assert.deepStrictEqual(readableSpan.status, {
+    assert.strictEqual(span.name, 'my-span');
+    assert.strictEqual(span.kind, SpanKind.INTERNAL);
+    assert.strictEqual(span.parentSpanId, parentId);
+    assert.strictEqual(span.spanContext.traceId, spanContext.traceId);
+    assert.deepStrictEqual(span.status, {
       code: CanonicalCode.OK,
     });
-    assert.deepStrictEqual(readableSpan.attributes, {});
-    assert.deepStrictEqual(readableSpan.links, []);
-    assert.deepStrictEqual(readableSpan.events, []);
+    assert.deepStrictEqual(span.attributes, {});
+    assert.deepStrictEqual(span.links, []);
+    assert.deepStrictEqual(span.events, []);
   });
 
   it('should return ReadableSpan with attributes', () => {
     const span = new Span(tracer, 'my-span', spanContext, SpanKind.CLIENT);
     span.setAttribute('attr1', 'value1');
-    let readableSpan = span.toReadableSpan();
-    assert.deepStrictEqual(readableSpan.attributes, { attr1: 'value1' });
+    assert.deepStrictEqual(span.attributes, { attr1: 'value1' });
 
     span.setAttributes({ attr2: 123, attr1: false });
-    readableSpan = span.toReadableSpan();
-    assert.deepStrictEqual(readableSpan.attributes, {
+    assert.deepStrictEqual(span.attributes, {
       attr1: false,
       attr2: 123,
     });
 
     span.end();
     // shouldn't add new attribute
     span.setAttribute('attr3', 'value3');
-    readableSpan = span.toReadableSpan();
-    assert.deepStrictEqual(readableSpan.attributes, {
+    assert.deepStrictEqual(span.attributes, {
       attr1: false,
       attr2: 123,
     });
@@ -271,9 +267,8 @@ describe('Span', () => {
         },
       ]
     );
-    const readableSpan = span.toReadableSpan();
-    assert.strictEqual(readableSpan.links.length, 2);
-    assert.deepStrictEqual(readableSpan.links, [
+    assert.strictEqual(span.links.length, 2);
+    assert.deepStrictEqual(span.links, [
       {
         context: linkContext,
       },
@@ -289,17 +284,15 @@ describe('Span', () => {
   it('should return ReadableSpan with events', () => {
     const span = new Span(tracer, 'my-span', spanContext, SpanKind.CLIENT);
     span.addEvent('sent');
-    let readableSpan = span.toReadableSpan();
-    assert.strictEqual(readableSpan.events.length, 1);
-    const [event] = readableSpan.events;
+    assert.strictEqual(span.events.length, 1);
+    const [event] = span.events;
     assert.deepStrictEqual(event.name, 'sent');
     assert.ok(!event.attributes);
     assert.ok(event.time[0] > 0);
 
     span.addEvent('rev', { attr1: 'value', attr2: 123, attr3: true });
-    readableSpan = span.toReadableSpan();
-    assert.strictEqual(readableSpan.events.length, 2);
-    const [event1, event2] = readableSpan.events;
+    assert.strictEqual(span.events.length, 2);
+    const [event1, event2] = span.events;
     assert.deepStrictEqual(event1.name, 'sent');
     assert.ok(!event1.attributes);
     assert.ok(event1.time[0] > 0);
@@ -314,9 +307,7 @@ describe('Span', () => {
     span.end();
     // shouldn't add new event
     span.addEvent('sent');
-    assert.strictEqual(readableSpan.events.length, 2);
-    readableSpan = span.toReadableSpan();
-    assert.strictEqual(readableSpan.events.length, 2);
+    assert.strictEqual(span.events.length, 2);
   });
 
   it('should return ReadableSpan with new status', () => {
@@ -325,12 +316,11 @@ describe('Span', () => {
       code: CanonicalCode.PERMISSION_DENIED,
       message: 'This is an error',
     });
-    const readableSpan = span.toReadableSpan();
     assert.strictEqual(
-      readableSpan.status.code,
+      span.status.code,
       CanonicalCode.PERMISSION_DENIED
     );
-    assert.strictEqual(readableSpan.status.message, 'This is an error');
+    assert.strictEqual(span.status.message, 'This is an error');
     span.end();
 
     // shouldn't update status