From 8efc277778b2f8e12dd547037613dac075c00a09 Mon Sep 17 00:00:00 2001 From: Jakub Malinowski Date: Thu, 5 Nov 2020 17:11:27 +0100 Subject: [PATCH] chore: env vars for span limit as per specification --- .../src/utils/environment.ts | 20 ++++++++++++++-- .../test/utils/environment.test.ts | 20 ++++++++++++++++ packages/opentelemetry-tracing/src/config.ts | 13 +++------- packages/opentelemetry-tracing/src/utility.ts | 24 +++++++------------ .../test/BasicTracerProvider.test.ts | 18 +++++++------- .../opentelemetry-tracing/test/Span.test.ts | 10 +++++--- 6 files changed, 65 insertions(+), 40 deletions(-) diff --git a/packages/opentelemetry-core/src/utils/environment.ts b/packages/opentelemetry-core/src/utils/environment.ts index 73cde2085f..fe5e45c6e9 100644 --- a/packages/opentelemetry-core/src/utils/environment.ts +++ b/packages/opentelemetry-core/src/utils/environment.ts @@ -25,10 +25,16 @@ export interface ENVIRONMENT { OTEL_LOG_LEVEL?: LogLevel; OTEL_NO_PATCH_MODULES?: string; OTEL_SAMPLING_PROBABILITY?: number; + OTEL_SPAN_ATTRIBUTE_COUNT_LIMIT?: number; + OTEL_SPAN_EVENT_COUNT_LIMIT?: number; + OTEL_SPAN_LINK_COUNT_LIMIT?: number; } const ENVIRONMENT_NUMBERS: Partial[] = [ 'OTEL_SAMPLING_PROBABILITY', + 'OTEL_SPAN_ATTRIBUTE_COUNT_LIMIT', + 'OTEL_SPAN_EVENT_COUNT_LIMIT', + 'OTEL_SPAN_LINK_COUNT_LIMIT', ]; /** @@ -38,6 +44,9 @@ export const DEFAULT_ENVIRONMENT: Required = { OTEL_NO_PATCH_MODULES: '', OTEL_LOG_LEVEL: LogLevel.INFO, OTEL_SAMPLING_PROBABILITY: 1, + OTEL_SPAN_ATTRIBUTE_COUNT_LIMIT: 1000, + OTEL_SPAN_EVENT_COUNT_LIMIT: 1000, + OTEL_SPAN_LINK_COUNT_LIMIT: 1000, }; /** @@ -57,8 +66,15 @@ function parseNumber( ) { if (typeof values[name] !== 'undefined') { const value = Number(values[name] as string); - if (!isNaN(value) && value >= min && value <= max) { - environment[name] = value; + + if (!isNaN(value)) { + if (value < min) { + environment[name] = min; + } else if (value > max) { + environment[name] = max; + } else { + environment[name] = value; + } } } } diff --git a/packages/opentelemetry-core/test/utils/environment.test.ts b/packages/opentelemetry-core/test/utils/environment.test.ts index 578fb97ca2..60fa403425 100644 --- a/packages/opentelemetry-core/test/utils/environment.test.ts +++ b/packages/opentelemetry-core/test/utils/environment.test.ts @@ -77,11 +77,31 @@ describe('environment', () => { OTEL_NO_PATCH_MODULES: 'a,b,c', OTEL_LOG_LEVEL: 'ERROR', OTEL_SAMPLING_PROBABILITY: '0.5', + OTEL_SPAN_ATTRIBUTE_COUNT_LIMIT: 10, + OTEL_SPAN_EVENT_COUNT_LIMIT: 20, + OTEL_SPAN_LINK_COUNT_LIMIT: 30, }); const env = getEnv(); assert.strictEqual(env.OTEL_NO_PATCH_MODULES, 'a,b,c'); assert.strictEqual(env.OTEL_LOG_LEVEL, LogLevel.ERROR); assert.strictEqual(env.OTEL_SAMPLING_PROBABILITY, 0.5); + assert.strictEqual(env.OTEL_SPAN_ATTRIBUTE_COUNT_LIMIT, 10); + assert.strictEqual(env.OTEL_SPAN_EVENT_COUNT_LIMIT, 20); + assert.strictEqual(env.OTEL_SPAN_LINK_COUNT_LIMIT, 30); + }); + + it('should match invalid values to closest valid equivalent', () => { + mockEnvironment({ + OTEL_SAMPLING_PROBABILITY: '-0.1', + }); + const minEnv = getEnv(); + assert.strictEqual(minEnv.OTEL_SAMPLING_PROBABILITY, 0); + + mockEnvironment({ + OTEL_SAMPLING_PROBABILITY: '1.1', + }); + const maxEnv = getEnv(); + assert.strictEqual(maxEnv.OTEL_SAMPLING_PROBABILITY, 1); }); it('should parse OTEL_LOG_LEVEL despite casing', () => { diff --git a/packages/opentelemetry-tracing/src/config.ts b/packages/opentelemetry-tracing/src/config.ts index 4bbb6e11cf..a2ef623fbd 100644 --- a/packages/opentelemetry-tracing/src/config.ts +++ b/packages/opentelemetry-tracing/src/config.ts @@ -16,13 +16,6 @@ import { AlwaysOnSampler, getEnv } from '@opentelemetry/core'; -/** Default limit for Message events per span */ -export const DEFAULT_MAX_EVENTS_PER_SPAN = 128; -/** Default limit for Attributes per span */ -export const DEFAULT_MAX_ATTRIBUTES_PER_SPAN = 32; -/** Default limit for Links per span */ -export const DEFAULT_MAX_LINKS_PER_SPAN = 32; - /** * Default configuration. For fields with primitive values, any user-provided * value will override the corresponding default value. For fields with @@ -33,9 +26,9 @@ export const DEFAULT_CONFIG = { logLevel: getEnv().OTEL_LOG_LEVEL, sampler: new AlwaysOnSampler(), traceParams: { - numberOfAttributesPerSpan: DEFAULT_MAX_ATTRIBUTES_PER_SPAN, - numberOfLinksPerSpan: DEFAULT_MAX_LINKS_PER_SPAN, - numberOfEventsPerSpan: DEFAULT_MAX_EVENTS_PER_SPAN, + numberOfAttributesPerSpan: getEnv().OTEL_SPAN_ATTRIBUTE_COUNT_LIMIT, + numberOfLinksPerSpan: getEnv().OTEL_SPAN_LINK_COUNT_LIMIT, + numberOfEventsPerSpan: getEnv().OTEL_SPAN_EVENT_COUNT_LIMIT, }, gracefulShutdown: true, }; diff --git a/packages/opentelemetry-tracing/src/utility.ts b/packages/opentelemetry-tracing/src/utility.ts index 98fe6fac55..0150623865 100644 --- a/packages/opentelemetry-tracing/src/utility.ts +++ b/packages/opentelemetry-tracing/src/utility.ts @@ -14,12 +14,7 @@ * limitations under the License. */ -import { - DEFAULT_CONFIG, - DEFAULT_MAX_ATTRIBUTES_PER_SPAN, - DEFAULT_MAX_EVENTS_PER_SPAN, - DEFAULT_MAX_LINKS_PER_SPAN, -} from './config'; +import { DEFAULT_CONFIG } from './config'; import { TracerConfig } from './types'; import { ParentBasedSampler, @@ -32,10 +27,10 @@ import { * user provided configurations. */ export function mergeConfig(userConfig: TracerConfig) { - const traceParams = userConfig.traceParams; const otelSamplingProbability = getEnv().OTEL_SAMPLING_PROBABILITY; const target = Object.assign( + {}, DEFAULT_CONFIG, // use default AlwaysOnSampler if otelSamplingProbability is 1 otelSamplingProbability !== undefined && otelSamplingProbability < 1 @@ -48,14 +43,11 @@ export function mergeConfig(userConfig: TracerConfig) { userConfig ); - // the user-provided value will be used to extend the default value. - if (traceParams) { - target.traceParams.numberOfAttributesPerSpan = - traceParams.numberOfAttributesPerSpan || DEFAULT_MAX_ATTRIBUTES_PER_SPAN; - target.traceParams.numberOfEventsPerSpan = - traceParams.numberOfEventsPerSpan || DEFAULT_MAX_EVENTS_PER_SPAN; - target.traceParams.numberOfLinksPerSpan = - traceParams.numberOfLinksPerSpan || DEFAULT_MAX_LINKS_PER_SPAN; - } + target.traceParams = Object.assign( + {}, + DEFAULT_CONFIG.traceParams, + userConfig.traceParams || {} + ); + return target; } diff --git a/packages/opentelemetry-tracing/test/BasicTracerProvider.test.ts b/packages/opentelemetry-tracing/test/BasicTracerProvider.test.ts index f378805bd5..fe212d351c 100644 --- a/packages/opentelemetry-tracing/test/BasicTracerProvider.test.ts +++ b/packages/opentelemetry-tracing/test/BasicTracerProvider.test.ts @@ -75,9 +75,9 @@ describe('BasicTracerProvider', () => { it('should construct an instance with default trace params', () => { const tracer = new BasicTracerProvider({}).getTracer('default'); assert.deepStrictEqual(tracer.getActiveTraceParams(), { - numberOfAttributesPerSpan: 32, - numberOfEventsPerSpan: 128, - numberOfLinksPerSpan: 32, + numberOfAttributesPerSpan: 1000, + numberOfEventsPerSpan: 1000, + numberOfLinksPerSpan: 1000, }); }); @@ -89,8 +89,8 @@ describe('BasicTracerProvider', () => { }).getTracer('default'); assert.deepStrictEqual(tracer.getActiveTraceParams(), { numberOfAttributesPerSpan: 100, - numberOfEventsPerSpan: 128, - numberOfLinksPerSpan: 32, + numberOfEventsPerSpan: 1000, + numberOfLinksPerSpan: 1000, }); }); @@ -101,9 +101,9 @@ describe('BasicTracerProvider', () => { }, }).getTracer('default'); assert.deepStrictEqual(tracer.getActiveTraceParams(), { - numberOfAttributesPerSpan: 32, + numberOfAttributesPerSpan: 1000, numberOfEventsPerSpan: 300, - numberOfLinksPerSpan: 32, + numberOfLinksPerSpan: 1000, }); }); @@ -114,8 +114,8 @@ describe('BasicTracerProvider', () => { }, }).getTracer('default'); assert.deepStrictEqual(tracer.getActiveTraceParams(), { - numberOfAttributesPerSpan: 32, - numberOfEventsPerSpan: 128, + numberOfAttributesPerSpan: 1000, + numberOfEventsPerSpan: 1000, numberOfLinksPerSpan: 10, }); }); diff --git a/packages/opentelemetry-tracing/test/Span.test.ts b/packages/opentelemetry-tracing/test/Span.test.ts index 07c7f2119d..a8533693fe 100644 --- a/packages/opentelemetry-tracing/test/Span.test.ts +++ b/packages/opentelemetry-tracing/test/Span.test.ts @@ -39,6 +39,10 @@ const performanceTimeOrigin = hrTime(); describe('Span', () => { const tracer = new BasicTracerProvider({ logger: new NoopLogger(), + traceParams: { + numberOfAttributesPerSpan: 100, + numberOfEventsPerSpan: 100, + }, }).getTracer('default'); const name = 'span1'; const spanContext: SpanContext = { @@ -327,7 +331,7 @@ describe('Span', () => { span.end(); }); - it('should drop extra links, attributes and events', () => { + it('should drop extra attributes and events', () => { const span = new Span( tracer, ROOT_CONTEXT, @@ -341,8 +345,8 @@ describe('Span', () => { } span.end(); - assert.strictEqual(span.events.length, 128); - assert.strictEqual(Object.keys(span.attributes).length, 32); + assert.strictEqual(span.events.length, 100); + assert.strictEqual(Object.keys(span.attributes).length, 100); assert.strictEqual(span.events[span.events.length - 1].name, 'sent149'); assert.strictEqual(span.attributes['foo0'], undefined); assert.strictEqual(span.attributes['foo149'], 'bar149');