From 240f852cc41707c751f28811b7ce3d243382e3dd Mon Sep 17 00:00:00 2001 From: Matthew Wear Date: Wed, 7 Oct 2020 12:45:37 -0700 Subject: [PATCH] feat: add global error handler (#1514) --- .../opentelemetry-api/src/common/Exception.ts | 6 +- .../src/common/global-error-handler.ts | 40 +++++++ .../src/common/logging-error-handler.ts | 66 +++++++++++ .../opentelemetry-core/src/common/types.ts | 6 + packages/opentelemetry-core/src/index.ts | 2 + .../test/common/global-error-handler.test.ts | 56 +++++++++ .../test/common/logging-error-handler.test.ts | 75 ++++++++++++ .../src/util.ts | 4 +- .../test/CollectorMetricExporter.test.ts | 14 ++- .../test/CollectorTraceExporter.test.ts | 13 +- .../src/CollectorExporterBase.ts | 10 +- .../src/platform/browser/util.ts | 18 +-- .../src/platform/node/util.ts | 15 +-- .../src/types.ts | 13 +- .../browser/CollectorMetricExporter.test.ts | 44 ++++--- .../browser/CollectorTraceExporter.test.ts | 52 ++++---- .../test/node/CollectorMetricExporter.test.ts | 15 ++- .../test/node/CollectorTraceExporter.test.ts | 13 +- .../package.json | 2 + .../src/jaeger.ts | 10 +- .../test/jaeger.test.ts | 111 ++++++++++-------- .../src/PrometheusExporter.ts | 10 +- .../src/platform/browser/util.ts | 8 +- .../src/platform/node/util.ts | 4 +- .../test/browser/zipkin.test.ts | 44 ++++++- .../test/node/zipkin.test.ts | 27 +++++ .../src/export/Controller.ts | 16 ++- .../src/MultiSpanProcessor.ts | 16 ++- .../test/MultiSpanProcessor.test.ts | 27 +++++ 29 files changed, 582 insertions(+), 155 deletions(-) create mode 100644 packages/opentelemetry-core/src/common/global-error-handler.ts create mode 100644 packages/opentelemetry-core/src/common/logging-error-handler.ts create mode 100644 packages/opentelemetry-core/test/common/global-error-handler.test.ts create mode 100644 packages/opentelemetry-core/test/common/logging-error-handler.test.ts diff --git a/packages/opentelemetry-api/src/common/Exception.ts b/packages/opentelemetry-api/src/common/Exception.ts index 0fa98e741a..5539b7e9f3 100644 --- a/packages/opentelemetry-api/src/common/Exception.ts +++ b/packages/opentelemetry-api/src/common/Exception.ts @@ -15,21 +15,21 @@ */ interface ExceptionWithCode { - code: string; + code: string | number; name?: string; message?: string; stack?: string; } interface ExceptionWithMessage { - code?: string; + code?: string | number; message: string; name?: string; stack?: string; } interface ExceptionWithName { - code?: string; + code?: string | number; message?: string; name: string; stack?: string; diff --git a/packages/opentelemetry-core/src/common/global-error-handler.ts b/packages/opentelemetry-core/src/common/global-error-handler.ts new file mode 100644 index 0000000000..e8461aba76 --- /dev/null +++ b/packages/opentelemetry-core/src/common/global-error-handler.ts @@ -0,0 +1,40 @@ +/* + * Copyright The OpenTelemetry Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { Exception } from '@opentelemetry/api'; +import { loggingErrorHandler } from './logging-error-handler'; +import { ErrorHandler } from './types'; + +/** The global error handler delegate */ +let delegateHandler = loggingErrorHandler(); + +/** + * Set the global error handler + * @param {ErrorHandler} handler + */ +export function setGlobalErrorHandler(handler: ErrorHandler) { + delegateHandler = handler; +} + +/** + * Return the global error handler + * @param {Exception} ex + */ +export const globalErrorHandler = (ex: Exception) => { + try { + delegateHandler(ex); + } catch {} // eslint-disable-line no-empty +}; diff --git a/packages/opentelemetry-core/src/common/logging-error-handler.ts b/packages/opentelemetry-core/src/common/logging-error-handler.ts new file mode 100644 index 0000000000..5925f4510b --- /dev/null +++ b/packages/opentelemetry-core/src/common/logging-error-handler.ts @@ -0,0 +1,66 @@ +/* + * Copyright The OpenTelemetry Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { Logger, Exception } from '@opentelemetry/api'; +import { ConsoleLogger } from './ConsoleLogger'; +import { ErrorHandler, LogLevel } from './types'; + +/** + * Returns a function that logs an error using the provided logger, or a + * console logger if one was not provided. + * @param {Logger} logger + */ +export function loggingErrorHandler(logger?: Logger): ErrorHandler { + logger = logger ?? new ConsoleLogger(LogLevel.ERROR); + return (ex: Exception) => { + logger!.error(stringifyException(ex)); + }; +} + +/** + * Converts an exception into a string representation + * @param {Exception} ex + */ +function stringifyException(ex: Exception | string): string { + if (typeof ex === 'string') { + return ex; + } else { + return JSON.stringify(flattenException(ex)); + } +} + +/** + * Flattens an exception into key-value pairs by traversing the prototype chain + * and coercing values to strings. Duplicate properties will not be overwritten; + * the first insert wins. + */ +function flattenException(ex: Exception): Record { + const result = {} as Record; + let current = ex; + + while (current !== null) { + Object.getOwnPropertyNames(current).forEach(propertyName => { + if (result[propertyName]) return; + const value = current[propertyName as keyof typeof current]; + if (value) { + result[propertyName] = String(value); + } + }); + current = Object.getPrototypeOf(current); + } + + return result; +} diff --git a/packages/opentelemetry-core/src/common/types.ts b/packages/opentelemetry-core/src/common/types.ts index 399ed9d5c1..58f654e416 100644 --- a/packages/opentelemetry-core/src/common/types.ts +++ b/packages/opentelemetry-core/src/common/types.ts @@ -13,6 +13,9 @@ * See the License for the specific language governing permissions and * limitations under the License. */ + +import { Exception } from '@opentelemetry/api'; + export enum LogLevel { ERROR, WARN, @@ -56,3 +59,6 @@ export interface InstrumentationLibrary { readonly name: string; readonly version: string; } + +/** Defines an error handler function */ +export type ErrorHandler = (ex: Exception) => void; diff --git a/packages/opentelemetry-core/src/index.ts b/packages/opentelemetry-core/src/index.ts index b4053a5e07..4ac46ce314 100644 --- a/packages/opentelemetry-core/src/index.ts +++ b/packages/opentelemetry-core/src/index.ts @@ -16,6 +16,8 @@ export * from './common/attributes'; export * from './common/ConsoleLogger'; +export * from './common/global-error-handler'; +export * from './common/logging-error-handler'; export * from './common/NoopLogger'; export * from './common/time'; export * from './common/types'; diff --git a/packages/opentelemetry-core/test/common/global-error-handler.test.ts b/packages/opentelemetry-core/test/common/global-error-handler.test.ts new file mode 100644 index 0000000000..9dcdb3036f --- /dev/null +++ b/packages/opentelemetry-core/test/common/global-error-handler.test.ts @@ -0,0 +1,56 @@ +/* + * Copyright The OpenTelemetry Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import * as assert from 'assert'; +import * as sinon from 'sinon'; +import { globalErrorHandler, setGlobalErrorHandler } from '../../src'; +import { Exception } from '@opentelemetry/api'; + +describe('globalErrorHandler', () => { + let defaultHandler: sinon.SinonSpy; + + beforeEach(() => { + defaultHandler = sinon.spy(); + setGlobalErrorHandler(defaultHandler); + }); + + it('receives errors', () => { + const err = new Error('this is bad'); + globalErrorHandler(err); + sinon.assert.calledOnceWithExactly(defaultHandler, err); + }); + + it('replaces delegate when handler is updated', () => { + const err = new Error('this is bad'); + const newHandler = sinon.spy(); + setGlobalErrorHandler(newHandler); + + globalErrorHandler(err); + + sinon.assert.calledOnceWithExactly(newHandler, err); + sinon.assert.notCalled(defaultHandler); + }); + + it('catches exceptions thrown in handler', () => { + setGlobalErrorHandler((ex: Exception) => { + throw new Error('bad things'); + }); + + assert.doesNotThrow(() => { + globalErrorHandler('an error'); + }); + }); +}); diff --git a/packages/opentelemetry-core/test/common/logging-error-handler.test.ts b/packages/opentelemetry-core/test/common/logging-error-handler.test.ts new file mode 100644 index 0000000000..ddef522d09 --- /dev/null +++ b/packages/opentelemetry-core/test/common/logging-error-handler.test.ts @@ -0,0 +1,75 @@ +/* + * Copyright The OpenTelemetry Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import * as assert from 'assert'; +import * as sinon from 'sinon'; +import { ErrorHandler, loggingErrorHandler } from '../../src'; + +describe('loggingErrorHandler', () => { + let handler: ErrorHandler; + const errorStub = sinon.fake(); + + beforeEach(() => { + handler = loggingErrorHandler({ + debug: sinon.fake(), + info: sinon.fake(), + warn: sinon.fake(), + error: errorStub, + }); + }); + + it('logs from string', () => { + const err = 'not found'; + handler(err); + assert.ok(errorStub.calledOnceWith(err)); + }); + + it('logs from an object', () => { + const err = { + name: 'NotFoundError', + message: 'not found', + randomString: 'random value', + randomNumber: 42, + randomArray: [1, 2, 3], + randomObject: { a: 'a' }, + stack: 'a stack', + }; + + handler(err); + + const [result] = errorStub.lastCall.args; + + assert.ok(result.includes(err.name)); + assert.ok(result.includes(err.message)); + assert.ok(result.includes(err.randomString)); + assert.ok(result.includes(err.randomNumber)); + assert.ok(result.includes(err.randomArray)); + assert.ok(result.includes(err.randomObject)); + assert.ok(result.includes(JSON.stringify(err.stack))); + }); + + it('logs from an error', () => { + const err = new Error('this is bad'); + + handler(err); + + const [result] = errorStub.lastCall.args; + + assert.ok(result.includes(err.name)); + assert.ok(result.includes(err.message)); + assert.ok(result.includes(JSON.stringify(err.stack))); + }); +}); diff --git a/packages/opentelemetry-exporter-collector-proto/src/util.ts b/packages/opentelemetry-exporter-collector-proto/src/util.ts index d43fc8645d..05e4ec00fc 100644 --- a/packages/opentelemetry-exporter-collector-proto/src/util.ts +++ b/packages/opentelemetry-exporter-collector-proto/src/util.ts @@ -80,8 +80,6 @@ export function send( ); } } else { - onError({ - message: 'No proto', - }); + onError(new collectorTypes.CollectorExporterError('No proto')); } } diff --git a/packages/opentelemetry-exporter-collector-proto/test/CollectorMetricExporter.test.ts b/packages/opentelemetry-exporter-collector-proto/test/CollectorMetricExporter.test.ts index 83f3f7a101..ac8c07cc4d 100644 --- a/packages/opentelemetry-exporter-collector-proto/test/CollectorMetricExporter.test.ts +++ b/packages/opentelemetry-exporter-collector-proto/test/CollectorMetricExporter.test.ts @@ -177,7 +177,14 @@ describe('CollectorMetricExporter - node with proto over http', () => { }); it('should log the error message', done => { - const spyLoggerError = sinon.stub(collectorExporter.logger, 'error'); + const spyLoggerError = sinon.spy(); + const handler = core.loggingErrorHandler({ + debug: sinon.fake(), + info: sinon.fake(), + warn: sinon.fake(), + error: spyLoggerError, + }); + core.setGlobalErrorHandler(handler); const responseSpy = sinon.spy(); collectorExporter.export(metrics, responseSpy); @@ -187,9 +194,8 @@ describe('CollectorMetricExporter - node with proto over http', () => { const callback = args[1]; callback(mockResError); setTimeout(() => { - const response: any = spyLoggerError.args[0][0]; - assert.strictEqual(response, 'statusCode: 400'); - + const response = spyLoggerError.args[0][0] as string; + assert.ok(response.includes('"code":"400"')); assert.strictEqual(responseSpy.args[0][0], 1); done(); }); diff --git a/packages/opentelemetry-exporter-collector-proto/test/CollectorTraceExporter.test.ts b/packages/opentelemetry-exporter-collector-proto/test/CollectorTraceExporter.test.ts index 662bd25e4c..6363bd4407 100644 --- a/packages/opentelemetry-exporter-collector-proto/test/CollectorTraceExporter.test.ts +++ b/packages/opentelemetry-exporter-collector-proto/test/CollectorTraceExporter.test.ts @@ -144,7 +144,14 @@ describe('CollectorTraceExporter - node with proto over http', () => { }); it('should log the error message', done => { - const spyLoggerError = sinon.stub(collectorExporter.logger, 'error'); + const spyLoggerError = sinon.spy(); + const handler = core.loggingErrorHandler({ + debug: sinon.fake(), + info: sinon.fake(), + warn: sinon.fake(), + error: spyLoggerError, + }); + core.setGlobalErrorHandler(handler); const responseSpy = sinon.spy(); collectorExporter.export(spans, responseSpy); @@ -154,9 +161,9 @@ describe('CollectorTraceExporter - node with proto over http', () => { const callback = args[1]; callback(mockResError); setTimeout(() => { - const response: any = spyLoggerError.args[0][0]; - assert.strictEqual(response, 'statusCode: 400'); + const response = spyLoggerError.args[0][0] as string; + assert.ok(response.includes('"code":"400"')); assert.strictEqual(responseSpy.args[0][0], 1); done(); }); diff --git a/packages/opentelemetry-exporter-collector/src/CollectorExporterBase.ts b/packages/opentelemetry-exporter-collector/src/CollectorExporterBase.ts index a0237eb056..2b3ed5b066 100644 --- a/packages/opentelemetry-exporter-collector/src/CollectorExporterBase.ts +++ b/packages/opentelemetry-exporter-collector/src/CollectorExporterBase.ts @@ -15,7 +15,11 @@ */ import { Attributes, Logger } from '@opentelemetry/api'; -import { ExportResult, NoopLogger } from '@opentelemetry/core'; +import { + ExportResult, + NoopLogger, + globalErrorHandler, +} from '@opentelemetry/core'; import { CollectorExporterError, CollectorExporterConfigBase, @@ -75,9 +79,7 @@ export abstract class CollectorExporterBase< resultCallback(ExportResult.SUCCESS); }) .catch((error: ExportServiceError) => { - if (error.message) { - this.logger.error(error.message); - } + globalErrorHandler(error); if (error.code && error.code < 500) { resultCallback(ExportResult.FAILED_NOT_RETRYABLE); } else { diff --git a/packages/opentelemetry-exporter-collector/src/platform/browser/util.ts b/packages/opentelemetry-exporter-collector/src/platform/browser/util.ts index 433490026f..03a4c9badd 100644 --- a/packages/opentelemetry-exporter-collector/src/platform/browser/util.ts +++ b/packages/opentelemetry-exporter-collector/src/platform/browser/util.ts @@ -33,8 +33,10 @@ export function sendWithBeacon( logger.debug('sendBeacon - can send', body); onSuccess(); } else { - logger.error('sendBeacon - cannot send', body); - onError({}); + const error = new collectorTypes.CollectorExporterError( + `sendBeacon - cannot send ${body}` + ); + onError(error); } } @@ -69,12 +71,12 @@ export function sendWithXhr( logger.debug('xhr success', body); onSuccess(); } else { - logger.error('body', body); - logger.error('xhr error', xhr); - onError({ - code: xhr.status, - message: xhr.responseText, - }); + const error = new collectorTypes.CollectorExporterError( + xhr.responseText, + xhr.status + ); + + onError(error); } } }; diff --git a/packages/opentelemetry-exporter-collector/src/platform/node/util.ts b/packages/opentelemetry-exporter-collector/src/platform/node/util.ts index eddb110167..0e20cfc8ce 100644 --- a/packages/opentelemetry-exporter-collector/src/platform/node/util.ts +++ b/packages/opentelemetry-exporter-collector/src/platform/node/util.ts @@ -54,19 +54,16 @@ export function sendWithHttp( collector.logger.debug(`statusCode: ${res.statusCode}`); onSuccess(); } else { - collector.logger.error(`statusCode: ${res.statusCode}`); - onError({ - code: res.statusCode, - message: res.statusMessage, - }); + const error = new collectorTypes.CollectorExporterError( + res.statusMessage, + res.statusCode + ); + onError(error); } }); req.on('error', (error: Error) => { - collector.logger.error('error', error.message); - onError({ - message: error.message, - }); + onError(error); }); req.write(data); req.end(); diff --git a/packages/opentelemetry-exporter-collector/src/types.ts b/packages/opentelemetry-exporter-collector/src/types.ts index eab6b6b193..cf63825af7 100644 --- a/packages/opentelemetry-exporter-collector/src/types.ts +++ b/packages/opentelemetry-exporter-collector/src/types.ts @@ -266,16 +266,21 @@ export namespace opentelemetryProto { /** * Interface for handling error */ -export interface CollectorExporterError { - code?: number; - message?: string; - stack?: string; +export class CollectorExporterError extends Error { + readonly code?: number; + readonly name: string = 'CollectorExporterError'; + + constructor(message?: string, code?: number) { + super(message); + this.code = code; + } } /** * Interface for handling export service errors */ export interface ExportServiceError { + name: string; code: number; details: string; metadata: { [key: string]: unknown }; diff --git a/packages/opentelemetry-exporter-collector/test/browser/CollectorMetricExporter.test.ts b/packages/opentelemetry-exporter-collector/test/browser/CollectorMetricExporter.test.ts index 50b6279498..2195ef2463 100644 --- a/packages/opentelemetry-exporter-collector/test/browser/CollectorMetricExporter.test.ts +++ b/packages/opentelemetry-exporter-collector/test/browser/CollectorMetricExporter.test.ts @@ -14,7 +14,11 @@ * limitations under the License. */ -import { NoopLogger } from '@opentelemetry/core'; +import { + NoopLogger, + setGlobalErrorHandler, + loggingErrorHandler, +} from '@opentelemetry/core'; import * as assert from 'assert'; import * as sinon from 'sinon'; import { CollectorMetricExporter } from '../../src/platform/browser/index'; @@ -176,16 +180,23 @@ describe('CollectorMetricExporter - web', () => { }); it('should log the error message', done => { + const spyLoggerError = sinon.spy(); + const handler = loggingErrorHandler({ + debug: sinon.fake(), + info: sinon.fake(), + warn: sinon.fake(), + error: spyLoggerError, + }); + setGlobalErrorHandler(handler); const spyLoggerDebug = sinon.stub(collectorExporter.logger, 'debug'); - const spyLoggerError = sinon.stub(collectorExporter.logger, 'error'); spyBeacon.restore(); spyBeacon = sinon.stub(window.navigator, 'sendBeacon').returns(false); collectorExporter.export(metrics, () => {}); setTimeout(() => { - const response: any = spyLoggerError.args[0][0]; - assert.strictEqual(response, 'sendBeacon - cannot send'); + const response: any = spyLoggerError.args[0][0] as string; + assert.ok(response.includes('sendBeacon - cannot send')); assert.strictEqual(spyLoggerDebug.args.length, 1); done(); @@ -305,21 +316,26 @@ describe('CollectorMetricExporter - web', () => { }); it('should log the error message', done => { - const spyLoggerError = sinon.stub(collectorExporter.logger, 'error'); + const spyLoggerError = sinon.spy(); + const handler = loggingErrorHandler({ + debug: sinon.fake(), + info: sinon.fake(), + warn: sinon.fake(), + error: spyLoggerError, + }); + setGlobalErrorHandler(handler); - collectorExporter.export(metrics, () => {}); + collectorExporter.export(metrics, () => { + const response = spyLoggerError.args[0][0] as string; + assert.ok(response.includes('"code":"400"')); + + assert.strictEqual(spyBeacon.callCount, 0); + done(); + }); setTimeout(() => { const request = server.requests[0]; request.respond(400); - - const response1: any = spyLoggerError.args[0][0]; - const response2: any = spyLoggerError.args[1][0]; - assert.strictEqual(response1, 'body'); - assert.strictEqual(response2, 'xhr error'); - - assert.strictEqual(spyBeacon.callCount, 0); - done(); }); }); it('should send custom headers', done => { diff --git a/packages/opentelemetry-exporter-collector/test/browser/CollectorTraceExporter.test.ts b/packages/opentelemetry-exporter-collector/test/browser/CollectorTraceExporter.test.ts index 018d9ec8f9..25e795f911 100644 --- a/packages/opentelemetry-exporter-collector/test/browser/CollectorTraceExporter.test.ts +++ b/packages/opentelemetry-exporter-collector/test/browser/CollectorTraceExporter.test.ts @@ -14,7 +14,11 @@ * limitations under the License. */ -import { NoopLogger } from '@opentelemetry/core'; +import { + NoopLogger, + setGlobalErrorHandler, + loggingErrorHandler, +} from '@opentelemetry/core'; import { ReadableSpan } from '@opentelemetry/tracing'; import * as assert from 'assert'; import * as sinon from 'sinon'; @@ -131,22 +135,27 @@ describe('CollectorTraceExporter - web', () => { }); it('should log the error message', done => { + const spyLoggerError = sinon.spy(); + const handler = loggingErrorHandler({ + debug: sinon.fake(), + info: sinon.fake(), + warn: sinon.fake(), + error: spyLoggerError, + }); + setGlobalErrorHandler(handler); + const spyLoggerDebug = sinon.stub( collectorTraceExporter.logger, 'debug' ); - const spyLoggerError = sinon.stub( - collectorTraceExporter.logger, - 'error' - ); spyBeacon.restore(); spyBeacon = sinon.stub(window.navigator, 'sendBeacon').returns(false); collectorTraceExporter.export(spans, () => {}); setTimeout(() => { - const response: any = spyLoggerError.args[0][0]; - assert.strictEqual(response, 'sendBeacon - cannot send'); + const response = spyLoggerError.args[0][0] as string; + assert.ok(response.includes('sendBeacon - cannot send')); assert.strictEqual(spyLoggerDebug.args.length, 1); done(); @@ -227,25 +236,28 @@ describe('CollectorTraceExporter - web', () => { }); it('should log the error message', done => { - const spyLoggerError = sinon.stub( - collectorTraceExporter.logger, - 'error' - ); - - collectorTraceExporter.export(spans, () => {}); + const spyLoggerError = sinon.spy(); + const handler = loggingErrorHandler({ + debug: sinon.fake(), + info: sinon.fake(), + warn: sinon.fake(), + error: spyLoggerError, + }); + setGlobalErrorHandler(handler); - setTimeout(() => { - const request = server.requests[0]; - request.respond(400); + collectorTraceExporter.export(spans, () => { + const response = spyLoggerError.args[0][0] as string; - const response1: any = spyLoggerError.args[0][0]; - const response2: any = spyLoggerError.args[1][0]; - assert.strictEqual(response1, 'body'); - assert.strictEqual(response2, 'xhr error'); + assert.ok(response.includes('"code":"400"')); assert.strictEqual(spyBeacon.callCount, 0); done(); }); + + setTimeout(() => { + const request = server.requests[0]; + request.respond(400); + }); }); it('should send custom headers', done => { diff --git a/packages/opentelemetry-exporter-collector/test/node/CollectorMetricExporter.test.ts b/packages/opentelemetry-exporter-collector/test/node/CollectorMetricExporter.test.ts index 372fe44133..6270c63c66 100644 --- a/packages/opentelemetry-exporter-collector/test/node/CollectorMetricExporter.test.ts +++ b/packages/opentelemetry-exporter-collector/test/node/CollectorMetricExporter.test.ts @@ -203,7 +203,14 @@ describe('CollectorMetricExporter - node with json over http', () => { }); it('should log the error message', done => { - const spyLoggerError = sinon.stub(collectorExporter.logger, 'error'); + const spyLoggerError = sinon.spy(); + const handler = core.loggingErrorHandler({ + debug: sinon.fake(), + info: sinon.fake(), + warn: sinon.fake(), + error: spyLoggerError, + }); + core.setGlobalErrorHandler(handler); const responseSpy = sinon.spy(); collectorExporter.export(metrics, responseSpy); @@ -213,9 +220,9 @@ describe('CollectorMetricExporter - node with json over http', () => { const callback = args[1]; callback(mockResError); setTimeout(() => { - const response: any = spyLoggerError.args[0][0]; - assert.strictEqual(response, 'statusCode: 400'); - + const response = spyLoggerError.args[0][0] as string; + console.log(response); + assert.ok(response.includes('"code":"400"')); assert.strictEqual(responseSpy.args[0][0], 1); done(); }); diff --git a/packages/opentelemetry-exporter-collector/test/node/CollectorTraceExporter.test.ts b/packages/opentelemetry-exporter-collector/test/node/CollectorTraceExporter.test.ts index c90dce9c8e..0429cb57f1 100644 --- a/packages/opentelemetry-exporter-collector/test/node/CollectorTraceExporter.test.ts +++ b/packages/opentelemetry-exporter-collector/test/node/CollectorTraceExporter.test.ts @@ -158,7 +158,14 @@ describe('CollectorTraceExporter - node with json over http', () => { }); it('should log the error message', done => { - const spyLoggerError = sinon.stub(collectorExporter.logger, 'error'); + const spyLoggerError = sinon.spy(); + const handler = core.loggingErrorHandler({ + debug: sinon.fake(), + info: sinon.fake(), + warn: sinon.fake(), + error: spyLoggerError, + }); + core.setGlobalErrorHandler(handler); const responseSpy = sinon.spy(); collectorExporter.export(spans, responseSpy); @@ -168,9 +175,9 @@ describe('CollectorTraceExporter - node with json over http', () => { const callback = args[1]; callback(mockResError); setTimeout(() => { - const response: any = spyLoggerError.args[0][0]; - assert.strictEqual(response, 'statusCode: 400'); + const response = spyLoggerError.args[0][0] as string; + assert.ok(response.includes('"code":"400"')); assert.strictEqual(responseSpy.args[0][0], 1); done(); }); diff --git a/packages/opentelemetry-exporter-jaeger/package.json b/packages/opentelemetry-exporter-jaeger/package.json index 2ac156ce11..914f1544a5 100644 --- a/packages/opentelemetry-exporter-jaeger/package.json +++ b/packages/opentelemetry-exporter-jaeger/package.json @@ -45,12 +45,14 @@ "@opentelemetry/resources": "^0.11.0", "@types/mocha": "8.0.2", "@types/node": "14.0.27", + "@types/sinon": "^9.0.8", "codecov": "3.7.2", "gts": "2.0.2", "mocha": "7.2.0", "nock": "12.0.3", "nyc": "15.1.0", "rimraf": "3.0.2", + "sinon": "^9.2.0", "ts-mocha": "7.0.0", "ts-node": "9.0.0", "typescript": "3.9.7" diff --git a/packages/opentelemetry-exporter-jaeger/src/jaeger.ts b/packages/opentelemetry-exporter-jaeger/src/jaeger.ts index def95ddcf7..0078b26644 100644 --- a/packages/opentelemetry-exporter-jaeger/src/jaeger.ts +++ b/packages/opentelemetry-exporter-jaeger/src/jaeger.ts @@ -15,7 +15,11 @@ */ import * as api from '@opentelemetry/api'; -import { ExportResult, NoopLogger } from '@opentelemetry/core'; +import { + ExportResult, + NoopLogger, + globalErrorHandler, +} from '@opentelemetry/core'; import { ReadableSpan, SpanExporter } from '@opentelemetry/tracing'; import { Socket } from 'dgram'; import { spanToThrift } from './transform'; @@ -81,7 +85,7 @@ export class JaegerExporter implements SpanExporter { } this._logger.debug('Jaeger exporter export'); this._sendSpans(spans, resultCallback).catch(err => { - this._logger.error(`JaegerExporter failed to export: ${err}`); + globalErrorHandler(err); }); } @@ -132,7 +136,7 @@ export class JaegerExporter implements SpanExporter { try { await this._append(span); } catch (err) { - this._logger.error(`failed to append span: ${err}`); + globalErrorHandler(err); // TODO right now we break out on first error, is that desirable? if (done) return done(ExportResult.FAILED_NOT_RETRYABLE); } diff --git a/packages/opentelemetry-exporter-jaeger/test/jaeger.test.ts b/packages/opentelemetry-exporter-jaeger/test/jaeger.test.ts index 1018b78126..b74b1cd2d4 100644 --- a/packages/opentelemetry-exporter-jaeger/test/jaeger.test.ts +++ b/packages/opentelemetry-exporter-jaeger/test/jaeger.test.ts @@ -15,8 +15,14 @@ */ import * as assert from 'assert'; +import * as sinon from 'sinon'; import { JaegerExporter } from '../src'; -import { ExportResult, NoopLogger } from '@opentelemetry/core'; +import { + ExportResult, + loggingErrorHandler, + NoopLogger, + setGlobalErrorHandler, +} from '@opentelemetry/core'; import * as api from '@opentelemetry/api'; import { ThriftProcess } from '../src/types'; import { ReadableSpan } from '@opentelemetry/tracing'; @@ -106,7 +112,33 @@ describe('JaegerExporter', () => { }); describe('export', () => { + const readableSpan: ReadableSpan = { + name: 'my-span1', + kind: api.SpanKind.CLIENT, + spanContext: { + traceId: 'd4cda95b652f4a1592b449d5929fda1b', + spanId: '6e0c63257de34c92', + traceFlags: TraceFlags.NONE, + }, + startTime: [1566156729, 709], + endTime: [1566156731, 709], + ended: true, + status: { + code: api.CanonicalCode.DATA_LOSS, + }, + attributes: {}, + links: [], + events: [], + duration: [32, 800000000], + resource: Resource.empty(), + instrumentationLibrary: { + name: 'default', + version: '0.0.1', + }, + }; + let exporter: JaegerExporter; + beforeEach(() => { exporter = new JaegerExporter({ serviceName: 'opentelemetry', @@ -124,32 +156,6 @@ describe('JaegerExporter', () => { }); it('should send spans to Jaeger backend and return with Success', () => { - const spanContext = { - traceId: 'd4cda95b652f4a1592b449d5929fda1b', - spanId: '6e0c63257de34c92', - traceFlags: TraceFlags.NONE, - }; - const readableSpan: ReadableSpan = { - name: 'my-span1', - kind: api.SpanKind.CLIENT, - spanContext, - startTime: [1566156729, 709], - endTime: [1566156731, 709], - ended: true, - status: { - code: api.CanonicalCode.DATA_LOSS, - }, - attributes: {}, - links: [], - events: [], - duration: [32, 800000000], - resource: Resource.empty(), - instrumentationLibrary: { - name: 'default', - version: '0.0.1', - }, - }; - exporter.export([readableSpan], (result: ExportResult) => { assert.strictEqual(result, ExportResult.SUCCESS); }); @@ -172,32 +178,33 @@ describe('JaegerExporter', () => { endpoint: mockedEndpoint, }); assert.strictEqual(exporter['_sender'].constructor.name, 'HTTPSender'); - const spanContext = { - traceId: 'd4cda95b652f4a1592b449d5929fda1b', - spanId: '6e0c63257de34c92', - traceFlags: TraceFlags.NONE, - }; - const readableSpan: ReadableSpan = { - name: 'my-span1', - kind: api.SpanKind.CLIENT, - spanContext, - startTime: [1566156729, 709], - endTime: [1566156731, 709], - ended: true, - status: { - code: api.CanonicalCode.DATA_LOSS, - }, - attributes: {}, - links: [], - events: [], - duration: [32, 800000000], - resource: Resource.empty(), - instrumentationLibrary: { - name: 'default', - version: '0.0.1', - }, - }; exporter.export([readableSpan], () => {}); }); + + it('should call globalErrorHandler on error', () => { + nock.cleanAll(); + const errorHandlerSpy = sinon.spy(); + setGlobalErrorHandler(errorHandlerSpy); + const expectedError = new Error('whoops'); + const mockedEndpoint = 'http://testendpoint'; + const scope = nock(mockedEndpoint) + .post('/') + .replyWithError(expectedError); + const exporter = new JaegerExporter({ + serviceName: 'opentelemetry', + endpoint: mockedEndpoint, + }); + + exporter.export([readableSpan], () => { + scope.done(); + assert.strictEqual(errorHandlerSpy.callCount, 1); + + const [[error]] = errorHandlerSpy.args; + assert.strictEqual(error, expectedError); + }); + + // reset global error handler + setGlobalErrorHandler(loggingErrorHandler()); + }); }); }); diff --git a/packages/opentelemetry-exporter-prometheus/src/PrometheusExporter.ts b/packages/opentelemetry-exporter-prometheus/src/PrometheusExporter.ts index f8c9c85370..42d658c256 100644 --- a/packages/opentelemetry-exporter-prometheus/src/PrometheusExporter.ts +++ b/packages/opentelemetry-exporter-prometheus/src/PrometheusExporter.ts @@ -15,7 +15,11 @@ */ import * as api from '@opentelemetry/api'; -import { ExportResult, NoopLogger } from '@opentelemetry/core'; +import { + ExportResult, + NoopLogger, + globalErrorHandler, +} from '@opentelemetry/core'; import { MetricExporter, MetricRecord } from '@opentelemetry/metrics'; import { createServer, IncomingMessage, Server, ServerResponse } from 'http'; import * as url from 'url'; @@ -121,9 +125,7 @@ export class PrometheusExporter implements MetricExporter { ((err as unknown) as { code: string }).code !== 'ERR_SERVER_NOT_RUNNING' ) { - this._logger.error( - `Error during stopping the Prometheus Exporter "${err.message}"` - ); + globalErrorHandler(err); } } resolve(); diff --git a/packages/opentelemetry-exporter-zipkin/src/platform/browser/util.ts b/packages/opentelemetry-exporter-zipkin/src/platform/browser/util.ts index d2782dad01..b72df3d665 100644 --- a/packages/opentelemetry-exporter-zipkin/src/platform/browser/util.ts +++ b/packages/opentelemetry-exporter-zipkin/src/platform/browser/util.ts @@ -15,7 +15,7 @@ */ import * as api from '@opentelemetry/api'; -import { ExportResult } from '@opentelemetry/core'; +import { ExportResult, globalErrorHandler } from '@opentelemetry/core'; import * as zipkinTypes from '../../types'; /** @@ -73,7 +73,7 @@ function sendWithBeacon( logger.debug('sendBeacon - can send', data); done(ExportResult.SUCCESS); } else { - logger.error('sendBeacon - cannot send', data); + globalErrorHandler(new Error(`sendBeacon - cannot send ${data}`)); done(ExportResult.FAILED_NOT_RETRYABLE); } } @@ -118,8 +118,8 @@ function sendWithXhr( } }; - xhr.onerror = err => { - logger.error('Zipkin request error', err); + xhr.onerror = msg => { + globalErrorHandler(new Error(`Zipkin request error: ${msg}`)); return done(ExportResult.FAILED_RETRYABLE); }; diff --git a/packages/opentelemetry-exporter-zipkin/src/platform/node/util.ts b/packages/opentelemetry-exporter-zipkin/src/platform/node/util.ts index 668d11bb48..9686f73a40 100644 --- a/packages/opentelemetry-exporter-zipkin/src/platform/node/util.ts +++ b/packages/opentelemetry-exporter-zipkin/src/platform/node/util.ts @@ -15,7 +15,7 @@ */ import * as api from '@opentelemetry/api'; -import { ExportResult } from '@opentelemetry/core'; +import { ExportResult, globalErrorHandler } from '@opentelemetry/core'; import * as http from 'http'; import * as https from 'https'; import * as url from 'url'; @@ -82,7 +82,7 @@ export function prepareSend( }); req.on('error', (err: Error) => { - logger.error('Zipkin request error', err); + globalErrorHandler(err); return done(ExportResult.FAILED_RETRYABLE); }); diff --git a/packages/opentelemetry-exporter-zipkin/test/browser/zipkin.test.ts b/packages/opentelemetry-exporter-zipkin/test/browser/zipkin.test.ts index c47e69fa57..0ed5ca6b0c 100644 --- a/packages/opentelemetry-exporter-zipkin/test/browser/zipkin.test.ts +++ b/packages/opentelemetry-exporter-zipkin/test/browser/zipkin.test.ts @@ -14,7 +14,11 @@ * limitations under the License. */ -import { NoopLogger } from '@opentelemetry/core'; +import { + NoopLogger, + setGlobalErrorHandler, + loggingErrorHandler, +} from '@opentelemetry/core'; import { ReadableSpan } from '@opentelemetry/tracing'; import * as assert from 'assert'; import * as sinon from 'sinon'; @@ -132,6 +136,25 @@ describe('Zipkin Exporter - web', () => { done(); }); }); + + it('should call globalErrorHandler on error', () => { + const errorHandlerSpy = sinon.spy(); + setGlobalErrorHandler(errorHandlerSpy); + + zipkinExporter.export(spans, () => { + const [[error]] = errorHandlerSpy.args; + assert.strictEqual(errorHandlerSpy.callCount, 1); + assert.ok(error.message.includes('Zipkin request error')); + + //reset global error handler + setGlobalErrorHandler(loggingErrorHandler()); + }); + + setTimeout(() => { + const request = server.requests[0]; + request.respond(400); + }); + }); }); describe('when "sendBeacon" is NOT available', () => { @@ -152,6 +175,25 @@ describe('Zipkin Exporter - web', () => { done(); }); }); + + it('should call globalErrorHandler on error', () => { + const errorHandlerSpy = sinon.spy(); + setGlobalErrorHandler(errorHandlerSpy); + + zipkinExporter.export(spans, () => { + const [[error]] = errorHandlerSpy.args; + assert.strictEqual(errorHandlerSpy.callCount, 1); + assert.ok(error.message.includes('sendBeacon - cannot send')); + + //reset global error handler + setGlobalErrorHandler(loggingErrorHandler()); + }); + + setTimeout(() => { + const request = server.requests[0]; + request.respond(400); + }); + }); }); }); }); diff --git a/packages/opentelemetry-exporter-zipkin/test/node/zipkin.test.ts b/packages/opentelemetry-exporter-zipkin/test/node/zipkin.test.ts index df2604f532..298cd04f26 100644 --- a/packages/opentelemetry-exporter-zipkin/test/node/zipkin.test.ts +++ b/packages/opentelemetry-exporter-zipkin/test/node/zipkin.test.ts @@ -16,11 +16,14 @@ import * as assert from 'assert'; import * as nock from 'nock'; +import * as sinon from 'sinon'; import { ReadableSpan } from '@opentelemetry/tracing'; import { ExportResult, NoopLogger, hrTimeToMicroseconds, + setGlobalErrorHandler, + loggingErrorHandler, } from '@opentelemetry/core'; import * as api from '@opentelemetry/api'; import { Resource } from '@opentelemetry/resources'; @@ -463,6 +466,30 @@ describe('Zipkin Exporter - node', () => { assert.equal(exporter['_serviceName'], resource_service_name); }); }); + + it('should call globalErrorHandler on error', () => { + const errorHandlerSpy = sinon.spy(); + setGlobalErrorHandler(errorHandlerSpy); + const expectedError = new Error('Whoops'); + const scope = nock('http://localhost:9411') + .post('/api/v2/spans') + .replyWithError(expectedError); + + const exporter = new ZipkinExporter({ + serviceName: 'my-service', + logger: new NoopLogger(), + }); + + exporter.export([getReadableSpan()], (result: ExportResult) => { + scope.done(); + }); + + const [[error]] = errorHandlerSpy.args; + + assert.strictEqual(errorHandlerSpy.callCount, 1); + assert.strictEqual(error, expectedError); + setGlobalErrorHandler(loggingErrorHandler()); + }); }); }); diff --git a/packages/opentelemetry-metrics/src/export/Controller.ts b/packages/opentelemetry-metrics/src/export/Controller.ts index 0b631777bf..662218c10a 100644 --- a/packages/opentelemetry-metrics/src/export/Controller.ts +++ b/packages/opentelemetry-metrics/src/export/Controller.ts @@ -14,7 +14,11 @@ * limitations under the License. */ -import { ExportResult, unrefTimer } from '@opentelemetry/core'; +import { + ExportResult, + unrefTimer, + globalErrorHandler, +} from '@opentelemetry/core'; import { Meter } from '../Meter'; import { MetricExporter } from './types'; @@ -49,12 +53,12 @@ export class PushController extends Controller { this._exporter.export( this._meter.getBatcher().checkPointSet(), result => { - if (result === ExportResult.SUCCESS) { - resolve(); - } else { - // @todo log error - reject(); + if (result !== ExportResult.SUCCESS) { + globalErrorHandler( + new Error('PushController: export failed in _collect') + ); } + resolve(); } ); }); diff --git a/packages/opentelemetry-tracing/src/MultiSpanProcessor.ts b/packages/opentelemetry-tracing/src/MultiSpanProcessor.ts index 97341a277a..179394bca2 100644 --- a/packages/opentelemetry-tracing/src/MultiSpanProcessor.ts +++ b/packages/opentelemetry-tracing/src/MultiSpanProcessor.ts @@ -14,6 +14,7 @@ * limitations under the License. */ +import { globalErrorHandler } from '@opentelemetry/core'; import { SpanProcessor } from './SpanProcessor'; import { ReadableSpan } from './export/ReadableSpan'; @@ -30,10 +31,17 @@ export class MultiSpanProcessor implements SpanProcessor { for (const spanProcessor of this._spanProcessors) { promises.push(spanProcessor.forceFlush()); } - return new Promise((resolve, reject) => { - Promise.all(promises).then(() => { - resolve(); - }, reject); + return new Promise(resolve => { + Promise.all(promises) + .then(() => { + resolve(); + }) + .catch(error => { + globalErrorHandler( + error || new Error('MultiSpanProcessor: forceFlush failed') + ); + resolve(); + }); }); } diff --git a/packages/opentelemetry-tracing/test/MultiSpanProcessor.test.ts b/packages/opentelemetry-tracing/test/MultiSpanProcessor.test.ts index d732917d5b..68723d2114 100644 --- a/packages/opentelemetry-tracing/test/MultiSpanProcessor.test.ts +++ b/packages/opentelemetry-tracing/test/MultiSpanProcessor.test.ts @@ -23,6 +23,10 @@ import { Span, SpanProcessor, } from '../src'; +import { + setGlobalErrorHandler, + loggingErrorHandler, +} from '@opentelemetry/core'; import { MultiSpanProcessor } from '../src/MultiSpanProcessor'; class TestProcessor implements SpanProcessor { @@ -179,4 +183,27 @@ describe('MultiSpanProcessor', () => { done(); }); }); + + it('should call globalErrorHandler in forceFlush', async () => { + const expectedError = new Error('whoops'); + const testProcessor = new TestProcessor(); + const forceFlush = Sinon.stub(testProcessor, 'forceFlush'); + forceFlush.rejects(expectedError); + + const multiSpanProcessor = new MultiSpanProcessor([testProcessor]); + const errorHandlerSpy = Sinon.spy(); + + setGlobalErrorHandler(errorHandlerSpy); + + await multiSpanProcessor.forceFlush(); + + forceFlush.restore(); + const [[error]] = errorHandlerSpy.args; + + assert.strictEqual(error, expectedError); + assert.strictEqual(errorHandlerSpy.callCount, 1); + + //reset global error handler + setGlobalErrorHandler(loggingErrorHandler()); + }); });