From b7fa9e8659378210a3a78a0a04e8f6cb9a67df8c Mon Sep 17 00:00:00 2001 From: Bartlomiej Obecny Date: Thu, 20 Feb 2020 20:42:27 +0100 Subject: [PATCH 1/3] chore: adding force flash to span processors --- .../src/CollectorExporter.ts | 7 ---- .../test/common/CollectorExporter.test.ts | 11 ------ .../src/types.ts | 2 - .../src/zipkin.ts | 7 ---- .../test/zipkin.test.ts | 16 -------- .../src/version.ts | 2 +- .../src/MultiSpanProcessor.ts | 4 ++ .../src/NoopSpanProcessor.ts | 1 + .../src/SpanProcessor.ts | 5 +++ .../src/export/BatchSpanProcessor.ts | 18 ++++++++- .../src/export/SimpleSpanProcessor.ts | 13 +++++++ .../test/MultiSpanProcessor.test.ts | 1 + .../test/export/BatchSpanProcessor.test.ts | 37 +++++++++++++++++++ 13 files changed, 79 insertions(+), 45 deletions(-) diff --git a/packages/opentelemetry-exporter-collector/src/CollectorExporter.ts b/packages/opentelemetry-exporter-collector/src/CollectorExporter.ts index f825c99f1e..82aee23399 100644 --- a/packages/opentelemetry-exporter-collector/src/CollectorExporter.ts +++ b/packages/opentelemetry-exporter-collector/src/CollectorExporter.ts @@ -123,12 +123,5 @@ export class CollectorExporter implements SpanExporter { // platform dependent onShutdown(this.shutdown); - - // @TODO get spans from span processor (batch) - this._exportSpans([]) - .then(() => { - this.logger.debug('shutdown completed'); - }) - .catch(() => {}); } } diff --git a/packages/opentelemetry-exporter-collector/test/common/CollectorExporter.test.ts b/packages/opentelemetry-exporter-collector/test/common/CollectorExporter.test.ts index 12b7985f1d..1c37f37b8e 100644 --- a/packages/opentelemetry-exporter-collector/test/common/CollectorExporter.test.ts +++ b/packages/opentelemetry-exporter-collector/test/common/CollectorExporter.test.ts @@ -152,17 +152,6 @@ describe('CollectorExporter - common', () => { onShutdownSpy.restore(); }); - it('should export spans once only', done => { - collectorExporter.shutdown(); - collectorExporter.shutdown(); - collectorExporter.shutdown(); - - setTimeout(() => { - assert.strictEqual(onShutdownSpy.callCount, 1); - done(); - }); - }); - it('should call onShutdown', done => { collectorExporter.shutdown(); setTimeout(() => { diff --git a/packages/opentelemetry-exporter-zipkin/src/types.ts b/packages/opentelemetry-exporter-zipkin/src/types.ts index 4efd641f89..343faf09b6 100644 --- a/packages/opentelemetry-exporter-zipkin/src/types.ts +++ b/packages/opentelemetry-exporter-zipkin/src/types.ts @@ -23,8 +23,6 @@ export interface ExporterConfig { logger?: types.Logger; serviceName: string; url?: string; - // Initiates a request with spans in memory to the backend. - forceFlush?: boolean; // Optional mapping overrides for OpenTelemetry status code and description. statusCodeTagName?: string; statusDescriptionTagName?: string; diff --git a/packages/opentelemetry-exporter-zipkin/src/zipkin.ts b/packages/opentelemetry-exporter-zipkin/src/zipkin.ts index 1939a9eb65..56a06469f2 100644 --- a/packages/opentelemetry-exporter-zipkin/src/zipkin.ts +++ b/packages/opentelemetry-exporter-zipkin/src/zipkin.ts @@ -33,7 +33,6 @@ import { OT_REQUEST_HEADER } from './utils'; */ export class ZipkinExporter implements SpanExporter { static readonly DEFAULT_URL = 'http://localhost:9411/api/v2/spans'; - private readonly _forceFlush: boolean; private readonly _logger: types.Logger; private readonly _serviceName: string; private readonly _statusCodeTagName: string; @@ -45,7 +44,6 @@ export class ZipkinExporter implements SpanExporter { const urlStr = config.url || ZipkinExporter.DEFAULT_URL; const urlOpts = url.parse(urlStr); - this._forceFlush = config.forceFlush || true; this._logger = config.logger || new NoopLogger(); this._reqOpts = Object.assign( { @@ -88,11 +86,6 @@ export class ZipkinExporter implements SpanExporter { return; } this._isShutdown = true; - // Make an optimistic flush. - if (this._forceFlush) { - // @todo get spans from span processor (batch) - this._sendSpans([]); - } } /** diff --git a/packages/opentelemetry-exporter-zipkin/test/zipkin.test.ts b/packages/opentelemetry-exporter-zipkin/test/zipkin.test.ts index b4b0a5177a..806f4fd038 100644 --- a/packages/opentelemetry-exporter-zipkin/test/zipkin.test.ts +++ b/packages/opentelemetry-exporter-zipkin/test/zipkin.test.ts @@ -72,14 +72,6 @@ describe('ZipkinExporter', () => { assert.ok(typeof exporter.export === 'function'); assert.ok(typeof exporter.shutdown === 'function'); }); - it('should construct an exporter with forceFlush', () => { - const exporter = new ZipkinExporter({ - serviceName: 'my-service', - forceFlush: false, - }); - assert.ok(typeof exporter.export === 'function'); - assert.ok(typeof exporter.shutdown === 'function'); - }); it('should construct an exporter with statusCodeTagName', () => { const exporter = new ZipkinExporter({ serviceName: 'my-service', @@ -338,13 +330,5 @@ describe('ZipkinExporter', () => { // @todo: implement it('should send by default'); - it('should not send with forceFlush=false', () => { - const exporter = new ZipkinExporter({ - serviceName: 'my-service', - forceFlush: false, - }); - - exporter.shutdown(); - }); }); }); diff --git a/packages/opentelemetry-plugin-express/src/version.ts b/packages/opentelemetry-plugin-express/src/version.ts index 2efbb00dcb..2c69dc0f79 100644 --- a/packages/opentelemetry-plugin-express/src/version.ts +++ b/packages/opentelemetry-plugin-express/src/version.ts @@ -15,4 +15,4 @@ */ // this is autogenerated file, see scripts/version-update.js -export const VERSION = '0.3.2'; +export const VERSION = '0.4.0'; diff --git a/packages/opentelemetry-tracing/src/MultiSpanProcessor.ts b/packages/opentelemetry-tracing/src/MultiSpanProcessor.ts index 54e31ac756..b4d4c9d3fa 100644 --- a/packages/opentelemetry-tracing/src/MultiSpanProcessor.ts +++ b/packages/opentelemetry-tracing/src/MultiSpanProcessor.ts @@ -24,6 +24,10 @@ import { SpanProcessor } from './SpanProcessor'; export class MultiSpanProcessor implements SpanProcessor { constructor(private readonly _spanProcessors: SpanProcessor[]) {} + forceFlush(): void { + // do nothing as all spans are being exported without waiting + } + onStart(span: Span): void { for (const spanProcessor of this._spanProcessors) { spanProcessor.onStart(span); diff --git a/packages/opentelemetry-tracing/src/NoopSpanProcessor.ts b/packages/opentelemetry-tracing/src/NoopSpanProcessor.ts index 9918d2394f..2898e77cfa 100644 --- a/packages/opentelemetry-tracing/src/NoopSpanProcessor.ts +++ b/packages/opentelemetry-tracing/src/NoopSpanProcessor.ts @@ -22,4 +22,5 @@ export class NoopSpanProcessor implements SpanProcessor { onStart(span: Span): void {} onEnd(span: Span): void {} shutdown(): void {} + forceFlush(): void {} } diff --git a/packages/opentelemetry-tracing/src/SpanProcessor.ts b/packages/opentelemetry-tracing/src/SpanProcessor.ts index 233d9fb611..0320455ae9 100644 --- a/packages/opentelemetry-tracing/src/SpanProcessor.ts +++ b/packages/opentelemetry-tracing/src/SpanProcessor.ts @@ -21,6 +21,11 @@ import { Span } from '@opentelemetry/api'; * for when a {@link Span} is started or when a {@link Span} is ended. */ export interface SpanProcessor { + /** + * Forces to export all finished spans + */ + forceFlush(): void; + /** * Called when a {@link Span} is started, if the `span.isRecording()` * returns true. diff --git a/packages/opentelemetry-tracing/src/export/BatchSpanProcessor.ts b/packages/opentelemetry-tracing/src/export/BatchSpanProcessor.ts index 80f1b33eab..0c32e1e1ca 100644 --- a/packages/opentelemetry-tracing/src/export/BatchSpanProcessor.ts +++ b/packages/opentelemetry-tracing/src/export/BatchSpanProcessor.ts @@ -35,12 +35,13 @@ export class BatchSpanProcessor implements SpanProcessor { private _finishedSpans: ReadableSpan[] = []; private _lastSpanFlush = Date.now(); private _timer: NodeJS.Timeout; + private _isShutdown = false; constructor(private readonly _exporter: SpanExporter, config?: BufferConfig) { this._bufferSize = config && config.bufferSize ? config.bufferSize : DEFAULT_BUFFER_SIZE; this._bufferTimeout = - config && config.bufferTimeout + config && typeof config.bufferTimeout === 'number' ? config.bufferTimeout : DEFAULT_BUFFER_TIMEOUT_MS; @@ -52,16 +53,31 @@ export class BatchSpanProcessor implements SpanProcessor { unrefTimer(this._timer); } + forceFlush(): void { + if (this._isShutdown) { + return; + } + this._flush(); + } + // does nothing. onStart(span: Span): void {} onEnd(span: Span): void { + if (this._isShutdown) { + return; + } if (span.context().traceFlags !== TraceFlags.SAMPLED) return; this._addToBuffer(span.toReadableSpan()); } shutdown(): void { + if (this._isShutdown) { + return; + } clearInterval(this._timer); + this.forceFlush(); + this._isShutdown = true; this._exporter.shutdown(); } diff --git a/packages/opentelemetry-tracing/src/export/SimpleSpanProcessor.ts b/packages/opentelemetry-tracing/src/export/SimpleSpanProcessor.ts index c92ae20294..dc49015d29 100644 --- a/packages/opentelemetry-tracing/src/export/SimpleSpanProcessor.ts +++ b/packages/opentelemetry-tracing/src/export/SimpleSpanProcessor.ts @@ -27,16 +27,29 @@ import { Span } from '../Span'; */ export class SimpleSpanProcessor implements SpanProcessor { constructor(private readonly _exporter: SpanExporter) {} + private _isShutdown = false; + + forceFlush(): void { + // do nothing as all spans are being exported without waiting + } // does nothing. onStart(span: Span): void {} onEnd(span: Span): void { + if (this._isShutdown) { + return; + } if (span.context().traceFlags !== TraceFlags.SAMPLED) return; this._exporter.export([span.toReadableSpan()], () => {}); } shutdown(): void { + if (this._isShutdown) { + return; + } + this._isShutdown = true; + this._exporter.shutdown(); } } diff --git a/packages/opentelemetry-tracing/test/MultiSpanProcessor.test.ts b/packages/opentelemetry-tracing/test/MultiSpanProcessor.test.ts index 66b0165255..62b33fe224 100644 --- a/packages/opentelemetry-tracing/test/MultiSpanProcessor.test.ts +++ b/packages/opentelemetry-tracing/test/MultiSpanProcessor.test.ts @@ -27,6 +27,7 @@ class TestProcessor implements SpanProcessor { shutdown(): void { this.spans = []; } + forceFlush(): void {} } describe('MultiSpanProcessor', () => { diff --git a/packages/opentelemetry-tracing/test/export/BatchSpanProcessor.test.ts b/packages/opentelemetry-tracing/test/export/BatchSpanProcessor.test.ts index 2fbb7d8754..50e477dcb4 100644 --- a/packages/opentelemetry-tracing/test/export/BatchSpanProcessor.test.ts +++ b/packages/opentelemetry-tracing/test/export/BatchSpanProcessor.test.ts @@ -78,6 +78,32 @@ describe('BatchSpanProcessor', () => { }); describe('.onStart/.onEnd/.shutdown', () => { + it('should do nothing after processor is shutdown', () => { + const processor = new BatchSpanProcessor(exporter, defaultBufferConfig); + const spy: sinon.SinonSpy = sinon.spy(exporter, 'export') as any; + + const span = createSampledSpan(`${name}_0`); + + processor.onEnd(span); + assert.strictEqual(processor['_finishedSpans'].length, 1); + + processor.forceFlush(); + assert.strictEqual(exporter.getFinishedSpans().length, 1); + + processor.onEnd(span); + assert.strictEqual(processor['_finishedSpans'].length, 1); + + assert.strictEqual(spy.args.length, 1); + processor.shutdown(); + assert.strictEqual(spy.args.length, 2); + assert.strictEqual(exporter.getFinishedSpans().length, 0); + + processor.onEnd(span); + assert.strictEqual(spy.args.length, 2); + assert.strictEqual(processor['_finishedSpans'].length, 0); + assert.strictEqual(exporter.getFinishedSpans().length, 0); + }); + it('should export the sampled spans with buffer size reached', () => { const processor = new BatchSpanProcessor(exporter, defaultBufferConfig); for (let i = 0; i < defaultBufferConfig.bufferSize; i++) { @@ -125,5 +151,16 @@ describe('BatchSpanProcessor', () => { clock.restore(); }); + + it('should force flush on demand', () => { + const processor = new BatchSpanProcessor(exporter, defaultBufferConfig); + for (let i = 0; i < defaultBufferConfig.bufferSize; i++) { + const span = createSampledSpan(`${name}_${i}`); + processor.onEnd(span); + } + assert.strictEqual(exporter.getFinishedSpans().length, 0); + processor.forceFlush(); + assert.strictEqual(exporter.getFinishedSpans().length, 5); + }); }); }); From 8e3ae1b15af5dc71b5fe71398c01c0d9e480b7eb Mon Sep 17 00:00:00 2001 From: Bartlomiej Obecny Date: Mon, 2 Mar 2020 23:27:51 +0100 Subject: [PATCH 2/3] chore: fixing merge --- packages/opentelemetry-tracing/src/export/BatchSpanProcessor.ts | 1 - .../opentelemetry-tracing/src/export/SimpleSpanProcessor.ts | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/packages/opentelemetry-tracing/src/export/BatchSpanProcessor.ts b/packages/opentelemetry-tracing/src/export/BatchSpanProcessor.ts index b188d24faf..a839415c1f 100644 --- a/packages/opentelemetry-tracing/src/export/BatchSpanProcessor.ts +++ b/packages/opentelemetry-tracing/src/export/BatchSpanProcessor.ts @@ -66,7 +66,6 @@ export class BatchSpanProcessor implements SpanProcessor { if (this._isShutdown) { return; } - if (span.context().traceFlags !== TraceFlags.SAMPLED) return; this._addToBuffer(span.toReadableSpan()); } diff --git a/packages/opentelemetry-tracing/src/export/SimpleSpanProcessor.ts b/packages/opentelemetry-tracing/src/export/SimpleSpanProcessor.ts index b9cede5c2f..15a3632327 100644 --- a/packages/opentelemetry-tracing/src/export/SimpleSpanProcessor.ts +++ b/packages/opentelemetry-tracing/src/export/SimpleSpanProcessor.ts @@ -1,3 +1,4 @@ + /*! * Copyright 2019, OpenTelemetry Authors * @@ -39,7 +40,6 @@ export class SimpleSpanProcessor implements SpanProcessor { if (this._isShutdown) { return; } - if (span.context().traceFlags !== TraceFlags.SAMPLED) return; this._exporter.export([span.toReadableSpan()], () => {}); } From 076d9ad42c60e41be13cfcb384b4103ecc5cd751 Mon Sep 17 00:00:00 2001 From: Bartlomiej Obecny Date: Mon, 2 Mar 2020 23:35:52 +0100 Subject: [PATCH 3/3] chore: fixing merge --- packages/opentelemetry-tracing/src/export/SimpleSpanProcessor.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/opentelemetry-tracing/src/export/SimpleSpanProcessor.ts b/packages/opentelemetry-tracing/src/export/SimpleSpanProcessor.ts index 15a3632327..e76797cfcf 100644 --- a/packages/opentelemetry-tracing/src/export/SimpleSpanProcessor.ts +++ b/packages/opentelemetry-tracing/src/export/SimpleSpanProcessor.ts @@ -1,4 +1,3 @@ - /*! * Copyright 2019, OpenTelemetry Authors *