feat(sdk-trace-base)!: do not read environment variables from window (#…

…5455) Co-authored-by: Trent Mick <[email protected]>
open-telemetry · Feb 17, 2025 · e4381fc · e4381fc
1 parent 1a5b57b
commit e4381fc
Show file tree

Hide file tree

Showing 15 changed files with 335 additions and 277 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -93,8 +93,12 @@ For semantic convention package changes, see the [semconv CHANGELOG](packages/se
   * (user-facing): `baggageUtils.parsePairKeyValue` was an internal utility function that was unintentionally exported. It has been removed without replacement.
   * (user-facing): `TimeOriginLegacy` has been removed without replacement.
   * (user-facing): `isAttributeKey` was an internal utility function that was unintentionally exported. It has been removed without replacement.
+* feat(sdk-trace-base)!: do not read environment variables from window in browsers [#5445](https://github.com/open-telemetry/opentelemetry-js/pull/5455) @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.
 * feat(exporter-zipkin)!: do not read environment variables from window in browsers [#5465](https://github.com/open-telemetry/opentelemetry-js/pull/5465) @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.
 * feat(resource)!: Remove resource class export in favor of functions and types only to aid in cross-version compatibility [#5421](https://github.com/open-telemetry/opentelemetry-js/pull/5421)
   * Renames `Resource` class to `ResourceImpl` and makes it package-private
   * Renames `IResource` interface to `Resource`

diff --git a/packages/opentelemetry-sdk-trace-base/karma.conf.js b/packages/opentelemetry-sdk-trace-base/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,
@@ -17,8 +17,12 @@
 const karmaWebpackConfig = require('../../karma.webpack');
 const karmaBaseConfig = require('../../karma.base');
 
-module.exports = (config) => {
-  config.set(Object.assign({}, karmaBaseConfig, {
-    webpack: karmaWebpackConfig,
-  }))
+module.exports = config => {
+  config.set(
+    Object.assign({}, karmaBaseConfig, {
+      webpack: karmaWebpackConfig,
+      files: ['test/browser/index-webpack.ts'],
+      preprocessors: { 'test/browser/index-webpack.ts': ['webpack'] },
+    })
+  );
 };
diff --git a/packages/opentelemetry-sdk-trace-base/karma.worker.js b/packages/opentelemetry-sdk-trace-base/karma.worker.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,
@@ -17,8 +17,17 @@
 const karmaWebpackConfig = require('../../karma.webpack');
 const karmaBaseConfig = require('../../karma.worker');
 
-module.exports = (config) => {
-  config.set(Object.assign({}, karmaBaseConfig, {
-    webpack: karmaWebpackConfig,
-  }))
+module.exports = config => {
+  config.set(
+    Object.assign({}, karmaBaseConfig, {
+      webpack: karmaWebpackConfig,
+      files: [
+        {
+          pattern: 'test/browser/index-webpack.worker.ts',
+          included: false,
+        },
+      ],
+      preprocessors: { 'test/browser/index-webpack.worker.ts': ['webpack'] },
+    })
+  );
 };
diff --git a/packages/opentelemetry-sdk-trace-base/src/config.ts b/packages/opentelemetry-sdk-trace-base/src/config.ts
@@ -15,7 +15,7 @@
  */
 
 import { diag } from '@opentelemetry/api';
-import { getEnv, ENVIRONMENT } from '@opentelemetry/core';
+import { getNumberFromEnv, getStringFromEnv } from '@opentelemetry/core';
 import { Sampler } from './Sampler';
 import { AlwaysOffSampler } from './sampler/AlwaysOffSampler';
 import { AlwaysOnSampler } from './sampler/AlwaysOnSampler';
@@ -44,35 +44,38 @@ const DEFAULT_RATIO = 1;
 // object needs to be wrapped in this function and called when needed otherwise
 // envs are parsed before tests are ran - causes tests using these envs to fail
 export function loadDefaultConfig() {
-  const env = getEnv();
-
   return {
-    sampler: buildSamplerFromEnv(env),
+    sampler: buildSamplerFromEnv(),
     forceFlushTimeoutMillis: 30000,
     generalLimits: {
-      attributeValueLengthLimit: env.OTEL_ATTRIBUTE_VALUE_LENGTH_LIMIT,
-      attributeCountLimit: env.OTEL_ATTRIBUTE_COUNT_LIMIT,
+      attributeValueLengthLimit:
+        getNumberFromEnv('OTEL_ATTRIBUTE_VALUE_LENGTH_LIMIT') ?? Infinity,
+      attributeCountLimit:
+        getNumberFromEnv('OTEL_ATTRIBUTE_COUNT_LIMIT') ?? 128,
     },
     spanLimits: {
-      attributeValueLengthLimit: env.OTEL_SPAN_ATTRIBUTE_VALUE_LENGTH_LIMIT,
-      attributeCountLimit: env.OTEL_SPAN_ATTRIBUTE_COUNT_LIMIT,
-      linkCountLimit: env.OTEL_SPAN_LINK_COUNT_LIMIT,
-      eventCountLimit: env.OTEL_SPAN_EVENT_COUNT_LIMIT,
+      attributeValueLengthLimit:
+        getNumberFromEnv('OTEL_SPAN_ATTRIBUTE_VALUE_LENGTH_LIMIT') ?? Infinity,
+      attributeCountLimit:
+        getNumberFromEnv('OTEL_SPAN_ATTRIBUTE_COUNT_LIMIT') ?? 128,
+      linkCountLimit: getNumberFromEnv('OTEL_SPAN_LINK_COUNT_LIMIT') ?? 128,
+      eventCountLimit: getNumberFromEnv('OTEL_SPAN_EVENT_COUNT_LIMIT') ?? 128,
       attributePerEventCountLimit:
-        env.OTEL_SPAN_ATTRIBUTE_PER_EVENT_COUNT_LIMIT,
-      attributePerLinkCountLimit: env.OTEL_SPAN_ATTRIBUTE_PER_LINK_COUNT_LIMIT,
+        getNumberFromEnv('OTEL_SPAN_ATTRIBUTE_PER_EVENT_COUNT_LIMIT') ?? 128,
+      attributePerLinkCountLimit:
+        getNumberFromEnv('OTEL_SPAN_ATTRIBUTE_PER_LINK_COUNT_LIMIT') ?? 128,
     },
   };
 }
 
 /**
  * Based on environment, builds a sampler, complies with specification.
- * @param environment optional, by default uses getEnv(), but allows passing a value to reuse parsed environment
  */
-export function buildSamplerFromEnv(
-  environment: Required<ENVIRONMENT> = getEnv()
-): Sampler {
-  switch (environment.OTEL_TRACES_SAMPLER) {
+export function buildSamplerFromEnv(): Sampler {
+  const sampler =
+    getStringFromEnv('OTEL_TRACES_SAMPLER') ??
+    TracesSamplerValues.ParentBasedAlwaysOn;
+  switch (sampler) {
     case TracesSamplerValues.AlwaysOn:
       return new AlwaysOnSampler();
     case TracesSamplerValues.AlwaysOff:
@@ -86,48 +89,31 @@ export function buildSamplerFromEnv(
         root: new AlwaysOffSampler(),
       });
     case TracesSamplerValues.TraceIdRatio:
-      return new TraceIdRatioBasedSampler(
-        getSamplerProbabilityFromEnv(environment)
-      );
+      return new TraceIdRatioBasedSampler(getSamplerProbabilityFromEnv());
     case TracesSamplerValues.ParentBasedTraceIdRatio:
       return new ParentBasedSampler({
-        root: new TraceIdRatioBasedSampler(
-          getSamplerProbabilityFromEnv(environment)
-        ),
+        root: new TraceIdRatioBasedSampler(getSamplerProbabilityFromEnv()),
       });
     default:
       diag.error(
-        `OTEL_TRACES_SAMPLER value "${environment.OTEL_TRACES_SAMPLER} invalid, defaulting to ${FALLBACK_OTEL_TRACES_SAMPLER}".`
+        `OTEL_TRACES_SAMPLER value "${sampler}" invalid, defaulting to "${FALLBACK_OTEL_TRACES_SAMPLER}".`
       );
       return new AlwaysOnSampler();
   }
 }
 
-function getSamplerProbabilityFromEnv(
-  environment: Required<ENVIRONMENT>
-): number | undefined {
-  if (
-    environment.OTEL_TRACES_SAMPLER_ARG === undefined ||
-    environment.OTEL_TRACES_SAMPLER_ARG === ''
-  ) {
+function getSamplerProbabilityFromEnv(): number | undefined {
+  const probability = getNumberFromEnv('OTEL_TRACES_SAMPLER_ARG');
+  if (probability == null) {
     diag.error(
       `OTEL_TRACES_SAMPLER_ARG is blank, defaulting to ${DEFAULT_RATIO}.`
     );
     return DEFAULT_RATIO;
   }
 
-  const probability = Number(environment.OTEL_TRACES_SAMPLER_ARG);
-
-  if (isNaN(probability)) {
-    diag.error(
-      `OTEL_TRACES_SAMPLER_ARG=${environment.OTEL_TRACES_SAMPLER_ARG} was given, but it is invalid, defaulting to ${DEFAULT_RATIO}.`
-    );
-    return DEFAULT_RATIO;
-  }
-
   if (probability < 0 || probability > 1) {
     diag.error(
-      `OTEL_TRACES_SAMPLER_ARG=${environment.OTEL_TRACES_SAMPLER_ARG} was given, but it is out of range ([0..1]), defaulting to ${DEFAULT_RATIO}.`
+      `OTEL_TRACES_SAMPLER_ARG=${probability} was given, but it is out of range ([0..1]), defaulting to ${DEFAULT_RATIO}.`
     );
     return DEFAULT_RATIO;
   }

diff --git a/packages/opentelemetry-sdk-trace-base/src/export/BatchSpanProcessorBase.ts b/packages/opentelemetry-sdk-trace-base/src/export/BatchSpanProcessorBase.ts
@@ -18,7 +18,7 @@ import { context, Context, diag, TraceFlags } from '@opentelemetry/api';
 import {
   BindOnceFuture,
   ExportResultCode,
-  getEnv,
+  getNumberFromEnv,
   globalErrorHandler,
   suppressTracing,
   unrefTimer,
@@ -51,23 +51,22 @@ export abstract class BatchSpanProcessorBase<T extends BufferConfig>
     private readonly _exporter: SpanExporter,
     config?: T
   ) {
-    const env = getEnv();
     this._maxExportBatchSize =
       typeof config?.maxExportBatchSize === 'number'
         ? config.maxExportBatchSize
-        : env.OTEL_BSP_MAX_EXPORT_BATCH_SIZE;
+        : (getNumberFromEnv('OTEL_BSP_MAX_EXPORT_BATCH_SIZE') ?? 512);
     this._maxQueueSize =
       typeof config?.maxQueueSize === 'number'
         ? config.maxQueueSize
-        : env.OTEL_BSP_MAX_QUEUE_SIZE;
+        : (getNumberFromEnv('OTEL_BSP_MAX_QUEUE_SIZE') ?? 2048);
     this._scheduledDelayMillis =
       typeof config?.scheduledDelayMillis === 'number'
         ? config.scheduledDelayMillis
-        : env.OTEL_BSP_SCHEDULE_DELAY;
+        : (getNumberFromEnv('OTEL_BSP_SCHEDULE_DELAY') ?? 5000);
     this._exportTimeoutMillis =
       typeof config?.exportTimeoutMillis === 'number'
         ? config.exportTimeoutMillis
-        : env.OTEL_BSP_EXPORT_TIMEOUT;
+        : (getNumberFromEnv('OTEL_BSP_EXPORT_TIMEOUT') ?? 30000);
 
     this._shutdownOnce = new BindOnceFuture(this._shutdown, this);
 

diff --git a/packages/opentelemetry-sdk-trace-base/src/utility.ts b/packages/opentelemetry-sdk-trace-base/src/utility.ts
@@ -20,7 +20,7 @@ import { SpanLimits, TracerConfig, GeneralLimits } from './types';
 import {
   DEFAULT_ATTRIBUTE_COUNT_LIMIT,
   DEFAULT_ATTRIBUTE_VALUE_LENGTH_LIMIT,
-  getEnvWithoutDefaults,
+  getNumberFromEnv,
 } from '@opentelemetry/core';
 
 /**
@@ -68,16 +68,14 @@ export function mergeConfig(userConfig: TracerConfig): TracerConfig & {
 export function reconfigureLimits(userConfig: TracerConfig): TracerConfig {
   const spanLimits = Object.assign({}, userConfig.spanLimits);
 
-  const parsedEnvConfig = getEnvWithoutDefaults();
-
   /**
    * Reassign span attribute count limit to use first non null value defined by user or use default value
    */
   spanLimits.attributeCountLimit =
     userConfig.spanLimits?.attributeCountLimit ??
     userConfig.generalLimits?.attributeCountLimit ??
-    parsedEnvConfig.OTEL_SPAN_ATTRIBUTE_COUNT_LIMIT ??
-    parsedEnvConfig.OTEL_ATTRIBUTE_COUNT_LIMIT ??
+    getNumberFromEnv('OTEL_SPAN_ATTRIBUTE_COUNT_LIMIT') ??
+    getNumberFromEnv('OTEL_ATTRIBUTE_COUNT_LIMIT') ??
     DEFAULT_ATTRIBUTE_COUNT_LIMIT;
 
   /**
@@ -86,8 +84,8 @@ export function reconfigureLimits(userConfig: TracerConfig): TracerConfig {
   spanLimits.attributeValueLengthLimit =
     userConfig.spanLimits?.attributeValueLengthLimit ??
     userConfig.generalLimits?.attributeValueLengthLimit ??
-    parsedEnvConfig.OTEL_SPAN_ATTRIBUTE_VALUE_LENGTH_LIMIT ??
-    parsedEnvConfig.OTEL_ATTRIBUTE_VALUE_LENGTH_LIMIT ??
+    getNumberFromEnv('OTEL_SPAN_ATTRIBUTE_VALUE_LENGTH_LIMIT') ??
+    getNumberFromEnv('OTEL_ATTRIBUTE_VALUE_LENGTH_LIMIT') ??
     DEFAULT_ATTRIBUTE_VALUE_LENGTH_LIMIT;
 
   return Object.assign({}, userConfig, { spanLimits });

diff --git a/...etry-sdk-trace-base/test/index-webpack.ts → ...-trace-base/test/browser/index-webpack.ts b/...etry-sdk-trace-base/test/index-webpack.ts → ...-trace-base/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);
diff --git a/...k-trace-base/test/index-webpack.worker.ts → ...base/test/browser/index-webpack.worker.ts b/...k-trace-base/test/index-webpack.worker.ts → ...base/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);