From 2ae80cf58aa8b41964e06042a237a1aaca827052 Mon Sep 17 00:00:00 2001 From: Daniel Dyla Date: Tue, 24 Mar 2020 16:57:45 -0400 Subject: [PATCH 1/6] chore: do not export empty span lists --- packages/opentelemetry-tracing/src/export/BatchSpanProcessor.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/opentelemetry-tracing/src/export/BatchSpanProcessor.ts b/packages/opentelemetry-tracing/src/export/BatchSpanProcessor.ts index a839415c1f..6d49b5e125 100644 --- a/packages/opentelemetry-tracing/src/export/BatchSpanProcessor.ts +++ b/packages/opentelemetry-tracing/src/export/BatchSpanProcessor.ts @@ -89,7 +89,7 @@ export class BatchSpanProcessor implements SpanProcessor { private _shouldFlush(): boolean { return ( - this._finishedSpans.length >= 0 && + this._finishedSpans.length > 0 && Date.now() - this._lastSpanFlush >= this._bufferTimeout ); } From 0311ad60b8463006165000c6d803035e9bba6bba Mon Sep 17 00:00:00 2001 From: Daniel Dyla Date: Tue, 24 Mar 2020 17:25:50 -0400 Subject: [PATCH 2/6] test: should not export empty span lists --- .../test/export/BatchSpanProcessor.test.ts | 31 +++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/packages/opentelemetry-tracing/test/export/BatchSpanProcessor.test.ts b/packages/opentelemetry-tracing/test/export/BatchSpanProcessor.test.ts index 07308a6da1..c4452bb069 100644 --- a/packages/opentelemetry-tracing/test/export/BatchSpanProcessor.test.ts +++ b/packages/opentelemetry-tracing/test/export/BatchSpanProcessor.test.ts @@ -45,6 +45,7 @@ describe('BatchSpanProcessor', () => { }); afterEach(() => { exporter.reset(); + sinon.restore(); }); describe('constructor', () => { @@ -142,5 +143,35 @@ describe('BatchSpanProcessor', () => { processor.forceFlush(); assert.strictEqual(exporter.getFinishedSpans().length, 5); }); + + it('should not export empty span lists', done => { + const spy = sinon.spy(exporter, 'export'); + const clock = sinon.useFakeTimers(); + + const tracer = new BasicTracerProvider({ + sampler: ALWAYS_SAMPLER, + }).getTracer('default'); + const processor = new BatchSpanProcessor(exporter, defaultBufferConfig); + + // start but do not end spans + for (let i = 0; i < defaultBufferConfig.bufferSize; i++) { + const span = tracer.startSpan('spanName'); + processor.onStart(span as Span); + } + + setTimeout(() => { + assert.strictEqual(exporter.getFinishedSpans().length, 0); + // after the timeout, export should not have been called + // because no spans are ended + sinon.assert.notCalled(spy); + done(); + }, defaultBufferConfig.bufferTimeout + 1000); + + // no spans have been finished + assert.strictEqual(exporter.getFinishedSpans().length, 0); + clock.tick(defaultBufferConfig.bufferTimeout + 1000); + + clock.restore(); + }); }); }); From 84e2f8a3a13623e868813cfab9cd783c06bfcb20 Mon Sep 17 00:00:00 2001 From: Daniel Dyla Date: Wed, 25 Mar 2020 17:15:43 -0400 Subject: [PATCH 3/6] chore: remove polling timer from batch span processor --- .../src/export/BatchSpanProcessor.ts | 42 ++++++++++--------- 1 file changed, 23 insertions(+), 19 deletions(-) diff --git a/packages/opentelemetry-tracing/src/export/BatchSpanProcessor.ts b/packages/opentelemetry-tracing/src/export/BatchSpanProcessor.ts index 6d49b5e125..84f657327b 100644 --- a/packages/opentelemetry-tracing/src/export/BatchSpanProcessor.ts +++ b/packages/opentelemetry-tracing/src/export/BatchSpanProcessor.ts @@ -31,9 +31,9 @@ const DEFAULT_BUFFER_TIMEOUT_MS = 20_000; export class BatchSpanProcessor implements SpanProcessor { private readonly _bufferSize: number; private readonly _bufferTimeout: number; + private _finishedSpans: ReadableSpan[] = []; - private _lastSpanFlush = Date.now(); - private _timer: NodeJS.Timeout; + private _timer: NodeJS.Timeout | undefined; private _isShutdown = false; constructor(private readonly _exporter: SpanExporter, config?: BufferConfig) { @@ -43,13 +43,6 @@ export class BatchSpanProcessor implements SpanProcessor { config && typeof config.bufferTimeout === 'number' ? config.bufferTimeout : DEFAULT_BUFFER_TIMEOUT_MS; - - this._timer = setInterval(() => { - if (this._shouldFlush()) { - this._flush(); - } - }, this._bufferTimeout); - unrefTimer(this._timer); } forceFlush(): void { @@ -60,7 +53,7 @@ export class BatchSpanProcessor implements SpanProcessor { } // does nothing. - onStart(span: Span): void {} + onStart(span: Span): void { } onEnd(span: Span): void { if (this._isShutdown) { @@ -73,7 +66,7 @@ export class BatchSpanProcessor implements SpanProcessor { if (this._isShutdown) { return; } - clearInterval(this._timer); + this._clearTimer(); this.forceFlush(); this._isShutdown = true; this._exporter.shutdown(); @@ -82,22 +75,33 @@ export class BatchSpanProcessor implements SpanProcessor { /** Add a span in the buffer. */ private _addToBuffer(span: ReadableSpan) { this._finishedSpans.push(span); + this._maybeStartTimer(); if (this._finishedSpans.length > this._bufferSize) { this._flush(); } } - private _shouldFlush(): boolean { - return ( - this._finishedSpans.length > 0 && - Date.now() - this._lastSpanFlush >= this._bufferTimeout - ); - } - /** Send the span data list to exporter */ private _flush() { + this._clearTimer(); + if (this._finishedSpans.length === 0) return; this._exporter.export(this._finishedSpans, () => {}); this._finishedSpans = []; - this._lastSpanFlush = Date.now(); + } + + private _maybeStartTimer() { + if (this._timer === undefined) { + this._timer = setTimeout(() => { + this._flush(); + }, this._bufferTimeout); + unrefTimer(this._timer); + } + } + + private _clearTimer() { + if (this._timer !== undefined) { + clearTimeout(this._timer); + this._timer = undefined; + } } } From fd03d9760fb0127bbd7cdcd6f0bfeeb7d678e3c9 Mon Sep 17 00:00:00 2001 From: Daniel Dyla Date: Thu, 26 Mar 2020 11:05:12 -0400 Subject: [PATCH 4/6] chore: lint --- packages/opentelemetry-tracing/src/export/BatchSpanProcessor.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/opentelemetry-tracing/src/export/BatchSpanProcessor.ts b/packages/opentelemetry-tracing/src/export/BatchSpanProcessor.ts index 84f657327b..2af2115add 100644 --- a/packages/opentelemetry-tracing/src/export/BatchSpanProcessor.ts +++ b/packages/opentelemetry-tracing/src/export/BatchSpanProcessor.ts @@ -53,7 +53,7 @@ export class BatchSpanProcessor implements SpanProcessor { } // does nothing. - onStart(span: Span): void { } + onStart(span: Span): void {} onEnd(span: Span): void { if (this._isShutdown) { From d04e8f2da274962425423f9c367550d9186ab54f Mon Sep 17 00:00:00 2001 From: Daniel Dyla Date: Thu, 26 Mar 2020 11:30:05 -0400 Subject: [PATCH 5/6] chore: flush clears the timer --- packages/opentelemetry-tracing/src/export/BatchSpanProcessor.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/opentelemetry-tracing/src/export/BatchSpanProcessor.ts b/packages/opentelemetry-tracing/src/export/BatchSpanProcessor.ts index 2af2115add..4fbb80f391 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; } - this._clearTimer(); this.forceFlush(); this._isShutdown = true; this._exporter.shutdown(); From 3e66950de193edfbcdb7a5b4201995cb890592c8 Mon Sep 17 00:00:00 2001 From: Daniel Dyla Date: Fri, 27 Mar 2020 08:20:54 -0400 Subject: [PATCH 6/6] chore: prefer early return --- .../src/export/BatchSpanProcessor.ts | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/packages/opentelemetry-tracing/src/export/BatchSpanProcessor.ts b/packages/opentelemetry-tracing/src/export/BatchSpanProcessor.ts index 4fbb80f391..87fd79bded 100644 --- a/packages/opentelemetry-tracing/src/export/BatchSpanProcessor.ts +++ b/packages/opentelemetry-tracing/src/export/BatchSpanProcessor.ts @@ -89,12 +89,12 @@ export class BatchSpanProcessor implements SpanProcessor { } private _maybeStartTimer() { - if (this._timer === undefined) { - this._timer = setTimeout(() => { - this._flush(); - }, this._bufferTimeout); - unrefTimer(this._timer); - } + if (this._timer !== undefined) return; + + this._timer = setTimeout(() => { + this._flush(); + }, this._bufferTimeout); + unrefTimer(this._timer); } private _clearTimer() {