diff --git a/CHANGELOG.md b/CHANGELOG.md index 5ca8b500d9..fff7a1c22e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -26,6 +26,7 @@ All notable changes to this project will be documented in this file. * fix(resources): fix browser compatibility for host and os detectors [#3004](https://github.com/open-telemetry/opentelemetry-js/pull/3004) @legendecas * fix(sdk-trace-base): fix crash on environments without global document [#3000](https://github.com/open-telemetry/opentelemetry-js/pull/3000) @legendecas +* fix(sdk-trace-base): fix spanLimits attribute length/count to consider env values [#3068](https://github.com/open-telemetry/opentelemetry-js/pull/3068) @svetlanabrennan ### :house: (Internal) diff --git a/packages/opentelemetry-core/src/utils/environment.ts b/packages/opentelemetry-core/src/utils/environment.ts index def0fe4846..05ba058f0f 100644 --- a/packages/opentelemetry-core/src/utils/environment.ts +++ b/packages/opentelemetry-core/src/utils/environment.ts @@ -16,6 +16,7 @@ import { DiagLogLevel } from '@opentelemetry/api'; import { TracesSamplerValues } from './sampling'; +import { _globalThis } from '../platform/browser/globalThis'; const DEFAULT_LIST_SEPARATOR = ','; @@ -283,3 +284,13 @@ export function parseEnvironment(values: RAW_ENVIRONMENT): ENVIRONMENT { return environment; } + +/** + * Get environment in node or browser without + * populating default values. + */ +export function getEnvWithoutDefaults(): ENVIRONMENT { + return typeof process !== 'undefined' ? + parseEnvironment(process.env as RAW_ENVIRONMENT) : + parseEnvironment(_globalThis as typeof globalThis & RAW_ENVIRONMENT); +} diff --git a/packages/opentelemetry-sdk-trace-base/src/BasicTracerProvider.ts b/packages/opentelemetry-sdk-trace-base/src/BasicTracerProvider.ts index b0560fc3f1..aae02ad1ad 100644 --- a/packages/opentelemetry-sdk-trace-base/src/BasicTracerProvider.ts +++ b/packages/opentelemetry-sdk-trace-base/src/BasicTracerProvider.ts @@ -31,7 +31,7 @@ import { } from '@opentelemetry/core'; import { Resource } from '@opentelemetry/resources'; import { SpanProcessor, Tracer } from '.'; -import { DEFAULT_CONFIG } from './config'; +import { loadDefaultConfig } from './config'; import { MultiSpanProcessor } from './MultiSpanProcessor'; import { NoopSpanProcessor } from './export/NoopSpanProcessor'; import { SDKRegistrationConfig, TracerConfig } from './types'; @@ -74,7 +74,7 @@ export class BasicTracerProvider implements TracerProvider { readonly resource: Resource; constructor(config: TracerConfig = {}) { - const mergedConfig = merge({}, DEFAULT_CONFIG, reconfigureLimits(config)); + const mergedConfig = merge({}, loadDefaultConfig(), reconfigureLimits(config)); this.resource = mergedConfig.resource ?? Resource.empty(); this.resource = Resource.default().merge(this.resource); this._config = Object.assign({}, mergedConfig, { diff --git a/packages/opentelemetry-sdk-trace-base/src/config.ts b/packages/opentelemetry-sdk-trace-base/src/config.ts index 4b364b3718..7895514597 100644 --- a/packages/opentelemetry-sdk-trace-base/src/config.ts +++ b/packages/opentelemetry-sdk-trace-base/src/config.ts @@ -30,25 +30,30 @@ const FALLBACK_OTEL_TRACES_SAMPLER = TracesSamplerValues.AlwaysOn; const DEFAULT_RATIO = 1; /** - * Default configuration. For fields with primitive values, any user-provided + * Load default configuration. For fields with primitive values, any user-provided * value will override the corresponding default value. For fields with * non-primitive values (like `spanLimits`), the user-provided value will be * used to extend the default value. */ -export const DEFAULT_CONFIG = { - sampler: buildSamplerFromEnv(env), - forceFlushTimeoutMillis: 30000, - generalLimits: { - attributeValueLengthLimit: getEnv().OTEL_ATTRIBUTE_VALUE_LENGTH_LIMIT, - attributeCountLimit: getEnv().OTEL_ATTRIBUTE_COUNT_LIMIT, - }, - spanLimits: { - attributeValueLengthLimit: getEnv().OTEL_SPAN_ATTRIBUTE_VALUE_LENGTH_LIMIT, - attributeCountLimit: getEnv().OTEL_SPAN_ATTRIBUTE_COUNT_LIMIT, - linkCountLimit: getEnv().OTEL_SPAN_LINK_COUNT_LIMIT, - eventCountLimit: getEnv().OTEL_SPAN_EVENT_COUNT_LIMIT, - }, -}; + +// 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() { + return { + sampler: buildSamplerFromEnv(env), + forceFlushTimeoutMillis: 30000, + generalLimits: { + attributeValueLengthLimit: getEnv().OTEL_ATTRIBUTE_VALUE_LENGTH_LIMIT, + attributeCountLimit: getEnv().OTEL_ATTRIBUTE_COUNT_LIMIT, + }, + spanLimits: { + attributeValueLengthLimit: getEnv().OTEL_SPAN_ATTRIBUTE_VALUE_LENGTH_LIMIT, + attributeCountLimit: getEnv().OTEL_SPAN_ATTRIBUTE_COUNT_LIMIT, + linkCountLimit: getEnv().OTEL_SPAN_LINK_COUNT_LIMIT, + eventCountLimit: getEnv().OTEL_SPAN_EVENT_COUNT_LIMIT, + }, + }; +} /** * Based on environment, builds a sampler, complies with specification. diff --git a/packages/opentelemetry-sdk-trace-base/src/utility.ts b/packages/opentelemetry-sdk-trace-base/src/utility.ts index 3492f3b050..684f68a210 100644 --- a/packages/opentelemetry-sdk-trace-base/src/utility.ts +++ b/packages/opentelemetry-sdk-trace-base/src/utility.ts @@ -15,8 +15,13 @@ */ import { Sampler } from '@opentelemetry/api'; -import { buildSamplerFromEnv, DEFAULT_CONFIG } from './config'; +import { buildSamplerFromEnv, loadDefaultConfig } from './config'; import { SpanLimits, TracerConfig, GeneralLimits } from './types'; +import { + DEFAULT_ATTRIBUTE_COUNT_LIMIT, + DEFAULT_ATTRIBUTE_VALUE_LENGTH_LIMIT, + getEnvWithoutDefaults, +} from '@opentelemetry/core'; /** * Function to merge Default configuration (as specified in './config') with @@ -31,6 +36,8 @@ export function mergeConfig(userConfig: TracerConfig): TracerConfig & { sampler: buildSamplerFromEnv(), }; + const DEFAULT_CONFIG = loadDefaultConfig(); + const target = Object.assign( {}, DEFAULT_CONFIG, @@ -61,21 +68,27 @@ export function mergeConfig(userConfig: TracerConfig): TracerConfig & { export function reconfigureLimits(userConfig: TracerConfig): TracerConfig { const spanLimits = Object.assign({}, userConfig.spanLimits); + const parsedEnvConfig = getEnvWithoutDefaults(); + /** - * When span attribute count limit is not defined, but general attribute count limit is defined - * Then, span attribute count limit will be same as general one + * Reassign span attribute count limit to use first non null value defined by user or use default value */ - if (spanLimits.attributeCountLimit == null && userConfig.generalLimits?.attributeCountLimit != null) { - spanLimits.attributeCountLimit = userConfig.generalLimits.attributeCountLimit; - } + spanLimits.attributeCountLimit = + userConfig.spanLimits?.attributeCountLimit ?? + userConfig.generalLimits?.attributeCountLimit ?? + parsedEnvConfig.OTEL_SPAN_ATTRIBUTE_COUNT_LIMIT ?? + parsedEnvConfig.OTEL_ATTRIBUTE_COUNT_LIMIT ?? + DEFAULT_ATTRIBUTE_COUNT_LIMIT; /** - * When span attribute value length limit is not defined, but general attribute value length limit is defined - * Then, span attribute value length limit will be same as general one + * Reassign span attribute value length limit to use first non null value defined by user or use default value */ - if (spanLimits.attributeValueLengthLimit == null && userConfig.generalLimits?.attributeValueLengthLimit != null) { - spanLimits.attributeValueLengthLimit = userConfig.generalLimits.attributeValueLengthLimit; - } + spanLimits.attributeValueLengthLimit = + userConfig.spanLimits?.attributeValueLengthLimit ?? + userConfig.generalLimits?.attributeValueLengthLimit ?? + parsedEnvConfig.OTEL_SPAN_ATTRIBUTE_VALUE_LENGTH_LIMIT ?? + parsedEnvConfig.OTEL_ATTRIBUTE_VALUE_LENGTH_LIMIT ?? + DEFAULT_ATTRIBUTE_VALUE_LENGTH_LIMIT; return Object.assign({}, userConfig, { spanLimits }); } diff --git a/packages/opentelemetry-sdk-trace-base/test/common/BasicTracerProvider.test.ts b/packages/opentelemetry-sdk-trace-base/test/common/BasicTracerProvider.test.ts index 8575b044f6..88ec6fa16a 100644 --- a/packages/opentelemetry-sdk-trace-base/test/common/BasicTracerProvider.test.ts +++ b/packages/opentelemetry-sdk-trace-base/test/common/BasicTracerProvider.test.ts @@ -208,6 +208,108 @@ describe('BasicTracerProvider', () => { }); }); + describe('when attribute value length limit is defined via env', () => { + it('should have general attribute value length limits value as defined with env', () => { + envSource.OTEL_ATTRIBUTE_VALUE_LENGTH_LIMIT = '115'; + const tracer = new BasicTracerProvider().getTracer('default'); + const generalLimits = tracer.getGeneralLimits(); + assert.strictEqual(generalLimits.attributeValueLengthLimit, 115); + delete envSource.OTEL_ATTRIBUTE_VALUE_LENGTH_LIMIT; + }); + it('should have span attribute value length limit value same as general limit value', () => { + envSource.OTEL_ATTRIBUTE_VALUE_LENGTH_LIMIT = '125'; + const tracer = new BasicTracerProvider().getTracer('default'); + const generalLimits = tracer.getGeneralLimits(); + const spanLimits = tracer.getSpanLimits(); + assert.strictEqual(generalLimits.attributeValueLengthLimit, 125); + assert.strictEqual(spanLimits.attributeValueLengthLimit, 125); + delete envSource.OTEL_ATTRIBUTE_VALUE_LENGTH_LIMIT; + }); + it('should have span and general attribute value length limits as defined in env', () => { + envSource.OTEL_ATTRIBUTE_VALUE_LENGTH_LIMIT = '125'; + envSource.OTEL_SPAN_ATTRIBUTE_VALUE_LENGTH_LIMIT = '109'; + const tracer = new BasicTracerProvider().getTracer('default'); + const spanLimits = tracer.getSpanLimits(); + const generalLimits = tracer.getGeneralLimits(); + assert.strictEqual(generalLimits.attributeValueLengthLimit, 125); + assert.strictEqual(spanLimits.attributeValueLengthLimit, 109); + delete envSource.OTEL_ATTRIBUTE_VALUE_LENGTH_LIMIT; + delete envSource.OTEL_SPAN_ATTRIBUTE_VALUE_LENGTH_LIMIT; + }); + it('should have span attribute value length limit as deafult of Infinity', () => { + envSource.OTEL_ATTRIBUTE_VALUE_LENGTH_LIMIT = '125'; + envSource.OTEL_SPAN_ATTRIBUTE_VALUE_LENGTH_LIMIT = 'Infinity'; + const tracer = new BasicTracerProvider().getTracer('default'); + const spanLimits = tracer.getSpanLimits(); + const generalLimits = tracer.getGeneralLimits(); + assert.strictEqual(generalLimits.attributeValueLengthLimit, 125); + assert.strictEqual(spanLimits.attributeValueLengthLimit, Infinity); + delete envSource.OTEL_ATTRIBUTE_VALUE_LENGTH_LIMIT; + delete envSource.OTEL_SPAN_ATTRIBUTE_VALUE_LENGTH_LIMIT; + }); + }); + + describe('when attribute value length limit is not defined via env', () => { + it('should use default value of Infinity', () => { + const tracer = new BasicTracerProvider().getTracer('default'); + const spanLimits = tracer.getSpanLimits(); + const generalLimits = tracer.getGeneralLimits(); + assert.strictEqual(generalLimits.attributeValueLengthLimit, Infinity); + assert.strictEqual(spanLimits.attributeValueLengthLimit, Infinity); + }); + }); + + describe('when attribute count limit is defined via env', () => { + it('should general attribute count limit as defined with env', () => { + envSource.OTEL_ATTRIBUTE_COUNT_LIMIT = '25'; + const tracer = new BasicTracerProvider({}).getTracer('default'); + const generalLimits = tracer.getGeneralLimits(); + assert.strictEqual(generalLimits.attributeCountLimit, 25); + delete envSource.OTEL_ATTRIBUTE_COUNT_LIMIT; + }); + it('should have span attribute count limit value same as general limit value', () => { + envSource.OTEL_ATTRIBUTE_COUNT_LIMIT = '20'; + const tracer = new BasicTracerProvider().getTracer('default'); + const generalLimits = tracer.getGeneralLimits(); + const spanLimits = tracer.getSpanLimits(); + assert.strictEqual(generalLimits.attributeCountLimit, 20); + assert.strictEqual(spanLimits.attributeCountLimit, 20); + delete envSource.OTEL_ATTRIBUTE_COUNT_LIMIT; + }); + it('should have span and general attribute count limits as defined in env', () => { + envSource.OTEL_ATTRIBUTE_COUNT_LIMIT = '20'; + envSource.OTEL_SPAN_ATTRIBUTE_COUNT_LIMIT = '35'; + const tracer = new BasicTracerProvider().getTracer('default'); + const spanLimits = tracer.getSpanLimits(); + const generalLimits = tracer.getGeneralLimits(); + assert.strictEqual(generalLimits.attributeCountLimit, 20); + assert.strictEqual(spanLimits.attributeCountLimit, 35); + delete envSource.OTEL_ATTRIBUTE_COUNT_LIMIT; + delete envSource.OTEL_SPAN_ATTRIBUTE_COUNT_LIMIT; + }); + it('should have span attribute count limit as default of 128', () => { + envSource.OTEL_ATTRIBUTE_COUNT_LIMIT = '20'; + envSource.OTEL_SPAN_ATTRIBUTE_COUNT_LIMIT = '128'; + const tracer = new BasicTracerProvider().getTracer('default'); + const spanLimits = tracer.getSpanLimits(); + const generalLimits = tracer.getGeneralLimits(); + assert.strictEqual(generalLimits.attributeCountLimit, 20); + assert.strictEqual(spanLimits.attributeCountLimit, 128); + delete envSource.OTEL_ATTRIBUTE_COUNT_LIMIT; + delete envSource.OTEL_SPAN_ATTRIBUTE_COUNT_LIMIT; + }); + }); + + describe('when attribute count limit is not defined via env', () => { + it('should use default value of 128', () => { + const tracer = new BasicTracerProvider().getTracer('default'); + const spanLimits = tracer.getSpanLimits(); + const generalLimits = tracer.getGeneralLimits(); + assert.strictEqual(generalLimits.attributeCountLimit, 128); + assert.strictEqual(spanLimits.attributeCountLimit, 128); + }); + }); + describe('when "eventCountLimit" is defined', () => { it('should have tracer with defined value', () => { const tracer = new BasicTracerProvider({