chore: ensure onStart is called with a writeable span (#1621)

packages/opentelemetry-plugin-xml-http-request/test/unmocked.test.ts

@@ -13,19 +13,20 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
-import { XMLHttpRequestPlugin } from '../src';
+import { Span } from '@opentelemetry/api';
+import { HttpAttribute } from '@opentelemetry/semantic-conventions';
 import { ReadableSpan, SpanProcessor } from '@opentelemetry/tracing';
 import { WebTracerProvider } from '@opentelemetry/web';
+import { XMLHttpRequestPlugin } from '../src';
 import assert = require('assert');
-import { HttpAttribute } from '@opentelemetry/semantic-conventions';
 
 class TestSpanProcessor implements SpanProcessor {
   spans: ReadableSpan[] = [];
 
   forceFlush(): Promise<void> {
     return Promise.resolve();
   }
-  onStart(span: ReadableSpan): void {}
+  onStart(span: Span): void {}
   shutdown(): Promise<void> {
     return Promise.resolve();
   }

packages/opentelemetry-tracing/src/MultiSpanProcessor.ts

@@ -15,8 +15,9 @@
  */
 
 import { globalErrorHandler } from '@opentelemetry/core';
-import { SpanProcessor } from './SpanProcessor';
 import { ReadableSpan } from './export/ReadableSpan';
+import { Span } from './Span';
+import { SpanProcessor } from './SpanProcessor';
 
 /**
  * Implementation of the {@link SpanProcessor} that simply forwards all
@@ -45,7 +46,7 @@ export class MultiSpanProcessor implements SpanProcessor {
     });
   }
 
-  onStart(span: ReadableSpan): void {
+  onStart(span: Span): void {
     for (const spanProcessor of this._spanProcessors) {
       spanProcessor.onStart(span);
     }

packages/opentelemetry-tracing/src/NoopSpanProcessor.ts

@@ -14,12 +14,13 @@
  * limitations under the License.
  */
 
-import { SpanProcessor } from './SpanProcessor';
 import { ReadableSpan } from './export/ReadableSpan';
+import { Span } from './Span';
+import { SpanProcessor } from './SpanProcessor';
 
 /** No-op implementation of SpanProcessor */
 export class NoopSpanProcessor implements SpanProcessor {
-  onStart(span: ReadableSpan): void {}
+  onStart(span: Span): void {}
   onEnd(span: ReadableSpan): void {}
   shutdown(): Promise<void> {
     return Promise.resolve();

packages/opentelemetry-tracing/src/SpanProcessor.ts

@@ -15,6 +15,7 @@
  */
 
 import { ReadableSpan } from './export/ReadableSpan';
+import { Span } from './Span';
 
 /**
  * SpanProcessor is the interface Tracer SDK uses to allow synchronous hooks
@@ -27,11 +28,11 @@ export interface SpanProcessor {
   forceFlush(): Promise<void>;
 
   /**
-   * Called when a {@link ReadableSpan} is started, if the `span.isRecording()`
+   * Called when a {@link Span} is started, if the `span.isRecording()`
    * returns true.
    * @param span the Span that just started.
    */
-  onStart(span: ReadableSpan): void;
+  onStart(span: Span): void;
 
   /**
    * Called when a {@link ReadableSpan} is ended, if the `span.isRecording()`

packages/opentelemetry-tracing/src/export/BatchSpanProcessor.ts

@@ -16,6 +16,7 @@
 
 import { context, suppressInstrumentation } from '@opentelemetry/api';
 import { ExportResult, unrefTimer } from '@opentelemetry/core';
+import { Span } from '../Span';
 import { SpanProcessor } from '../SpanProcessor';
 import { BufferConfig } from '../types';
 import { ReadableSpan } from './ReadableSpan';
@@ -54,7 +55,7 @@ export class BatchSpanProcessor implements SpanProcessor {
   }
 
   // does nothing.
-  onStart(span: ReadableSpan): void {}
+  onStart(span: Span): void {}
 
   onEnd(span: ReadableSpan): void {
     if (this._isShutdown) {

packages/opentelemetry-tracing/src/export/SimpleSpanProcessor.ts

@@ -14,10 +14,11 @@
  * limitations under the License.
  */
 
+import { context, suppressInstrumentation } from '@opentelemetry/api';
+import { Span } from '../Span';
 import { SpanProcessor } from '../SpanProcessor';
-import { SpanExporter } from './SpanExporter';
 import { ReadableSpan } from './ReadableSpan';
-import { context, suppressInstrumentation } from '@opentelemetry/api';
+import { SpanExporter } from './SpanExporter';
 
 /**
  * An implementation of the {@link SpanProcessor} that converts the {@link Span}
@@ -37,7 +38,7 @@ export class SimpleSpanProcessor implements SpanProcessor {
   }
 
   // does nothing.
-  onStart(span: ReadableSpan): void {}
+  onStart(span: Span): void {}
 
   onEnd(span: ReadableSpan): void {
     if (this._isShutdown) {

packages/opentelemetry-tracing/test/Span.test.ts

@@ -14,24 +14,24 @@
  * limitations under the License.
  */
 
-import { ExceptionAttribute } from '@opentelemetry/semantic-conventions';
-import * as assert from 'assert';
 import {
-  SpanKind,
   CanonicalCode,
-  TraceFlags,
-  SpanContext,
-  LinkContext,
   Exception,
+  LinkContext,
+  SpanContext,
+  SpanKind,
+  TraceFlags,
 } from '@opentelemetry/api';
-import { BasicTracerProvider, Span } from '../src';
 import {
   hrTime,
-  hrTimeToNanoseconds,
+  hrTimeDuration,
   hrTimeToMilliseconds,
+  hrTimeToNanoseconds,
   NoopLogger,
-  hrTimeDuration,
 } from '@opentelemetry/core';
+import { ExceptionAttribute } from '@opentelemetry/semantic-conventions';
+import * as assert from 'assert';
+import { BasicTracerProvider, Span, SpanProcessor } from '../src';
 
 const performanceTimeOrigin = hrTime();
 
@@ -412,6 +412,70 @@ describe('Span', () => {
     assert.strictEqual(span.ended, true);
   });
 
+  describe('span processor', () => {
+    it('should call onStart synchronously when span is started', () => {
+      let started = false;
+      const processor: SpanProcessor = {
+        onStart: () => {
+          started = true;
+        },
+        forceFlush: () => Promise.resolve(),
+        onEnd() {},
+        shutdown: () => Promise.resolve(),
+      };
+
+      const provider = new BasicTracerProvider({
+        logger: new NoopLogger(),
+      });
+
+      provider.addSpanProcessor(processor);
+
+      provider.getTracer('default').startSpan('test');
+      assert.ok(started);
+    });
+
+    it('should call onEnd synchronously when span is ended', () => {
+      let ended = false;
+      const processor: SpanProcessor = {
+        onStart: () => {},
+        forceFlush: () => Promise.resolve(),
+        onEnd() {
+          ended = true;
+        },
+        shutdown: () => Promise.resolve(),
+      };
+
+      const provider = new BasicTracerProvider({
+        logger: new NoopLogger(),
+      });
+
+      provider.addSpanProcessor(processor);
+
+      provider.getTracer('default').startSpan('test').end();
+      assert.ok(ended);
+    });
+
+    it('should call onStart with a writeable span', () => {
+      const processor: SpanProcessor = {
+        onStart: span => {
+          span.setAttribute('attr', true);
+        },
+        forceFlush: () => Promise.resolve(),
+        onEnd() {},
+        shutdown: () => Promise.resolve(),
+      };
+
+      const provider = new BasicTracerProvider({
+        logger: new NoopLogger(),
+      });
+
+      provider.addSpanProcessor(processor);
+
+      const s = provider.getTracer('default').startSpan('test') as Span;
+      assert.ok(s.attributes.attr);
+    });
+  });
+
   describe('recordException', () => {
     const invalidExceptions: any[] = [
       1,