From 6364bc6d6b6cb3adab2b576bd01186d133feab8a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Johannes=20W=C3=BCrbach?= Date: Sat, 24 Oct 2020 21:07:18 +0200 Subject: [PATCH] fix(tracing): use globalErrorHandler when flushing fails --- .../src/export/BatchSpanProcessor.ts | 17 ++++--- .../src/export/SimpleSpanProcessor.ts | 14 ++++- .../test/export/BatchSpanProcessor.test.ts | 33 +++++++++++- .../test/export/SimpleSpanProcessor.test.ts | 51 +++++++++++++++++++ 4 files changed, 106 insertions(+), 9 deletions(-) diff --git a/packages/opentelemetry-tracing/src/export/BatchSpanProcessor.ts b/packages/opentelemetry-tracing/src/export/BatchSpanProcessor.ts index ba151c4c542..8e1237c007f 100644 --- a/packages/opentelemetry-tracing/src/export/BatchSpanProcessor.ts +++ b/packages/opentelemetry-tracing/src/export/BatchSpanProcessor.ts @@ -15,7 +15,11 @@ */ import { context, suppressInstrumentation } from '@opentelemetry/api'; -import { ExportResult, unrefTimer } from '@opentelemetry/core'; +import { + ExportResult, + globalErrorHandler, + unrefTimer, +} from '@opentelemetry/core'; import { SpanProcessor } from '../SpanProcessor'; import { BufferConfig } from '../types'; import { ReadableSpan } from './ReadableSpan'; @@ -99,16 +103,17 @@ export class BatchSpanProcessor implements SpanProcessor { if (this._finishedSpans.length === 0) { return Promise.resolve(); } - return new Promise((resolve, reject) => { + return new Promise((resolve) => { // prevent downstream exporter calls from generating spans context.with(suppressInstrumentation(context.active()), () => { this._exporter.export(this._finishedSpans, result => { this._finishedSpans = []; - if (result === ExportResult.SUCCESS) { - resolve(); - } else { - reject(result); + if (result !== ExportResult.SUCCESS) { + globalErrorHandler( + new Error('BatchSpanProcessor: span export failed') + ); } + resolve(); }); }); }); diff --git a/packages/opentelemetry-tracing/src/export/SimpleSpanProcessor.ts b/packages/opentelemetry-tracing/src/export/SimpleSpanProcessor.ts index b0cbbe873ec..95d6f2166e3 100644 --- a/packages/opentelemetry-tracing/src/export/SimpleSpanProcessor.ts +++ b/packages/opentelemetry-tracing/src/export/SimpleSpanProcessor.ts @@ -17,7 +17,10 @@ import { SpanProcessor } from '../SpanProcessor'; import { SpanExporter } from './SpanExporter'; import { ReadableSpan } from './ReadableSpan'; -import { context, suppressInstrumentation } from '@opentelemetry/api'; +import { context, suppressInstrumentation } from '@opentelemetry/api';import { + ExportResult, + globalErrorHandler, +} from '@opentelemetry/core'; /** * An implementation of the {@link SpanProcessor} that converts the {@link Span} @@ -46,7 +49,14 @@ export class SimpleSpanProcessor implements SpanProcessor { // prevent downstream exporter calls from generating spans context.with(suppressInstrumentation(context.active()), () => { - this._exporter.export([span], () => {}); + this._exporter.export([span], result => { + console.log('export', result) + if (result !== ExportResult.SUCCESS) { + globalErrorHandler( + new Error('SimpleSpanProcessor: span export failed') + ); + } + }); }); } diff --git a/packages/opentelemetry-tracing/test/export/BatchSpanProcessor.test.ts b/packages/opentelemetry-tracing/test/export/BatchSpanProcessor.test.ts index 4b0f6590931..431b80e821b 100644 --- a/packages/opentelemetry-tracing/test/export/BatchSpanProcessor.test.ts +++ b/packages/opentelemetry-tracing/test/export/BatchSpanProcessor.test.ts @@ -14,7 +14,12 @@ * limitations under the License. */ -import { AlwaysOnSampler, ExportResult } from '@opentelemetry/core'; +import { + AlwaysOnSampler, + ExportResult, + loggingErrorHandler, + setGlobalErrorHandler, +} from '@opentelemetry/core'; import * as assert from 'assert'; import * as sinon from 'sinon'; import { @@ -228,6 +233,32 @@ describe('BatchSpanProcessor', () => { done(); }); }); + + it('should call globalErrorHandler when exporting fails', async () => { + const expectedError = new Error( + 'BatchSpanProcessor: span export failed' + ); + sinon.stub(exporter, 'export').callsFake((_, callback) => { + setTimeout(() => { + callback(ExportResult.FAILED_NOT_RETRYABLE); + }, 0); + }); + + const errorHandlerSpy = sinon.spy(); + + setGlobalErrorHandler(errorHandlerSpy); + + await processor.forceFlush(); + + assert.strictEqual(errorHandlerSpy.callCount, 1); + + const [[error]] = errorHandlerSpy.args; + + assert.deepStrictEqual(error, expectedError); + + //reset global error handler + setGlobalErrorHandler(loggingErrorHandler()); + }); }); describe('flushing spans with exporter triggering instrumentation', () => { diff --git a/packages/opentelemetry-tracing/test/export/SimpleSpanProcessor.test.ts b/packages/opentelemetry-tracing/test/export/SimpleSpanProcessor.test.ts index c62799428a1..54876aaf87b 100644 --- a/packages/opentelemetry-tracing/test/export/SimpleSpanProcessor.test.ts +++ b/packages/opentelemetry-tracing/test/export/SimpleSpanProcessor.test.ts @@ -15,12 +15,18 @@ */ import * as assert from 'assert'; +import * as sinon from 'sinon'; import { Span, BasicTracerProvider, InMemorySpanExporter, SimpleSpanProcessor, } from '../../src'; +import { + ExportResult, + setGlobalErrorHandler, + loggingErrorHandler, +} from '@opentelemetry/core'; import { SpanContext, SpanKind, TraceFlags, context } from '@opentelemetry/api'; import { TestTracingSpanExporter } from './TestTracingSpanExporter'; import { TestStackContextManager } from './TestStackContextManager'; @@ -82,6 +88,51 @@ describe('SimpleSpanProcessor', () => { await processor.shutdown(); assert.strictEqual(exporter.getFinishedSpans().length, 0); }); + + it('should call globalErrorHandler when exporting fails', async () => { + const expectedError = new Error( + 'SimpleSpanProcessor: span export failed' + ); + const processor = new SimpleSpanProcessor(exporter); + const spanContext: SpanContext = { + traceId: 'a3cda95b652f4a1592b449d5929fda1b', + spanId: '5e0c63257de34c92', + traceFlags: TraceFlags.NONE, + }; + const span = new Span( + provider.getTracer('default'), + 'span-name', + spanContext, + SpanKind.CLIENT + ); + + sinon.stub(exporter, 'export').callsFake((_, callback) => { + setTimeout(() => { + callback(ExportResult.FAILED_NOT_RETRYABLE); + }, 0); + }); + + const errorHandlerSpy = sinon.spy(); + + setGlobalErrorHandler(errorHandlerSpy); + + processor.onEnd(span); + + await new Promise((resolve) => { + setTimeout(() => { + resolve(); + }, 0); + }) + + assert.strictEqual(errorHandlerSpy.callCount, 1); + + const [[error]] = errorHandlerSpy.args; + + assert.deepStrictEqual(error, expectedError); + + //reset global error handler + setGlobalErrorHandler(loggingErrorHandler()); + }); }); describe('force flush', () => {