feat: implement on ending in span processor (#6024)

Author: majanjua-amzn

CHANGELOG.md

@@ -14,6 +14,9 @@ For notes on migrating to 2.x / 0.200.x see [the upgrade guide](doc/upgrade-to-2
 
 ### :rocket: Features
 
+* feat(sdk-trace-base): implement on ending in span processor [#6024](https://github.com/open-telemetry/opentelemetry-js/pull/6024) @majanjua-amzn
+  * note: this feature is experimental and subject to change
+
 ### :bug: Bug Fixes
 
 ### :books: Documentation

packages/opentelemetry-sdk-trace-base/src/MultiSpanProcessor.ts

@@ -53,6 +53,14 @@ export class MultiSpanProcessor implements SpanProcessor {
     }
   }
 
+  onEnding(span: Span): void {
+    for (const spanProcessor of this._spanProcessors) {
+      if (spanProcessor.onEnding) {
+        spanProcessor.onEnding(span);
+      }
+    }
+  }
+
   onEnd(span: ReadableSpan): void {
     for (const spanProcessor of this._spanProcessors) {
       spanProcessor.onEnd(span);

packages/opentelemetry-sdk-trace-base/src/Span.ts

@@ -270,8 +270,6 @@ export class SpanImpl implements Span {
       );
       return;
     }
-    this._ended = true;
-
     this.endTime = this._getTime(endTime);
     this._duration = hrTimeDuration(this.startTime, this.endTime);
 
@@ -290,7 +288,11 @@ export class SpanImpl implements Span {
         `Dropped ${this._droppedEventsCount} events because eventCountLimit reached`
       );
     }
+    if (this._spanProcessor.onEnding) {
+      this._spanProcessor.onEnding(this);
+    }
 
+    this._ended = true;
     this._spanProcessor.onEnd(this);
   }
 

packages/opentelemetry-sdk-trace-base/src/SpanProcessor.ts

@@ -35,6 +35,15 @@ export interface SpanProcessor {
    */
   onStart(span: Span, parentContext: Context): void;
 
+  /**
+   * Called when a {@link Span} is ending, if the `span.isRecording()`
+   * returns true.
+   * @param span the Span that is ending.
+   *
+   * @experimental This method is experimental and may break in minor versions of this package
+   */
+  onEnding?(span: Span): void;
+
   /**
    * Called when a {@link ReadableSpan} is ended, if the `span.isRecording()`
    * returns true.

packages/opentelemetry-sdk-trace-base/test/common/MultiSpanProcessor.test.ts

@@ -30,20 +30,39 @@ import {
 import { MultiSpanProcessor } from '../../src/MultiSpanProcessor';
 
 class TestProcessor implements SpanProcessor {
+  static events: string[] = [];
+  name: string;
   spans: Span[] = [];
-  onStart(span: Span): void {}
+
+  constructor(name: string) {
+    this.name = name;
+  }
+
+  onStart(span: Span): void {
+    TestProcessor.events.push(this.name + ':start');
+  }
+
   onEnd(span: Span): void {
+    TestProcessor.events.push(this.name + ':end');
     this.spans.push(span);
   }
+
   shutdown(): Promise<void> {
-    this.spans = [];
+    TestProcessor.events = [];
     return Promise.resolve();
   }
+
   forceFlush(): Promise<void> {
     return Promise.resolve();
   }
 }
 
+class ExtendedTestProcessor extends TestProcessor {
+  onEnding(span: Span): void {
+    TestProcessor.events.push(this.name + ':ending');
+  }
+}
+
 describe('MultiSpanProcessor', () => {
   let removeEvent: (() => void) | undefined;
   afterEach(() => {
@@ -54,63 +73,71 @@ describe('MultiSpanProcessor', () => {
   });
 
   it('should handle one span processor', () => {
-    const processor1 = new TestProcessor();
+    const processor1 = new TestProcessor('sp1');
     const tracerProvider = new BasicTracerProvider({
       spanProcessors: [processor1],
     });
     const tracer = tracerProvider.getTracer('default');
     const span = tracer.startSpan('one');
-    assert.strictEqual(processor1.spans.length, 0);
+    assert.deepStrictEqual(TestProcessor.events, ['sp1:start']);
     span.end();
-    assert.strictEqual(processor1.spans.length, 1);
+    assert.deepStrictEqual(TestProcessor.events, ['sp1:start', 'sp1:end']);
     tracerProvider['_activeSpanProcessor'].shutdown();
   });
 
-  it('should handle two span processor', async () => {
-    const processor1 = new TestProcessor();
-    const processor2 = new TestProcessor();
+  it('should handle one span processor with on ending', () => {
+    TestProcessor.events = [];
+    const processor1 = new ExtendedTestProcessor('sp1');
     const tracerProvider = new BasicTracerProvider({
-      spanProcessors: [processor1, processor2],
+      spanProcessors: [processor1],
     });
     const tracer = tracerProvider.getTracer('default');
     const span = tracer.startSpan('one');
-    assert.strictEqual(processor1.spans.length, 0);
-    assert.strictEqual(processor1.spans.length, processor2.spans.length);
-
+    assert.deepStrictEqual(TestProcessor.events, ['sp1:start']);
     span.end();
-    assert.strictEqual(processor1.spans.length, 1);
-    assert.strictEqual(processor1.spans.length, processor2.spans.length);
-
-    tracerProvider.shutdown().then(() => {
-      assert.strictEqual(processor1.spans.length, 0);
-      assert.strictEqual(processor1.spans.length, processor2.spans.length);
-    });
+    assert.deepStrictEqual(TestProcessor.events, [
+      'sp1:start',
+      'sp1:ending',
+      'sp1:end',
+    ]);
+    tracerProvider['_activeSpanProcessor'].shutdown();
   });
 
-  it('should export spans on manual shutdown from two span processor', () => {
-    const processor1 = new TestProcessor();
-    const processor2 = new TestProcessor();
+  it('should handle two span processor', async () => {
+    TestProcessor.events = [];
+    const processor1 = new TestProcessor('p1');
+    const processor2 = new ExtendedTestProcessor('p2');
     const tracerProvider = new BasicTracerProvider({
       spanProcessors: [processor1, processor2],
     });
     const tracer = tracerProvider.getTracer('default');
     const span = tracer.startSpan('one');
     assert.strictEqual(processor1.spans.length, 0);
-    assert.strictEqual(processor1.spans.length, processor2.spans.length);
+    assert.strictEqual(processor2.spans.length, processor2.spans.length);
+    assert.deepStrictEqual(TestProcessor.events, ['p1:start', 'p2:start']);
 
     span.end();
     assert.strictEqual(processor1.spans.length, 1);
     assert.strictEqual(processor1.spans.length, processor2.spans.length);
+    assert.deepStrictEqual(TestProcessor.events, [
+      'p1:start',
+      'p2:start',
+      'p2:ending',
+      'p1:end',
+      'p2:end',
+    ]);
 
     tracerProvider.shutdown().then(() => {
       assert.strictEqual(processor1.spans.length, 0);
       assert.strictEqual(processor1.spans.length, processor2.spans.length);
+      assert.deepStrictEqual(TestProcessor.events.length, 0);
     });
   });
 
   it('should export spans on manual shutdown from two span processor', () => {
-    const processor1 = new TestProcessor();
-    const processor2 = new TestProcessor();
+    TestProcessor.events = [];
+    const processor1 = new TestProcessor('p1');
+    const processor2 = new ExtendedTestProcessor('p2');
     const tracerProvider = new BasicTracerProvider({
       spanProcessors: [processor1, processor2],
     });
@@ -172,7 +199,8 @@ describe('MultiSpanProcessor', () => {
 
   it('should call globalErrorHandler in forceFlush', async () => {
     const expectedError = new Error('whoops');
-    const testProcessor = new TestProcessor();
+    TestProcessor.events = [];
+    const testProcessor = new TestProcessor('p1');
     const forceFlush = Sinon.stub(testProcessor, 'forceFlush');
     forceFlush.rejects(expectedError);
 

packages/opentelemetry-sdk-trace-base/test/common/Span.test.ts

@@ -1346,6 +1346,29 @@ describe('Span', () => {
       const s = provider.getTracer('default').startSpan('test') as Span;
       assert.ok(s.attributes.attr);
     });
+
+    it('should call onEnding with a writeable span', () => {
+      const processor: SpanProcessor = {
+        onStart: span => {
+          span.setAttribute('attr', true);
+        },
+        forceFlush: () => Promise.resolve(),
+        onEnding: span => {
+          span.setAttribute('attr2', true);
+        },
+        onEnd() {},
+        shutdown: () => Promise.resolve(),
+      };
+
+      const provider = new BasicTracerProvider({
+        spanProcessors: [processor],
+      });
+
+      const s = provider.getTracer('default').startSpan('test') as Span;
+      s.end();
+      assert.ok(s.attributes.attr);
+      assert.ok(s.attributes.attr2);
+    });
   });
 
   describe('recordException', () => {