feat(sdk-logs)!: do not read environment variables from window (#5472)

Author: pichlermarc

experimental/CHANGELOG.md

@@ -16,6 +16,9 @@ All notable changes to experimental packages in this project will be documented
 * chore!: Raise the minimum supported Node.js version to `^18.19.0 || >=20.6.0`. Support for Node.js 14, 16, and early minor versions of 18 and 20 have been dropped. This applies to all packages except the 'api' and 'semantic-conventions' packages. [#5395](https://github.com/open-telemetry/opentelemetry-js/issues/5395) @trentm
 * feat(sdk-node)!: use `IMetricReader` over `MetricReader` [#5311](https://github.com/open-telemetry/opentelemetry-js/pull/5311)
   * (user-facing): `NodeSDKConfiguration` now provides the more general `IMetricReader` type over `MetricReader`
+* feat(sdk-logs)!: do not read environment variables from window in browsers [#5472](https://github.com/open-telemetry/opentelemetry-js/pull/5472) @pichlermarc
+  * (user-facing): all configuration previously possible via `window.OTEL_*` is now not supported anymore, please pass configuration options to constructors instead.
+    * Note: Node.js environment variable configuration continues to work as-is.
 
 ### :rocket: (Enhancement)
 

experimental/packages/sdk-logs/karma.conf.js

@@ -1,11 +1,11 @@
-/*!
+/*
  * 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
  *
- *      http://www.apache.org/licenses/LICENSE-2.0
+ *      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,
@@ -21,6 +21,8 @@ module.exports = config => {
   config.set(
     Object.assign({}, karmaBaseConfig, {
       webpack: karmaWebpackConfig,
+      files: ['test/browser/index-webpack.ts'],
+      preprocessors: { 'test/browser/index-webpack.ts': ['webpack'] },
     })
   );
 };

experimental/packages/sdk-logs/src/config.ts

@@ -14,21 +14,18 @@
  * limitations under the License.
  */
 
-import {
-  DEFAULT_ATTRIBUTE_COUNT_LIMIT,
-  DEFAULT_ATTRIBUTE_VALUE_LENGTH_LIMIT,
-  getEnv,
-  getEnvWithoutDefaults,
-} from '@opentelemetry/core';
+import { getNumberFromEnv } from '@opentelemetry/core';
 import { LogRecordLimits } from './types';
 
 export function loadDefaultConfig() {
   return {
     forceFlushTimeoutMillis: 30000,
     logRecordLimits: {
       attributeValueLengthLimit:
-        getEnv().OTEL_LOGRECORD_ATTRIBUTE_VALUE_LENGTH_LIMIT,
-      attributeCountLimit: getEnv().OTEL_LOGRECORD_ATTRIBUTE_COUNT_LIMIT,
+        getNumberFromEnv('OTEL_LOGRECORD_ATTRIBUTE_VALUE_LENGTH_LIMIT') ??
+        Infinity,
+      attributeCountLimit:
+        getNumberFromEnv('OTEL_LOGRECORD_ATTRIBUTE_COUNT_LIMIT') ?? 128,
     },
     includeTraceContext: true,
   };
@@ -42,24 +39,22 @@ export function loadDefaultConfig() {
 export function reconfigureLimits(
   logRecordLimits: LogRecordLimits
 ): Required<LogRecordLimits> {
-  const parsedEnvConfig = getEnvWithoutDefaults();
-
   return {
     /**
      * Reassign log record attribute count limit to use first non null value defined by user or use default value
      */
     attributeCountLimit:
       logRecordLimits.attributeCountLimit ??
-      parsedEnvConfig.OTEL_LOGRECORD_ATTRIBUTE_COUNT_LIMIT ??
-      parsedEnvConfig.OTEL_ATTRIBUTE_COUNT_LIMIT ??
-      DEFAULT_ATTRIBUTE_COUNT_LIMIT,
+      getNumberFromEnv('OTEL_LOGRECORD_ATTRIBUTE_COUNT_LIMIT') ??
+      getNumberFromEnv('OTEL_ATTRIBUTE_COUNT_LIMIT') ??
+      128,
     /**
      * Reassign log record attribute value length limit to use first non null value defined by user or use default value
      */
     attributeValueLengthLimit:
       logRecordLimits.attributeValueLengthLimit ??
-      parsedEnvConfig.OTEL_LOGRECORD_ATTRIBUTE_VALUE_LENGTH_LIMIT ??
-      parsedEnvConfig.OTEL_ATTRIBUTE_VALUE_LENGTH_LIMIT ??
-      DEFAULT_ATTRIBUTE_VALUE_LENGTH_LIMIT,
+      getNumberFromEnv('OTEL_LOGRECORD_ATTRIBUTE_VALUE_LENGTH_LIMIT') ??
+      getNumberFromEnv('OTEL_ATTRIBUTE_VALUE_LENGTH_LIMIT') ??
+      Infinity,
   };
 }

experimental/packages/sdk-logs/src/export/BatchLogRecordProcessorBase.ts

@@ -14,11 +14,10 @@
  * limitations under the License.
  */
 
-import type { ExportResult } from '@opentelemetry/core';
+import { ExportResult, getNumberFromEnv } from '@opentelemetry/core';
 import { diag } from '@opentelemetry/api';
 import {
   ExportResultCode,
-  getEnv,
   globalErrorHandler,
   unrefTimer,
   BindOnceFuture,
@@ -47,14 +46,22 @@ export abstract class BatchLogRecordProcessorBase<T extends BufferConfig>
     private readonly _exporter: LogRecordExporter,
     config?: T
   ) {
-    const env = getEnv();
     this._maxExportBatchSize =
-      config?.maxExportBatchSize ?? env.OTEL_BLRP_MAX_EXPORT_BATCH_SIZE;
-    this._maxQueueSize = config?.maxQueueSize ?? env.OTEL_BLRP_MAX_QUEUE_SIZE;
+      config?.maxExportBatchSize ??
+      getNumberFromEnv('OTEL_BLRP_MAX_EXPORT_BATCH_SIZE') ??
+      512;
+    this._maxQueueSize =
+      config?.maxQueueSize ??
+      getNumberFromEnv('OTEL_BLRP_MAX_QUEUE_SIZE') ??
+      2048;
     this._scheduledDelayMillis =
-      config?.scheduledDelayMillis ?? env.OTEL_BLRP_SCHEDULE_DELAY;
+      config?.scheduledDelayMillis ??
+      getNumberFromEnv('OTEL_BLRP_SCHEDULE_DELAY') ??
+      5000;
     this._exportTimeoutMillis =
-      config?.exportTimeoutMillis ?? env.OTEL_BLRP_EXPORT_TIMEOUT;
+      config?.exportTimeoutMillis ??
+      getNumberFromEnv('OTEL_BLRP_EXPORT_TIMEOUT') ??
+      30000;
 
     this._shutdownOnce = new BindOnceFuture(this._shutdown, this);
 

experimental/packages/sdk-logs/test/index-webpack.ts renamed to experimental/packages/sdk-logs/test/browser/index-webpack.ts

@@ -13,8 +13,11 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
+const testsContext = require.context('../browser', true, /test$/);
+testsContext.keys().forEach(testsContext);
 
-{
-  const testsContext = require.context('./', true, /test$/);
-  testsContext.keys().forEach(testsContext);
-}
+const testsContextCommon = require.context('../common', true, /test$/);
+testsContextCommon.keys().forEach(testsContextCommon);
+
+const srcContext = require.context('.', true, /src$/);
+srcContext.keys().forEach(srcContext);

experimental/packages/sdk-logs/test/index-webpack.worker.ts renamed to experimental/packages/sdk-logs/test/browser/index-webpack.worker.ts

@@ -13,8 +13,11 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
+const testsContext = require.context('../browser', true, /test$/);
+testsContext.keys().forEach(testsContext);
 
-{
-  const testsContext = require.context('./', true, /test$/);
-  testsContext.keys().forEach(testsContext);
-}
+const testsContextCommon = require.context('../common', true, /test$/);
+testsContextCommon.keys().forEach(testsContextCommon);
+
+const srcContext = require.context('.', true, /src$/);
+srcContext.keys().forEach(srcContext);

experimental/packages/sdk-logs/test/common/LoggerProvider.test.ts

@@ -29,14 +29,6 @@ import { MultiLogRecordProcessor } from '../../src/MultiLogRecordProcessor';
 import { Logger } from '../../src/Logger';
 
 describe('LoggerProvider', () => {
-  let envSource: Record<string, any>;
-
-  if (global.process?.versions?.node === undefined) {
-    envSource = globalThis as unknown as Record<string, any>;
-  } else {
-    envSource = process.env as Record<string, any>;
-  }
-
   beforeEach(() => {
     // to avoid actually registering the LoggerProvider and leaking env to other tests
     sinon.stub(logs, 'setGlobalLoggerProvider');
@@ -134,21 +126,7 @@ describe('LoggerProvider', () => {
         });
       });
 
-      describe('when attribute value length limit is defined via env', () => {
-        it('should have attribute value length limit as default of Infinity', () => {
-          envSource.OTEL_LOGRECORD_ATTRIBUTE_VALUE_LENGTH_LIMIT = 'Infinity';
-          const loggerProvider = new LoggerProvider();
-          const logRecordLimits =
-            loggerProvider['_sharedState'].logRecordLimits;
-          assert.strictEqual(
-            logRecordLimits.attributeValueLengthLimit,
-            Infinity
-          );
-          delete envSource.OTEL_LOGRECORD_ATTRIBUTE_VALUE_LENGTH_LIMIT;
-        });
-      });
-
-      describe('when attribute value length limit is not defined via env', () => {
+      describe('when attribute value length limit is not defined', () => {
         it('should use default value of Infinity', () => {
           const loggerProvider = new LoggerProvider();
           const logRecordLimits =
@@ -160,26 +138,7 @@ describe('LoggerProvider', () => {
         });
       });
 
-      describe('when attribute count limit is defined via env', () => {
-        it('should have attribute count limits as defined in env', () => {
-          envSource.OTEL_LOGRECORD_ATTRIBUTE_COUNT_LIMIT = '35';
-          const loggerProvider = new LoggerProvider();
-          const logRecordLimits =
-            loggerProvider['_sharedState'].logRecordLimits;
-          assert.strictEqual(logRecordLimits.attributeCountLimit, 35);
-          delete envSource.OTEL_LOGRECORD_ATTRIBUTE_COUNT_LIMIT;
-        });
-        it('should have attribute count limit as default of 128', () => {
-          envSource.OTEL_LOGRECORD_ATTRIBUTE_COUNT_LIMIT = '128';
-          const loggerProvider = new LoggerProvider();
-          const logRecordLimits =
-            loggerProvider['_sharedState'].logRecordLimits;
-          assert.strictEqual(logRecordLimits.attributeCountLimit, 128);
-          delete envSource.OTEL_LOGRECORD_ATTRIBUTE_COUNT_LIMIT;
-        });
-      });
-
-      describe('when attribute count limit is not defined via env', () => {
+      describe('when attribute count limit is not defined', () => {
         it('should use default value of 128', () => {
           const loggerProvider = new LoggerProvider();
           const logRecordLimits =

experimental/packages/sdk-logs/test/common/export/BatchLogRecordProcessor.test.ts

@@ -18,7 +18,6 @@ import * as assert from 'assert';
 import * as sinon from 'sinon';
 import {
   ExportResultCode,
-  getEnv,
   loggingErrorHandler,
   setGlobalErrorHandler,
 } from '@opentelemetry/core';
@@ -117,26 +116,11 @@ describe('BatchLogRecordProcessorBase', () => {
     it('should create a BatchLogRecordProcessor instance with empty config', () => {
       const processor = new BatchLogRecordProcessor(exporter);
 
-      const {
-        OTEL_BSP_MAX_EXPORT_BATCH_SIZE,
-        OTEL_BSP_MAX_QUEUE_SIZE,
-        OTEL_BSP_SCHEDULE_DELAY,
-        OTEL_BSP_EXPORT_TIMEOUT,
-      } = getEnv();
       assert.ok(processor instanceof BatchLogRecordProcessor);
-      assert.strictEqual(
-        processor['_maxExportBatchSize'],
-        OTEL_BSP_MAX_EXPORT_BATCH_SIZE
-      );
-      assert.strictEqual(processor['_maxQueueSize'], OTEL_BSP_MAX_QUEUE_SIZE);
-      assert.strictEqual(
-        processor['_scheduledDelayMillis'],
-        OTEL_BSP_SCHEDULE_DELAY
-      );
-      assert.strictEqual(
-        processor['_exportTimeoutMillis'],
-        OTEL_BSP_EXPORT_TIMEOUT
-      );
+      assert.strictEqual(processor['_maxExportBatchSize'], 512);
+      assert.strictEqual(processor['_maxQueueSize'], 2048);
+      assert.strictEqual(processor['_scheduledDelayMillis'], 5000);
+      assert.strictEqual(processor['_exportTimeoutMillis'], 30000);
       processor.shutdown();
     });
 

experimental/packages/sdk-logs/test/node/LoggerProvider.test.ts

@@ -0,0 +1,68 @@
+/*
+ * 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 { logs } from '@opentelemetry/api-logs';
+import * as assert from 'assert';
+import * as sinon from 'sinon';
+
+import { LoggerProvider } from '../../src';
+
+describe('LoggerProvider - node', () => {
+  beforeEach(() => {
+    // to avoid actually registering the LoggerProvider and leaking env to other tests
+    sinon.stub(logs, 'setGlobalLoggerProvider');
+  });
+
+  afterEach(() => {
+    sinon.restore();
+  });
+
+  describe('constructor', () => {
+    describe('logRecordLimits', () => {
+      describe('when attribute value length limit is defined via env', () => {
+        it('should have attribute value length limit as default of Infinity', () => {
+          process.env.OTEL_LOGRECORD_ATTRIBUTE_VALUE_LENGTH_LIMIT = 'Infinity';
+          const loggerProvider = new LoggerProvider();
+          const logRecordLimits =
+            loggerProvider['_sharedState'].logRecordLimits;
+          assert.strictEqual(
+            logRecordLimits.attributeValueLengthLimit,
+            Infinity
+          );
+          delete process.env.OTEL_LOGRECORD_ATTRIBUTE_VALUE_LENGTH_LIMIT;
+        });
+      });
+
+      describe('when attribute count limit is defined via env', () => {
+        it('should have attribute count limits as defined in env', () => {
+          process.env.OTEL_LOGRECORD_ATTRIBUTE_COUNT_LIMIT = '35';
+          const loggerProvider = new LoggerProvider();
+          const logRecordLimits =
+            loggerProvider['_sharedState'].logRecordLimits;
+          assert.strictEqual(logRecordLimits.attributeCountLimit, 35);
+          delete process.env.OTEL_LOGRECORD_ATTRIBUTE_COUNT_LIMIT;
+        });
+        it('should have attribute count limit as default of 128', () => {
+          process.env.OTEL_LOGRECORD_ATTRIBUTE_COUNT_LIMIT = '128';
+          const loggerProvider = new LoggerProvider();
+          const logRecordLimits =
+            loggerProvider['_sharedState'].logRecordLimits;
+          assert.strictEqual(logRecordLimits.attributeCountLimit, 128);
+          delete process.env.OTEL_LOGRECORD_ATTRIBUTE_COUNT_LIMIT;
+        });
+      });
+    });
+  });
+});