From 4e81f83a53d2d954a92ef26bf86604e10c4c0056 Mon Sep 17 00:00:00 2001 From: Svetlana Brennan Date: Mon, 24 Jan 2022 17:45:34 -0600 Subject: [PATCH 01/46] feat(otlp-exporter): add timeout env var Signed-off-by: Svetlana Brennan --- .../src/OTLPExporterBase.ts | 19 ++++++++++++++++++- .../exporter-trace-otlp-http/src/types.ts | 3 +++ .../src/utils/environment.ts | 2 ++ 3 files changed, 23 insertions(+), 1 deletion(-) diff --git a/packages/exporter-trace-otlp-http/src/OTLPExporterBase.ts b/packages/exporter-trace-otlp-http/src/OTLPExporterBase.ts index 2dc095bcc5..80b905d008 100644 --- a/packages/exporter-trace-otlp-http/src/OTLPExporterBase.ts +++ b/packages/exporter-trace-otlp-http/src/OTLPExporterBase.ts @@ -15,7 +15,7 @@ */ import { SpanAttributes, diag } from '@opentelemetry/api'; -import { ExportResult, ExportResultCode, BindOnceFuture } from '@opentelemetry/core'; +import { ExportResult, ExportResultCode, BindOnceFuture, getEnv } from '@opentelemetry/core'; import { OTLPExporterError, OTLPExporterConfigBase, @@ -36,6 +36,7 @@ export abstract class OTLPExporterBase< protected _concurrencyLimit: number; protected _sendingPromises: Promise[] = []; protected _shutdownOnce: BindOnceFuture; + private readonly _timeoutMillis: number; /** * @param config @@ -55,6 +56,12 @@ export abstract class OTLPExporterBase< typeof config.concurrencyLimit === 'number' ? config.concurrencyLimit : Infinity; + this._timeoutMillis = + typeof config.timeoutMillis === 'number' + ? config.timeoutMillis < 0 + ? this._invalidTimeout(config.timeoutMillis) + : config.timeoutMillis + : getEnv().OTEL_EXPORTER_OTLP_TIMEOUT; // platform dependent this.onInit(config); @@ -93,6 +100,11 @@ export abstract class OTLPExporterBase< private _export(items: ExportItem[]): Promise { return new Promise((resolve, reject) => { + setTimeout(() => { + // don't wait anymore for export + reject(new Error('Timeout')); + }, this._timeoutMillis); + try { diag.debug('items to be sent', items); this.send(items, resolve, reject); @@ -102,6 +114,11 @@ export abstract class OTLPExporterBase< }); } + private _invalidTimeout(timeout: number): number { + diag.warn('Timeout must be non-negative', timeout); + return getEnv().OTEL_EXPORTER_OTLP_TIMEOUT; + } + /** * Shutdown the exporter. */ diff --git a/packages/exporter-trace-otlp-http/src/types.ts b/packages/exporter-trace-otlp-http/src/types.ts index 3f2bd27a01..dfb16a511f 100644 --- a/packages/exporter-trace-otlp-http/src/types.ts +++ b/packages/exporter-trace-otlp-http/src/types.ts @@ -347,6 +347,9 @@ export interface OTLPExporterConfigBase { attributes?: SpanAttributes; url?: string; concurrencyLimit?: number; + /** Maximum time the OTLP exporter will wait for each batch export. + * The default value is 10000ms. */ + timeoutMillis?: number; } /** diff --git a/packages/opentelemetry-core/src/utils/environment.ts b/packages/opentelemetry-core/src/utils/environment.ts index 7df0be698c..e6035edc7a 100644 --- a/packages/opentelemetry-core/src/utils/environment.ts +++ b/packages/opentelemetry-core/src/utils/environment.ts @@ -34,6 +34,7 @@ const ENVIRONMENT_NUMBERS_KEYS = [ 'OTEL_SPAN_ATTRIBUTE_COUNT_LIMIT', 'OTEL_SPAN_EVENT_COUNT_LIMIT', 'OTEL_SPAN_LINK_COUNT_LIMIT', + 'OTEL_EXPORTER_OTLP_TIMEOUT' ] as const; type ENVIRONMENT_NUMBERS = { @@ -118,6 +119,7 @@ export const DEFAULT_ENVIRONMENT: Required = { OTEL_EXPORTER_OTLP_HEADERS: '', OTEL_EXPORTER_OTLP_TRACES_HEADERS: '', OTEL_EXPORTER_OTLP_METRICS_HEADERS: '', + OTEL_EXPORTER_OTLP_TIMEOUT: 10000, OTEL_EXPORTER_ZIPKIN_ENDPOINT: 'http://localhost:9411/api/v2/spans', OTEL_LOG_LEVEL: DiagLogLevel.INFO, OTEL_NO_PATCH_MODULES: [], From 11ea7701fa87ce041be9cf0af3fe43733e21c8e2 Mon Sep 17 00:00:00 2001 From: Svetlana Brennan Date: Tue, 25 Jan 2022 17:15:37 -0600 Subject: [PATCH 02/46] feat(otlp-exporter): add timeout env var test Signed-off-by: Svetlana Brennan --- packages/opentelemetry-core/test/utils/environment.test.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/packages/opentelemetry-core/test/utils/environment.test.ts b/packages/opentelemetry-core/test/utils/environment.test.ts index 4ab3518645..3863ed7279 100644 --- a/packages/opentelemetry-core/test/utils/environment.test.ts +++ b/packages/opentelemetry-core/test/utils/environment.test.ts @@ -92,6 +92,7 @@ describe('environment', () => { OTEL_SPAN_LINK_COUNT_LIMIT: 30, OTEL_TRACES_SAMPLER: 'always_on', OTEL_TRACES_SAMPLER_ARG: '0.5', + OTEL_EXPORTER_OTLP_TIMEOUT: 10000, }); const env = getEnv(); assert.deepStrictEqual(env.OTEL_NO_PATCH_MODULES, ['a', 'b', 'c']); @@ -126,6 +127,7 @@ describe('environment', () => { assert.strictEqual(env.OTEL_BSP_SCHEDULE_DELAY, 50); assert.strictEqual(env.OTEL_TRACES_SAMPLER, 'always_on'); assert.strictEqual(env.OTEL_TRACES_SAMPLER_ARG, '0.5'); + assert.strictEqual(env.OTEL_EXPORTER_OTLP_TIMEOUT, 10000); }); it('should parse OTEL_LOG_LEVEL despite casing', () => { From 2bc61e438119a085fb07c1e751dc162ed4e299e3 Mon Sep 17 00:00:00 2001 From: Svetlana Brennan Date: Wed, 26 Jan 2022 13:35:33 -0600 Subject: [PATCH 03/46] feat(otlp-exporter): add trace_timeout env var Signed-off-by: Svetlana Brennan --- .../src/OTLPExporterBase.ts | 27 +++++++++++++------ .../src/utils/environment.ts | 4 ++- .../test/utils/environment.test.ts | 2 ++ 3 files changed, 24 insertions(+), 9 deletions(-) diff --git a/packages/exporter-trace-otlp-http/src/OTLPExporterBase.ts b/packages/exporter-trace-otlp-http/src/OTLPExporterBase.ts index 80b905d008..5bc46c17e1 100644 --- a/packages/exporter-trace-otlp-http/src/OTLPExporterBase.ts +++ b/packages/exporter-trace-otlp-http/src/OTLPExporterBase.ts @@ -56,12 +56,8 @@ export abstract class OTLPExporterBase< typeof config.concurrencyLimit === 'number' ? config.concurrencyLimit : Infinity; - this._timeoutMillis = - typeof config.timeoutMillis === 'number' - ? config.timeoutMillis < 0 - ? this._invalidTimeout(config.timeoutMillis) - : config.timeoutMillis - : getEnv().OTEL_EXPORTER_OTLP_TIMEOUT; + + this._timeoutMillis = this._configureTimeout(config.timeoutMillis); // platform dependent this.onInit(config); @@ -114,9 +110,24 @@ export abstract class OTLPExporterBase< }); } - private _invalidTimeout(timeout: number): number { + private _configureTimeout(timeoutMillis: number | undefined): number { + let defaultTimeout = getEnv().OTEL_EXPORTER_OTLP_TRACES_TIMEOUT; + + if ( defaultTimeout === null) { + defaultTimeout = getEnv().OTEL_EXPORTER_OTLP_TIMEOUT; + } + + return typeof timeoutMillis === 'number' + ? timeoutMillis < 0 + ? this._invalidTimeout(timeoutMillis, defaultTimeout) + : timeoutMillis + : defaultTimeout; + } + + private _invalidTimeout(timeout: number, defaultTimeout: number): number { diag.warn('Timeout must be non-negative', timeout); - return getEnv().OTEL_EXPORTER_OTLP_TIMEOUT; + + return defaultTimeout; } /** diff --git a/packages/opentelemetry-core/src/utils/environment.ts b/packages/opentelemetry-core/src/utils/environment.ts index e6035edc7a..4522384988 100644 --- a/packages/opentelemetry-core/src/utils/environment.ts +++ b/packages/opentelemetry-core/src/utils/environment.ts @@ -34,7 +34,8 @@ const ENVIRONMENT_NUMBERS_KEYS = [ 'OTEL_SPAN_ATTRIBUTE_COUNT_LIMIT', 'OTEL_SPAN_EVENT_COUNT_LIMIT', 'OTEL_SPAN_LINK_COUNT_LIMIT', - 'OTEL_EXPORTER_OTLP_TIMEOUT' + 'OTEL_EXPORTER_OTLP_TIMEOUT', + 'OTEL_EXPORTER_OTLP_TRACES_TIMEOUT' ] as const; type ENVIRONMENT_NUMBERS = { @@ -120,6 +121,7 @@ export const DEFAULT_ENVIRONMENT: Required = { OTEL_EXPORTER_OTLP_TRACES_HEADERS: '', OTEL_EXPORTER_OTLP_METRICS_HEADERS: '', OTEL_EXPORTER_OTLP_TIMEOUT: 10000, + OTEL_EXPORTER_OTLP_TRACES_TIMEOUT: 10000, OTEL_EXPORTER_ZIPKIN_ENDPOINT: 'http://localhost:9411/api/v2/spans', OTEL_LOG_LEVEL: DiagLogLevel.INFO, OTEL_NO_PATCH_MODULES: [], diff --git a/packages/opentelemetry-core/test/utils/environment.test.ts b/packages/opentelemetry-core/test/utils/environment.test.ts index 3863ed7279..708e0e6abe 100644 --- a/packages/opentelemetry-core/test/utils/environment.test.ts +++ b/packages/opentelemetry-core/test/utils/environment.test.ts @@ -93,6 +93,7 @@ describe('environment', () => { OTEL_TRACES_SAMPLER: 'always_on', OTEL_TRACES_SAMPLER_ARG: '0.5', OTEL_EXPORTER_OTLP_TIMEOUT: 10000, + OTEL_EXPORTER_OTLP_TRACES_TIMEOUT: 10000, }); const env = getEnv(); assert.deepStrictEqual(env.OTEL_NO_PATCH_MODULES, ['a', 'b', 'c']); @@ -128,6 +129,7 @@ describe('environment', () => { assert.strictEqual(env.OTEL_TRACES_SAMPLER, 'always_on'); assert.strictEqual(env.OTEL_TRACES_SAMPLER_ARG, '0.5'); assert.strictEqual(env.OTEL_EXPORTER_OTLP_TIMEOUT, 10000); + assert.strictEqual(env.OTEL_EXPORTER_OTLP_TRACES_TIMEOUT, 10000); }); it('should parse OTEL_LOG_LEVEL despite casing', () => { From da3211f44bf32d4568fe6ceee660b4996d897eed Mon Sep 17 00:00:00 2001 From: Svetlana Brennan Date: Wed, 26 Jan 2022 17:17:46 -0600 Subject: [PATCH 04/46] feat(otlp-exporter): add how timeout is selected Signed-off-by: Svetlana Brennan --- .../src/OTLPExporterBase.ts | 22 +++++++++++-------- 1 file changed, 13 insertions(+), 9 deletions(-) diff --git a/packages/exporter-trace-otlp-http/src/OTLPExporterBase.ts b/packages/exporter-trace-otlp-http/src/OTLPExporterBase.ts index 5bc46c17e1..8efb531de9 100644 --- a/packages/exporter-trace-otlp-http/src/OTLPExporterBase.ts +++ b/packages/exporter-trace-otlp-http/src/OTLPExporterBase.ts @@ -111,17 +111,21 @@ export abstract class OTLPExporterBase< } private _configureTimeout(timeoutMillis: number | undefined): number { - let defaultTimeout = getEnv().OTEL_EXPORTER_OTLP_TRACES_TIMEOUT; - - if ( defaultTimeout === null) { - defaultTimeout = getEnv().OTEL_EXPORTER_OTLP_TIMEOUT; - } + const timeout = this._selectATimeout(); return typeof timeoutMillis === 'number' - ? timeoutMillis < 0 - ? this._invalidTimeout(timeoutMillis, defaultTimeout) - : timeoutMillis - : defaultTimeout; + ? timeoutMillis < 0 + ? this._invalidTimeout(timeoutMillis, timeout) + : timeoutMillis + : timeout; + } + + private _selectATimeout(): number { + const definedTimeout = + Number(process.env.OTEL_EXPORTER_OTLP_TRACES_TIMEOUT || + process.env.OTEL_EXPORTER_OTLP_TIMEOUT); + + return definedTimeout ? definedTimeout : getEnv().OTEL_EXPORTER_OTLP_TRACES_TIMEOUT; } private _invalidTimeout(timeout: number, defaultTimeout: number): number { From 0c1011d97dbcca8de24038b1298da844f034f187 Mon Sep 17 00:00:00 2001 From: Svetlana Brennan Date: Thu, 27 Jan 2022 15:00:58 -0600 Subject: [PATCH 05/46] feat(otlp-exporter): add negative value check to env vars Signed-off-by: Svetlana Brennan --- .../src/OTLPExporterBase.ts | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/packages/exporter-trace-otlp-http/src/OTLPExporterBase.ts b/packages/exporter-trace-otlp-http/src/OTLPExporterBase.ts index 8efb531de9..5fef70384d 100644 --- a/packages/exporter-trace-otlp-http/src/OTLPExporterBase.ts +++ b/packages/exporter-trace-otlp-http/src/OTLPExporterBase.ts @@ -111,13 +111,11 @@ export abstract class OTLPExporterBase< } private _configureTimeout(timeoutMillis: number | undefined): number { - const timeout = this._selectATimeout(); - return typeof timeoutMillis === 'number' ? timeoutMillis < 0 - ? this._invalidTimeout(timeoutMillis, timeout) - : timeoutMillis - : timeout; + ? this._invalidTimeout(timeoutMillis, 10000) + : timeoutMillis + : this._selectATimeout(); } private _selectATimeout(): number { @@ -125,7 +123,11 @@ export abstract class OTLPExporterBase< Number(process.env.OTEL_EXPORTER_OTLP_TRACES_TIMEOUT || process.env.OTEL_EXPORTER_OTLP_TIMEOUT); - return definedTimeout ? definedTimeout : getEnv().OTEL_EXPORTER_OTLP_TRACES_TIMEOUT; + return definedTimeout + ? definedTimeout < 0 + ? this._invalidTimeout(definedTimeout, 10000) + : definedTimeout + : getEnv().OTEL_EXPORTER_OTLP_TRACES_TIMEOUT; } private _invalidTimeout(timeout: number, defaultTimeout: number): number { From ed4079f9c69f5684a48c1ca44ff2f5756907dec0 Mon Sep 17 00:00:00 2001 From: Svetlana Brennan Date: Thu, 27 Jan 2022 15:01:41 -0600 Subject: [PATCH 06/46] feat(otlp-exporter): add negative value check to env vars Signed-off-by: Svetlana Brennan --- .../exporter-trace-otlp-http/src/OTLPExporterBase.ts | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/packages/exporter-trace-otlp-http/src/OTLPExporterBase.ts b/packages/exporter-trace-otlp-http/src/OTLPExporterBase.ts index 5fef70384d..d66976a723 100644 --- a/packages/exporter-trace-otlp-http/src/OTLPExporterBase.ts +++ b/packages/exporter-trace-otlp-http/src/OTLPExporterBase.ts @@ -113,9 +113,9 @@ export abstract class OTLPExporterBase< private _configureTimeout(timeoutMillis: number | undefined): number { return typeof timeoutMillis === 'number' ? timeoutMillis < 0 - ? this._invalidTimeout(timeoutMillis, 10000) - : timeoutMillis - : this._selectATimeout(); + ? this._invalidTimeout(timeoutMillis, 10000) + : timeoutMillis + : this._selectATimeout(); } private _selectATimeout(): number { @@ -125,9 +125,9 @@ export abstract class OTLPExporterBase< return definedTimeout ? definedTimeout < 0 - ? this._invalidTimeout(definedTimeout, 10000) - : definedTimeout - : getEnv().OTEL_EXPORTER_OTLP_TRACES_TIMEOUT; + ? this._invalidTimeout(definedTimeout, 10000) + : definedTimeout + : getEnv().OTEL_EXPORTER_OTLP_TRACES_TIMEOUT; } private _invalidTimeout(timeout: number, defaultTimeout: number): number { From fc486a1cc41dcb5b43de4adce7ec878123d46550 Mon Sep 17 00:00:00 2001 From: Svetlana Brennan Date: Fri, 28 Jan 2022 11:14:24 -0600 Subject: [PATCH 07/46] feat(otlp-exporter): add timeout config information to readme Signed-off-by: Svetlana Brennan --- packages/exporter-trace-otlp-grpc/README.md | 23 ++++++++++++++++++++ packages/exporter-trace-otlp-http/README.md | 23 ++++++++++++++++++++ packages/exporter-trace-otlp-proto/README.md | 23 ++++++++++++++++++++ 3 files changed, 69 insertions(+) diff --git a/packages/exporter-trace-otlp-grpc/README.md b/packages/exporter-trace-otlp-grpc/README.md index f54acbe43b..8402641030 100644 --- a/packages/exporter-trace-otlp-grpc/README.md +++ b/packages/exporter-trace-otlp-grpc/README.md @@ -107,6 +107,29 @@ provider.register(); Note, that this will only work if TLS is also configured on the server. +## Exporter Timeout Configuration + +The OTLPTraceExporter has a timeout configuration option which is the maximum time, in milliseconds, the OTLP exporter will wait for each batch export. The default value is 10000ms. + +To override the default timeout duration, use the following options: + ++ Set with environment variables: + + | Environment variable | Description | + |----------------------|-------------| + | OTEL_EXPORTER_OTLP_TRACES_TIMEOUT | The maximum waiting time, in milliseconds, allowed to send each OTLP trace batch. Default is 10000. | + | OTEL_EXPORTER_OTLP_TIMEOUT | The maximum waiting time, in milliseconds, allowed to send each OTLP trace and metric batch. Default is 10000. | + + > `OTEL_EXPORTER_OTLP_TRACES_TIMEOUT` takes precedence and overrides `OTEL_EXPORTER_OTLP_TIMEOUT`. + ++ Provide `timeoutMillis` to OTLPTraceExporter with `collectorOptions`: + ```js + const collectorOptions = { + timeoutMillis: 15000 + }; + ``` + > Providing `timeoutMillis` with `collectorOptions` takes precedence and overrides timeout set with environment variables. + ## Running opentelemetry-collector locally to see the traces 1. Go to examples/otlp-exporter-node diff --git a/packages/exporter-trace-otlp-http/README.md b/packages/exporter-trace-otlp-http/README.md index f03ca83ff5..668fcfbaac 100644 --- a/packages/exporter-trace-otlp-http/README.md +++ b/packages/exporter-trace-otlp-http/README.md @@ -106,6 +106,29 @@ OTEL_EXPORTER_OTLP_METRICS_ENDPOINT=https://metric-service:4317/v1/metrics For more details, see [OpenTelemetry Specification on Protocol Exporter][opentelemetry-spec-protocol-exporter]. +## Exporter Timeout Configuration + +The OTLPTraceExporter has a timeout configuration option which is the maximum time, in milliseconds, the OTLP exporter will wait for each batch export. The default value is 10000ms. + +To override the default timeout duration, use the following options: + ++ Set with environment variables: + + | Environment variable | Description | + |----------------------|-------------| + | OTEL_EXPORTER_OTLP_TRACES_TIMEOUT | The maximum waiting time, in milliseconds, allowed to send each OTLP trace batch. Default is 10000. | + | OTEL_EXPORTER_OTLP_TIMEOUT | The maximum waiting time, in milliseconds, allowed to send each OTLP trace and metric batch. Default is 10000. | + + > `OTEL_EXPORTER_OTLP_TRACES_TIMEOUT` takes precedence and overrides `OTEL_EXPORTER_OTLP_TIMEOUT`. + ++ Provide `timeoutMillis` to OTLPTraceExporter with `collectorOptions`: + ```js + const collectorOptions = { + timeoutMillis: 15000 + }; + ``` + > Providing `timeoutMillis` with `collectorOptions` takes precedence and overrides timeout set with environment variables. + ## Running opentelemetry-collector locally to see the traces 1. Go to examples/otlp-exporter-node diff --git a/packages/exporter-trace-otlp-proto/README.md b/packages/exporter-trace-otlp-proto/README.md index 19e91f0f46..c86359632b 100644 --- a/packages/exporter-trace-otlp-proto/README.md +++ b/packages/exporter-trace-otlp-proto/README.md @@ -38,6 +38,29 @@ provider.register(); ``` +## Exporter Timeout Configuration + +The OTLPTraceExporter has a timeout configuration option which is the maximum time, in milliseconds, the OTLP exporter will wait for each batch export. The default value is 10000ms. + +To override the default timeout duration, use the following options: + ++ Set with environment variables: + + | Environment variable | Description | + |----------------------|-------------| + | OTEL_EXPORTER_OTLP_TRACES_TIMEOUT | The maximum waiting time, in milliseconds, allowed to send each OTLP trace batch. Default is 10000. | + | OTEL_EXPORTER_OTLP_TIMEOUT | The maximum waiting time, in milliseconds, allowed to send each OTLP trace and metric batch. Default is 10000. | + + > `OTEL_EXPORTER_OTLP_TRACES_TIMEOUT` takes precedence and overrides `OTEL_EXPORTER_OTLP_TIMEOUT`. + ++ Provide `timeoutMillis` to OTLPTraceExporter with `collectorOptions`: + ```js + const collectorOptions = { + timeoutMillis: 15000 + }; + ``` + > Providing `timeoutMillis` with `collectorOptions` takes precedence and overrides timeout set with environment variables. + ## Running opentelemetry-collector locally to see the traces 1. Go to examples/otlp-exporter-node From 7292244845de071bd5536779cae0fa50b90777c5 Mon Sep 17 00:00:00 2001 From: Svetlana Brennan Date: Fri, 28 Jan 2022 13:56:25 -0600 Subject: [PATCH 08/46] feat(otlp-exporter): add const var for default timeout value Signed-off-by: Svetlana Brennan --- packages/exporter-trace-otlp-http/src/OTLPExporterBase.ts | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/packages/exporter-trace-otlp-http/src/OTLPExporterBase.ts b/packages/exporter-trace-otlp-http/src/OTLPExporterBase.ts index d66976a723..dc56f43644 100644 --- a/packages/exporter-trace-otlp-http/src/OTLPExporterBase.ts +++ b/packages/exporter-trace-otlp-http/src/OTLPExporterBase.ts @@ -22,6 +22,8 @@ import { ExportServiceError, } from './types'; +const DEFAULT_TIMEOUT = 10000; + /** * Collector Exporter abstract base class */ @@ -113,7 +115,7 @@ export abstract class OTLPExporterBase< private _configureTimeout(timeoutMillis: number | undefined): number { return typeof timeoutMillis === 'number' ? timeoutMillis < 0 - ? this._invalidTimeout(timeoutMillis, 10000) + ? this._invalidTimeout(timeoutMillis, DEFAULT_TIMEOUT) : timeoutMillis : this._selectATimeout(); } @@ -125,7 +127,7 @@ export abstract class OTLPExporterBase< return definedTimeout ? definedTimeout < 0 - ? this._invalidTimeout(definedTimeout, 10000) + ? this._invalidTimeout(definedTimeout, DEFAULT_TIMEOUT) : definedTimeout : getEnv().OTEL_EXPORTER_OTLP_TRACES_TIMEOUT; } From e8dc21b7f7ec46ac8a75d078da98936fe5699ede Mon Sep 17 00:00:00 2001 From: Svetlana Brennan Date: Tue, 1 Feb 2022 17:26:32 -0600 Subject: [PATCH 09/46] feat(otlp-exporter): refactor code based on feedback Signed-off-by: Svetlana Brennan --- .../src/OTLPExporterBase.ts | 35 ++++++++++++------- .../test/utils/environment.test.ts | 8 ++--- 2 files changed, 27 insertions(+), 16 deletions(-) diff --git a/packages/exporter-trace-otlp-http/src/OTLPExporterBase.ts b/packages/exporter-trace-otlp-http/src/OTLPExporterBase.ts index dc56f43644..ad3fb80e12 100644 --- a/packages/exporter-trace-otlp-http/src/OTLPExporterBase.ts +++ b/packages/exporter-trace-otlp-http/src/OTLPExporterBase.ts @@ -98,7 +98,7 @@ export abstract class OTLPExporterBase< private _export(items: ExportItem[]): Promise { return new Promise((resolve, reject) => { - setTimeout(() => { + const timer = setTimeout(() => { // don't wait anymore for export reject(new Error('Timeout')); }, this._timeoutMillis); @@ -108,30 +108,41 @@ export abstract class OTLPExporterBase< this.send(items, resolve, reject); } catch (e) { reject(e); + } finally { + timer.unref(); } }); } private _configureTimeout(timeoutMillis: number | undefined): number { - return typeof timeoutMillis === 'number' - ? timeoutMillis < 0 - ? this._invalidTimeout(timeoutMillis, DEFAULT_TIMEOUT) - : timeoutMillis - : this._selectATimeout(); + if (typeof timeoutMillis === 'number') { + if (timeoutMillis < 0) { + // OTLP exporter configured timeout - using default value of 10000ms + return this._invalidTimeout(timeoutMillis, DEFAULT_TIMEOUT); + } + return timeoutMillis; + } else { + return this._getTimeoutFromEnv(); + } } - private _selectATimeout(): number { + private _getTimeoutFromEnv(): number { const definedTimeout = Number(process.env.OTEL_EXPORTER_OTLP_TRACES_TIMEOUT || process.env.OTEL_EXPORTER_OTLP_TIMEOUT); - return definedTimeout - ? definedTimeout < 0 - ? this._invalidTimeout(definedTimeout, DEFAULT_TIMEOUT) - : definedTimeout - : getEnv().OTEL_EXPORTER_OTLP_TRACES_TIMEOUT; + if (definedTimeout) { + if (definedTimeout < 0) { + // OTLP exporter configured timeout - using default value of 10000ms + return this._invalidTimeout(definedTimeout, DEFAULT_TIMEOUT); + } + return definedTimeout; + } else { + return getEnv().OTEL_EXPORTER_OTLP_TRACES_TIMEOUT; + } } + // OTLP exporter configured timeout - using default value of 10000ms private _invalidTimeout(timeout: number, defaultTimeout: number): number { diag.warn('Timeout must be non-negative', timeout); diff --git a/packages/opentelemetry-core/test/utils/environment.test.ts b/packages/opentelemetry-core/test/utils/environment.test.ts index 708e0e6abe..96904f0ca0 100644 --- a/packages/opentelemetry-core/test/utils/environment.test.ts +++ b/packages/opentelemetry-core/test/utils/environment.test.ts @@ -92,8 +92,8 @@ describe('environment', () => { OTEL_SPAN_LINK_COUNT_LIMIT: 30, OTEL_TRACES_SAMPLER: 'always_on', OTEL_TRACES_SAMPLER_ARG: '0.5', - OTEL_EXPORTER_OTLP_TIMEOUT: 10000, - OTEL_EXPORTER_OTLP_TRACES_TIMEOUT: 10000, + OTEL_EXPORTER_OTLP_TIMEOUT: 15000, + OTEL_EXPORTER_OTLP_TRACES_TIMEOUT: 12000, }); const env = getEnv(); assert.deepStrictEqual(env.OTEL_NO_PATCH_MODULES, ['a', 'b', 'c']); @@ -128,8 +128,8 @@ describe('environment', () => { assert.strictEqual(env.OTEL_BSP_SCHEDULE_DELAY, 50); assert.strictEqual(env.OTEL_TRACES_SAMPLER, 'always_on'); assert.strictEqual(env.OTEL_TRACES_SAMPLER_ARG, '0.5'); - assert.strictEqual(env.OTEL_EXPORTER_OTLP_TIMEOUT, 10000); - assert.strictEqual(env.OTEL_EXPORTER_OTLP_TRACES_TIMEOUT, 10000); + assert.strictEqual(env.OTEL_EXPORTER_OTLP_TIMEOUT, 15000); + assert.strictEqual(env.OTEL_EXPORTER_OTLP_TRACES_TIMEOUT, 12000); }); it('should parse OTEL_LOG_LEVEL despite casing', () => { From 111cb83ee3bcbb45f12431d2afff11d1590dcfaa Mon Sep 17 00:00:00 2001 From: Svetlana Brennan Date: Thu, 3 Feb 2022 14:52:52 -0600 Subject: [PATCH 10/46] feat(otlp-exporter): move configuring timeout to utils and tests Signed-off-by: Svetlana Brennan --- .../src/OTLPExporterBase.ts | 42 ++---------------- packages/exporter-trace-otlp-http/src/util.ts | 44 +++++++++++++++++++ .../test/common/utils.test.ts | 31 +++++++++++++ .../test/node/CollectorTraceExporter.test.ts | 1 + 4 files changed, 79 insertions(+), 39 deletions(-) diff --git a/packages/exporter-trace-otlp-http/src/OTLPExporterBase.ts b/packages/exporter-trace-otlp-http/src/OTLPExporterBase.ts index ad3fb80e12..a55fcc4fbf 100644 --- a/packages/exporter-trace-otlp-http/src/OTLPExporterBase.ts +++ b/packages/exporter-trace-otlp-http/src/OTLPExporterBase.ts @@ -15,14 +15,13 @@ */ import { SpanAttributes, diag } from '@opentelemetry/api'; -import { ExportResult, ExportResultCode, BindOnceFuture, getEnv } from '@opentelemetry/core'; +import { ExportResult, ExportResultCode, BindOnceFuture } from '@opentelemetry/core'; import { OTLPExporterError, OTLPExporterConfigBase, ExportServiceError, } from './types'; - -const DEFAULT_TIMEOUT = 10000; +import { configureExporterTimeout } from './util'; /** * Collector Exporter abstract base class @@ -59,7 +58,7 @@ export abstract class OTLPExporterBase< ? config.concurrencyLimit : Infinity; - this._timeoutMillis = this._configureTimeout(config.timeoutMillis); + this._timeoutMillis = configureExporterTimeout(config.timeoutMillis); // platform dependent this.onInit(config); @@ -114,41 +113,6 @@ export abstract class OTLPExporterBase< }); } - private _configureTimeout(timeoutMillis: number | undefined): number { - if (typeof timeoutMillis === 'number') { - if (timeoutMillis < 0) { - // OTLP exporter configured timeout - using default value of 10000ms - return this._invalidTimeout(timeoutMillis, DEFAULT_TIMEOUT); - } - return timeoutMillis; - } else { - return this._getTimeoutFromEnv(); - } - } - - private _getTimeoutFromEnv(): number { - const definedTimeout = - Number(process.env.OTEL_EXPORTER_OTLP_TRACES_TIMEOUT || - process.env.OTEL_EXPORTER_OTLP_TIMEOUT); - - if (definedTimeout) { - if (definedTimeout < 0) { - // OTLP exporter configured timeout - using default value of 10000ms - return this._invalidTimeout(definedTimeout, DEFAULT_TIMEOUT); - } - return definedTimeout; - } else { - return getEnv().OTEL_EXPORTER_OTLP_TRACES_TIMEOUT; - } - } - - // OTLP exporter configured timeout - using default value of 10000ms - private _invalidTimeout(timeout: number, defaultTimeout: number): number { - diag.warn('Timeout must be non-negative', timeout); - - return defaultTimeout; - } - /** * Shutdown the exporter. */ diff --git a/packages/exporter-trace-otlp-http/src/util.ts b/packages/exporter-trace-otlp-http/src/util.ts index b018000110..173259e576 100644 --- a/packages/exporter-trace-otlp-http/src/util.ts +++ b/packages/exporter-trace-otlp-http/src/util.ts @@ -15,6 +15,9 @@ */ import { diag } from '@opentelemetry/api'; +import { getEnv } from '@opentelemetry/core'; + +const DEFAULT_TRACE_TIMEOUT = 10000; /** * Parses headers from config leaving only those that have defined values @@ -39,3 +42,44 @@ export function appendResourcePathToUrlIfNotPresent(url: string, path: string): return url + path; } + +/** + * Configure exporter trace timeout value from passed in value or environment variables + * @param timeoutMillis + * @returns timeout value in milliseconds + */ + +export function configureExporterTimeout(timeoutMillis: number | undefined): number { + if (typeof timeoutMillis === 'number') { + if (timeoutMillis <= 0) { + // OTLP exporter configured timeout - using default value of 10000ms + return invalidTimeout(timeoutMillis, DEFAULT_TRACE_TIMEOUT); + } + return timeoutMillis; + } else { + return getExporterTimeoutFromEnv(); + } +} + +function getExporterTimeoutFromEnv(): number { + const definedTimeout = + Number(process.env.OTEL_EXPORTER_OTLP_TRACES_TIMEOUT || + process.env.OTEL_EXPORTER_OTLP_TIMEOUT); + + if (definedTimeout) { + if (definedTimeout <= 0) { + // OTLP exporter configured timeout - using default value of 10000ms + return invalidTimeout(definedTimeout, DEFAULT_TRACE_TIMEOUT); + } + return definedTimeout; + } else { + return getEnv().OTEL_EXPORTER_OTLP_TRACES_TIMEOUT; + } +} + +// OTLP exporter configured timeout - using default value of 10000ms +function invalidTimeout(timeout: number, defaultTimeout: number): number { + diag.warn('Timeout must be non-negative', timeout); + + return defaultTimeout; +} diff --git a/packages/exporter-trace-otlp-http/test/common/utils.test.ts b/packages/exporter-trace-otlp-http/test/common/utils.test.ts index e241495214..61aec6b5f8 100644 --- a/packages/exporter-trace-otlp-http/test/common/utils.test.ts +++ b/packages/exporter-trace-otlp-http/test/common/utils.test.ts @@ -18,6 +18,7 @@ import * as assert from 'assert'; import * as sinon from 'sinon'; import { diag } from '@opentelemetry/api'; import { parseHeaders } from '../../src/util'; +import { configureExporterTimeout } from '../../src/util'; describe('utils', () => { afterEach(() => { @@ -50,4 +51,34 @@ describe('utils', () => { assert.deepStrictEqual(result, {}); }); }); + + describe('configureExporterTimeout', () => { + const envSource = process.env; + it('should use default trace export timeout env variable value when timeoutMillis parameter is undefined', () => { + const exporterTimeout = configureExporterTimeout(undefined); + assert.strictEqual(exporterTimeout, 10000); + }); + it('should use default trace export timeout env variable value when timeoutMillis parameter is negative', () => { + const exporterTimeout = configureExporterTimeout(-18000); + assert.strictEqual(exporterTimeout, 10000); + }); + it('should use trace export timeout value defined in env', () => { + envSource.OTEL_EXPORTER_OTLP_TRACES_TIMEOUT = '15000'; + const exporterTimeout = configureExporterTimeout(undefined); + assert.strictEqual(exporterTimeout, 15000); + delete envSource.OTEL_EXPORTER_OTLP_TRACES_TIMEOUT; + }); + it('should use default trace export timeout env variable value when trace export timeout value defined in env is negative', () => { + envSource.OTEL_EXPORTER_OTLP_TRACES_TIMEOUT = '-15000'; + const exporterTimeout = configureExporterTimeout(undefined); + assert.strictEqual(exporterTimeout, 10000); + delete envSource.OTEL_EXPORTER_OTLP_TRACES_TIMEOUT; + }); + it('should use timeoutMillis parameter over trace export timeout value defined in env', () => { + envSource.OTEL_EXPORTER_OTLP_TRACES_TIMEOUT = '11000'; + const exporterTimeout = configureExporterTimeout(9000); + assert.strictEqual(exporterTimeout, 9000); + delete envSource.OTEL_EXPORTER_OTLP_TRACES_TIMEOUT; + }); + }); }); diff --git a/packages/exporter-trace-otlp-http/test/node/CollectorTraceExporter.test.ts b/packages/exporter-trace-otlp-http/test/node/CollectorTraceExporter.test.ts index af543bae22..8c39978adc 100644 --- a/packages/exporter-trace-otlp-http/test/node/CollectorTraceExporter.test.ts +++ b/packages/exporter-trace-otlp-http/test/node/CollectorTraceExporter.test.ts @@ -232,6 +232,7 @@ describe('OTLPTraceExporter - node with json over http', () => { const mockRes = new MockedResponse(200); const args = stubRequest.args[0]; const callback = args[1]; + callback(mockRes); mockRes.send('success'); setTimeout(() => { From bbb75cb07be4b88c36a35cf922a423d1677d2229 Mon Sep 17 00:00:00 2001 From: Svetlana Brennan Date: Tue, 8 Feb 2022 15:11:40 -0600 Subject: [PATCH 11/46] feat(otlp-exporter): add timeout to http and xhr request Signed-off-by: Svetlana Brennan --- .../src/OTLPExporterBase.ts | 9 +--- .../browser/OTLPExporterBrowserBase.ts | 2 +- .../src/platform/browser/util.ts | 15 +++++- .../src/platform/node/util.ts | 12 ++++- .../test/browser/util.test.ts | 16 +++++-- .../test/node/CollectorTraceExporter.test.ts | 47 +++++++++++++++++++ 6 files changed, 86 insertions(+), 15 deletions(-) diff --git a/packages/exporter-trace-otlp-http/src/OTLPExporterBase.ts b/packages/exporter-trace-otlp-http/src/OTLPExporterBase.ts index a55fcc4fbf..bd7c025d03 100644 --- a/packages/exporter-trace-otlp-http/src/OTLPExporterBase.ts +++ b/packages/exporter-trace-otlp-http/src/OTLPExporterBase.ts @@ -37,7 +37,7 @@ export abstract class OTLPExporterBase< protected _concurrencyLimit: number; protected _sendingPromises: Promise[] = []; protected _shutdownOnce: BindOnceFuture; - private readonly _timeoutMillis: number; + public readonly _timeoutMillis: number; /** * @param config @@ -97,18 +97,11 @@ export abstract class OTLPExporterBase< private _export(items: ExportItem[]): Promise { return new Promise((resolve, reject) => { - const timer = setTimeout(() => { - // don't wait anymore for export - reject(new Error('Timeout')); - }, this._timeoutMillis); - try { diag.debug('items to be sent', items); this.send(items, resolve, reject); } catch (e) { reject(e); - } finally { - timer.unref(); } }); } diff --git a/packages/exporter-trace-otlp-http/src/platform/browser/OTLPExporterBrowserBase.ts b/packages/exporter-trace-otlp-http/src/platform/browser/OTLPExporterBrowserBase.ts index 708702300b..080cadc5db 100644 --- a/packages/exporter-trace-otlp-http/src/platform/browser/OTLPExporterBrowserBase.ts +++ b/packages/exporter-trace-otlp-http/src/platform/browser/OTLPExporterBrowserBase.ts @@ -78,7 +78,7 @@ export abstract class OTLPExporterBrowserBase< const promise = new Promise((resolve, reject) => { if (this._useXHR) { - sendWithXhr(body, this.url, this._headers, resolve, reject); + sendWithXhr(body, this.url, this._headers, resolve, reject, this._timeoutMillis); } else { sendWithBeacon(body, this.url, { type: 'application/json' }, resolve, reject); } diff --git a/packages/exporter-trace-otlp-http/src/platform/browser/util.ts b/packages/exporter-trace-otlp-http/src/platform/browser/util.ts index 8030552c17..760fa907e2 100644 --- a/packages/exporter-trace-otlp-http/src/platform/browser/util.ts +++ b/packages/exporter-trace-otlp-http/src/platform/browser/util.ts @@ -52,7 +52,8 @@ export function sendWithXhr( url: string, headers: Record, onSuccess: () => void, - onError: (error: otlpTypes.OTLPExporterError) => void + onError: (error: otlpTypes.OTLPExporterError) => void, + exporterTimeout: number ): void { const xhr = new XMLHttpRequest(); xhr.open('POST', url); @@ -62,6 +63,8 @@ export function sendWithXhr( 'Content-Type': 'application/json', }; + xhr.timeout = exporterTimeout; + Object.entries({ ...defaultHeaders, ...headers, @@ -69,6 +72,10 @@ export function sendWithXhr( xhr.setRequestHeader(k, v); }); + xhr.ontimeout = function () { + xhr.abort(); + }; + xhr.send(body); xhr.onreadystatechange = () => { @@ -84,6 +91,12 @@ export function sendWithXhr( onError(error); } + } else if (xhr.readyState === XMLHttpRequest.UNSENT) { + const error = new otlpTypes.OTLPExporterError( + 'Request Timeout', xhr.status + ); + + onError(error); } }; } diff --git a/packages/exporter-trace-otlp-http/src/platform/node/util.ts b/packages/exporter-trace-otlp-http/src/platform/node/util.ts index 0e40b585db..0098121259 100644 --- a/packages/exporter-trace-otlp-http/src/platform/node/util.ts +++ b/packages/exporter-trace-otlp-http/src/platform/node/util.ts @@ -41,6 +41,7 @@ export function sendWithHttp( onSuccess: () => void, onError: (error: otlpTypes.OTLPExporterError) => void ): void { + const exporterTimeout = collector._timeoutMillis; const parsedUrl = new url.URL(collector.url); const options: http.RequestOptions | https.RequestOptions = { @@ -76,9 +77,16 @@ export function sendWithHttp( }); }); + req.setTimeout(exporterTimeout, function requestTimeout () { + req.destroy(); + }); - req.on('error', (error: Error) => { - onError(error); + req.on('error', (error: Error | any) => { + if (error.code === 'ECONNRESET') { + onError(new Error('Request Timeout')); + } else { + onError(error); + } }); switch (collector.compression) { diff --git a/packages/exporter-trace-otlp-http/test/browser/util.test.ts b/packages/exporter-trace-otlp-http/test/browser/util.test.ts index 3cb6aa4474..c03f28f986 100644 --- a/packages/exporter-trace-otlp-http/test/browser/util.test.ts +++ b/packages/exporter-trace-otlp-http/test/browser/util.test.ts @@ -17,6 +17,7 @@ import * as sinon from 'sinon'; import { sendWithXhr } from '../../src/platform/browser/util'; import { ensureHeadersContain } from '../traceHelper'; +import { OTLPTraceExporter } from '../../src/platform/node'; describe('util - browser', () => { let server: any; @@ -51,7 +52,10 @@ describe('util - browser', () => { const explicitContentType = { 'Content-Type': 'application/json', }; - sendWithXhr(body, url, explicitContentType, onSuccessStub, onErrorStub); + // use default exporter timeout + const collectorExporter = new OTLPTraceExporter(); + const exporterTimeout = collectorExporter._timeoutMillis; + sendWithXhr(body, url, explicitContentType, onSuccessStub, onErrorStub, exporterTimeout); }); it('Request Headers should contain "Content-Type" header', done => { @@ -74,7 +78,10 @@ describe('util - browser', () => { describe('and empty headers are set', () => { beforeEach(()=>{ const emptyHeaders = {}; - sendWithXhr(body, url, emptyHeaders, onSuccessStub, onErrorStub); + // use default exporter timeout + const collectorExporter = new OTLPTraceExporter(); + const exporterTimeout = collectorExporter._timeoutMillis; + sendWithXhr(body, url, emptyHeaders, onSuccessStub, onErrorStub, exporterTimeout); }); it('Request Headers should contain "Content-Type" header', done => { @@ -97,7 +104,10 @@ describe('util - browser', () => { let customHeaders: Record; beforeEach(()=>{ customHeaders = { aHeader: 'aValue', bHeader: 'bValue' }; - sendWithXhr(body, url, customHeaders, onSuccessStub, onErrorStub); + // use default exporter timeout + const collectorExporter = new OTLPTraceExporter(); + const exporterTimeout = collectorExporter._timeoutMillis; + sendWithXhr(body, url, customHeaders, onSuccessStub, onErrorStub, exporterTimeout); }); it('Request Headers should contain "Content-Type" header', done => { diff --git a/packages/exporter-trace-otlp-http/test/node/CollectorTraceExporter.test.ts b/packages/exporter-trace-otlp-http/test/node/CollectorTraceExporter.test.ts index 8c39978adc..df74ad7dc6 100644 --- a/packages/exporter-trace-otlp-http/test/node/CollectorTraceExporter.test.ts +++ b/packages/exporter-trace-otlp-http/test/node/CollectorTraceExporter.test.ts @@ -117,6 +117,7 @@ describe('OTLPTraceExporter - node with json over http', () => { describe('export', () => { beforeEach(() => { stubRequest = sinon.stub(http, 'request').returns(fakeRequest as any); + (fakeRequest as any).setTimeout = sinon.spy(); collectorExporterConfig = { headers: { foo: 'bar', @@ -272,6 +273,7 @@ describe('OTLPTraceExporter - node with json over http', () => { describe('export - with compression', () => { beforeEach(() => { stubRequest = sinon.stub(http, 'request').returns(fakeRequest as any); + (fakeRequest as any).setTimeout = sinon.spy(); spySetHeader = sinon.spy(); (fakeRequest as any).setHeader = spySetHeader; collectorExporterConfig = { @@ -341,4 +343,49 @@ describe('OTLPTraceExporter - node with json over http', () => { }); }); }); + + describe('export - with timeout', () => { + beforeEach(() => { + stubRequest = sinon.stub(http, 'request').returns(fakeRequest as any); + (fakeRequest as any).setTimeout = sinon.spy(); + spySetHeader = sinon.spy(); + (fakeRequest as any).setHeader = spySetHeader; + collectorExporterConfig = { + headers: { + foo: 'bar', + }, + hostname: 'foo', + attributes: {}, + url: 'http://foo.bar.com', + keepAlive: true, + compression: CompressionAlgorithm.GZIP, + httpAgentOptions: { keepAliveMsecs: 2000 }, + }; + collectorExporter = new OTLPTraceExporter(collectorExporterConfig); + spans = []; + spans.push(Object.assign({}, mockedReadableSpan)); + }); + it('should log the timeout error message', done => { + const responseSpy = sinon.spy(); + collectorExporter.export(spans, responseSpy); + + const timeoutFunc = (fakeRequest as any).setTimeout.args[0][1]; + timeoutFunc(); + + setTimeout(() => { + const mockResError = new MockedResponse(400); + fakeRequest.emit('error', { code: 'ECONNRESET'}); + mockResError.send('failed'); + + setTimeout(() => { + const result = responseSpy.args[0][0] as core.ExportResult; + assert.strictEqual(result.code, core.ExportResultCode.FAILED); + const error = result.error as otlpTypes.OTLPExporterError; + assert.ok(error !== undefined); + assert.strictEqual(error.message, 'Request Timeout'); + done(); + }); + }); + }); + }); }); From b8543c0ce988b67ada138cf9f6c25a12af51aa8c Mon Sep 17 00:00:00 2001 From: Svetlana Brennan <50715937+svetlanabrennan@users.noreply.github.com> Date: Tue, 8 Feb 2022 16:30:43 -0600 Subject: [PATCH 12/46] Fix typo in environment.ts --- packages/opentelemetry-core/src/utils/environment.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/opentelemetry-core/src/utils/environment.ts b/packages/opentelemetry-core/src/utils/environment.ts index 7b3fa4695c..71152502ce 100644 --- a/packages/opentelemetry-core/src/utils/environment.ts +++ b/packages/opentelemetry-core/src/utils/environment.ts @@ -35,7 +35,7 @@ const ENVIRONMENT_NUMBERS_KEYS = [ 'OTEL_SPAN_EVENT_COUNT_LIMIT', 'OTEL_SPAN_LINK_COUNT_LIMIT', 'OTEL_EXPORTER_OTLP_TIMEOUT', - 'OTEL_EXPORTER_OTLP_TRACES_TIMEOUT' + 'OTEL_EXPORTER_OTLP_TRACES_TIMEOUT', 'OTEL_EXPORTER_JAEGER_AGENT_PORT', ] as const; From 3873a8505b8c683195afd4c59021140094a8f23b Mon Sep 17 00:00:00 2001 From: Svetlana Brennan Date: Mon, 14 Feb 2022 11:24:50 -0600 Subject: [PATCH 13/46] feat(otlp-exporter): update timeout to general timers Signed-off-by: Svetlana Brennan --- .../exporter-trace-otlp-grpc/src/types.ts | 1 + packages/exporter-trace-otlp-grpc/src/util.ts | 2 ++ .../browser/OTLPExporterBrowserBase.ts | 2 +- .../src/platform/browser/util.ts | 31 +++++++++++-------- .../src/platform/node/util.ts | 19 ++++++++---- .../test/browser/util.test.ts | 6 ++-- 6 files changed, 38 insertions(+), 23 deletions(-) diff --git a/packages/exporter-trace-otlp-grpc/src/types.ts b/packages/exporter-trace-otlp-grpc/src/types.ts index e9513819e7..f08107f33d 100644 --- a/packages/exporter-trace-otlp-grpc/src/types.ts +++ b/packages/exporter-trace-otlp-grpc/src/types.ts @@ -34,6 +34,7 @@ export interface ServiceClient extends grpc.Client { export: ( request: any, metadata: grpc.Metadata, + options: object, callback: Function ) => {}; } diff --git a/packages/exporter-trace-otlp-grpc/src/util.ts b/packages/exporter-trace-otlp-grpc/src/util.ts index e0021ce896..b055541fbc 100644 --- a/packages/exporter-trace-otlp-grpc/src/util.ts +++ b/packages/exporter-trace-otlp-grpc/src/util.ts @@ -84,10 +84,12 @@ export function send( ): void { if (collector.serviceClient) { const serviceRequest = collector.convert(objects); + const deadline = new Date().setSeconds(new Date().getSeconds() + (collector._timeoutMillis / 1000)); collector.serviceClient.export( serviceRequest, collector.metadata || new grpc.Metadata(), + {deadline: deadline}, (err: otlpTypes.ExportServiceError) => { if (err) { diag.error('Service request', serviceRequest); diff --git a/packages/exporter-trace-otlp-http/src/platform/browser/OTLPExporterBrowserBase.ts b/packages/exporter-trace-otlp-http/src/platform/browser/OTLPExporterBrowserBase.ts index 080cadc5db..471efb58ff 100644 --- a/packages/exporter-trace-otlp-http/src/platform/browser/OTLPExporterBrowserBase.ts +++ b/packages/exporter-trace-otlp-http/src/platform/browser/OTLPExporterBrowserBase.ts @@ -78,7 +78,7 @@ export abstract class OTLPExporterBrowserBase< const promise = new Promise((resolve, reject) => { if (this._useXHR) { - sendWithXhr(body, this.url, this._headers, resolve, reject, this._timeoutMillis); + sendWithXhr(body, this.url, this._headers, this._timeoutMillis, resolve, reject, ); } else { sendWithBeacon(body, this.url, { type: 'application/json' }, resolve, reject); } diff --git a/packages/exporter-trace-otlp-http/src/platform/browser/util.ts b/packages/exporter-trace-otlp-http/src/platform/browser/util.ts index 760fa907e2..e0c35b5474 100644 --- a/packages/exporter-trace-otlp-http/src/platform/browser/util.ts +++ b/packages/exporter-trace-otlp-http/src/platform/browser/util.ts @@ -51,10 +51,17 @@ export function sendWithXhr( body: string, url: string, headers: Record, + exporterTimeout: number, onSuccess: () => void, onError: (error: otlpTypes.OTLPExporterError) => void, - exporterTimeout: number ): void { + let reqIsDestroyed: boolean; + + const exporterTimer = setTimeout(() => { + reqIsDestroyed = true; + xhr.abort(); + }, exporterTimeout); + const xhr = new XMLHttpRequest(); xhr.open('POST', url); @@ -63,8 +70,6 @@ export function sendWithXhr( 'Content-Type': 'application/json', }; - xhr.timeout = exporterTimeout; - Object.entries({ ...defaultHeaders, ...headers, @@ -72,15 +77,21 @@ export function sendWithXhr( xhr.setRequestHeader(k, v); }); - xhr.ontimeout = function () { - xhr.abort(); - }; + xhr.addEventListener('abort', () => { + if (reqIsDestroyed) { + const error = new otlpTypes.OTLPExporterError( + 'Request Timeout', xhr.status + ); + onError(error); + } + }); xhr.send(body); xhr.onreadystatechange = () => { if (xhr.readyState === XMLHttpRequest.DONE) { if (xhr.status >= 200 && xhr.status <= 299) { + clearTimeout(exporterTimer); diag.debug('xhr success', body); onSuccess(); } else { @@ -88,15 +99,9 @@ export function sendWithXhr( `Failed to export with XHR (status: ${xhr.status})`, xhr.status ); - + clearTimeout(exporterTimer); onError(error); } - } else if (xhr.readyState === XMLHttpRequest.UNSENT) { - const error = new otlpTypes.OTLPExporterError( - 'Request Timeout', xhr.status - ); - - onError(error); } }; } diff --git a/packages/exporter-trace-otlp-http/src/platform/node/util.ts b/packages/exporter-trace-otlp-http/src/platform/node/util.ts index 0098121259..0ca61eeca9 100644 --- a/packages/exporter-trace-otlp-http/src/platform/node/util.ts +++ b/packages/exporter-trace-otlp-http/src/platform/node/util.ts @@ -43,6 +43,12 @@ export function sendWithHttp( ): void { const exporterTimeout = collector._timeoutMillis; const parsedUrl = new url.URL(collector.url); + let reqIsDestroyed: boolean; + + const exporterTimer = setTimeout(() => { + reqIsDestroyed = true; + req.destroy(); + }, exporterTimeout); const options: http.RequestOptions | https.RequestOptions = { hostname: parsedUrl.hostname, @@ -65,6 +71,7 @@ export function sendWithHttp( res.on('end', () => { if (res.statusCode && res.statusCode < 299) { diag.debug(`statusCode: ${res.statusCode}`, responseData); + clearTimeout(exporterTimer); onSuccess(); } else { const error = new otlpTypes.OTLPExporterError( @@ -72,18 +79,18 @@ export function sendWithHttp( res.statusCode, responseData ); + clearTimeout(exporterTimer); onError(error); } }); }); - req.setTimeout(exporterTimeout, function requestTimeout () { - req.destroy(); - }); - req.on('error', (error: Error | any) => { - if (error.code === 'ECONNRESET') { - onError(new Error('Request Timeout')); + if (reqIsDestroyed) { + const err = new otlpTypes.OTLPExporterError( + 'Request Timeout', error.code + ); + onError(err); } else { onError(error); } diff --git a/packages/exporter-trace-otlp-http/test/browser/util.test.ts b/packages/exporter-trace-otlp-http/test/browser/util.test.ts index c03f28f986..1618666488 100644 --- a/packages/exporter-trace-otlp-http/test/browser/util.test.ts +++ b/packages/exporter-trace-otlp-http/test/browser/util.test.ts @@ -55,7 +55,7 @@ describe('util - browser', () => { // use default exporter timeout const collectorExporter = new OTLPTraceExporter(); const exporterTimeout = collectorExporter._timeoutMillis; - sendWithXhr(body, url, explicitContentType, onSuccessStub, onErrorStub, exporterTimeout); + sendWithXhr(body, url, explicitContentType, exporterTimeout, onSuccessStub, onErrorStub); }); it('Request Headers should contain "Content-Type" header', done => { @@ -81,7 +81,7 @@ describe('util - browser', () => { // use default exporter timeout const collectorExporter = new OTLPTraceExporter(); const exporterTimeout = collectorExporter._timeoutMillis; - sendWithXhr(body, url, emptyHeaders, onSuccessStub, onErrorStub, exporterTimeout); + sendWithXhr(body, url, emptyHeaders, exporterTimeout, onSuccessStub, onErrorStub); }); it('Request Headers should contain "Content-Type" header', done => { @@ -107,7 +107,7 @@ describe('util - browser', () => { // use default exporter timeout const collectorExporter = new OTLPTraceExporter(); const exporterTimeout = collectorExporter._timeoutMillis; - sendWithXhr(body, url, customHeaders, onSuccessStub, onErrorStub, exporterTimeout); + sendWithXhr(body, url, customHeaders, exporterTimeout, onSuccessStub, onErrorStub); }); it('Request Headers should contain "Content-Type" header', done => { From 1fe18476feeaf1675452fb19c8de9890e7d764e7 Mon Sep 17 00:00:00 2001 From: Svetlana Brennan Date: Tue, 15 Feb 2022 11:53:24 -0600 Subject: [PATCH 14/46] feat(otlp-exporter): update node http timeout tests Signed-off-by: Svetlana Brennan --- .../test/node/CollectorTraceExporter.test.ts | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/packages/exporter-trace-otlp-http/test/node/CollectorTraceExporter.test.ts b/packages/exporter-trace-otlp-http/test/node/CollectorTraceExporter.test.ts index df74ad7dc6..db0f1728a8 100644 --- a/packages/exporter-trace-otlp-http/test/node/CollectorTraceExporter.test.ts +++ b/packages/exporter-trace-otlp-http/test/node/CollectorTraceExporter.test.ts @@ -117,7 +117,6 @@ describe('OTLPTraceExporter - node with json over http', () => { describe('export', () => { beforeEach(() => { stubRequest = sinon.stub(http, 'request').returns(fakeRequest as any); - (fakeRequest as any).setTimeout = sinon.spy(); collectorExporterConfig = { headers: { foo: 'bar', @@ -273,7 +272,6 @@ describe('OTLPTraceExporter - node with json over http', () => { describe('export - with compression', () => { beforeEach(() => { stubRequest = sinon.stub(http, 'request').returns(fakeRequest as any); - (fakeRequest as any).setTimeout = sinon.spy(); spySetHeader = sinon.spy(); (fakeRequest as any).setHeader = spySetHeader; collectorExporterConfig = { @@ -343,11 +341,9 @@ describe('OTLPTraceExporter - node with json over http', () => { }); }); }); - describe('export - with timeout', () => { beforeEach(() => { stubRequest = sinon.stub(http, 'request').returns(fakeRequest as any); - (fakeRequest as any).setTimeout = sinon.spy(); spySetHeader = sinon.spy(); (fakeRequest as any).setHeader = spySetHeader; collectorExporterConfig = { @@ -365,12 +361,13 @@ describe('OTLPTraceExporter - node with json over http', () => { spans = []; spans.push(Object.assign({}, mockedReadableSpan)); }); - it('should log the timeout error message', done => { + it('should log the timeout request error message', done => { const responseSpy = sinon.spy(); + const clock = sinon.useFakeTimers(); collectorExporter.export(spans, responseSpy); - const timeoutFunc = (fakeRequest as any).setTimeout.args[0][1]; - timeoutFunc(); + clock.tick(10000); + clock.restore(); setTimeout(() => { const mockResError = new MockedResponse(400); From 991beeb4f9c3adbafcc50f50142bd349469293df Mon Sep 17 00:00:00 2001 From: Svetlana Brennan Date: Tue, 15 Feb 2022 13:37:46 -0600 Subject: [PATCH 15/46] feat(otlp-exporter): fix dependency version for http exporter Signed-off-by: Svetlana Brennan --- packages/exporter-trace-otlp-grpc/package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/exporter-trace-otlp-grpc/package.json b/packages/exporter-trace-otlp-grpc/package.json index dba48e1593..d089e1d441 100644 --- a/packages/exporter-trace-otlp-grpc/package.json +++ b/packages/exporter-trace-otlp-grpc/package.json @@ -69,7 +69,7 @@ "@grpc/grpc-js": "^1.3.7", "@grpc/proto-loader": "^0.6.4", "@opentelemetry/core": "1.0.1", - "@opentelemetry/exporter-trace-otlp-http": "0.27.0", + "@opentelemetry/exporter-trace-otlp-http": "1.0.1", "@opentelemetry/resources": "1.0.1", "@opentelemetry/sdk-trace-base": "1.0.1" } From 4e5af76242ba778c635cd44adec3c140dc6004e2 Mon Sep 17 00:00:00 2001 From: Svetlana Brennan Date: Tue, 15 Feb 2022 16:30:51 -0600 Subject: [PATCH 16/46] feat(otlp-exporter): add grpc deadline test Signed-off-by: Svetlana Brennan --- .../test/OTLPTraceExporter.test.ts | 29 +++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/packages/exporter-trace-otlp-grpc/test/OTLPTraceExporter.test.ts b/packages/exporter-trace-otlp-grpc/test/OTLPTraceExporter.test.ts index 2d17bf9f78..7ad27ef857 100644 --- a/packages/exporter-trace-otlp-grpc/test/OTLPTraceExporter.test.ts +++ b/packages/exporter-trace-otlp-grpc/test/OTLPTraceExporter.test.ts @@ -36,6 +36,8 @@ import { mockedReadableSpan, } from './traceHelper'; +import * as core from '@opentelemetry/core'; + const traceServiceProtoPath = 'opentelemetry/proto/collector/trace/v1/trace_service.proto'; const includeDirs = [path.resolve(__dirname, '../protos')]; @@ -197,6 +199,33 @@ const testCollectorExporter = (params: TestParams) => done(); }, 200); }); + it('should log deadline exceeded error', done => { + const credentials = params.useTLS + ? grpc.credentials.createSsl( + fs.readFileSync('./test/certs/ca.crt'), + fs.readFileSync('./test/certs/client.key'), + fs.readFileSync('./test/certs/client.crt') + ) + : undefined; + + const collectorExporterWithTimeout = new OTLPTraceExporter({ + url: 'grpcs://' + address, + credentials, + metadata: params.metadata, + timeoutMillis: 100, + }); + + const responseSpy = sinon.spy(); + const spans = [Object.assign({}, mockedReadableSpan)]; + collectorExporterWithTimeout.export(spans, responseSpy); + + setTimeout(() => { + const result = responseSpy.args[0][0] as core.ExportResult; + assert.strictEqual(result.code, core.ExportResultCode.FAILED); + assert.strictEqual(responseSpy.args[0][0].error.details, 'Deadline exceeded'); + done(); + }, 300); + }); }); }); From b27ca5fb2ace749dd68d8afe9dba5e5b3ba3ba33 Mon Sep 17 00:00:00 2001 From: Svetlana Brennan Date: Tue, 15 Feb 2022 17:52:22 -0600 Subject: [PATCH 17/46] feat(otlp-exporter): fix typos Signed-off-by: Svetlana Brennan --- .../src/platform/browser/OTLPExporterBrowserBase.ts | 2 +- packages/exporter-trace-otlp-http/src/platform/browser/util.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/exporter-trace-otlp-http/src/platform/browser/OTLPExporterBrowserBase.ts b/packages/exporter-trace-otlp-http/src/platform/browser/OTLPExporterBrowserBase.ts index 471efb58ff..8f05816547 100644 --- a/packages/exporter-trace-otlp-http/src/platform/browser/OTLPExporterBrowserBase.ts +++ b/packages/exporter-trace-otlp-http/src/platform/browser/OTLPExporterBrowserBase.ts @@ -78,7 +78,7 @@ export abstract class OTLPExporterBrowserBase< const promise = new Promise((resolve, reject) => { if (this._useXHR) { - sendWithXhr(body, this.url, this._headers, this._timeoutMillis, resolve, reject, ); + sendWithXhr(body, this.url, this._headers, this._timeoutMillis, resolve, reject); } else { sendWithBeacon(body, this.url, { type: 'application/json' }, resolve, reject); } diff --git a/packages/exporter-trace-otlp-http/src/platform/browser/util.ts b/packages/exporter-trace-otlp-http/src/platform/browser/util.ts index e0c35b5474..de646dba2b 100644 --- a/packages/exporter-trace-otlp-http/src/platform/browser/util.ts +++ b/packages/exporter-trace-otlp-http/src/platform/browser/util.ts @@ -53,7 +53,7 @@ export function sendWithXhr( headers: Record, exporterTimeout: number, onSuccess: () => void, - onError: (error: otlpTypes.OTLPExporterError) => void, + onError: (error: otlpTypes.OTLPExporterError) => void ): void { let reqIsDestroyed: boolean; From 21f596e14247a7e263d210b8c2fa06ffed0740e7 Mon Sep 17 00:00:00 2001 From: Svetlana Brennan Date: Fri, 18 Feb 2022 11:48:12 -0600 Subject: [PATCH 18/46] feat(otlp-exporter): add browser timeout test Signed-off-by: Svetlana Brennan --- .../src/platform/browser/util.ts | 14 ++---- packages/exporter-trace-otlp-http/src/util.ts | 4 +- .../browser/CollectorTraceExporter.test.ts | 17 +++++++ .../test/browser/util.test.ts | 2 +- .../test/common/utils.test.ts | 31 ------------ .../test/node/utils.test.ts | 50 +++++++++++++++++++ .../src/utils/environment.ts | 2 + 7 files changed, 77 insertions(+), 43 deletions(-) create mode 100644 packages/exporter-trace-otlp-http/test/node/utils.test.ts diff --git a/packages/exporter-trace-otlp-http/src/platform/browser/util.ts b/packages/exporter-trace-otlp-http/src/platform/browser/util.ts index de646dba2b..c937c61d1b 100644 --- a/packages/exporter-trace-otlp-http/src/platform/browser/util.ts +++ b/packages/exporter-trace-otlp-http/src/platform/browser/util.ts @@ -77,15 +77,6 @@ export function sendWithXhr( xhr.setRequestHeader(k, v); }); - xhr.addEventListener('abort', () => { - if (reqIsDestroyed) { - const error = new otlpTypes.OTLPExporterError( - 'Request Timeout', xhr.status - ); - onError(error); - } - }); - xhr.send(body); xhr.onreadystatechange = () => { @@ -94,6 +85,11 @@ export function sendWithXhr( clearTimeout(exporterTimer); diag.debug('xhr success', body); onSuccess(); + } else if (reqIsDestroyed) { + const error = new otlpTypes.OTLPExporterError( + 'Request Timeout', xhr.status + ); + onError(error); } else { const error = new otlpTypes.OTLPExporterError( `Failed to export with XHR (status: ${xhr.status})`, diff --git a/packages/exporter-trace-otlp-http/src/util.ts b/packages/exporter-trace-otlp-http/src/util.ts index 173259e576..51056fdb71 100644 --- a/packages/exporter-trace-otlp-http/src/util.ts +++ b/packages/exporter-trace-otlp-http/src/util.ts @@ -63,8 +63,8 @@ export function configureExporterTimeout(timeoutMillis: number | undefined): num function getExporterTimeoutFromEnv(): number { const definedTimeout = - Number(process.env.OTEL_EXPORTER_OTLP_TRACES_TIMEOUT || - process.env.OTEL_EXPORTER_OTLP_TIMEOUT); + Number(getEnv().OTEL_EXPORTER_OTLP_TRACES_TIMEOUT || + getEnv().OTEL_EXPORTER_OTLP_TIMEOUT); if (definedTimeout) { if (definedTimeout <= 0) { diff --git a/packages/exporter-trace-otlp-http/test/browser/CollectorTraceExporter.test.ts b/packages/exporter-trace-otlp-http/test/browser/CollectorTraceExporter.test.ts index a66a7c59d0..1233675d8c 100644 --- a/packages/exporter-trace-otlp-http/test/browser/CollectorTraceExporter.test.ts +++ b/packages/exporter-trace-otlp-http/test/browser/CollectorTraceExporter.test.ts @@ -15,6 +15,7 @@ */ import { diag } from '@opentelemetry/api'; +import * as core from '@opentelemetry/core'; import { ExportResultCode } from '@opentelemetry/core'; import { ReadableSpan } from '@opentelemetry/sdk-trace-base'; import * as assert from 'assert'; @@ -284,6 +285,22 @@ describe('OTLPTraceExporter - web', () => { done(); }); }); + it('should log the timeout request error message', done => { + const responseSpy = sinon.spy(); + const clock = sinon.useFakeTimers(); + collectorTraceExporter.export(spans, responseSpy); + clock.tick(10000); + clock.restore(); + + setTimeout(() => { + const result = responseSpy.args[0][0] as core.ExportResult; + assert.strictEqual(result.code, core.ExportResultCode.FAILED); + const error = result.error as otlpTypes.OTLPExporterError; + assert.ok(error !== undefined); + assert.strictEqual(error.message, 'Request Timeout'); + done(); + }); + }); }); }); }); diff --git a/packages/exporter-trace-otlp-http/test/browser/util.test.ts b/packages/exporter-trace-otlp-http/test/browser/util.test.ts index 1618666488..9994d5875e 100644 --- a/packages/exporter-trace-otlp-http/test/browser/util.test.ts +++ b/packages/exporter-trace-otlp-http/test/browser/util.test.ts @@ -17,7 +17,7 @@ import * as sinon from 'sinon'; import { sendWithXhr } from '../../src/platform/browser/util'; import { ensureHeadersContain } from '../traceHelper'; -import { OTLPTraceExporter } from '../../src/platform/node'; +import { OTLPTraceExporter } from '../../src/platform/browser/index'; describe('util - browser', () => { let server: any; diff --git a/packages/exporter-trace-otlp-http/test/common/utils.test.ts b/packages/exporter-trace-otlp-http/test/common/utils.test.ts index 61aec6b5f8..e241495214 100644 --- a/packages/exporter-trace-otlp-http/test/common/utils.test.ts +++ b/packages/exporter-trace-otlp-http/test/common/utils.test.ts @@ -18,7 +18,6 @@ import * as assert from 'assert'; import * as sinon from 'sinon'; import { diag } from '@opentelemetry/api'; import { parseHeaders } from '../../src/util'; -import { configureExporterTimeout } from '../../src/util'; describe('utils', () => { afterEach(() => { @@ -51,34 +50,4 @@ describe('utils', () => { assert.deepStrictEqual(result, {}); }); }); - - describe('configureExporterTimeout', () => { - const envSource = process.env; - it('should use default trace export timeout env variable value when timeoutMillis parameter is undefined', () => { - const exporterTimeout = configureExporterTimeout(undefined); - assert.strictEqual(exporterTimeout, 10000); - }); - it('should use default trace export timeout env variable value when timeoutMillis parameter is negative', () => { - const exporterTimeout = configureExporterTimeout(-18000); - assert.strictEqual(exporterTimeout, 10000); - }); - it('should use trace export timeout value defined in env', () => { - envSource.OTEL_EXPORTER_OTLP_TRACES_TIMEOUT = '15000'; - const exporterTimeout = configureExporterTimeout(undefined); - assert.strictEqual(exporterTimeout, 15000); - delete envSource.OTEL_EXPORTER_OTLP_TRACES_TIMEOUT; - }); - it('should use default trace export timeout env variable value when trace export timeout value defined in env is negative', () => { - envSource.OTEL_EXPORTER_OTLP_TRACES_TIMEOUT = '-15000'; - const exporterTimeout = configureExporterTimeout(undefined); - assert.strictEqual(exporterTimeout, 10000); - delete envSource.OTEL_EXPORTER_OTLP_TRACES_TIMEOUT; - }); - it('should use timeoutMillis parameter over trace export timeout value defined in env', () => { - envSource.OTEL_EXPORTER_OTLP_TRACES_TIMEOUT = '11000'; - const exporterTimeout = configureExporterTimeout(9000); - assert.strictEqual(exporterTimeout, 9000); - delete envSource.OTEL_EXPORTER_OTLP_TRACES_TIMEOUT; - }); - }); }); diff --git a/packages/exporter-trace-otlp-http/test/node/utils.test.ts b/packages/exporter-trace-otlp-http/test/node/utils.test.ts new file mode 100644 index 0000000000..35b0ba8b0b --- /dev/null +++ b/packages/exporter-trace-otlp-http/test/node/utils.test.ts @@ -0,0 +1,50 @@ +/* + * 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 { configureExporterTimeout } from '../../src/util'; + + +describe('configureExporterTimeout', () => { + const envSource = process.env; + it('should use default trace export timeout env variable value when timeoutMillis parameter is undefined', () => { + const exporterTimeout = configureExporterTimeout(undefined); + assert.strictEqual(exporterTimeout, 10000); + }); + it('should use default trace export timeout env variable value when timeoutMillis parameter is negative', () => { + const exporterTimeout = configureExporterTimeout(-18000); + assert.strictEqual(exporterTimeout, 10000); + }); + it('should use trace export timeout value defined in env', () => { + envSource.OTEL_EXPORTER_OTLP_TRACES_TIMEOUT = '15000'; + const exporterTimeout = configureExporterTimeout(undefined); + assert.strictEqual(exporterTimeout, 15000); + delete envSource.OTEL_EXPORTER_OTLP_TRACES_TIMEOUT; + }); + it('should use default trace export timeout env variable value when trace export timeout value defined in env is negative', () => { + envSource.OTEL_EXPORTER_OTLP_TRACES_TIMEOUT = '-15000'; + const exporterTimeout = configureExporterTimeout(undefined); + assert.strictEqual(exporterTimeout, 10000); + delete envSource.OTEL_EXPORTER_OTLP_TRACES_TIMEOUT; + }); + it('should use timeoutMillis parameter over trace export timeout value defined in env', () => { + envSource.OTEL_EXPORTER_OTLP_TRACES_TIMEOUT = '11000'; + const exporterTimeout = configureExporterTimeout(9000); + assert.strictEqual(exporterTimeout, 9000); + delete envSource.OTEL_EXPORTER_OTLP_TRACES_TIMEOUT; + }); +}); + diff --git a/packages/opentelemetry-core/src/utils/environment.ts b/packages/opentelemetry-core/src/utils/environment.ts index 71152502ce..9fcc429b1c 100644 --- a/packages/opentelemetry-core/src/utils/environment.ts +++ b/packages/opentelemetry-core/src/utils/environment.ts @@ -36,6 +36,7 @@ const ENVIRONMENT_NUMBERS_KEYS = [ 'OTEL_SPAN_LINK_COUNT_LIMIT', 'OTEL_EXPORTER_OTLP_TIMEOUT', 'OTEL_EXPORTER_OTLP_TRACES_TIMEOUT', + 'OTEL_EXPORTER_OTLP_METRICS_TIMEOUT', 'OTEL_EXPORTER_JAEGER_AGENT_PORT', ] as const; @@ -124,6 +125,7 @@ export const DEFAULT_ENVIRONMENT: Required = { OTEL_EXPORTER_OTLP_METRICS_HEADERS: '', OTEL_EXPORTER_OTLP_TIMEOUT: 10000, OTEL_EXPORTER_OTLP_TRACES_TIMEOUT: 10000, + OTEL_EXPORTER_OTLP_METRICS_TIMEOUT: 10000, OTEL_EXPORTER_ZIPKIN_ENDPOINT: 'http://localhost:9411/api/v2/spans', OTEL_LOG_LEVEL: DiagLogLevel.INFO, OTEL_NO_PATCH_MODULES: [], From d66cb4a734f7cfa416c445628c1b280fd761950d Mon Sep 17 00:00:00 2001 From: Svetlana Brennan Date: Fri, 18 Feb 2022 13:44:07 -0600 Subject: [PATCH 19/46] feat(otlp-exporter): fix lint errors Signed-off-by: Svetlana Brennan --- packages/exporter-trace-otlp-grpc/README.md | 2 ++ packages/exporter-trace-otlp-http/README.md | 2 ++ packages/exporter-trace-otlp-proto/README.md | 2 ++ 3 files changed, 6 insertions(+) diff --git a/packages/exporter-trace-otlp-grpc/README.md b/packages/exporter-trace-otlp-grpc/README.md index ef075d07bf..e249e16544 100644 --- a/packages/exporter-trace-otlp-grpc/README.md +++ b/packages/exporter-trace-otlp-grpc/README.md @@ -123,11 +123,13 @@ To override the default timeout duration, use the following options: > `OTEL_EXPORTER_OTLP_TRACES_TIMEOUT` takes precedence and overrides `OTEL_EXPORTER_OTLP_TIMEOUT`. + Provide `timeoutMillis` to OTLPTraceExporter with `collectorOptions`: + ```js const collectorOptions = { timeoutMillis: 15000 }; ``` + > Providing `timeoutMillis` with `collectorOptions` takes precedence and overrides timeout set with environment variables. ## Running opentelemetry-collector locally to see the traces diff --git a/packages/exporter-trace-otlp-http/README.md b/packages/exporter-trace-otlp-http/README.md index 668fcfbaac..47a81bc3fe 100644 --- a/packages/exporter-trace-otlp-http/README.md +++ b/packages/exporter-trace-otlp-http/README.md @@ -122,11 +122,13 @@ To override the default timeout duration, use the following options: > `OTEL_EXPORTER_OTLP_TRACES_TIMEOUT` takes precedence and overrides `OTEL_EXPORTER_OTLP_TIMEOUT`. + Provide `timeoutMillis` to OTLPTraceExporter with `collectorOptions`: + ```js const collectorOptions = { timeoutMillis: 15000 }; ``` + > Providing `timeoutMillis` with `collectorOptions` takes precedence and overrides timeout set with environment variables. ## Running opentelemetry-collector locally to see the traces diff --git a/packages/exporter-trace-otlp-proto/README.md b/packages/exporter-trace-otlp-proto/README.md index c86359632b..a3483d869c 100644 --- a/packages/exporter-trace-otlp-proto/README.md +++ b/packages/exporter-trace-otlp-proto/README.md @@ -54,11 +54,13 @@ To override the default timeout duration, use the following options: > `OTEL_EXPORTER_OTLP_TRACES_TIMEOUT` takes precedence and overrides `OTEL_EXPORTER_OTLP_TIMEOUT`. + Provide `timeoutMillis` to OTLPTraceExporter with `collectorOptions`: + ```js const collectorOptions = { timeoutMillis: 15000 }; ``` + > Providing `timeoutMillis` with `collectorOptions` takes precedence and overrides timeout set with environment variables. ## Running opentelemetry-collector locally to see the traces From 10880beae0f79e27b8663ec8eb7f6f82d51d2cf0 Mon Sep 17 00:00:00 2001 From: Svetlana Brennan Date: Fri, 25 Feb 2022 17:13:31 -0600 Subject: [PATCH 20/46] feat(otlp-exporter): fix tests after main merge Signed-off-by: Svetlana Brennan --- .../browser/CollectorTraceExporter.test.ts | 45 ++++++++++++++----- .../test/browser/util.test.ts | 27 ++++++++--- .../test/node/CollectorTraceExporter.test.ts | 11 +++-- .../test/OTLPTraceExporter.test.ts | 13 +++--- 4 files changed, 65 insertions(+), 31 deletions(-) diff --git a/packages/exporter-trace-otlp-http/test/browser/CollectorTraceExporter.test.ts b/packages/exporter-trace-otlp-http/test/browser/CollectorTraceExporter.test.ts index 1233675d8c..9178fa84d6 100644 --- a/packages/exporter-trace-otlp-http/test/browser/CollectorTraceExporter.test.ts +++ b/packages/exporter-trace-otlp-http/test/browser/CollectorTraceExporter.test.ts @@ -23,6 +23,7 @@ import * as sinon from 'sinon'; import { OTLPTraceExporter } from '../../src/platform/browser/index'; import { OTLPExporterConfigBase } from '../../src/types'; import * as otlpTypes from '../../src/types'; +import { nextTick } from 'process'; import { ensureSpanIsCorrect, @@ -133,7 +134,12 @@ describe('OTLPTraceExporter - web', () => { describe('when "sendBeacon" is NOT available', () => { let server: any; + let clock: sinon.SinonFakeTimers; beforeEach(() => { + // fakeTimers is used to replace the next setTimeout which is + // located in sendWithXhr function called by the export method + clock = sinon.useFakeTimers(); + (window.navigator as any).sendBeacon = false; collectorTraceExporter = new OTLPTraceExporter( collectorExporterConfig @@ -147,7 +153,7 @@ describe('OTLPTraceExporter - web', () => { it('should successfully send the spans using XMLHttpRequest', done => { collectorTraceExporter.export(spans, () => { }); - setTimeout(() => { + nextTick(() => { const request = server.requests[0]; assert.strictEqual(request.method, 'POST'); assert.strictEqual(request.url, 'http://foo.bar.com'); @@ -171,9 +177,9 @@ describe('OTLPTraceExporter - web', () => { } assert.strictEqual(stubBeacon.callCount, 0); - ensureExportTraceServiceRequestIsSet(json); + clock.restore(); done(); }); }); @@ -185,15 +191,15 @@ describe('OTLPTraceExporter - web', () => { collectorTraceExporter.export(spans, () => { }); - setTimeout(() => { + nextTick(() => { const request = server.requests[0]; request.respond(200); - const response: any = spyLoggerDebug.args[1][0]; assert.strictEqual(response, 'xhr success'); assert.strictEqual(spyLoggerError.args.length, 0); - assert.strictEqual(stubBeacon.callCount, 0); + + clock.restore(); done(); }); }); @@ -205,20 +211,23 @@ describe('OTLPTraceExporter - web', () => { done(); }); - setTimeout(() => { + nextTick(() => { const request = server.requests[0]; request.respond(400); + clock.restore(); + done(); }); }); it('should send custom headers', done => { collectorTraceExporter.export(spans, () => { }); - setTimeout(() => { + nextTick(() => { const request = server.requests[0]; request.respond(200); assert.strictEqual(stubBeacon.callCount, 0); + clock.restore(); done(); }); }); @@ -244,7 +253,12 @@ describe('OTLPTraceExporter - web', () => { }); describe('when "sendBeacon" is available', () => { + let clock: sinon.SinonFakeTimers; beforeEach(() => { + // fakeTimers is used to replace the next setTimeout which is + // located in sendWithXhr function called by the export method + clock = sinon.useFakeTimers(); + collectorTraceExporter = new OTLPTraceExporter( collectorExporterConfig ); @@ -252,20 +266,26 @@ describe('OTLPTraceExporter - web', () => { it('should successfully send custom headers using XMLHTTPRequest', done => { collectorTraceExporter.export(spans, () => { }); - setTimeout(() => { + nextTick(() => { const [{ requestHeaders }] = server.requests; ensureHeadersContain(requestHeaders, customHeaders); assert.strictEqual(stubBeacon.callCount, 0); assert.strictEqual(stubOpen.callCount, 0); + clock.restore(); done(); }); }); }); describe('when "sendBeacon" is NOT available', () => { + let clock: sinon.SinonFakeTimers; beforeEach(() => { + // fakeTimers is used to replace the next setTimeout which is + // located in sendWithXhr function called by the export method + clock = sinon.useFakeTimers(); + (window.navigator as any).sendBeacon = false; collectorTraceExporter = new OTLPTraceExporter( collectorExporterConfig @@ -275,29 +295,30 @@ describe('OTLPTraceExporter - web', () => { it('should successfully send spans using XMLHttpRequest', done => { collectorTraceExporter.export(spans, () => { }); - setTimeout(() => { + nextTick(() => { const [{ requestHeaders }] = server.requests; ensureHeadersContain(requestHeaders, customHeaders); assert.strictEqual(stubBeacon.callCount, 0); assert.strictEqual(stubOpen.callCount, 0); + clock.restore(); done(); }); }); it('should log the timeout request error message', done => { const responseSpy = sinon.spy(); - const clock = sinon.useFakeTimers(); collectorTraceExporter.export(spans, responseSpy); clock.tick(10000); - clock.restore(); - setTimeout(() => { + nextTick(() => { const result = responseSpy.args[0][0] as core.ExportResult; assert.strictEqual(result.code, core.ExportResultCode.FAILED); const error = result.error as otlpTypes.OTLPExporterError; assert.ok(error !== undefined); assert.strictEqual(error.message, 'Request Timeout'); + + clock.restore(); done(); }); }); diff --git a/packages/exporter-trace-otlp-http/test/browser/util.test.ts b/packages/exporter-trace-otlp-http/test/browser/util.test.ts index 9994d5875e..c17ff4c0b9 100644 --- a/packages/exporter-trace-otlp-http/test/browser/util.test.ts +++ b/packages/exporter-trace-otlp-http/test/browser/util.test.ts @@ -18,6 +18,7 @@ import * as sinon from 'sinon'; import { sendWithXhr } from '../../src/platform/browser/util'; import { ensureHeadersContain } from '../traceHelper'; import { OTLPTraceExporter } from '../../src/platform/browser/index'; +import { nextTick } from 'process'; describe('util - browser', () => { let server: any; @@ -40,7 +41,12 @@ describe('util - browser', () => { describe('when XMLHTTPRequest is used', () => { let expectedHeaders: Record; + let clock: sinon.SinonFakeTimers; beforeEach(()=>{ + // fakeTimers is used to replace the next setTimeout which is + // located in sendWithXhr function called by the export method + clock = sinon.useFakeTimers(); + expectedHeaders = { // ;charset=utf-8 is applied by sinon.fakeServer 'Content-Type': 'application/json;charset=utf-8', @@ -59,17 +65,19 @@ describe('util - browser', () => { }); it('Request Headers should contain "Content-Type" header', done => { - setTimeout(() => { + nextTick(() => { const { requestHeaders } = server.requests[0]; ensureHeadersContain(requestHeaders, expectedHeaders); + clock.restore(); done(); }); }); it('Request Headers should contain "Accept" header', done => { - setTimeout(() => { + nextTick(() => { const { requestHeaders } = server.requests[0]; ensureHeadersContain(requestHeaders, expectedHeaders); + clock.restore(); done(); }); }); @@ -85,17 +93,19 @@ describe('util - browser', () => { }); it('Request Headers should contain "Content-Type" header', done => { - setTimeout(() => { + nextTick(() => { const { requestHeaders } = server.requests[0]; ensureHeadersContain(requestHeaders, expectedHeaders); + clock.restore(); done(); }); }); it('Request Headers should contain "Accept" header', done => { - setTimeout(() => { + nextTick(() => { const { requestHeaders } = server.requests[0]; ensureHeadersContain(requestHeaders, expectedHeaders); + clock.restore(); done(); }); }); @@ -111,25 +121,28 @@ describe('util - browser', () => { }); it('Request Headers should contain "Content-Type" header', done => { - setTimeout(() => { + nextTick(() => { const { requestHeaders } = server.requests[0]; ensureHeadersContain(requestHeaders, expectedHeaders); + clock.restore(); done(); }); }); it('Request Headers should contain "Accept" header', done => { - setTimeout(() => { + nextTick(() => { const { requestHeaders } = server.requests[0]; ensureHeadersContain(requestHeaders, expectedHeaders); + clock.restore(); done(); }); }); it('Request Headers should contain custom headers', done => { - setTimeout(() => { + nextTick(() => { const { requestHeaders } = server.requests[0]; ensureHeadersContain(requestHeaders, customHeaders); + clock.restore(); done(); }); }); diff --git a/packages/exporter-trace-otlp-http/test/node/CollectorTraceExporter.test.ts b/packages/exporter-trace-otlp-http/test/node/CollectorTraceExporter.test.ts index db0f1728a8..b63069e359 100644 --- a/packages/exporter-trace-otlp-http/test/node/CollectorTraceExporter.test.ts +++ b/packages/exporter-trace-otlp-http/test/node/CollectorTraceExporter.test.ts @@ -356,6 +356,7 @@ describe('OTLPTraceExporter - node with json over http', () => { keepAlive: true, compression: CompressionAlgorithm.GZIP, httpAgentOptions: { keepAliveMsecs: 2000 }, + timeoutMillis: 3000, }; collectorExporter = new OTLPTraceExporter(collectorExporterConfig); spans = []; @@ -366,13 +367,12 @@ describe('OTLPTraceExporter - node with json over http', () => { const clock = sinon.useFakeTimers(); collectorExporter.export(spans, responseSpy); - clock.tick(10000); + clock.tick(3000); clock.restore(); - setTimeout(() => { - const mockResError = new MockedResponse(400); - fakeRequest.emit('error', { code: 'ECONNRESET'}); - mockResError.send('failed'); + const mockResError = new MockedResponse(400); + fakeRequest.emit('error', { code: 'ECONNRESET'}); + mockResError.send('failed'); setTimeout(() => { const result = responseSpy.args[0][0] as core.ExportResult; @@ -382,7 +382,6 @@ describe('OTLPTraceExporter - node with json over http', () => { assert.strictEqual(error.message, 'Request Timeout'); done(); }); - }); }); }); }); diff --git a/packages/exporter-trace-otlp-proto/test/OTLPTraceExporter.test.ts b/packages/exporter-trace-otlp-proto/test/OTLPTraceExporter.test.ts index 67b8c17868..b4f5d37b9a 100644 --- a/packages/exporter-trace-otlp-proto/test/OTLPTraceExporter.test.ts +++ b/packages/exporter-trace-otlp-proto/test/OTLPTraceExporter.test.ts @@ -25,7 +25,7 @@ import { ReadableSpan } from '@opentelemetry/sdk-trace-base'; import * as assert from 'assert'; import * as http from 'http'; import * as sinon from 'sinon'; -import { Stream } from 'stream'; +import { Stream, PassThrough } from 'stream'; import * as zlib from 'zlib'; import { OTLPTraceExporter } from '../src'; import { getExportRequestProto } from '../src/util'; @@ -36,17 +36,18 @@ import { MockedResponse, } from './traceHelper'; -const fakeRequest = { - end: function () { }, - on: function () { }, - write: function () { }, -}; +let fakeRequest: PassThrough; describe('OTLPTraceExporter - node with proto over http', () => { let collectorExporter: OTLPTraceExporter; let collectorExporterConfig: OTLPExporterNodeConfigBase; let spans: ReadableSpan[]; + afterEach(() => { + fakeRequest = new Stream.PassThrough(); + sinon.restore(); + }); + describe('when configuring via environment', () => { const envSource = process.env; it('should use url defined in env', () => { From 3e8360d2031b60ea511f4b27a6d6a342f27d0dae Mon Sep 17 00:00:00 2001 From: Svetlana Brennan Date: Fri, 25 Feb 2022 18:19:08 -0600 Subject: [PATCH 21/46] feat(otlp-exporter): fix one test and lint on readme Signed-off-by: Svetlana Brennan --- packages/exporter-trace-otlp-grpc/README.md | 6 +++--- packages/exporter-trace-otlp-http/README.md | 6 +++--- .../test/node/CollectorTraceExporter.test.ts | 17 +++++++++-------- packages/exporter-trace-otlp-proto/README.md | 6 +++--- 4 files changed, 18 insertions(+), 17 deletions(-) diff --git a/packages/exporter-trace-otlp-grpc/README.md b/packages/exporter-trace-otlp-grpc/README.md index e249e16544..ede2ff37f7 100644 --- a/packages/exporter-trace-otlp-grpc/README.md +++ b/packages/exporter-trace-otlp-grpc/README.md @@ -140,9 +140,9 @@ To override the default timeout duration, use the following options: ## Useful links -- For more information on OpenTelemetry, visit: -- For more about OpenTelemetry JavaScript: -- For help or feedback on this project, join us in [GitHub Discussions][discussions-url] ++ For more information on OpenTelemetry, visit: ++ For more about OpenTelemetry JavaScript: ++ For help or feedback on this project, join us in [GitHub Discussions][discussions-url] ## License diff --git a/packages/exporter-trace-otlp-http/README.md b/packages/exporter-trace-otlp-http/README.md index 47a81bc3fe..39d2f40191 100644 --- a/packages/exporter-trace-otlp-http/README.md +++ b/packages/exporter-trace-otlp-http/README.md @@ -139,9 +139,9 @@ To override the default timeout duration, use the following options: ## Useful links -- For more information on OpenTelemetry, visit: -- For more about OpenTelemetry JavaScript: -- For help or feedback on this project, join us in [GitHub Discussions][discussions-url] ++ For more information on OpenTelemetry, visit: ++ For more about OpenTelemetry JavaScript: ++ For help or feedback on this project, join us in [GitHub Discussions][discussions-url] ## License diff --git a/packages/exporter-trace-otlp-http/test/node/CollectorTraceExporter.test.ts b/packages/exporter-trace-otlp-http/test/node/CollectorTraceExporter.test.ts index b63069e359..d622d78e83 100644 --- a/packages/exporter-trace-otlp-http/test/node/CollectorTraceExporter.test.ts +++ b/packages/exporter-trace-otlp-http/test/node/CollectorTraceExporter.test.ts @@ -343,6 +343,7 @@ describe('OTLPTraceExporter - node with json over http', () => { }); describe('export - with timeout', () => { beforeEach(() => { + fakeRequest = new Stream.PassThrough(); stubRequest = sinon.stub(http, 'request').returns(fakeRequest as any); spySetHeader = sinon.spy(); (fakeRequest as any).setHeader = spySetHeader; @@ -356,7 +357,7 @@ describe('OTLPTraceExporter - node with json over http', () => { keepAlive: true, compression: CompressionAlgorithm.GZIP, httpAgentOptions: { keepAliveMsecs: 2000 }, - timeoutMillis: 3000, + timeoutMillis: 100, }; collectorExporter = new OTLPTraceExporter(collectorExporterConfig); spans = []; @@ -364,15 +365,13 @@ describe('OTLPTraceExporter - node with json over http', () => { }); it('should log the timeout request error message', done => { const responseSpy = sinon.spy(); - const clock = sinon.useFakeTimers(); - collectorExporter.export(spans, responseSpy); - clock.tick(3000); - clock.restore(); + collectorExporter.export(spans, responseSpy); - const mockResError = new MockedResponse(400); - fakeRequest.emit('error', { code: 'ECONNRESET'}); - mockResError.send('failed'); + setTimeout(() => { + const mockResError = new MockedResponse(400); + fakeRequest.emit('error', { code: 'ECONNRESET'}); + mockResError.send('failed'); setTimeout(() => { const result = responseSpy.args[0][0] as core.ExportResult; @@ -380,8 +379,10 @@ describe('OTLPTraceExporter - node with json over http', () => { const error = result.error as otlpTypes.OTLPExporterError; assert.ok(error !== undefined); assert.strictEqual(error.message, 'Request Timeout'); + done(); }); + }, 100); }); }); }); diff --git a/packages/exporter-trace-otlp-proto/README.md b/packages/exporter-trace-otlp-proto/README.md index a3483d869c..6725c1deca 100644 --- a/packages/exporter-trace-otlp-proto/README.md +++ b/packages/exporter-trace-otlp-proto/README.md @@ -71,9 +71,9 @@ To override the default timeout duration, use the following options: ## Useful links -- For more information on OpenTelemetry, visit: -- For more about OpenTelemetry JavaScript: -- For help or feedback on this project, join us in [GitHub Discussions][discussions-url] ++ For more information on OpenTelemetry, visit: ++ For more about OpenTelemetry JavaScript: ++ For help or feedback on this project, join us in [GitHub Discussions][discussions-url] ## License From 481795db2c18f78b6217692e718aedbd23b2b0e7 Mon Sep 17 00:00:00 2001 From: Svetlana Brennan Date: Tue, 1 Mar 2022 13:14:10 -0600 Subject: [PATCH 22/46] feat(otlp-exporter): fix tests that call export method Signed-off-by: Svetlana Brennan --- .../src/platform/node/util.ts | 1 + .../test/node/CollectorTraceExporter.test.ts | 68 ++++++++++++++++--- .../test/OTLPTraceExporter.test.ts | 24 ++++++- 3 files changed, 81 insertions(+), 12 deletions(-) diff --git a/packages/exporter-trace-otlp-http/src/platform/node/util.ts b/packages/exporter-trace-otlp-http/src/platform/node/util.ts index 0ca61eeca9..39ced8ff9e 100644 --- a/packages/exporter-trace-otlp-http/src/platform/node/util.ts +++ b/packages/exporter-trace-otlp-http/src/platform/node/util.ts @@ -92,6 +92,7 @@ export function sendWithHttp( ); onError(err); } else { + clearTimeout(exporterTimer); onError(error); } }); diff --git a/packages/exporter-trace-otlp-http/test/node/CollectorTraceExporter.test.ts b/packages/exporter-trace-otlp-http/test/node/CollectorTraceExporter.test.ts index d622d78e83..b742d6e0c2 100644 --- a/packages/exporter-trace-otlp-http/test/node/CollectorTraceExporter.test.ts +++ b/packages/exporter-trace-otlp-http/test/node/CollectorTraceExporter.test.ts @@ -35,6 +35,7 @@ import { ensureSpanIsCorrect, mockedReadableSpan, } from '../traceHelper'; +import { nextTick } from 'process'; let fakeRequest: PassThrough; @@ -136,7 +137,11 @@ describe('OTLPTraceExporter - node with json over http', () => { collectorExporter.export(spans, () => { }); setTimeout(() => { + const mockRes = new MockedResponse(200); const args = stubRequest.args[0]; + const callback = args[1]; + callback(mockRes); + mockRes.send('success'); const options = args[0]; assert.strictEqual(options.hostname, 'foo.bar.com'); @@ -150,7 +155,12 @@ describe('OTLPTraceExporter - node with json over http', () => { collectorExporter.export(spans, () => { }); setTimeout(() => { + const mockRes = new MockedResponse(200); const args = stubRequest.args[0]; + const callback = args[1]; + callback(mockRes); + mockRes.send('success'); + const options = args[0]; assert.strictEqual(options.headers['foo'], 'bar'); done(); @@ -161,7 +171,12 @@ describe('OTLPTraceExporter - node with json over http', () => { collectorExporter.export(spans, () => { }); setTimeout(() => { + const mockRes = new MockedResponse(200); const args = stubRequest.args[0]; + const callback = args[1]; + callback(mockRes); + mockRes.send('success'); + const options = args[0]; assert.strictEqual(options.headers['Content-Encoding'], undefined); done(); @@ -172,7 +187,12 @@ describe('OTLPTraceExporter - node with json over http', () => { collectorExporter.export(spans, () => { }); setTimeout(() => { + const mockRes = new MockedResponse(200); const args = stubRequest.args[0]; + const callback = args[1]; + callback(mockRes); + mockRes.send('success'); + const options = args[0]; const agent = options.agent; assert.strictEqual(agent.keepAlive, true); @@ -182,14 +202,34 @@ describe('OTLPTraceExporter - node with json over http', () => { }); it('different http export requests should use the same agent', done => { - collectorExporter.export(spans, () => { }); + const clock = sinon.useFakeTimers(); collectorExporter.export(spans, () => { }); - setTimeout(() => { + const mockRes = new MockedResponse(200); + const args = stubRequest.args[0]; + const callback = args[1]; + + callback(mockRes); + mockRes.send('success'); + clock.restore(); + + nextTick(() => { + const clock = sinon.useFakeTimers(); + collectorExporter.export(spans, () => { }); + + const mockRes2 = new MockedResponse(200); + const args2 = stubRequest.args[1]; + const callback2 = args2[1]; + + callback2(mockRes); + mockRes2.send('success'); + const [firstExportAgent, secondExportAgent] = stubRequest.args.map( a => a[0].agent ); + assert.strictEqual(firstExportAgent, secondExportAgent); + clock.restore(); done(); }); }); @@ -220,6 +260,13 @@ describe('OTLPTraceExporter - node with json over http', () => { }); collectorExporter.export(spans, () => { }); + + const mockRes = new MockedResponse(200); + const args = stubRequest.args[0]; + const callback = args[1]; + + callback(mockRes); + mockRes.send('success'); }); it('should log the successful message', done => { @@ -256,6 +303,7 @@ describe('OTLPTraceExporter - node with json over http', () => { const callback = args[1]; callback(mockResError); mockResError.send('failed'); + setTimeout(() => { const result = responseSpy.args[0][0] as core.ExportResult; assert.strictEqual(result.code, core.ExportResultCode.FAILED); @@ -291,7 +339,6 @@ describe('OTLPTraceExporter - node with json over http', () => { }); it('should successfully send the spans', done => { - collectorExporter.export(spans, () => { }); let buff = Buffer.from(''); fakeRequest.on('end', () => { @@ -309,15 +356,22 @@ describe('OTLPTraceExporter - node with json over http', () => { ensureExportTraceServiceRequestIsSet(json); assert.ok(spySetHeader.calledWith('Content-Encoding', 'gzip')); - done(); }); fakeRequest.on('data', chunk => { buff = Buffer.concat([buff, chunk]); }); - }); + collectorExporter.export(spans,() => { }); + + const mockRes = new MockedResponse(200); + const args = stubRequest.args[0]; + const callback = args[1]; + + callback(mockRes); + mockRes.send('success'); + }); }); describe('OTLPTraceExporter - node (getDefaultUrl)', () => { @@ -355,7 +409,6 @@ describe('OTLPTraceExporter - node with json over http', () => { attributes: {}, url: 'http://foo.bar.com', keepAlive: true, - compression: CompressionAlgorithm.GZIP, httpAgentOptions: { keepAliveMsecs: 2000 }, timeoutMillis: 100, }; @@ -365,13 +418,10 @@ describe('OTLPTraceExporter - node with json over http', () => { }); it('should log the timeout request error message', done => { const responseSpy = sinon.spy(); - collectorExporter.export(spans, responseSpy); setTimeout(() => { - const mockResError = new MockedResponse(400); fakeRequest.emit('error', { code: 'ECONNRESET'}); - mockResError.send('failed'); setTimeout(() => { const result = responseSpy.args[0][0] as core.ExportResult; diff --git a/packages/exporter-trace-otlp-proto/test/OTLPTraceExporter.test.ts b/packages/exporter-trace-otlp-proto/test/OTLPTraceExporter.test.ts index b4f5d37b9a..9d58be3acd 100644 --- a/packages/exporter-trace-otlp-proto/test/OTLPTraceExporter.test.ts +++ b/packages/exporter-trace-otlp-proto/test/OTLPTraceExporter.test.ts @@ -119,10 +119,14 @@ describe('OTLPTraceExporter - node with proto over http', () => { it('should open the connection', done => { collectorExporter.export(spans, () => { }); - sinon.stub(http, 'request').callsFake((options: any) => { + sinon.stub(http, 'request').callsFake((options: any, cb: any) => { assert.strictEqual(options.hostname, 'foo.bar.com'); assert.strictEqual(options.method, 'POST'); assert.strictEqual(options.path, '/'); + + const mockRes = new MockedResponse(200); + cb(mockRes); + mockRes.send('success'); done(); return fakeRequest as any; }); @@ -131,8 +135,12 @@ describe('OTLPTraceExporter - node with proto over http', () => { it('should set custom headers', done => { collectorExporter.export(spans, () => { }); - sinon.stub(http, 'request').callsFake((options: any) => { + sinon.stub(http, 'request').callsFake((options: any, cb: any) => { assert.strictEqual(options.headers['foo'], 'bar'); + + const mockRes = new MockedResponse(200); + cb(mockRes); + mockRes.send('success'); done(); return fakeRequest as any; }); @@ -141,9 +149,13 @@ describe('OTLPTraceExporter - node with proto over http', () => { it('should have keep alive and keepAliveMsecs option set', done => { collectorExporter.export(spans, () => { }); - sinon.stub(http, 'request').callsFake((options: any) => { + sinon.stub(http, 'request').callsFake((options: any, cb: any) => { assert.strictEqual(options.agent.keepAlive, true); assert.strictEqual(options.agent.options.keepAliveMsecs, 2000); + + const mockRes = new MockedResponse(200); + cb(mockRes); + mockRes.send('success'); done(); return fakeRequest as any; }); @@ -174,7 +186,10 @@ describe('OTLPTraceExporter - node with proto over http', () => { buff = Buffer.concat([buff, chunk]); }); + const clock = sinon.useFakeTimers(); collectorExporter.export(spans, () => { }); + clock.tick(200); + clock.restore(); }); it('should log the successful message', done => { @@ -262,7 +277,10 @@ describe('OTLPTraceExporter - node with proto over http', () => { buff = Buffer.concat([buff, chunk]); }); + const clock = sinon.useFakeTimers(); collectorExporter.export(spans, () => { }); + clock.tick(200); + clock.restore(); }); }); }); From d9a665f374a53035c93719255d281d34b0882f5b Mon Sep 17 00:00:00 2001 From: Svetlana Brennan Date: Wed, 2 Mar 2022 14:01:32 -0600 Subject: [PATCH 23/46] feat(otlp-exporter): update various items based on feedback Signed-off-by: Svetlana Brennan --- packages/exporter-trace-otlp-grpc/README.md | 7 ++++++- packages/exporter-trace-otlp-grpc/src/types.ts | 2 +- packages/exporter-trace-otlp-grpc/src/util.ts | 2 +- .../test/OTLPTraceExporter.test.ts | 2 +- packages/exporter-trace-otlp-http/README.md | 9 ++++++++- .../exporter-trace-otlp-http/src/OTLPExporterBase.ts | 4 ++-- .../src/platform/browser/OTLPExporterBrowserBase.ts | 2 +- .../exporter-trace-otlp-http/src/platform/node/util.ts | 5 ++--- packages/exporter-trace-otlp-http/src/util.ts | 6 +++--- .../test/browser/CollectorTraceExporter.test.ts | 2 +- .../exporter-trace-otlp-http/test/browser/util.test.ts | 6 +++--- .../test/node/CollectorTraceExporter.test.ts | 2 +- packages/exporter-trace-otlp-proto/README.md | 8 +++++++- 13 files changed, 37 insertions(+), 20 deletions(-) diff --git a/packages/exporter-trace-otlp-grpc/README.md b/packages/exporter-trace-otlp-grpc/README.md index ede2ff37f7..a32c1f3ad5 100644 --- a/packages/exporter-trace-otlp-grpc/README.md +++ b/packages/exporter-trace-otlp-grpc/README.md @@ -126,8 +126,13 @@ To override the default timeout duration, use the following options: ```js const collectorOptions = { - timeoutMillis: 15000 + timeoutMillis: 15000, + // url is optional and can be omitted - default is localhost:4317 + url: ':', + metadata, // // an optional grpc.Metadata object to be sent with each request }; + + const exporter = new OTLPTraceExporter(collectorOptions); ``` > Providing `timeoutMillis` with `collectorOptions` takes precedence and overrides timeout set with environment variables. diff --git a/packages/exporter-trace-otlp-grpc/src/types.ts b/packages/exporter-trace-otlp-grpc/src/types.ts index f08107f33d..288dafc449 100644 --- a/packages/exporter-trace-otlp-grpc/src/types.ts +++ b/packages/exporter-trace-otlp-grpc/src/types.ts @@ -34,7 +34,7 @@ export interface ServiceClient extends grpc.Client { export: ( request: any, metadata: grpc.Metadata, - options: object, + options: grpc.CallOptions, callback: Function ) => {}; } diff --git a/packages/exporter-trace-otlp-grpc/src/util.ts b/packages/exporter-trace-otlp-grpc/src/util.ts index b055541fbc..d1585eb826 100644 --- a/packages/exporter-trace-otlp-grpc/src/util.ts +++ b/packages/exporter-trace-otlp-grpc/src/util.ts @@ -84,7 +84,7 @@ export function send( ): void { if (collector.serviceClient) { const serviceRequest = collector.convert(objects); - const deadline = new Date().setSeconds(new Date().getSeconds() + (collector._timeoutMillis / 1000)); + const deadline = new Date().setSeconds(new Date().getSeconds() + (collector.timeoutMillis / 1000)); collector.serviceClient.export( serviceRequest, diff --git a/packages/exporter-trace-otlp-grpc/test/OTLPTraceExporter.test.ts b/packages/exporter-trace-otlp-grpc/test/OTLPTraceExporter.test.ts index 7ad27ef857..f61bbcab0b 100644 --- a/packages/exporter-trace-otlp-grpc/test/OTLPTraceExporter.test.ts +++ b/packages/exporter-trace-otlp-grpc/test/OTLPTraceExporter.test.ts @@ -197,7 +197,7 @@ const testCollectorExporter = (params: TestParams) => ensureMetadataIsCorrect(reqMetadata, params.metadata); } done(); - }, 200); + }, 500); }); it('should log deadline exceeded error', done => { const credentials = params.useTLS diff --git a/packages/exporter-trace-otlp-http/README.md b/packages/exporter-trace-otlp-http/README.md index 39d2f40191..777e3658e2 100644 --- a/packages/exporter-trace-otlp-http/README.md +++ b/packages/exporter-trace-otlp-http/README.md @@ -125,8 +125,15 @@ To override the default timeout duration, use the following options: ```js const collectorOptions = { - timeoutMillis: 15000 + timeoutMillis: 15000, + url: '', // url is optional and can be omitted - default is http://localhost:4318/v1/traces + headers: { + foo: 'bar' + }, // an optional object containing custom headers to be sent with each request will only work with http + concurrencyLimit: 10, // an optional limit on pending requests }; + + const exporter = new OTLPTraceExporter(collectorOptions); ``` > Providing `timeoutMillis` with `collectorOptions` takes precedence and overrides timeout set with environment variables. diff --git a/packages/exporter-trace-otlp-http/src/OTLPExporterBase.ts b/packages/exporter-trace-otlp-http/src/OTLPExporterBase.ts index 3e07141d53..a3e96019bc 100644 --- a/packages/exporter-trace-otlp-http/src/OTLPExporterBase.ts +++ b/packages/exporter-trace-otlp-http/src/OTLPExporterBase.ts @@ -34,10 +34,10 @@ export abstract class OTLPExporterBase< public readonly url: string; public readonly hostname: string | undefined; public readonly attributes?: SpanAttributes; + public readonly timeoutMillis: number; protected _concurrencyLimit: number; protected _sendingPromises: Promise[] = []; protected _shutdownOnce: BindOnceFuture; - public readonly _timeoutMillis: number; /** * @param config @@ -58,7 +58,7 @@ export abstract class OTLPExporterBase< ? config.concurrencyLimit : Infinity; - this._timeoutMillis = configureExporterTimeout(config.timeoutMillis); + this.timeoutMillis = configureExporterTimeout(config.timeoutMillis); // platform dependent this.onInit(config); diff --git a/packages/exporter-trace-otlp-http/src/platform/browser/OTLPExporterBrowserBase.ts b/packages/exporter-trace-otlp-http/src/platform/browser/OTLPExporterBrowserBase.ts index 8f05816547..46948167b2 100644 --- a/packages/exporter-trace-otlp-http/src/platform/browser/OTLPExporterBrowserBase.ts +++ b/packages/exporter-trace-otlp-http/src/platform/browser/OTLPExporterBrowserBase.ts @@ -78,7 +78,7 @@ export abstract class OTLPExporterBrowserBase< const promise = new Promise((resolve, reject) => { if (this._useXHR) { - sendWithXhr(body, this.url, this._headers, this._timeoutMillis, resolve, reject); + sendWithXhr(body, this.url, this._headers, this.timeoutMillis, resolve, reject); } else { sendWithBeacon(body, this.url, { type: 'application/json' }, resolve, reject); } diff --git a/packages/exporter-trace-otlp-http/src/platform/node/util.ts b/packages/exporter-trace-otlp-http/src/platform/node/util.ts index d287104d9c..134688e0da 100644 --- a/packages/exporter-trace-otlp-http/src/platform/node/util.ts +++ b/packages/exporter-trace-otlp-http/src/platform/node/util.ts @@ -42,7 +42,7 @@ export function sendWithHttp( onSuccess: () => void, onError: (error: otlpTypes.OTLPExporterError) => void ): void { - const exporterTimeout = collector._timeoutMillis; + const exporterTimeout = collector.timeoutMillis; const parsedUrl = new url.URL(collector.url); let reqIsDestroyed: boolean; @@ -72,7 +72,6 @@ export function sendWithHttp( res.on('end', () => { if (res.statusCode && res.statusCode < 299) { diag.debug(`statusCode: ${res.statusCode}`, responseData); - clearTimeout(exporterTimer); onSuccess(); } else { const error = new otlpTypes.OTLPExporterError( @@ -80,9 +79,9 @@ export function sendWithHttp( res.statusCode, responseData ); - clearTimeout(exporterTimer); onError(error); } + clearTimeout(exporterTimer); }); }); diff --git a/packages/exporter-trace-otlp-http/src/util.ts b/packages/exporter-trace-otlp-http/src/util.ts index 51056fdb71..dd12a8c05b 100644 --- a/packages/exporter-trace-otlp-http/src/util.ts +++ b/packages/exporter-trace-otlp-http/src/util.ts @@ -63,10 +63,10 @@ export function configureExporterTimeout(timeoutMillis: number | undefined): num function getExporterTimeoutFromEnv(): number { const definedTimeout = - Number(getEnv().OTEL_EXPORTER_OTLP_TRACES_TIMEOUT || + Number(getEnv().OTEL_EXPORTER_OTLP_TRACES_TIMEOUT ?? getEnv().OTEL_EXPORTER_OTLP_TIMEOUT); - if (definedTimeout) { + if (definedTimeout !== undefined) { if (definedTimeout <= 0) { // OTLP exporter configured timeout - using default value of 10000ms return invalidTimeout(definedTimeout, DEFAULT_TRACE_TIMEOUT); @@ -79,7 +79,7 @@ function getExporterTimeoutFromEnv(): number { // OTLP exporter configured timeout - using default value of 10000ms function invalidTimeout(timeout: number, defaultTimeout: number): number { - diag.warn('Timeout must be non-negative', timeout); + diag.warn('Timeout must be greater than 0', timeout); return defaultTimeout; } diff --git a/packages/exporter-trace-otlp-http/test/browser/CollectorTraceExporter.test.ts b/packages/exporter-trace-otlp-http/test/browser/CollectorTraceExporter.test.ts index 9178fa84d6..a0f57f7709 100644 --- a/packages/exporter-trace-otlp-http/test/browser/CollectorTraceExporter.test.ts +++ b/packages/exporter-trace-otlp-http/test/browser/CollectorTraceExporter.test.ts @@ -255,7 +255,7 @@ describe('OTLPTraceExporter - web', () => { describe('when "sendBeacon" is available', () => { let clock: sinon.SinonFakeTimers; beforeEach(() => { - // fakeTimers is used to replace the next setTimeout which is + // fakeTimers is used to replace the next setTimeout which is // located in sendWithXhr function called by the export method clock = sinon.useFakeTimers(); diff --git a/packages/exporter-trace-otlp-http/test/browser/util.test.ts b/packages/exporter-trace-otlp-http/test/browser/util.test.ts index c17ff4c0b9..351ab4db23 100644 --- a/packages/exporter-trace-otlp-http/test/browser/util.test.ts +++ b/packages/exporter-trace-otlp-http/test/browser/util.test.ts @@ -60,7 +60,7 @@ describe('util - browser', () => { }; // use default exporter timeout const collectorExporter = new OTLPTraceExporter(); - const exporterTimeout = collectorExporter._timeoutMillis; + const exporterTimeout = collectorExporter.timeoutMillis; sendWithXhr(body, url, explicitContentType, exporterTimeout, onSuccessStub, onErrorStub); }); it('Request Headers should contain "Content-Type" header', done => { @@ -88,7 +88,7 @@ describe('util - browser', () => { const emptyHeaders = {}; // use default exporter timeout const collectorExporter = new OTLPTraceExporter(); - const exporterTimeout = collectorExporter._timeoutMillis; + const exporterTimeout = collectorExporter.timeoutMillis; sendWithXhr(body, url, emptyHeaders, exporterTimeout, onSuccessStub, onErrorStub); }); it('Request Headers should contain "Content-Type" header', done => { @@ -116,7 +116,7 @@ describe('util - browser', () => { customHeaders = { aHeader: 'aValue', bHeader: 'bValue' }; // use default exporter timeout const collectorExporter = new OTLPTraceExporter(); - const exporterTimeout = collectorExporter._timeoutMillis; + const exporterTimeout = collectorExporter.timeoutMillis; sendWithXhr(body, url, customHeaders, exporterTimeout, onSuccessStub, onErrorStub); }); it('Request Headers should contain "Content-Type" header', done => { diff --git a/packages/exporter-trace-otlp-http/test/node/CollectorTraceExporter.test.ts b/packages/exporter-trace-otlp-http/test/node/CollectorTraceExporter.test.ts index 01cad639a4..465589726d 100644 --- a/packages/exporter-trace-otlp-http/test/node/CollectorTraceExporter.test.ts +++ b/packages/exporter-trace-otlp-http/test/node/CollectorTraceExporter.test.ts @@ -446,7 +446,7 @@ describe('OTLPTraceExporter - node with json over http', () => { done(); }); - }, 100); + }, 300); }); }); }); diff --git a/packages/exporter-trace-otlp-proto/README.md b/packages/exporter-trace-otlp-proto/README.md index 6725c1deca..094b03ca47 100644 --- a/packages/exporter-trace-otlp-proto/README.md +++ b/packages/exporter-trace-otlp-proto/README.md @@ -57,8 +57,14 @@ To override the default timeout duration, use the following options: ```js const collectorOptions = { - timeoutMillis: 15000 + timeoutMillis: 15000, + url: '', // url is optional and can be omitted - default is http://localhost:4318/v1/traces + headers: { + foo: 'bar' + }, //an optional object containing custom headers to be sent with each request will only work with http }; + + const exporter = new OTLPTraceExporter(collectorOptions); ``` > Providing `timeoutMillis` with `collectorOptions` takes precedence and overrides timeout set with environment variables. From 4222cc6d375eef969efc63473046ff45369f9633 Mon Sep 17 00:00:00 2001 From: Svetlana Brennan Date: Wed, 2 Mar 2022 14:44:26 -0600 Subject: [PATCH 24/46] feat(otlp-exporter): add additional tests Signed-off-by: Svetlana Brennan --- packages/exporter-trace-otlp-http/src/util.ts | 13 +++---- .../test/node/utils.test.ts | 39 ++++++++++++++++++- 2 files changed, 43 insertions(+), 9 deletions(-) diff --git a/packages/exporter-trace-otlp-http/src/util.ts b/packages/exporter-trace-otlp-http/src/util.ts index dd12a8c05b..47629d9a8e 100644 --- a/packages/exporter-trace-otlp-http/src/util.ts +++ b/packages/exporter-trace-otlp-http/src/util.ts @@ -66,19 +66,16 @@ function getExporterTimeoutFromEnv(): number { Number(getEnv().OTEL_EXPORTER_OTLP_TRACES_TIMEOUT ?? getEnv().OTEL_EXPORTER_OTLP_TIMEOUT); - if (definedTimeout !== undefined) { - if (definedTimeout <= 0) { - // OTLP exporter configured timeout - using default value of 10000ms - return invalidTimeout(definedTimeout, DEFAULT_TRACE_TIMEOUT); - } - return definedTimeout; + if (definedTimeout <= 0) { + // OTLP exporter configured timeout - using default value of 10000ms + return invalidTimeout(definedTimeout, DEFAULT_TRACE_TIMEOUT); } else { - return getEnv().OTEL_EXPORTER_OTLP_TRACES_TIMEOUT; + return definedTimeout; } } // OTLP exporter configured timeout - using default value of 10000ms -function invalidTimeout(timeout: number, defaultTimeout: number): number { +export function invalidTimeout(timeout: number, defaultTimeout: number): number { diag.warn('Timeout must be greater than 0', timeout); return defaultTimeout; diff --git a/packages/exporter-trace-otlp-http/test/node/utils.test.ts b/packages/exporter-trace-otlp-http/test/node/utils.test.ts index 6a7f802834..a4fed1543c 100644 --- a/packages/exporter-trace-otlp-http/test/node/utils.test.ts +++ b/packages/exporter-trace-otlp-http/test/node/utils.test.ts @@ -15,12 +15,18 @@ */ import * as assert from 'assert'; -import { configureExporterTimeout } from '../../src/util'; +import { configureExporterTimeout, invalidTimeout } from '../../src/util'; import { CompressionAlgorithm} from '../../src/platform/node/types'; import { configureCompression} from '../../src/platform/node/util'; +import { diag } from '@opentelemetry/api'; +import * as sinon from 'sinon'; describe('configureExporterTimeout', () => { const envSource = process.env; + it('should use timeoutMillis parameter as export timeout value', () => { + const exporterTimeout = configureExporterTimeout(9000); + assert.strictEqual(exporterTimeout, 9000); + }); it('should use default trace export timeout env variable value when timeoutMillis parameter is undefined', () => { const exporterTimeout = configureExporterTimeout(undefined); assert.strictEqual(exporterTimeout, 10000); @@ -41,12 +47,43 @@ describe('configureExporterTimeout', () => { assert.strictEqual(exporterTimeout, 10000); delete envSource.OTEL_EXPORTER_OTLP_TRACES_TIMEOUT; }); + it('should use default trace export timeout when timeoutMillis parameter is negative', () => { + const exporterTimeout = configureExporterTimeout(-15000); + assert.strictEqual(exporterTimeout, 10000); + }); it('should use timeoutMillis parameter over trace export timeout value defined in env', () => { envSource.OTEL_EXPORTER_OTLP_TRACES_TIMEOUT = '11000'; const exporterTimeout = configureExporterTimeout(9000); assert.strictEqual(exporterTimeout, 9000); delete envSource.OTEL_EXPORTER_OTLP_TRACES_TIMEOUT; }); + it('should use default value when both timeoutMillis parameter export timeout value defined in env are negative', () => { + envSource.OTEL_EXPORTER_OTLP_TRACES_TIMEOUT = '-11000'; + envSource.OTEL_EXPORTER_OTLP_TIMEOUT = '-9000'; + const exporterTimeout = configureExporterTimeout(-5000); + assert.strictEqual(exporterTimeout, 10000); + delete envSource.OTEL_EXPORTER_OTLP_TRACES_TIMEOUT; + delete envSource.OTEL_EXPORTER_OTLP_TIMEOUT; + }); + it('should warn user about invalid timeout', () => { + const spyLoggerWarn = sinon.stub(diag, 'warn'); + configureExporterTimeout(-15000); + const args = spyLoggerWarn.args[0]; + assert.strictEqual(args[0], 'Timeout must be greater than 0'); + assert.strictEqual(args[1], -15000); + sinon.restore(); + }); +}); + +describe('invalidTimeout', () => { + it('should warn user about invalid timeout', () => { + const spyLoggerWarn = sinon.stub(diag, 'warn'); + invalidTimeout(-9000, 10000); + const args = spyLoggerWarn.args[0]; + assert.strictEqual(args[0], 'Timeout must be greater than 0'); + assert.strictEqual(args[1], -9000); + sinon.restore(); + }); }); describe('configureCompression', () => { From 0d9de49f9b2544d6cf57ac4e7b46e1743869677a Mon Sep 17 00:00:00 2001 From: Svetlana Brennan Date: Wed, 2 Mar 2022 15:15:38 -0600 Subject: [PATCH 25/46] feat(otlp-exporter): add two more tests Signed-off-by: Svetlana Brennan --- .../exporter-trace-otlp-http/test/node/utils.test.ts | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/packages/exporter-trace-otlp-http/test/node/utils.test.ts b/packages/exporter-trace-otlp-http/test/node/utils.test.ts index a4fed1543c..c87a58675b 100644 --- a/packages/exporter-trace-otlp-http/test/node/utils.test.ts +++ b/packages/exporter-trace-otlp-http/test/node/utils.test.ts @@ -84,6 +84,16 @@ describe('invalidTimeout', () => { assert.strictEqual(args[1], -9000); sinon.restore(); }); + it('diag warn was called', () => { + const spyLoggerWarn = sinon.stub(diag, 'warn'); + invalidTimeout(-9000, 10000); + assert(spyLoggerWarn.calledOnce); + sinon.restore(); + }); + it('should return default timeout', () => { + const defaultTimeout = invalidTimeout(-9000, 10000); + assert.strictEqual(defaultTimeout, 10000); + }); }); describe('configureCompression', () => { From eff32980f742961e1741766893decd278b7699c8 Mon Sep 17 00:00:00 2001 From: Svetlana Brennan Date: Fri, 11 Mar 2022 10:39:21 -0600 Subject: [PATCH 26/46] feat(otlp-exporter): add 1 more test Signed-off-by: Svetlana Brennan --- .../exporter-trace-otlp-http/test/node/utils.test.ts | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/packages/exporter-trace-otlp-http/test/node/utils.test.ts b/packages/exporter-trace-otlp-http/test/node/utils.test.ts index c87a58675b..b50e1329ca 100644 --- a/packages/exporter-trace-otlp-http/test/node/utils.test.ts +++ b/packages/exporter-trace-otlp-http/test/node/utils.test.ts @@ -57,7 +57,7 @@ describe('configureExporterTimeout', () => { assert.strictEqual(exporterTimeout, 9000); delete envSource.OTEL_EXPORTER_OTLP_TRACES_TIMEOUT; }); - it('should use default value when both timeoutMillis parameter export timeout value defined in env are negative', () => { + it('should use default value when both timeoutMillis parameter and export timeout values defined in env are negative', () => { envSource.OTEL_EXPORTER_OTLP_TRACES_TIMEOUT = '-11000'; envSource.OTEL_EXPORTER_OTLP_TIMEOUT = '-9000'; const exporterTimeout = configureExporterTimeout(-5000); @@ -65,6 +65,14 @@ describe('configureExporterTimeout', () => { delete envSource.OTEL_EXPORTER_OTLP_TRACES_TIMEOUT; delete envSource.OTEL_EXPORTER_OTLP_TIMEOUT; }); + it('should use default value export timeout value defined in env are negative', () => { + envSource.OTEL_EXPORTER_OTLP_TRACES_TIMEOUT = '-11000'; + envSource.OTEL_EXPORTER_OTLP_TIMEOUT = '-9000'; + const exporterTimeout = configureExporterTimeout(undefined); + assert.strictEqual(exporterTimeout, 10000); + delete envSource.OTEL_EXPORTER_OTLP_TRACES_TIMEOUT; + delete envSource.OTEL_EXPORTER_OTLP_TIMEOUT; + }); it('should warn user about invalid timeout', () => { const spyLoggerWarn = sinon.stub(diag, 'warn'); configureExporterTimeout(-15000); From 44eeb565abda1befa9f2171974176f857eb96944 Mon Sep 17 00:00:00 2001 From: Svetlana Brennan Date: Thu, 24 Mar 2022 13:38:21 -0500 Subject: [PATCH 27/46] feat(otlp-exporter): add logic around request aborted after response received Signed-off-by: Svetlana Brennan --- .../src/platform/node/util.ts | 44 ++++++++--- .../test/node/CollectorTraceExporter.test.ts | 76 +++++++++++++++++++ 2 files changed, 108 insertions(+), 12 deletions(-) diff --git a/packages/exporter-trace-otlp-http/src/platform/node/util.ts b/packages/exporter-trace-otlp-http/src/platform/node/util.ts index 134688e0da..1e0fc8ed99 100644 --- a/packages/exporter-trace-otlp-http/src/platform/node/util.ts +++ b/packages/exporter-trace-otlp-http/src/platform/node/util.ts @@ -45,10 +45,15 @@ export function sendWithHttp( const exporterTimeout = collector.timeoutMillis; const parsedUrl = new url.URL(collector.url); let reqIsDestroyed: boolean; + const nodeVersion = Number(process.versions.node.split('.')[0]); const exporterTimer = setTimeout(() => { reqIsDestroyed = true; - req.destroy(); + if (Number(nodeVersion) >= 14) { + req.destroy(); + } else { + req.abort(); + } }, exporterTimeout); const options: http.RequestOptions | https.RequestOptions = { @@ -69,19 +74,34 @@ export function sendWithHttp( const req = request(options, (res: http.IncomingMessage) => { let responseData = ''; res.on('data', chunk => (responseData += chunk)); - res.on('end', () => { - if (res.statusCode && res.statusCode < 299) { - diag.debug(`statusCode: ${res.statusCode}`, responseData); - onSuccess(); - } else { - const error = new otlpTypes.OTLPExporterError( - res.statusMessage, - res.statusCode, - responseData + + res.on('aborted', () => { + if (reqIsDestroyed) { + const err = new otlpTypes.OTLPExporterError( + 'Request Timeout' ); - onError(error); + onError(err); + } + }); + + res.on('end', () => { + // node version <= 12 will emit 'end' event even if request was aborted + // no need to handle promise here if request was aborted since promise + // was already rejected in res.on('aborted') + if (!reqIsDestroyed) { + if (res.statusCode && res.statusCode < 299) { + diag.debug(`statusCode: ${res.statusCode}`, responseData); + onSuccess(); + } else { + const error = new otlpTypes.OTLPExporterError( + res.statusMessage, + res.statusCode, + responseData + ); + onError(error); + } + clearTimeout(exporterTimer); } - clearTimeout(exporterTimer); }); }); diff --git a/packages/exporter-trace-otlp-http/test/node/CollectorTraceExporter.test.ts b/packages/exporter-trace-otlp-http/test/node/CollectorTraceExporter.test.ts index 465589726d..8331daf80a 100644 --- a/packages/exporter-trace-otlp-http/test/node/CollectorTraceExporter.test.ts +++ b/packages/exporter-trace-otlp-http/test/node/CollectorTraceExporter.test.ts @@ -449,4 +449,80 @@ describe('OTLPTraceExporter - node with json over http', () => { }, 300); }); }); + describe('export - real http request destroyed before response received', () => { + const server = http.createServer((_, res) => { + setTimeout(() => { + res.statusCode = 200; + res.end(); + }, 200); + }); + beforeEach(() => { + server.listen(8080); + }); + afterEach(() => { + server.close(); + }); + it('should log the timeout request error message when timeout is 1', done => { + collectorExporterConfig = { + url: 'http://localhost:8080', + timeoutMillis: 1, + }; + collectorExporter = new OTLPTraceExporter(collectorExporterConfig); + spans = []; + spans.push(Object.assign({}, mockedReadableSpan)); + + collectorExporter.export(spans, result => { + assert.strictEqual(result.code, core.ExportResultCode.FAILED); + const error = result.error as otlpTypes.OTLPExporterError; + assert.ok(error !== undefined); + assert.strictEqual(error.message, 'Request Timeout'); + done(); + }); + }); + it('should log the timeout request error message when timeout is 100', done => { + collectorExporterConfig = { + url: 'http://localhost:8080', + timeoutMillis: 100, + }; + collectorExporter = new OTLPTraceExporter(collectorExporterConfig); + spans = []; + spans.push(Object.assign({}, mockedReadableSpan)); + + collectorExporter.export(spans, result => { + assert.strictEqual(result.code, core.ExportResultCode.FAILED); + const error = result.error as otlpTypes.OTLPExporterError; + assert.ok(error !== undefined); + assert.strictEqual(error.message, 'Request Timeout'); + done(); + }); + }); + }); + describe('export - real http request destroyed after response received', () => { + const server = http.createServer((_, res) => { + res.write('writing something'); + }); + beforeEach(() => { + server.listen(8080); + }); + afterEach(() => { + server.close(); + }); + it('should log the timeout request error message', done => { + collectorExporterConfig = { + url: 'http://localhost:8080', + timeoutMillis: 300, + }; + collectorExporter = new OTLPTraceExporter(collectorExporterConfig); + spans = []; + spans.push(Object.assign({}, mockedReadableSpan)); + + collectorExporter.export(spans, result => { + assert.strictEqual(result.code, core.ExportResultCode.FAILED); + const error = result.error as otlpTypes.OTLPExporterError; + assert.ok(error !== undefined); + assert.strictEqual(error.message, 'Request Timeout'); + done(); + }); + }); + }); }); From cfeb18209af34c64f88365d4220e52349fabd867 Mon Sep 17 00:00:00 2001 From: Svetlana Brennan Date: Thu, 24 Mar 2022 14:48:29 -0500 Subject: [PATCH 28/46] feat(otlp-exporter): update grpc deadline calculation Signed-off-by: Svetlana Brennan --- packages/exporter-trace-otlp-grpc/src/util.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/exporter-trace-otlp-grpc/src/util.ts b/packages/exporter-trace-otlp-grpc/src/util.ts index d1585eb826..5daa275c57 100644 --- a/packages/exporter-trace-otlp-grpc/src/util.ts +++ b/packages/exporter-trace-otlp-grpc/src/util.ts @@ -84,7 +84,7 @@ export function send( ): void { if (collector.serviceClient) { const serviceRequest = collector.convert(objects); - const deadline = new Date().setSeconds(new Date().getSeconds() + (collector.timeoutMillis / 1000)); + const deadline = Date.now() + collector.timeoutMillis; collector.serviceClient.export( serviceRequest, From 8b6a0054eaf5f6b8742009f268893ea5de8d0346 Mon Sep 17 00:00:00 2001 From: Svetlana Brennan Date: Thu, 24 Mar 2022 14:50:07 -0500 Subject: [PATCH 29/46] feat(otlp-exporter): add real http request test to proto exporter Signed-off-by: Svetlana Brennan --- .../test/OTLPTraceExporter.test.ts | 76 +++++++++++++++++++ 1 file changed, 76 insertions(+) diff --git a/packages/exporter-trace-otlp-proto/test/OTLPTraceExporter.test.ts b/packages/exporter-trace-otlp-proto/test/OTLPTraceExporter.test.ts index 9d58be3acd..f56e20c7ad 100644 --- a/packages/exporter-trace-otlp-proto/test/OTLPTraceExporter.test.ts +++ b/packages/exporter-trace-otlp-proto/test/OTLPTraceExporter.test.ts @@ -283,4 +283,80 @@ describe('OTLPTraceExporter - node with proto over http', () => { clock.restore(); }); }); + describe('export - real http request destroyed before response received', () => { + const server = http.createServer((_, res) => { + setTimeout(() => { + res.statusCode = 200; + res.end(); + }, 200); + }); + beforeEach(() => { + server.listen(8080); + }); + afterEach(() => { + server.close(); + }); + it('should log the timeout request error message when timeout is 1', done => { + collectorExporterConfig = { + url: 'http://localhost:8080', + timeoutMillis: 1, + }; + collectorExporter = new OTLPTraceExporter(collectorExporterConfig); + spans = []; + spans.push(Object.assign({}, mockedReadableSpan)); + + collectorExporter.export(spans, result => { + assert.strictEqual(result.code, ExportResultCode.FAILED); + const error = result.error as otlpTypes.OTLPExporterError; + assert.ok(error !== undefined); + assert.strictEqual(error.message, 'Request Timeout'); + done(); + }); + }); + it('should log the timeout request error message when timeout is 100', done => { + collectorExporterConfig = { + url: 'http://localhost:8080', + timeoutMillis: 100, + }; + collectorExporter = new OTLPTraceExporter(collectorExporterConfig); + spans = []; + spans.push(Object.assign({}, mockedReadableSpan)); + + collectorExporter.export(spans, result => { + assert.strictEqual(result.code, ExportResultCode.FAILED); + const error = result.error as otlpTypes.OTLPExporterError; + assert.ok(error !== undefined); + assert.strictEqual(error.message, 'Request Timeout'); + done(); + }); + }); + }); + describe('export - real http request destroyed after response received', () => { + const server = http.createServer((_, res) => { + res.write('writing something'); + }); + beforeEach(() => { + server.listen(8080); + }); + afterEach(() => { + server.close(); + }); + it('should log the timeout request error message', done => { + collectorExporterConfig = { + url: 'http://localhost:8080', + timeoutMillis: 300, + }; + collectorExporter = new OTLPTraceExporter(collectorExporterConfig); + spans = []; + spans.push(Object.assign({}, mockedReadableSpan)); + + collectorExporter.export(spans, result => { + assert.strictEqual(result.code, ExportResultCode.FAILED); + const error = result.error as otlpTypes.OTLPExporterError; + assert.ok(error !== undefined); + assert.strictEqual(error.message, 'Request Timeout'); + done(); + }); + }); + }); }); From 5547628dda740d07a47edfd9cbaf64fa17bd95b8 Mon Sep 17 00:00:00 2001 From: Svetlana Brennan Date: Thu, 24 Mar 2022 16:41:11 -0500 Subject: [PATCH 30/46] feat(otlp-exporter): fix timeout test Signed-off-by: Svetlana Brennan --- .../test/node/CollectorTraceExporter.test.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/exporter-trace-otlp-http/test/node/CollectorTraceExporter.test.ts b/packages/exporter-trace-otlp-http/test/node/CollectorTraceExporter.test.ts index 8331daf80a..97e215317f 100644 --- a/packages/exporter-trace-otlp-http/test/node/CollectorTraceExporter.test.ts +++ b/packages/exporter-trace-otlp-http/test/node/CollectorTraceExporter.test.ts @@ -415,6 +415,7 @@ describe('OTLPTraceExporter - node with json over http', () => { stubRequest = sinon.stub(http, 'request').returns(fakeRequest as any); spySetHeader = sinon.spy(); (fakeRequest as any).setHeader = spySetHeader; + (fakeRequest as any).abort = sinon.spy(); collectorExporterConfig = { headers: { foo: 'bar', From bc6bb16ee6c426966635409eeeb414aed4ebfcfe Mon Sep 17 00:00:00 2001 From: Svetlana Brennan Date: Thu, 24 Mar 2022 16:57:09 -0500 Subject: [PATCH 31/46] feat(otlp-exporter): fix grpc readme Signed-off-by: Svetlana Brennan --- packages/exporter-trace-otlp-grpc/README.md | 29 +++++++-------------- 1 file changed, 10 insertions(+), 19 deletions(-) diff --git a/packages/exporter-trace-otlp-grpc/README.md b/packages/exporter-trace-otlp-grpc/README.md index b91d580de1..1dc4bb3da1 100644 --- a/packages/exporter-trace-otlp-grpc/README.md +++ b/packages/exporter-trace-otlp-grpc/README.md @@ -111,18 +111,7 @@ Note, that this will only work if TLS is also configured on the server. The OTLPTraceExporter has a timeout configuration option which is the maximum time, in milliseconds, the OTLP exporter will wait for each batch export. The default value is 10000ms. -To override the default timeout duration, use the following options: - -+ Set with environment variables: - - | Environment variable | Description | - |----------------------|-------------| - | OTEL_EXPORTER_OTLP_TRACES_TIMEOUT | The maximum waiting time, in milliseconds, allowed to send each OTLP trace batch. Default is 10000. | - | OTEL_EXPORTER_OTLP_TIMEOUT | The maximum waiting time, in milliseconds, allowed to send each OTLP trace and metric batch. Default is 10000. | - - > `OTEL_EXPORTER_OTLP_TRACES_TIMEOUT` takes precedence and overrides `OTEL_EXPORTER_OTLP_TIMEOUT`. - -+ Provide `timeoutMillis` to OTLPTraceExporter with `collectorOptions`: ++ To override the default timeout duration, provide `timeoutMillis` to OTLPTraceExporter with `collectorOptions`: ```js const collectorOptions = { @@ -137,6 +126,8 @@ To override the default timeout duration, use the following options: > Providing `timeoutMillis` with `collectorOptions` takes precedence and overrides timeout set with environment variables. +## Exporter Compression Configuration + By default no compression will be used. To use compression, set it programmatically in `collectorOptions` or with environment variables. Supported compression options: `gzip` and `none`. ```js @@ -155,13 +146,13 @@ const exporter = new OTLPTraceExporter(collectorOptions); ## Environment Variable Configuration -Set compression with environment variables. - -```shell -OTEL_EXPORTER_OTLP_TRACES_COMPRESSION=gzip -``` - - > Compression set programatically in `collectorOptions` takes precedence over compression set with environment variables. `OTEL_EXPORTER_OTLP_TRACES_COMPRESSION` takes precedence and overrides `OTEL_EXPORTER_OTLP_COMPRESSION`. + | Environment variable | Description | + |----------------------|-------------| + | OTEL_EXPORTER_OTLP_TRACES_TIMEOUT | The maximum waiting time, in milliseconds, allowed to send each OTLP trace batch. Default is 10000. | + | OTEL_EXPORTER_OTLP_TIMEOUT | The maximum waiting time, in milliseconds, allowed to send each OTLP trace and metric batch. Default is 10000. | + | OTEL_EXPORTER_OTLP_TRACES_COMPRESSION | The compression type to use on OTLP trace requests. Options include gzip. By default no compression will be used. | + | OTEL_EXPORTER_OTLP_COMPRESSION | The compression type to use on OTLP trace, metric, and log requests. Options include gzip. By default no compression will be used. | + > The per-signal environment variables (`OTEL_EXPORTER_OTLP_TRACES_TIMEOUT`) takes precedence and non-per-signal environment variable (`OTEL_EXPORTER_OTLP_TIMEOUT`). ## Running opentelemetry-collector locally to see the traces From 8c0eda7d1644ee15e2a02245dd773818e2c85673 Mon Sep 17 00:00:00 2001 From: Svetlana Brennan <50715937+svetlanabrennan@users.noreply.github.com> Date: Mon, 28 Mar 2022 15:08:48 -0500 Subject: [PATCH 32/46] Update packages/exporter-trace-otlp-http/src/platform/node/util.ts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Gerhard Stöbich --- packages/exporter-trace-otlp-http/src/platform/node/util.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/exporter-trace-otlp-http/src/platform/node/util.ts b/packages/exporter-trace-otlp-http/src/platform/node/util.ts index 1e0fc8ed99..2670b51f0b 100644 --- a/packages/exporter-trace-otlp-http/src/platform/node/util.ts +++ b/packages/exporter-trace-otlp-http/src/platform/node/util.ts @@ -49,7 +49,7 @@ export function sendWithHttp( const exporterTimer = setTimeout(() => { reqIsDestroyed = true; - if (Number(nodeVersion) >= 14) { + if (nodeVersion >= 14) { req.destroy(); } else { req.abort(); From 7f103cbc2d2c3b20688fe7b13b38ef4adef2b984 Mon Sep 17 00:00:00 2001 From: Svetlana Brennan Date: Thu, 31 Mar 2022 14:53:15 -0500 Subject: [PATCH 33/46] feat(otlp-exporter): remove comment from sendWithHttp Signed-off-by: Svetlana Brennan --- .../exporter-trace-otlp-http/src/platform/node/util.ts | 3 --- 1 file changed, 3 deletions(-) diff --git a/experimental/packages/exporter-trace-otlp-http/src/platform/node/util.ts b/experimental/packages/exporter-trace-otlp-http/src/platform/node/util.ts index 2670b51f0b..7529057c81 100644 --- a/experimental/packages/exporter-trace-otlp-http/src/platform/node/util.ts +++ b/experimental/packages/exporter-trace-otlp-http/src/platform/node/util.ts @@ -85,9 +85,6 @@ export function sendWithHttp( }); res.on('end', () => { - // node version <= 12 will emit 'end' event even if request was aborted - // no need to handle promise here if request was aborted since promise - // was already rejected in res.on('aborted') if (!reqIsDestroyed) { if (res.statusCode && res.statusCode < 299) { diag.debug(`statusCode: ${res.statusCode}`, responseData); From f0055a0cdae0803b8db9348a14291a9318134723 Mon Sep 17 00:00:00 2001 From: Svetlana Brennan Date: Tue, 19 Apr 2022 14:55:05 -0500 Subject: [PATCH 34/46] feat(otlp-exporter): fix metric tests and add changelong Signed-off-by: Svetlana Brennan --- CHANGELOG.md | 1 + .../test/node/CollectorMetricExporter.test.ts | 45 +++++++++++++------ 2 files changed, 33 insertions(+), 13 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 58c9a285f2..2e1e162dfa 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,7 @@ All notable changes to this project will be documented in this file. ### :boom: Breaking Change ### :rocket: (Enhancement) +* feat(otlp-exporter): add timeout env var [#2738](https://github.com/open-telemetry/opentelemetry-js/pull/2738) @svetlanabrennan ### :bug: (Bug Fix) diff --git a/experimental/packages/opentelemetry-exporter-metrics-otlp-http/test/node/CollectorMetricExporter.test.ts b/experimental/packages/opentelemetry-exporter-metrics-otlp-http/test/node/CollectorMetricExporter.test.ts index 1e11f733d4..1da66cc629 100644 --- a/experimental/packages/opentelemetry-exporter-metrics-otlp-http/test/node/CollectorMetricExporter.test.ts +++ b/experimental/packages/opentelemetry-exporter-metrics-otlp-http/test/node/CollectorMetricExporter.test.ts @@ -40,15 +40,9 @@ import { } from '../metricsHelper'; import { MockedResponse } from './nodeHelpers'; import { AggregationTemporality, ResourceMetrics } from '@opentelemetry/sdk-metrics-base'; +import { Stream, PassThrough } from 'stream'; -const fakeRequest = { - end: function () { - }, - on: function () { - }, - write: function () { - }, -}; +let fakeRequest: PassThrough; const address = 'localhost:1501'; @@ -56,7 +50,6 @@ describe('OTLPMetricExporter - node with json over http', () => { let collectorExporter: OTLPMetricExporter; let collectorExporterConfig: OTLPExporterNodeConfigBase & OTLPMetricExporterOptions; let stubRequest: sinon.SinonStub; - let stubWrite: sinon.SinonStub; let metrics: ResourceMetrics; beforeEach(async () => { @@ -64,6 +57,7 @@ describe('OTLPMetricExporter - node with json over http', () => { }); afterEach(async () => { + fakeRequest = new Stream.PassThrough(); await shutdown(); sinon.restore(); }); @@ -152,7 +146,6 @@ describe('OTLPMetricExporter - node with json over http', () => { describe('export', () => { beforeEach(async () => { stubRequest = sinon.stub(http, 'request').returns(fakeRequest as any); - stubWrite = sinon.stub(fakeRequest, 'end'); collectorExporterConfig = { headers: { foo: 'bar', @@ -187,7 +180,11 @@ describe('OTLPMetricExporter - node with json over http', () => { }); setTimeout(() => { + const mockRes = new MockedResponse(200); const args = stubRequest.args[0]; + const callback = args[1]; + callback(mockRes); + mockRes.send('success'); const options = args[0]; assert.strictEqual(options.hostname, 'foo.bar.com'); @@ -202,7 +199,11 @@ describe('OTLPMetricExporter - node with json over http', () => { }); setTimeout(() => { + const mockRes = new MockedResponse(200); const args = stubRequest.args[0]; + const callback = args[1]; + callback(mockRes); + mockRes.send('success'); const options = args[0]; assert.strictEqual(options.headers['foo'], 'bar'); done(); @@ -214,7 +215,11 @@ describe('OTLPMetricExporter - node with json over http', () => { }); setTimeout(() => { + const mockRes = new MockedResponse(200); const args = stubRequest.args[0]; + const callback = args[1]; + callback(mockRes); + mockRes.send('success'); const options = args[0]; const agent = options.agent; assert.strictEqual(agent.keepAlive, true); @@ -224,13 +229,16 @@ describe('OTLPMetricExporter - node with json over http', () => { }); it('should successfully send metrics', done => { + let buff = Buffer.from(''); + collectorExporter.export(metrics, () => { }); - setTimeout(() => { - const writeArgs = stubWrite.args[0]; + fakeRequest.on('end', () => { + const responseBody = buff.toString(); + const json = JSON.parse( - writeArgs[0] + responseBody ) as otlpTypes.opentelemetryProto.collector.metrics.v1.ExportMetricsServiceRequest; const metric1 = json.resourceMetrics[0].instrumentationLibraryMetrics[0].metrics[0]; @@ -265,7 +273,18 @@ describe('OTLPMetricExporter - node with json over http', () => { ensureExportMetricsServiceRequestIsSet(json); done(); + }) + + fakeRequest.on('data', chunk => { + buff = Buffer.concat([buff, chunk]); }); + + const mockRes = new MockedResponse(200); + const args = stubRequest.args[0]; + const callback = args[1]; + + callback(mockRes); + mockRes.send('success'); }); it('should log the successful message', done => { From c79c521938606cf76f483554737789a12c23705f Mon Sep 17 00:00:00 2001 From: Svetlana Brennan Date: Tue, 19 Apr 2022 15:00:06 -0500 Subject: [PATCH 35/46] feat(otlp-exporter): fix lint Signed-off-by: Svetlana Brennan --- CHANGELOG.md | 1 - experimental/CHANGELOG.md | 1 + .../test/node/CollectorMetricExporter.test.ts | 2 +- 3 files changed, 2 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2e1e162dfa..58c9a285f2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,7 +7,6 @@ All notable changes to this project will be documented in this file. ### :boom: Breaking Change ### :rocket: (Enhancement) -* feat(otlp-exporter): add timeout env var [#2738](https://github.com/open-telemetry/opentelemetry-js/pull/2738) @svetlanabrennan ### :bug: (Bug Fix) diff --git a/experimental/CHANGELOG.md b/experimental/CHANGELOG.md index 58681558d4..ecd8555319 100644 --- a/experimental/CHANGELOG.md +++ b/experimental/CHANGELOG.md @@ -40,6 +40,7 @@ All notable changes to experimental packages in this project will be documented * feat(proto): add @opentelemetry/otlp-transformer package with hand-rolled transformation #2746 @dyladan * feat(sdk-metrics-base): shutdown and forceflush on MeterProvider #2890 @legendecas * feat(sdk-metrics-base): return the same meter for identical input to getMeter #2901 @legendecas +* feat(otlp-exporter): add timeout env var #2738 @svetlanabrennan ### :bug: (Bug Fix) diff --git a/experimental/packages/opentelemetry-exporter-metrics-otlp-http/test/node/CollectorMetricExporter.test.ts b/experimental/packages/opentelemetry-exporter-metrics-otlp-http/test/node/CollectorMetricExporter.test.ts index 1da66cc629..cac763ec27 100644 --- a/experimental/packages/opentelemetry-exporter-metrics-otlp-http/test/node/CollectorMetricExporter.test.ts +++ b/experimental/packages/opentelemetry-exporter-metrics-otlp-http/test/node/CollectorMetricExporter.test.ts @@ -273,7 +273,7 @@ describe('OTLPMetricExporter - node with json over http', () => { ensureExportMetricsServiceRequestIsSet(json); done(); - }) + }); fakeRequest.on('data', chunk => { buff = Buffer.concat([buff, chunk]); From d18c587888c06fe1f987bd9c880d15b2877f6788 Mon Sep 17 00:00:00 2001 From: Svetlana Brennan Date: Wed, 20 Apr 2022 11:00:14 -0500 Subject: [PATCH 36/46] feat(otlp-exporter): fix metric tests Signed-off-by: Svetlana Brennan --- .../test/OTLPMetricExporter.test.ts | 128 ++++++++++-------- 1 file changed, 73 insertions(+), 55 deletions(-) diff --git a/experimental/packages/opentelemetry-exporter-metrics-otlp-proto/test/OTLPMetricExporter.test.ts b/experimental/packages/opentelemetry-exporter-metrics-otlp-proto/test/OTLPMetricExporter.test.ts index 429096aff0..6854e77a31 100644 --- a/experimental/packages/opentelemetry-exporter-metrics-otlp-proto/test/OTLPMetricExporter.test.ts +++ b/experimental/packages/opentelemetry-exporter-metrics-otlp-proto/test/OTLPMetricExporter.test.ts @@ -38,21 +38,20 @@ import { } from './metricsHelper'; import { AggregationTemporality, ResourceMetrics } from '@opentelemetry/sdk-metrics-base'; import { OTLPMetricExporterOptions } from '@opentelemetry/exporter-metrics-otlp-http'; +import { Stream, PassThrough } from 'stream'; -const fakeRequest = { - end: function () { - }, - on: function () { - }, - write: function () { - }, -}; +let fakeRequest: PassThrough; describe('OTLPMetricExporter - node with proto over http', () => { let collectorExporter: OTLPMetricExporter; let collectorExporterConfig: OTLPExporterNodeConfigBase & OTLPMetricExporterOptions; let metrics: ResourceMetrics; + afterEach(() => { + fakeRequest = new Stream.PassThrough(); + sinon.restore(); + }); + describe('when configuring via environment', () => { const envSource = process.env; it('should use url defined in env', () => { @@ -140,10 +139,14 @@ describe('OTLPMetricExporter - node with proto over http', () => { collectorExporter.export(metrics, () => { }); - sinon.stub(http, 'request').callsFake((options: any) => { + sinon.stub(http, 'request').callsFake((options: any, cb: any) => { assert.strictEqual(options.hostname, 'foo.bar.com'); assert.strictEqual(options.method, 'POST'); assert.strictEqual(options.path, '/'); + + const mockRes = new MockedResponse(200); + cb(mockRes); + mockRes.send('success'); done(); return fakeRequest as any; }); @@ -153,8 +156,13 @@ describe('OTLPMetricExporter - node with proto over http', () => { collectorExporter.export(metrics, () => { }); - sinon.stub(http, 'request').callsFake((options: any) => { + sinon.stub(http, 'request').callsFake((options: any, cb: any) => { assert.strictEqual(options.headers['foo'], 'bar'); + + const mockRes = new MockedResponse(200); + cb(mockRes); + mockRes.send('success'); + done(); return fakeRequest as any; }); @@ -164,62 +172,72 @@ describe('OTLPMetricExporter - node with proto over http', () => { collectorExporter.export(metrics, () => { }); - sinon.stub(http, 'request').callsFake((options: any) => { + sinon.stub(http, 'request').callsFake((options: any, cb: any) => { assert.strictEqual(options.agent.keepAlive, true); assert.strictEqual(options.agent.options.keepAliveMsecs, 2000); + + const mockRes = new MockedResponse(200); + cb(mockRes); + mockRes.send('success'); + done(); return fakeRequest as any; }); }); it('should successfully send metrics', done => { + const fakeRequest = new Stream.PassThrough(); + sinon.stub(http, 'request').returns(fakeRequest as any); + collectorExporter.export(metrics, () => { }); - sinon.stub(http, 'request').returns({ - write: () => {}, - on: () => {}, - end: (...writeArgs: any[]) => { - const ExportTraceServiceRequestProto = getExportRequestProto(); - const data = ExportTraceServiceRequestProto?.decode(writeArgs[0]); - const json = data?.toJSON() as otlpTypes.opentelemetryProto.collector.metrics.v1.ExportMetricsServiceRequest; - - const metric1 = - json.resourceMetrics[0].instrumentationLibraryMetrics[0].metrics[0]; - const metric2 = - json.resourceMetrics[0].instrumentationLibraryMetrics[0].metrics[1]; - const metric3 = - json.resourceMetrics[0].instrumentationLibraryMetrics[0].metrics[2]; - - assert.ok(typeof metric1 !== 'undefined', "counter doesn't exist"); - ensureExportedCounterIsCorrect( - metric1, - metric1.intSum?.dataPoints[0].timeUnixNano, - metric1.intSum?.dataPoints[0].startTimeUnixNano - ); - assert.ok(typeof metric2 !== 'undefined', "observable gauge doesn't exist"); - ensureExportedObservableGaugeIsCorrect( - metric2, - metric2.doubleGauge?.dataPoints[0].timeUnixNano, - metric2.doubleGauge?.dataPoints[0].startTimeUnixNano - ); - assert.ok( - typeof metric3 !== 'undefined', - "value recorder doesn't exist" - ); - ensureExportedHistogramIsCorrect( - metric3, - metric3.intHistogram?.dataPoints[0].timeUnixNano, - metric3.intHistogram?.dataPoints[0].startTimeUnixNano, - [0, 100], - ['0', '2', '0'] - ); - - ensureExportMetricsServiceRequestIsSet(json); - - done(); - }, - } as any); + let buff = Buffer.from(''); + + fakeRequest.on('end', () => { + const ExportTraceServiceRequestProto = getExportRequestProto(); + const data = ExportTraceServiceRequestProto?.decode(buff); + const json = data?.toJSON() as otlpTypes.opentelemetryProto.collector.metrics.v1.ExportMetricsServiceRequest; + + const metric1 = + json.resourceMetrics[0].instrumentationLibraryMetrics[0].metrics[0]; + const metric2 = + json.resourceMetrics[0].instrumentationLibraryMetrics[0].metrics[1]; + const metric3 = + json.resourceMetrics[0].instrumentationLibraryMetrics[0].metrics[2]; + + assert.ok(typeof metric1 !== 'undefined', "counter doesn't exist"); + ensureExportedCounterIsCorrect( + metric1, + metric1.intSum?.dataPoints[0].timeUnixNano, + metric1.intSum?.dataPoints[0].startTimeUnixNano + ); + assert.ok(typeof metric2 !== 'undefined', "observable gauge doesn't exist"); + ensureExportedObservableGaugeIsCorrect( + metric2, + metric2.doubleGauge?.dataPoints[0].timeUnixNano, + metric2.doubleGauge?.dataPoints[0].startTimeUnixNano + ); + assert.ok( + typeof metric3 !== 'undefined', + "value recorder doesn't exist" + ); + ensureExportedHistogramIsCorrect( + metric3, + metric3.intHistogram?.dataPoints[0].timeUnixNano, + metric3.intHistogram?.dataPoints[0].startTimeUnixNano, + [0, 100], + ['0', '2', '0'] + ); + + ensureExportMetricsServiceRequestIsSet(json); + + done(); + }); + + fakeRequest.on('data', chunk => { + buff = Buffer.concat([buff, chunk]); + }); }); it('should log the successful message', done => { From b97a418a056ad9732ed91d22188f2ded25fabfa7 Mon Sep 17 00:00:00 2001 From: Svetlana Brennan Date: Wed, 20 Apr 2022 11:20:05 -0500 Subject: [PATCH 37/46] feat(otlp-exporter): fix proto tests Signed-off-by: Svetlana Brennan --- .../test/node/CollectorTraceExporter.test.ts | 4 ++-- .../test/OTLPTraceExporter.test.ts | 4 ++-- .../test/OTLPMetricExporter.test.ts | 8 +++++--- 3 files changed, 9 insertions(+), 7 deletions(-) diff --git a/experimental/packages/exporter-trace-otlp-http/test/node/CollectorTraceExporter.test.ts b/experimental/packages/exporter-trace-otlp-http/test/node/CollectorTraceExporter.test.ts index 97e215317f..ce21c38b46 100644 --- a/experimental/packages/exporter-trace-otlp-http/test/node/CollectorTraceExporter.test.ts +++ b/experimental/packages/exporter-trace-otlp-http/test/node/CollectorTraceExporter.test.ts @@ -460,8 +460,8 @@ describe('OTLPTraceExporter - node with json over http', () => { beforeEach(() => { server.listen(8080); }); - afterEach(() => { - server.close(); + afterEach((done) => { + server.close(done); }); it('should log the timeout request error message when timeout is 1', done => { collectorExporterConfig = { diff --git a/experimental/packages/exporter-trace-otlp-proto/test/OTLPTraceExporter.test.ts b/experimental/packages/exporter-trace-otlp-proto/test/OTLPTraceExporter.test.ts index f56e20c7ad..2c86e10050 100644 --- a/experimental/packages/exporter-trace-otlp-proto/test/OTLPTraceExporter.test.ts +++ b/experimental/packages/exporter-trace-otlp-proto/test/OTLPTraceExporter.test.ts @@ -293,8 +293,8 @@ describe('OTLPTraceExporter - node with proto over http', () => { beforeEach(() => { server.listen(8080); }); - afterEach(() => { - server.close(); + afterEach((done) => { + server.close(done); }); it('should log the timeout request error message when timeout is 1', done => { collectorExporterConfig = { diff --git a/experimental/packages/opentelemetry-exporter-metrics-otlp-proto/test/OTLPMetricExporter.test.ts b/experimental/packages/opentelemetry-exporter-metrics-otlp-proto/test/OTLPMetricExporter.test.ts index 6854e77a31..53d6bb628d 100644 --- a/experimental/packages/opentelemetry-exporter-metrics-otlp-proto/test/OTLPMetricExporter.test.ts +++ b/experimental/packages/opentelemetry-exporter-metrics-otlp-proto/test/OTLPMetricExporter.test.ts @@ -189,9 +189,6 @@ describe('OTLPMetricExporter - node with proto over http', () => { const fakeRequest = new Stream.PassThrough(); sinon.stub(http, 'request').returns(fakeRequest as any); - collectorExporter.export(metrics, () => { - }); - let buff = Buffer.from(''); fakeRequest.on('end', () => { @@ -238,6 +235,11 @@ describe('OTLPMetricExporter - node with proto over http', () => { fakeRequest.on('data', chunk => { buff = Buffer.concat([buff, chunk]); }); + + const clock = sinon.useFakeTimers(); + collectorExporter.export(metrics, () => { }); + clock.tick(200); + clock.restore(); }); it('should log the successful message', done => { From 0b8f7da3a25724d17a0bc6538d74ee9322e54191 Mon Sep 17 00:00:00 2001 From: Svetlana Brennan Date: Wed, 20 Apr 2022 11:30:18 -0500 Subject: [PATCH 38/46] feat(otlp-exporter): fix lint Signed-off-by: Svetlana Brennan --- .../test/node/CollectorTraceExporter.test.ts | 2 +- .../exporter-trace-otlp-proto/test/OTLPTraceExporter.test.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/experimental/packages/exporter-trace-otlp-http/test/node/CollectorTraceExporter.test.ts b/experimental/packages/exporter-trace-otlp-http/test/node/CollectorTraceExporter.test.ts index ce21c38b46..79b4671716 100644 --- a/experimental/packages/exporter-trace-otlp-http/test/node/CollectorTraceExporter.test.ts +++ b/experimental/packages/exporter-trace-otlp-http/test/node/CollectorTraceExporter.test.ts @@ -460,7 +460,7 @@ describe('OTLPTraceExporter - node with json over http', () => { beforeEach(() => { server.listen(8080); }); - afterEach((done) => { + afterEach(done => { server.close(done); }); it('should log the timeout request error message when timeout is 1', done => { diff --git a/experimental/packages/exporter-trace-otlp-proto/test/OTLPTraceExporter.test.ts b/experimental/packages/exporter-trace-otlp-proto/test/OTLPTraceExporter.test.ts index 2c86e10050..087a5103f3 100644 --- a/experimental/packages/exporter-trace-otlp-proto/test/OTLPTraceExporter.test.ts +++ b/experimental/packages/exporter-trace-otlp-proto/test/OTLPTraceExporter.test.ts @@ -293,7 +293,7 @@ describe('OTLPTraceExporter - node with proto over http', () => { beforeEach(() => { server.listen(8080); }); - afterEach((done) => { + afterEach(done => { server.close(done); }); it('should log the timeout request error message when timeout is 1', done => { From 78d4e93bb24db98096cfe92e346fe3d1b4ab86b4 Mon Sep 17 00:00:00 2001 From: Svetlana Brennan Date: Fri, 22 Apr 2022 16:23:58 -0500 Subject: [PATCH 39/46] feat(otlp-exporter): fix tests after main merge Signed-off-by: Svetlana Brennan --- .../test/browser/CollectorTraceExporter.test.ts | 3 ++- .../test/node/CollectorTraceExporter.test.ts | 8 ++++---- .../test/OTLPTraceExporter.test.ts | 7 ++++--- .../otlp-exporter-base/test/browser/util.test.ts | 13 +++---------- 4 files changed, 13 insertions(+), 18 deletions(-) diff --git a/experimental/packages/exporter-trace-otlp-http/test/browser/CollectorTraceExporter.test.ts b/experimental/packages/exporter-trace-otlp-http/test/browser/CollectorTraceExporter.test.ts index ca824f2696..8b0880b1ce 100644 --- a/experimental/packages/exporter-trace-otlp-http/test/browser/CollectorTraceExporter.test.ts +++ b/experimental/packages/exporter-trace-otlp-http/test/browser/CollectorTraceExporter.test.ts @@ -22,6 +22,7 @@ import * as assert from 'assert'; import * as sinon from 'sinon'; import { OTLPTraceExporter } from '../../src/platform/browser/index'; import * as otlpTypes from '../../src/types'; +import { OTLPExporterError } from '@opentelemetry/otlp-exporter-base'; import { nextTick } from 'process'; import { @@ -451,7 +452,7 @@ describe('OTLPTraceExporter - web', () => { nextTick(() => { const result = responseSpy.args[0][0] as core.ExportResult; assert.strictEqual(result.code, core.ExportResultCode.FAILED); - const error = result.error as otlpTypes.OTLPExporterError; + const error = result.error as OTLPExporterError; assert.ok(error !== undefined); assert.strictEqual(error.message, 'Request Timeout'); diff --git a/experimental/packages/exporter-trace-otlp-http/test/node/CollectorTraceExporter.test.ts b/experimental/packages/exporter-trace-otlp-http/test/node/CollectorTraceExporter.test.ts index 615a4d6f4a..859838d3a3 100644 --- a/experimental/packages/exporter-trace-otlp-http/test/node/CollectorTraceExporter.test.ts +++ b/experimental/packages/exporter-trace-otlp-http/test/node/CollectorTraceExporter.test.ts @@ -454,7 +454,7 @@ describe('OTLPTraceExporter - node with json over http', () => { setTimeout(() => { const result = responseSpy.args[0][0] as core.ExportResult; assert.strictEqual(result.code, core.ExportResultCode.FAILED); - const error = result.error as otlpTypes.OTLPExporterError; + const error = result.error as OTLPExporterError; assert.ok(error !== undefined); assert.strictEqual(error.message, 'Request Timeout'); @@ -487,7 +487,7 @@ describe('OTLPTraceExporter - node with json over http', () => { collectorExporter.export(spans, result => { assert.strictEqual(result.code, core.ExportResultCode.FAILED); - const error = result.error as otlpTypes.OTLPExporterError; + const error = result.error as OTLPExporterError; assert.ok(error !== undefined); assert.strictEqual(error.message, 'Request Timeout'); done(); @@ -504,7 +504,7 @@ describe('OTLPTraceExporter - node with json over http', () => { collectorExporter.export(spans, result => { assert.strictEqual(result.code, core.ExportResultCode.FAILED); - const error = result.error as otlpTypes.OTLPExporterError; + const error = result.error as OTLPExporterError; assert.ok(error !== undefined); assert.strictEqual(error.message, 'Request Timeout'); done(); @@ -532,7 +532,7 @@ describe('OTLPTraceExporter - node with json over http', () => { collectorExporter.export(spans, result => { assert.strictEqual(result.code, core.ExportResultCode.FAILED); - const error = result.error as otlpTypes.OTLPExporterError; + const error = result.error as OTLPExporterError; assert.ok(error !== undefined); assert.strictEqual(error.message, 'Request Timeout'); done(); diff --git a/experimental/packages/exporter-trace-otlp-proto/test/OTLPTraceExporter.test.ts b/experimental/packages/exporter-trace-otlp-proto/test/OTLPTraceExporter.test.ts index caa5d9e850..ffda7949b6 100644 --- a/experimental/packages/exporter-trace-otlp-proto/test/OTLPTraceExporter.test.ts +++ b/experimental/packages/exporter-trace-otlp-proto/test/OTLPTraceExporter.test.ts @@ -34,6 +34,7 @@ import { } from './traceHelper'; import { CompressionAlgorithm, OTLPExporterNodeConfigBase } from '@opentelemetry/otlp-exporter-base'; import { getExportRequestProto } from '@opentelemetry/otlp-proto-exporter-base'; +import { OTLPExporterError } from '@opentelemetry/otlp-exporter-base'; let fakeRequest: PassThrough; @@ -306,7 +307,7 @@ describe('OTLPTraceExporter - node with proto over http', () => { collectorExporter.export(spans, result => { assert.strictEqual(result.code, ExportResultCode.FAILED); - const error = result.error as otlpTypes.OTLPExporterError; + const error = result.error as OTLPExporterError; assert.ok(error !== undefined); assert.strictEqual(error.message, 'Request Timeout'); done(); @@ -323,7 +324,7 @@ describe('OTLPTraceExporter - node with proto over http', () => { collectorExporter.export(spans, result => { assert.strictEqual(result.code, ExportResultCode.FAILED); - const error = result.error as otlpTypes.OTLPExporterError; + const error = result.error as OTLPExporterError; assert.ok(error !== undefined); assert.strictEqual(error.message, 'Request Timeout'); done(); @@ -351,7 +352,7 @@ describe('OTLPTraceExporter - node with proto over http', () => { collectorExporter.export(spans, result => { assert.strictEqual(result.code, ExportResultCode.FAILED); - const error = result.error as otlpTypes.OTLPExporterError; + const error = result.error as OTLPExporterError; assert.ok(error !== undefined); assert.strictEqual(error.message, 'Request Timeout'); done(); diff --git a/experimental/packages/otlp-exporter-base/test/browser/util.test.ts b/experimental/packages/otlp-exporter-base/test/browser/util.test.ts index 8e50c21f1c..8238b4ddec 100644 --- a/experimental/packages/otlp-exporter-base/test/browser/util.test.ts +++ b/experimental/packages/otlp-exporter-base/test/browser/util.test.ts @@ -16,8 +16,6 @@ import * as sinon from 'sinon'; import { sendWithXhr } from '../../src/platform/browser/util'; - -import { OTLPTraceExporter } from '@opentelemetry/exporter-trace-otlp-http'; import { nextTick } from 'process'; import { ensureHeadersContain } from '../testHelper'; @@ -59,9 +57,7 @@ describe('util - browser', () => { const explicitContentType = { 'Content-Type': 'application/json', }; - // use default exporter timeout - const collectorExporter = new OTLPTraceExporter(); - const exporterTimeout = collectorExporter.timeoutMillis; + const exporterTimeout = 10000; sendWithXhr(body, url, explicitContentType, exporterTimeout, onSuccessStub, onErrorStub); }); it('Request Headers should contain "Content-Type" header', done => { @@ -88,8 +84,7 @@ describe('util - browser', () => { beforeEach(()=>{ const emptyHeaders = {}; // use default exporter timeout - const collectorExporter = new OTLPTraceExporter(); - const exporterTimeout = collectorExporter.timeoutMillis; + const exporterTimeout = 10000; sendWithXhr(body, url, emptyHeaders, exporterTimeout, onSuccessStub, onErrorStub); }); it('Request Headers should contain "Content-Type" header', done => { @@ -115,9 +110,7 @@ describe('util - browser', () => { let customHeaders: Record; beforeEach(()=>{ customHeaders = { aHeader: 'aValue', bHeader: 'bValue' }; - // use default exporter timeout - const collectorExporter = new OTLPTraceExporter(); - const exporterTimeout = collectorExporter.timeoutMillis; + const exporterTimeout = 10000; sendWithXhr(body, url, customHeaders, exporterTimeout, onSuccessStub, onErrorStub); }); it('Request Headers should contain "Content-Type" header', done => { From e81f9dd458be66975d090dc42ea7599dd12eb3dd Mon Sep 17 00:00:00 2001 From: Svetlana Brennan Date: Tue, 26 Apr 2022 12:34:36 -0500 Subject: [PATCH 40/46] feat(otlp-exporter): fix lint Signed-off-by: Svetlana Brennan --- .../exporter-trace-otlp-grpc/test/OTLPTraceExporter.test.ts | 4 ++-- .../packages/otlp-exporter-base/test/browser/util.test.ts | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/experimental/packages/exporter-trace-otlp-grpc/test/OTLPTraceExporter.test.ts b/experimental/packages/exporter-trace-otlp-grpc/test/OTLPTraceExporter.test.ts index 0b508c7e9e..9d2c668369 100644 --- a/experimental/packages/exporter-trace-otlp-grpc/test/OTLPTraceExporter.test.ts +++ b/experimental/packages/exporter-trace-otlp-grpc/test/OTLPTraceExporter.test.ts @@ -202,12 +202,12 @@ const testCollectorExporter = (params: TestParams) => }); it('should log deadline exceeded error', done => { const credentials = params.useTLS - ? grpc.credentials.createSsl( + ? grpc.credentials.createSsl( fs.readFileSync('./test/certs/ca.crt'), fs.readFileSync('./test/certs/client.key'), fs.readFileSync('./test/certs/client.crt') ) - : undefined; + : undefined; const collectorExporterWithTimeout = new OTLPTraceExporter({ url: 'grpcs://' + address, diff --git a/experimental/packages/otlp-exporter-base/test/browser/util.test.ts b/experimental/packages/otlp-exporter-base/test/browser/util.test.ts index 8238b4ddec..e72b8618f5 100644 --- a/experimental/packages/otlp-exporter-base/test/browser/util.test.ts +++ b/experimental/packages/otlp-exporter-base/test/browser/util.test.ts @@ -55,8 +55,8 @@ describe('util - browser', () => { describe('and Content-Type header is set', () => { beforeEach(()=>{ const explicitContentType = { - 'Content-Type': 'application/json', - }; + 'Content-Type': 'application/json', + }; const exporterTimeout = 10000; sendWithXhr(body, url, explicitContentType, exporterTimeout, onSuccessStub, onErrorStub); }); From fbaa4574d10c3f1bbc1fbeaedb63eb67633556b4 Mon Sep 17 00:00:00 2001 From: Svetlana Brennan Date: Thu, 5 May 2022 11:53:45 -0500 Subject: [PATCH 41/46] feat(otlp-exporter): fix accidentally removed code after main merge Signed-off-by: Svetlana Brennan --- .../browser/CollectorTraceExporter.test.ts | 2 +- .../test/node/CollectorTraceExporter.test.ts | 4 +- .../test/OTLPTraceExporter.test.ts | 4 +- .../test/node/CollectorMetricExporter.test.ts | 7 +- .../test/OTLPMetricExporter.test.ts | 68 +++++++++---------- 5 files changed, 43 insertions(+), 42 deletions(-) diff --git a/experimental/packages/exporter-trace-otlp-http/test/browser/CollectorTraceExporter.test.ts b/experimental/packages/exporter-trace-otlp-http/test/browser/CollectorTraceExporter.test.ts index 01288cc8c2..0f64268fb8 100644 --- a/experimental/packages/exporter-trace-otlp-http/test/browser/CollectorTraceExporter.test.ts +++ b/experimental/packages/exporter-trace-otlp-http/test/browser/CollectorTraceExporter.test.ts @@ -29,7 +29,7 @@ import { ensureHeadersContain, mockedReadableSpan, } from '../traceHelper'; -import { OTLPExporterConfigBase } from '@opentelemetry/otlp-exporter-base'; +import { OTLPExporterConfigBase, OTLPExporterError } from '@opentelemetry/otlp-exporter-base'; import { IExportTraceServiceRequest } from '@opentelemetry/otlp-transformer'; describe('OTLPTraceExporter - web', () => { diff --git a/experimental/packages/exporter-trace-otlp-http/test/node/CollectorTraceExporter.test.ts b/experimental/packages/exporter-trace-otlp-http/test/node/CollectorTraceExporter.test.ts index faf9f2b174..c8c8730cd7 100644 --- a/experimental/packages/exporter-trace-otlp-http/test/node/CollectorTraceExporter.test.ts +++ b/experimental/packages/exporter-trace-otlp-http/test/node/CollectorTraceExporter.test.ts @@ -508,8 +508,8 @@ describe('OTLPTraceExporter - node with json over http', () => { beforeEach(() => { server.listen(8080); }); - afterEach(() => { - server.close(); + afterEach(done => { + server.close(done); }); it('should log the timeout request error message', done => { collectorExporterConfig = { diff --git a/experimental/packages/exporter-trace-otlp-proto/test/OTLPTraceExporter.test.ts b/experimental/packages/exporter-trace-otlp-proto/test/OTLPTraceExporter.test.ts index 124f1a1524..b3930a240e 100644 --- a/experimental/packages/exporter-trace-otlp-proto/test/OTLPTraceExporter.test.ts +++ b/experimental/packages/exporter-trace-otlp-proto/test/OTLPTraceExporter.test.ts @@ -329,8 +329,8 @@ describe('OTLPTraceExporter - node with proto over http', () => { beforeEach(() => { server.listen(8080); }); - afterEach(() => { - server.close(); + afterEach(done => { + server.close(done); }); it('should log the timeout request error message', done => { collectorExporterConfig = { diff --git a/experimental/packages/opentelemetry-exporter-metrics-otlp-http/test/node/CollectorMetricExporter.test.ts b/experimental/packages/opentelemetry-exporter-metrics-otlp-http/test/node/CollectorMetricExporter.test.ts index 92c9592563..4512a36474 100644 --- a/experimental/packages/opentelemetry-exporter-metrics-otlp-http/test/node/CollectorMetricExporter.test.ts +++ b/experimental/packages/opentelemetry-exporter-metrics-otlp-http/test/node/CollectorMetricExporter.test.ts @@ -236,12 +236,13 @@ describe('OTLPMetricExporter - node with json over http', () => { }); fakeRequest.on('end', () => { - const writeArgs = stubWrite.args[0]; - const json = JSON.parse(writeArgs[0]) as IExportMetricsServiceRequest; + const responseBody = buff.toString(); + + const json = JSON.parse(responseBody) as IExportMetricsServiceRequest; const metric1 = json.resourceMetrics[0].scopeMetrics[0].metrics[0]; const metric2 = json.resourceMetrics[0].scopeMetrics[0].metrics[1]; const metric3 = json.resourceMetrics[0].scopeMetrics[0].metrics[2]; - + assert.ok(typeof metric1 !== 'undefined', "counter doesn't exist"); ensureCounterIsCorrect( metric1, diff --git a/experimental/packages/opentelemetry-exporter-metrics-otlp-proto/test/OTLPMetricExporter.test.ts b/experimental/packages/opentelemetry-exporter-metrics-otlp-proto/test/OTLPMetricExporter.test.ts index 6e558bdb0f..44495e023e 100644 --- a/experimental/packages/opentelemetry-exporter-metrics-otlp-proto/test/OTLPMetricExporter.test.ts +++ b/experimental/packages/opentelemetry-exporter-metrics-otlp-proto/test/OTLPMetricExporter.test.ts @@ -190,40 +190,40 @@ describe('OTLPMetricExporter - node with proto over http', () => { let buff = Buffer.from(''); fakeRequest.on('end', () => { - const ExportTraceServiceRequestProto = getExportRequestProto(); - const data = ExportTraceServiceRequestProto?.decode(writeArgs[0]); - const json = data?.toJSON() as IExportMetricsServiceRequest; - - const metric1 = json.resourceMetrics[0].scopeMetrics[0].metrics[0]; - const metric2 = json.resourceMetrics[0].scopeMetrics[0].metrics[1]; - const metric3 = json.resourceMetrics[0].scopeMetrics[0].metrics[2]; - - assert.ok(typeof metric1 !== 'undefined', "counter doesn't exist"); - ensureExportedCounterIsCorrect( - metric1, - metric1.sum?.dataPoints[0].timeUnixNano, - metric1.sum?.dataPoints[0].startTimeUnixNano - ); - assert.ok(typeof metric2 !== 'undefined', "observable gauge doesn't exist"); - ensureExportedObservableGaugeIsCorrect( - metric2, - metric2.gauge?.dataPoints[0].timeUnixNano, - metric2.gauge?.dataPoints[0].startTimeUnixNano - ); - assert.ok( - typeof metric3 !== 'undefined', - "value recorder doesn't exist" - ); - ensureExportedHistogramIsCorrect( - metric3, - metric3.histogram?.dataPoints[0].timeUnixNano, - metric3.histogram?.dataPoints[0].startTimeUnixNano, - [0, 100], - ['0', '2', '0'] - ); - - ensureExportMetricsServiceRequestIsSet(json); - done(); + const ExportTraceServiceRequestProto = getExportRequestProto(); + const data = ExportTraceServiceRequestProto?.decode(buff); + const json = data?.toJSON() as IExportMetricsServiceRequest; + + const metric1 = json.resourceMetrics[0].scopeMetrics[0].metrics[0]; + const metric2 = json.resourceMetrics[0].scopeMetrics[0].metrics[1]; + const metric3 = json.resourceMetrics[0].scopeMetrics[0].metrics[2]; + + assert.ok(typeof metric1 !== 'undefined', "counter doesn't exist"); + ensureExportedCounterIsCorrect( + metric1, + metric1.sum?.dataPoints[0].timeUnixNano, + metric1.sum?.dataPoints[0].startTimeUnixNano + ); + assert.ok(typeof metric2 !== 'undefined', "observable gauge doesn't exist"); + ensureExportedObservableGaugeIsCorrect( + metric2, + metric2.gauge?.dataPoints[0].timeUnixNano, + metric2.gauge?.dataPoints[0].startTimeUnixNano + ); + assert.ok( + typeof metric3 !== 'undefined', + "value recorder doesn't exist" + ); + ensureExportedHistogramIsCorrect( + metric3, + metric3.histogram?.dataPoints[0].timeUnixNano, + metric3.histogram?.dataPoints[0].startTimeUnixNano, + [0, 100], + ['0', '2', '0'] + ); + + ensureExportMetricsServiceRequestIsSet(json); + done(); }); fakeRequest.on('data', chunk => { From 14ba7d321f093e4b179e7ca39cdff50319e1be5e Mon Sep 17 00:00:00 2001 From: Svetlana Brennan Date: Mon, 9 May 2022 15:42:03 -0500 Subject: [PATCH 42/46] feat(otlp-exporter): replace nextTick with queueMicroTask for browser tests Signed-off-by: Svetlana Brennan --- .../test/browser/CollectorTraceExporter.test.ts | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/experimental/packages/exporter-trace-otlp-http/test/browser/CollectorTraceExporter.test.ts b/experimental/packages/exporter-trace-otlp-http/test/browser/CollectorTraceExporter.test.ts index 0f64268fb8..7970005c08 100644 --- a/experimental/packages/exporter-trace-otlp-http/test/browser/CollectorTraceExporter.test.ts +++ b/experimental/packages/exporter-trace-otlp-http/test/browser/CollectorTraceExporter.test.ts @@ -21,7 +21,6 @@ import { ReadableSpan } from '@opentelemetry/sdk-trace-base'; import * as assert from 'assert'; import * as sinon from 'sinon'; import { OTLPTraceExporter } from '../../src/platform/browser/index'; -import { nextTick } from 'process'; import { ensureSpanIsCorrect, ensureExportTraceServiceRequestIsSet, @@ -197,7 +196,7 @@ describe('OTLPTraceExporter - web', () => { collectorTraceExporter.export(spans, () => { }); - nextTick(() => { + queueMicrotask(() => { const request = server.requests[0]; assert.strictEqual(request.method, 'POST'); assert.strictEqual(request.url, 'http://foo.bar.com'); @@ -242,7 +241,7 @@ describe('OTLPTraceExporter - web', () => { collectorTraceExporter.export(spans, () => { }); - nextTick(() => { + queueMicrotask(() => { const request = server.requests[0]; request.respond(200); const response: any = spyLoggerDebug.args[2][0]; @@ -262,7 +261,7 @@ describe('OTLPTraceExporter - web', () => { done(); }); - nextTick(() => { + queueMicrotask(() => { const request = server.requests[0]; request.respond(400); clock.restore(); @@ -274,7 +273,7 @@ describe('OTLPTraceExporter - web', () => { collectorTraceExporter.export(spans, () => { }); - nextTick(() => { + queueMicrotask(() => { const request = server.requests[0]; request.respond(200); @@ -396,7 +395,7 @@ describe('OTLPTraceExporter - web', () => { collectorTraceExporter.export(spans, () => { }); - nextTick(() => { + queueMicrotask(() => { const [{ requestHeaders }] = server.requests; ensureHeadersContain(requestHeaders, customHeaders); @@ -426,7 +425,7 @@ describe('OTLPTraceExporter - web', () => { collectorTraceExporter.export(spans, () => { }); - nextTick(() => { + queueMicrotask(() => { const [{ requestHeaders }] = server.requests; ensureHeadersContain(requestHeaders, customHeaders); @@ -441,15 +440,15 @@ describe('OTLPTraceExporter - web', () => { const responseSpy = sinon.spy(); collectorTraceExporter.export(spans, responseSpy); clock.tick(10000); + clock.restore(); - nextTick(() => { + setTimeout(() => { const result = responseSpy.args[0][0] as core.ExportResult; assert.strictEqual(result.code, core.ExportResultCode.FAILED); const error = result.error as OTLPExporterError; assert.ok(error !== undefined); assert.strictEqual(error.message, 'Request Timeout'); - clock.restore(); done(); }); }); From 1ebc461396f53b5413630f679b810bde5aba03c9 Mon Sep 17 00:00:00 2001 From: Svetlana Brennan Date: Mon, 9 May 2022 15:47:47 -0500 Subject: [PATCH 43/46] feat(otlp-exporter): add more details to changelog item Signed-off-by: Svetlana Brennan --- experimental/CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/experimental/CHANGELOG.md b/experimental/CHANGELOG.md index 80e3761738..54870e2574 100644 --- a/experimental/CHANGELOG.md +++ b/experimental/CHANGELOG.md @@ -60,7 +60,7 @@ All notable changes to experimental packages in this project will be documented * feat(proto): add @opentelemetry/otlp-transformer package with hand-rolled transformation #2746 @dyladan * feat(sdk-metrics-base): shutdown and forceflush on MeterProvider #2890 @legendecas * feat(sdk-metrics-base): return the same meter for identical input to getMeter #2901 @legendecas -* feat(otlp-exporter): add timeout env var #2738 @svetlanabrennan +* feat(otlp-exporter): add [OTEL_EXPORTER_OTLP_TIMEOUT](https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/protocol/exporter.md#configuration-options) env var to otlp exporters #2738 @svetlanabrennan ### :bug: (Bug Fix) From 511999a66cede9982440bfe0bf3a027f04de3242 Mon Sep 17 00:00:00 2001 From: Svetlana Brennan Date: Tue, 10 May 2022 10:35:44 -0500 Subject: [PATCH 44/46] feat(otlp-exporter): fix server timing issue in real http request tests with settimeout Signed-off-by: Svetlana Brennan --- .../test/node/CollectorTraceExporter.test.ts | 48 +++++++++++-------- 1 file changed, 27 insertions(+), 21 deletions(-) diff --git a/experimental/packages/exporter-trace-otlp-http/test/node/CollectorTraceExporter.test.ts b/experimental/packages/exporter-trace-otlp-http/test/node/CollectorTraceExporter.test.ts index c8c8730cd7..8c55a0cfc0 100644 --- a/experimental/packages/exporter-trace-otlp-http/test/node/CollectorTraceExporter.test.ts +++ b/experimental/packages/exporter-trace-otlp-http/test/node/CollectorTraceExporter.test.ts @@ -475,13 +475,15 @@ describe('OTLPTraceExporter - node with json over http', () => { spans = []; spans.push(Object.assign({}, mockedReadableSpan)); - collectorExporter.export(spans, result => { - assert.strictEqual(result.code, core.ExportResultCode.FAILED); - const error = result.error as OTLPExporterError; - assert.ok(error !== undefined); - assert.strictEqual(error.message, 'Request Timeout'); - done(); - }); + setTimeout(() => { + collectorExporter.export(spans, result => { + assert.strictEqual(result.code, core.ExportResultCode.FAILED); + const error = result.error as OTLPExporterError; + assert.ok(error !== undefined); + assert.strictEqual(error.message, 'Request Timeout'); + done(); + }); + }, 0); }); it('should log the timeout request error message when timeout is 100', done => { collectorExporterConfig = { @@ -492,13 +494,15 @@ describe('OTLPTraceExporter - node with json over http', () => { spans = []; spans.push(Object.assign({}, mockedReadableSpan)); - collectorExporter.export(spans, result => { - assert.strictEqual(result.code, core.ExportResultCode.FAILED); - const error = result.error as OTLPExporterError; - assert.ok(error !== undefined); - assert.strictEqual(error.message, 'Request Timeout'); - done(); - }); + setTimeout(() => { + collectorExporter.export(spans, result => { + assert.strictEqual(result.code, core.ExportResultCode.FAILED); + const error = result.error as OTLPExporterError; + assert.ok(error !== undefined); + assert.strictEqual(error.message, 'Request Timeout'); + done(); + }); + }, 0); }); }); describe('export - real http request destroyed after response received', () => { @@ -520,13 +524,15 @@ describe('OTLPTraceExporter - node with json over http', () => { spans = []; spans.push(Object.assign({}, mockedReadableSpan)); - collectorExporter.export(spans, result => { - assert.strictEqual(result.code, core.ExportResultCode.FAILED); - const error = result.error as OTLPExporterError; - assert.ok(error !== undefined); - assert.strictEqual(error.message, 'Request Timeout'); - done(); - }); + setTimeout(() => { + collectorExporter.export(spans, result => { + assert.strictEqual(result.code, core.ExportResultCode.FAILED); + const error = result.error as OTLPExporterError; + assert.ok(error !== undefined); + assert.strictEqual(error.message, 'Request Timeout'); + done(); + }); + }, 0); }); }); }); From b553759a732123dd9d17e4d0ef07aee45afdc047 Mon Sep 17 00:00:00 2001 From: Svetlana Brennan Date: Tue, 10 May 2022 15:48:30 -0500 Subject: [PATCH 45/46] feat(otlp-exporter): fix real http request tests Signed-off-by: Svetlana Brennan --- .../test/node/CollectorTraceExporter.test.ts | 164 ++++++++++-------- .../test/OTLPTraceExporter.test.ts | 146 ++++++++-------- 2 files changed, 165 insertions(+), 145 deletions(-) diff --git a/experimental/packages/exporter-trace-otlp-http/test/node/CollectorTraceExporter.test.ts b/experimental/packages/exporter-trace-otlp-http/test/node/CollectorTraceExporter.test.ts index 8c55a0cfc0..3e9da9370e 100644 --- a/experimental/packages/exporter-trace-otlp-http/test/node/CollectorTraceExporter.test.ts +++ b/experimental/packages/exporter-trace-otlp-http/test/node/CollectorTraceExporter.test.ts @@ -453,86 +453,96 @@ describe('OTLPTraceExporter - node with json over http', () => { }, 300); }); }); - describe('export - real http request destroyed before response received', () => { - const server = http.createServer((_, res) => { - setTimeout(() => { - res.statusCode = 200; - res.end(); - }, 200); - }); - beforeEach(() => { - server.listen(8080); - }); - afterEach(done => { - server.close(done); - }); - it('should log the timeout request error message when timeout is 1', done => { - collectorExporterConfig = { - url: 'http://localhost:8080', - timeoutMillis: 1, - }; - collectorExporter = new OTLPTraceExporter(collectorExporterConfig); - spans = []; - spans.push(Object.assign({}, mockedReadableSpan)); +}); - setTimeout(() => { - collectorExporter.export(spans, result => { - assert.strictEqual(result.code, core.ExportResultCode.FAILED); - const error = result.error as OTLPExporterError; - assert.ok(error !== undefined); - assert.strictEqual(error.message, 'Request Timeout'); - done(); - }); - }, 0); - }); - it('should log the timeout request error message when timeout is 100', done => { - collectorExporterConfig = { - url: 'http://localhost:8080', - timeoutMillis: 100, - }; - collectorExporter = new OTLPTraceExporter(collectorExporterConfig); - spans = []; - spans.push(Object.assign({}, mockedReadableSpan)); +describe('export - real http request destroyed before response received', () => { + let collectorExporter: OTLPTraceExporter; + let collectorExporterConfig: OTLPExporterNodeConfigBase; + let spans: ReadableSpan[]; - setTimeout(() => { - collectorExporter.export(spans, result => { - assert.strictEqual(result.code, core.ExportResultCode.FAILED); - const error = result.error as OTLPExporterError; - assert.ok(error !== undefined); - assert.strictEqual(error.message, 'Request Timeout'); - done(); - }); - }, 0); - }); + const server = http.createServer((_, res) => { + setTimeout(() => { + res.statusCode = 200; + res.end(); + }, 200); }); - describe('export - real http request destroyed after response received', () => { - const server = http.createServer((_, res) => { - res.write('writing something'); - }); - beforeEach(() => { - server.listen(8080); - }); - afterEach(done => { - server.close(done); - }); - it('should log the timeout request error message', done => { - collectorExporterConfig = { - url: 'http://localhost:8080', - timeoutMillis: 300, - }; - collectorExporter = new OTLPTraceExporter(collectorExporterConfig); - spans = []; - spans.push(Object.assign({}, mockedReadableSpan)); + before(done => { + server.listen(8081, done); + }); + after(done => { + server.close(done); + }); + it('should log the timeout request error message when timeout is 1', done => { + collectorExporterConfig = { + url: 'http://localhost:8081', + timeoutMillis: 1, + }; + collectorExporter = new OTLPTraceExporter(collectorExporterConfig); + spans = []; + spans.push(Object.assign({}, mockedReadableSpan)); + + setTimeout(() => { + collectorExporter.export(spans, result => { + assert.strictEqual(result.code, core.ExportResultCode.FAILED); + const error = result.error as OTLPExporterError; + assert.ok(error !== undefined); + assert.strictEqual(error.message, 'Request Timeout'); + done(); + }); + }, 0); + }); + it('should log the timeout request error message when timeout is 100', done => { + collectorExporterConfig = { + url: 'http://localhost:8081', + timeoutMillis: 100, + }; + collectorExporter = new OTLPTraceExporter(collectorExporterConfig); + spans = []; + spans.push(Object.assign({}, mockedReadableSpan)); + + setTimeout(() => { + collectorExporter.export(spans, result => { + assert.strictEqual(result.code, core.ExportResultCode.FAILED); + const error = result.error as OTLPExporterError; + assert.ok(error !== undefined); + assert.strictEqual(error.message, 'Request Timeout'); + done(); + }); + }, 0); + }); +}); - setTimeout(() => { - collectorExporter.export(spans, result => { - assert.strictEqual(result.code, core.ExportResultCode.FAILED); - const error = result.error as OTLPExporterError; - assert.ok(error !== undefined); - assert.strictEqual(error.message, 'Request Timeout'); - done(); - }); - }, 0); - }); +describe('export - real http request destroyed after response received', () => { + let collectorExporter: OTLPTraceExporter; + let collectorExporterConfig: OTLPExporterNodeConfigBase; + let spans: ReadableSpan[]; + + const server = http.createServer((_, res) => { + res.write('writing something'); + }); + before(done => { + server.listen(8081, done); + }); + after(done => { + server.close(done); + }); + it('should log the timeout request error message', done => { + collectorExporterConfig = { + url: 'http://localhost:8081', + timeoutMillis: 300, + }; + collectorExporter = new OTLPTraceExporter(collectorExporterConfig); + spans = []; + spans.push(Object.assign({}, mockedReadableSpan)); + + setTimeout(() => { + collectorExporter.export(spans, result => { + assert.strictEqual(result.code, core.ExportResultCode.FAILED); + const error = result.error as OTLPExporterError; + assert.ok(error !== undefined); + assert.strictEqual(error.message, 'Request Timeout'); + done(); + }); + }, 0); }); }); diff --git a/experimental/packages/exporter-trace-otlp-proto/test/OTLPTraceExporter.test.ts b/experimental/packages/exporter-trace-otlp-proto/test/OTLPTraceExporter.test.ts index b3930a240e..e68a02e762 100644 --- a/experimental/packages/exporter-trace-otlp-proto/test/OTLPTraceExporter.test.ts +++ b/experimental/packages/exporter-trace-otlp-proto/test/OTLPTraceExporter.test.ts @@ -274,80 +274,90 @@ describe('OTLPTraceExporter - node with proto over http', () => { clock.restore(); }); }); - describe('export - real http request destroyed before response received', () => { - const server = http.createServer((_, res) => { - setTimeout(() => { - res.statusCode = 200; - res.end(); - }, 200); - }); - beforeEach(() => { - server.listen(8080); - }); - afterEach(done => { - server.close(done); - }); - it('should log the timeout request error message when timeout is 1', done => { - collectorExporterConfig = { - url: 'http://localhost:8080', - timeoutMillis: 1, - }; - collectorExporter = new OTLPTraceExporter(collectorExporterConfig); - spans = []; - spans.push(Object.assign({}, mockedReadableSpan)); +}); - collectorExporter.export(spans, result => { - assert.strictEqual(result.code, ExportResultCode.FAILED); - const error = result.error as OTLPExporterError; - assert.ok(error !== undefined); - assert.strictEqual(error.message, 'Request Timeout'); - done(); - }); - }); - it('should log the timeout request error message when timeout is 100', done => { - collectorExporterConfig = { - url: 'http://localhost:8080', - timeoutMillis: 100, - }; - collectorExporter = new OTLPTraceExporter(collectorExporterConfig); - spans = []; - spans.push(Object.assign({}, mockedReadableSpan)); - collectorExporter.export(spans, result => { - assert.strictEqual(result.code, ExportResultCode.FAILED); - const error = result.error as OTLPExporterError; - assert.ok(error !== undefined); - assert.strictEqual(error.message, 'Request Timeout'); - done(); - }); - }); +describe('export - real http request destroyed before response received', () => { + let collectorExporter: OTLPTraceExporter; + let collectorExporterConfig: OTLPExporterNodeConfigBase; + let spans: ReadableSpan[]; + const server = http.createServer((_, res) => { + setTimeout(() => { + res.statusCode = 200; + res.end(); + }, 200); }); - describe('export - real http request destroyed after response received', () => { - const server = http.createServer((_, res) => { - res.write('writing something'); - }); - beforeEach(() => { - server.listen(8080); + before(done => { + server.listen(8080, done); + }); + after(done => { + server.close(done); + }); + it('should log the timeout request error message when timeout is 1', done => { + collectorExporterConfig = { + url: 'http://localhost:8080', + timeoutMillis: 1, + }; + collectorExporter = new OTLPTraceExporter(collectorExporterConfig); + spans = []; + spans.push(Object.assign({}, mockedReadableSpan)); + + collectorExporter.export(spans, result => { + assert.strictEqual(result.code, ExportResultCode.FAILED); + const error = result.error as OTLPExporterError; + assert.ok(error !== undefined); + assert.strictEqual(error.message, 'Request Timeout'); + done(); }); - afterEach(done => { - server.close(done); + }); + it('should log the timeout request error message when timeout is 100', done => { + collectorExporterConfig = { + url: 'http://localhost:8080', + timeoutMillis: 100, + }; + collectorExporter = new OTLPTraceExporter(collectorExporterConfig); + spans = []; + spans.push(Object.assign({}, mockedReadableSpan)); + + collectorExporter.export(spans, result => { + assert.strictEqual(result.code, ExportResultCode.FAILED); + const error = result.error as OTLPExporterError; + assert.ok(error !== undefined); + assert.strictEqual(error.message, 'Request Timeout'); + done(); }); - it('should log the timeout request error message', done => { - collectorExporterConfig = { - url: 'http://localhost:8080', - timeoutMillis: 300, - }; - collectorExporter = new OTLPTraceExporter(collectorExporterConfig); - spans = []; - spans.push(Object.assign({}, mockedReadableSpan)); + }); +}); - collectorExporter.export(spans, result => { - assert.strictEqual(result.code, ExportResultCode.FAILED); - const error = result.error as OTLPExporterError; - assert.ok(error !== undefined); - assert.strictEqual(error.message, 'Request Timeout'); - done(); - }); +describe('export - real http request destroyed after response received', () => { + let collectorExporter: OTLPTraceExporter; + let collectorExporterConfig: OTLPExporterNodeConfigBase; + let spans: ReadableSpan[]; + + const server = http.createServer((_, res) => { + res.write('writing something'); + }); + before(done => { + server.listen(8080, done); + }); + after(done => { + server.close(done); + }); + it('should log the timeout request error message', done => { + collectorExporterConfig = { + url: 'http://localhost:8080', + timeoutMillis: 300, + }; + collectorExporter = new OTLPTraceExporter(collectorExporterConfig); + spans = []; + spans.push(Object.assign({}, mockedReadableSpan)); + + collectorExporter.export(spans, result => { + assert.strictEqual(result.code, ExportResultCode.FAILED); + const error = result.error as OTLPExporterError; + assert.ok(error !== undefined); + assert.strictEqual(error.message, 'Request Timeout'); + done(); }); }); }); From 5da6ce22dcb4f03339ec51f10b96458bd91d81cd Mon Sep 17 00:00:00 2001 From: Svetlana Brennan Date: Wed, 11 May 2022 11:26:48 -0500 Subject: [PATCH 46/46] feat(otlp-exporter): add comment to two ways of destroying request Signed-off-by: Svetlana Brennan --- .../packages/otlp-exporter-base/src/platform/node/util.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/experimental/packages/otlp-exporter-base/src/platform/node/util.ts b/experimental/packages/otlp-exporter-base/src/platform/node/util.ts index 18346eb5e5..f5b391866e 100644 --- a/experimental/packages/otlp-exporter-base/src/platform/node/util.ts +++ b/experimental/packages/otlp-exporter-base/src/platform/node/util.ts @@ -49,6 +49,7 @@ export function sendWithHttp( const exporterTimer = setTimeout(() => { reqIsDestroyed = true; + // req.abort() was deprecated since v14 if (nodeVersion >= 14) { req.destroy(); } else {