Skip to content

Commit

Permalink
fix(tracing): use globalErrorHandler when flushing fails
Browse files Browse the repository at this point in the history
  • Loading branch information
johanneswuerbach committed Oct 24, 2020
1 parent dc8082a commit c52d478
Show file tree
Hide file tree
Showing 4 changed files with 102 additions and 8 deletions.
17 changes: 11 additions & 6 deletions packages/opentelemetry-tracing/src/export/BatchSpanProcessor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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();
});
});
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import { SpanProcessor } from '../SpanProcessor';
import { SpanExporter } from './SpanExporter';
import { ReadableSpan } from './ReadableSpan';
import { context, suppressInstrumentation } from '@opentelemetry/api';
import { ExportResult, globalErrorHandler } from '@opentelemetry/core';

/**
* An implementation of the {@link SpanProcessor} that converts the {@link Span}
Expand Down Expand Up @@ -46,7 +47,13 @@ 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 => {
if (result !== ExportResult.SUCCESS) {
globalErrorHandler(
new Error('SimpleSpanProcessor: span export failed')
);
}
});
});
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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', () => {
Expand Down

0 comments on commit c52d478

Please sign in to comment.