From cb5d41643300826f5a47df7b50e623392e5d2b01 Mon Sep 17 00:00:00 2001 From: Gerhard Stoebich <18708370+Flarna@users.noreply.github.com> Date: Wed, 13 Nov 2019 08:40:32 +0100 Subject: [PATCH 1/3] fix: zipkin-exporter: don't export after shutdown According to spec and exporter should return FailedNotRetryable error after shutdown was called. --- .../opentelemetry-exporter-zipkin/src/zipkin.ts | 10 ++++++++++ .../test/zipkin.test.ts | 14 ++++++++++++++ 2 files changed, 24 insertions(+) diff --git a/packages/opentelemetry-exporter-zipkin/src/zipkin.ts b/packages/opentelemetry-exporter-zipkin/src/zipkin.ts index fdad1f2ef1..1201eeba4d 100644 --- a/packages/opentelemetry-exporter-zipkin/src/zipkin.ts +++ b/packages/opentelemetry-exporter-zipkin/src/zipkin.ts @@ -39,6 +39,7 @@ export class ZipkinExporter implements SpanExporter { private readonly _statusCodeTagName: string; private readonly _statusDescriptionTagName: string; private readonly _reqOpts: http.RequestOptions; + private _shutdown: boolean; constructor(config: zipkinTypes.ExporterConfig) { const urlStr = config.url || ZipkinExporter.DEFAULT_URL; @@ -60,6 +61,7 @@ export class ZipkinExporter implements SpanExporter { this._statusCodeTagName = config.statusCodeTagName || statusCodeTagName; this._statusDescriptionTagName = config.statusDescriptionTagName || statusDescriptionTagName; + this._shutdown = false; } /** @@ -70,6 +72,10 @@ export class ZipkinExporter implements SpanExporter { resultCallback: (result: ExportResult) => void ) { this._logger.debug('Zipkin exporter export'); + if (this._shutdown) { + setImmediate(resultCallback, ExportResult.FAILED_NOT_RETRYABLE); + return; + } return this._sendSpans(spans, resultCallback); } @@ -78,6 +84,10 @@ export class ZipkinExporter implements SpanExporter { */ shutdown() { this._logger.debug('Zipkin exporter shutdown'); + if (this._shutdown) { + return; + } + this._shutdown = true; // Make an optimistic flush. if (this._forceFlush) { // @todo get spans from span processor (batch) diff --git a/packages/opentelemetry-exporter-zipkin/test/zipkin.test.ts b/packages/opentelemetry-exporter-zipkin/test/zipkin.test.ts index dfc1cf08e9..2a09e37230 100644 --- a/packages/opentelemetry-exporter-zipkin/test/zipkin.test.ts +++ b/packages/opentelemetry-exporter-zipkin/test/zipkin.test.ts @@ -311,6 +311,20 @@ describe('ZipkinExporter', () => { assert.strictEqual(result, ExportResult.FAILED_RETRYABLE); }); }); + + it('should return FailedNonRetryable after shutdown', done => { + const exporter = new ZipkinExporter({ + serviceName: 'my-service', + logger: new NoopLogger(), + }); + + exporter.shutdown(); + + exporter.export([getReadableSpan()], (result: ExportResult) => { + assert.strictEqual(result, ExportResult.FAILED_NOT_RETRYABLE); + done(); + }); + }); }); describe('shutdown', () => { From 808ad65454a8b70f37f433829341f37e17583391 Mon Sep 17 00:00:00 2001 From: Gerhard Stoebich <18708370+Flarna@users.noreply.github.com> Date: Wed, 13 Nov 2019 16:28:24 +0100 Subject: [PATCH 2/3] chore: use setTimeout instead setImmediate for browsers --- packages/opentelemetry-exporter-zipkin/src/zipkin.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/opentelemetry-exporter-zipkin/src/zipkin.ts b/packages/opentelemetry-exporter-zipkin/src/zipkin.ts index 1201eeba4d..37d9225258 100644 --- a/packages/opentelemetry-exporter-zipkin/src/zipkin.ts +++ b/packages/opentelemetry-exporter-zipkin/src/zipkin.ts @@ -73,7 +73,7 @@ export class ZipkinExporter implements SpanExporter { ) { this._logger.debug('Zipkin exporter export'); if (this._shutdown) { - setImmediate(resultCallback, ExportResult.FAILED_NOT_RETRYABLE); + setTimeout(() => resultCallback(ExportResult.FAILED_NOT_RETRYABLE)); return; } return this._sendSpans(spans, resultCallback); From 8f85cd2d67c3bea7ad5558aa84ee8a59cdd7d721 Mon Sep 17 00:00:00 2001 From: Gerhard Stoebich <18708370+Flarna@users.noreply.github.com> Date: Fri, 15 Nov 2019 21:33:50 +0100 Subject: [PATCH 3/3] chore: rename shutdown to isShutdown --- packages/opentelemetry-exporter-zipkin/src/zipkin.ts | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/packages/opentelemetry-exporter-zipkin/src/zipkin.ts b/packages/opentelemetry-exporter-zipkin/src/zipkin.ts index 37d9225258..2be9f91fcf 100644 --- a/packages/opentelemetry-exporter-zipkin/src/zipkin.ts +++ b/packages/opentelemetry-exporter-zipkin/src/zipkin.ts @@ -39,7 +39,7 @@ export class ZipkinExporter implements SpanExporter { private readonly _statusCodeTagName: string; private readonly _statusDescriptionTagName: string; private readonly _reqOpts: http.RequestOptions; - private _shutdown: boolean; + private _isShutdown: boolean; constructor(config: zipkinTypes.ExporterConfig) { const urlStr = config.url || ZipkinExporter.DEFAULT_URL; @@ -61,7 +61,7 @@ export class ZipkinExporter implements SpanExporter { this._statusCodeTagName = config.statusCodeTagName || statusCodeTagName; this._statusDescriptionTagName = config.statusDescriptionTagName || statusDescriptionTagName; - this._shutdown = false; + this._isShutdown = false; } /** @@ -72,7 +72,7 @@ export class ZipkinExporter implements SpanExporter { resultCallback: (result: ExportResult) => void ) { this._logger.debug('Zipkin exporter export'); - if (this._shutdown) { + if (this._isShutdown) { setTimeout(() => resultCallback(ExportResult.FAILED_NOT_RETRYABLE)); return; } @@ -84,10 +84,10 @@ export class ZipkinExporter implements SpanExporter { */ shutdown() { this._logger.debug('Zipkin exporter shutdown'); - if (this._shutdown) { + if (this._isShutdown) { return; } - this._shutdown = true; + this._isShutdown = true; // Make an optimistic flush. if (this._forceFlush) { // @todo get spans from span processor (batch)