From 88a8e601311742748aa2ad646c134e87ea1b9fc1 Mon Sep 17 00:00:00 2001 From: Sami Musallam Date: Mon, 5 Sep 2022 19:48:28 +0300 Subject: [PATCH 1/5] fix(param-validation): validate maxExportBatchSize in BatchSpanProcessorBase --- .../src/export/BatchSpanProcessorBase.ts | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/packages/opentelemetry-sdk-trace-base/src/export/BatchSpanProcessorBase.ts b/packages/opentelemetry-sdk-trace-base/src/export/BatchSpanProcessorBase.ts index e6417d84afc..654fe319fb4 100644 --- a/packages/opentelemetry-sdk-trace-base/src/export/BatchSpanProcessorBase.ts +++ b/packages/opentelemetry-sdk-trace-base/src/export/BatchSpanProcessorBase.ts @@ -14,7 +14,7 @@ * limitations under the License. */ -import { context, Context, TraceFlags } from '@opentelemetry/api'; +import {context, Context, diag, TraceFlags} from '@opentelemetry/api'; import { BindOnceFuture, ExportResultCode, @@ -22,6 +22,7 @@ import { globalErrorHandler, suppressTracing, unrefTimer, + DEFAULT_ENVIRONMENT } from '@opentelemetry/core'; import { Span } from '../Span'; import { SpanProcessor } from '../SpanProcessor'; @@ -63,6 +64,12 @@ export abstract class BatchSpanProcessorBase implements : env.OTEL_BSP_EXPORT_TIMEOUT; this._shutdownOnce = new BindOnceFuture(this._shutdown, this); + + if (this._maxExportBatchSize > this._maxQueueSize) { + diag.warn('BatchSpanProcessor: maxExportBatchSize must be smaller or equal to maxQueueSize, using default values'); + this._maxExportBatchSize = DEFAULT_ENVIRONMENT.OTEL_BSP_MAX_EXPORT_BATCH_SIZE; + this._maxQueueSize = DEFAULT_ENVIRONMENT.OTEL_BSP_MAX_QUEUE_SIZE; + } } forceFlush(): Promise { From 48647dca928d29811efd759cc6123a088b2f5aa1 Mon Sep 17 00:00:00 2001 From: Sami Musallam Date: Thu, 8 Sep 2022 14:44:52 +0300 Subject: [PATCH 2/5] fix(param-validation): added BatchSpanProcessorBase validation test --- .../export/BatchSpanProcessorBase.test.ts | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/packages/opentelemetry-sdk-trace-base/test/common/export/BatchSpanProcessorBase.test.ts b/packages/opentelemetry-sdk-trace-base/test/common/export/BatchSpanProcessorBase.test.ts index 8d8de40d32d..a3bb3d2cc91 100644 --- a/packages/opentelemetry-sdk-trace-base/test/common/export/BatchSpanProcessorBase.test.ts +++ b/packages/opentelemetry-sdk-trace-base/test/common/export/BatchSpanProcessorBase.test.ts @@ -19,6 +19,7 @@ import { ExportResultCode, loggingErrorHandler, setGlobalErrorHandler, + DEFAULT_ENVIRONMENT } from '@opentelemetry/core'; import * as assert from 'assert'; import * as sinon from 'sinon'; @@ -435,4 +436,22 @@ describe('BatchSpanProcessorBase', () => { }); }); }); + + describe('maxExportBatchSize', () => { + let processor: BatchSpanProcessor; + + describe('when "maxExportBatchSize" is greater than "maxQueueSize"', () => { + beforeEach(() => { + processor = new BatchSpanProcessor( + exporter,{ + maxExportBatchSize: 7, + maxQueueSize: 6, + }); + }); + it('should use default values', () => { + assert.equal(processor['_maxExportBatchSize'], DEFAULT_ENVIRONMENT.OTEL_BSP_MAX_EXPORT_BATCH_SIZE); + assert.equal(processor['_maxQueueSize'], DEFAULT_ENVIRONMENT.OTEL_BSP_MAX_QUEUE_SIZE); + }); + }); + }); }); From 6712ffeecc3cf1f95e9dec0dafae754a102fedc3 Mon Sep 17 00:00:00 2001 From: Sami Musallam Date: Sun, 11 Sep 2022 14:39:42 +0300 Subject: [PATCH 3/5] fix(param-validation): added changes to CHANGELOG.md --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7633d06998e..e4608edf9cc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,8 @@ All notable changes to this project will be documented in this file. * fix(sdk-trace-base): make span start times resistant to hrtime clock drift [#3129](https://github.com/open-telemetry/opentelemetry-js/issues/3129) +* fix(param-validation): validate maxExportBatchSize in BatchSpanProcessorBase + [#3232](https://github.com/open-telemetry/opentelemetry-js/issues/3232) ### :books: (Refine Doc) * docs(metrics): add missing metrics packages to SDK reference documentation [#3239](https://github.com/open-telemetry/opentelemetry-js/pull/3239) @dyladan From 8b24d38eaaeff9be9f148b06b7c141871961db36 Mon Sep 17 00:00:00 2001 From: Sami Musallam Date: Sun, 11 Sep 2022 14:51:06 +0300 Subject: [PATCH 4/5] fix(param-validation): updated CHANGELOG.md entry --- CHANGELOG.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e4608edf9cc..006546c101e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,8 +13,9 @@ All notable changes to this project will be documented in this file. * fix(sdk-trace-base): make span start times resistant to hrtime clock drift [#3129](https://github.com/open-telemetry/opentelemetry-js/issues/3129) -* fix(param-validation): validate maxExportBatchSize in BatchSpanProcessorBase +* fix(sdk-trace-base): validate maxExportBatchSize in BatchSpanProcessorBase [#3232](https://github.com/open-telemetry/opentelemetry-js/issues/3232) + ### :books: (Refine Doc) * docs(metrics): add missing metrics packages to SDK reference documentation [#3239](https://github.com/open-telemetry/opentelemetry-js/pull/3239) @dyladan From 8a3d40e1880fb4c8ed49982b99c340292ef3bf9f Mon Sep 17 00:00:00 2001 From: Sami Musallam Date: Thu, 15 Sep 2022 12:31:15 +0300 Subject: [PATCH 5/5] fix(param-validation): set maxQueueSize's value to maxExportBatchSize instead of using defaults --- .../src/export/BatchSpanProcessorBase.ts | 8 +++----- .../test/common/export/BatchSpanProcessorBase.test.ts | 8 +++----- 2 files changed, 6 insertions(+), 10 deletions(-) diff --git a/packages/opentelemetry-sdk-trace-base/src/export/BatchSpanProcessorBase.ts b/packages/opentelemetry-sdk-trace-base/src/export/BatchSpanProcessorBase.ts index 654fe319fb4..8978fc7d79b 100644 --- a/packages/opentelemetry-sdk-trace-base/src/export/BatchSpanProcessorBase.ts +++ b/packages/opentelemetry-sdk-trace-base/src/export/BatchSpanProcessorBase.ts @@ -21,8 +21,7 @@ import { getEnv, globalErrorHandler, suppressTracing, - unrefTimer, - DEFAULT_ENVIRONMENT + unrefTimer } from '@opentelemetry/core'; import { Span } from '../Span'; import { SpanProcessor } from '../SpanProcessor'; @@ -66,9 +65,8 @@ export abstract class BatchSpanProcessorBase implements this._shutdownOnce = new BindOnceFuture(this._shutdown, this); if (this._maxExportBatchSize > this._maxQueueSize) { - diag.warn('BatchSpanProcessor: maxExportBatchSize must be smaller or equal to maxQueueSize, using default values'); - this._maxExportBatchSize = DEFAULT_ENVIRONMENT.OTEL_BSP_MAX_EXPORT_BATCH_SIZE; - this._maxQueueSize = DEFAULT_ENVIRONMENT.OTEL_BSP_MAX_QUEUE_SIZE; + diag.warn('BatchSpanProcessor: maxExportBatchSize must be smaller or equal to maxQueueSize, setting maxExportBatchSize to match maxQueueSize'); + this._maxExportBatchSize = this._maxQueueSize; } } diff --git a/packages/opentelemetry-sdk-trace-base/test/common/export/BatchSpanProcessorBase.test.ts b/packages/opentelemetry-sdk-trace-base/test/common/export/BatchSpanProcessorBase.test.ts index a3bb3d2cc91..229e2bdb3cc 100644 --- a/packages/opentelemetry-sdk-trace-base/test/common/export/BatchSpanProcessorBase.test.ts +++ b/packages/opentelemetry-sdk-trace-base/test/common/export/BatchSpanProcessorBase.test.ts @@ -18,8 +18,7 @@ import { diag, ROOT_CONTEXT } from '@opentelemetry/api'; import { ExportResultCode, loggingErrorHandler, - setGlobalErrorHandler, - DEFAULT_ENVIRONMENT + setGlobalErrorHandler } from '@opentelemetry/core'; import * as assert from 'assert'; import * as sinon from 'sinon'; @@ -448,9 +447,8 @@ describe('BatchSpanProcessorBase', () => { maxQueueSize: 6, }); }); - it('should use default values', () => { - assert.equal(processor['_maxExportBatchSize'], DEFAULT_ENVIRONMENT.OTEL_BSP_MAX_EXPORT_BATCH_SIZE); - assert.equal(processor['_maxQueueSize'], DEFAULT_ENVIRONMENT.OTEL_BSP_MAX_QUEUE_SIZE); + it('should match maxQueueSize', () => { + assert.equal(processor['_maxExportBatchSize'], processor['_maxQueueSize']); }); }); });