From 11adf3e01cb52e310f1642e3d923a2ddd9a9f286 Mon Sep 17 00:00:00 2001 From: Jakub Malinowski Date: Wed, 14 Apr 2021 19:30:30 +0200 Subject: [PATCH 1/3] feat: handle OTEL_TRACES_SAMPLER env var Adds support for OTEL_TRACES_SAMPLER and OTEL_TRACES_SAMPLER_ARG, in favor of OTEL_SAMPLING_PROBABILITY. --- .../src/utils/environment.ts | 10 +- .../test/utils/environment.test.ts | 24 +--- packages/opentelemetry-tracing/README.md | 3 +- packages/opentelemetry-tracing/src/config.ts | 78 +++++++++++- packages/opentelemetry-tracing/src/utility.ts | 20 +--- .../test/BasicTracerProvider.test.ts | 4 +- .../opentelemetry-tracing/test/Tracer.test.ts | 111 ++++++++---------- .../opentelemetry-tracing/test/config.test.ts | 102 ++++++++++++++++ .../test/export/ConsoleSpanExporter.test.ts | 5 +- 9 files changed, 250 insertions(+), 107 deletions(-) create mode 100644 packages/opentelemetry-tracing/test/config.test.ts diff --git a/packages/opentelemetry-core/src/utils/environment.ts b/packages/opentelemetry-core/src/utils/environment.ts index ac85665699..8786cd3e1c 100644 --- a/packages/opentelemetry-core/src/utils/environment.ts +++ b/packages/opentelemetry-core/src/utils/environment.ts @@ -27,7 +27,6 @@ const ENVIRONMENT_NUMBERS_KEYS = [ 'OTEL_BSP_MAX_EXPORT_BATCH_SIZE', 'OTEL_BSP_MAX_QUEUE_SIZE', 'OTEL_BSP_SCHEDULE_DELAY', - 'OTEL_SAMPLING_PROBABILITY', 'OTEL_SPAN_ATTRIBUTE_COUNT_LIMIT', 'OTEL_SPAN_EVENT_COUNT_LIMIT', 'OTEL_SPAN_LINK_COUNT_LIMIT', @@ -70,6 +69,8 @@ export type ENVIRONMENT = { OTEL_EXPORTER_ZIPKIN_ENDPOINT?: string; OTEL_LOG_LEVEL?: DiagLogLevel; OTEL_RESOURCE_ATTRIBUTES?: string; + OTEL_TRACES_SAMPLER_ARG?: string; + OTEL_TRACES_SAMPLER?: string; } & ENVIRONMENT_NUMBERS & ENVIRONMENT_LISTS; @@ -100,10 +101,11 @@ export const DEFAULT_ENVIRONMENT: Required = { OTEL_NO_PATCH_MODULES: [], OTEL_PROPAGATORS: ['tracecontext', 'baggage'], OTEL_RESOURCE_ATTRIBUTES: '', - OTEL_SAMPLING_PROBABILITY: 1, OTEL_SPAN_ATTRIBUTE_COUNT_LIMIT: 128, OTEL_SPAN_EVENT_COUNT_LIMIT: 128, OTEL_SPAN_LINK_COUNT_LIMIT: 128, + OTEL_TRACES_SAMPLER_ARG: '', + OTEL_TRACES_SAMPLER: 'parentbased_always_on', }; /** @@ -196,10 +198,6 @@ export function parseEnvironment(values: RAW_ENVIRONMENT): ENVIRONMENT { const key = env as keyof ENVIRONMENT; switch (key) { - case 'OTEL_SAMPLING_PROBABILITY': - parseNumber(key, environment, values, 0, 1); - break; - case 'OTEL_LOG_LEVEL': setLogLevelFromEnv(key, environment, values); break; diff --git a/packages/opentelemetry-core/test/utils/environment.test.ts b/packages/opentelemetry-core/test/utils/environment.test.ts index 38cd58a8f6..d2112e69df 100644 --- a/packages/opentelemetry-core/test/utils/environment.test.ts +++ b/packages/opentelemetry-core/test/utils/environment.test.ts @@ -83,15 +83,15 @@ describe('environment', () => { OTEL_LOG_LEVEL: 'ERROR', OTEL_NO_PATCH_MODULES: 'a,b,c', OTEL_RESOURCE_ATTRIBUTES: '', - OTEL_SAMPLING_PROBABILITY: '0.5', OTEL_SPAN_ATTRIBUTE_COUNT_LIMIT: 10, OTEL_SPAN_EVENT_COUNT_LIMIT: 20, OTEL_SPAN_LINK_COUNT_LIMIT: 30, + OTEL_TRACES_SAMPLER: 'always_on', + OTEL_TRACES_SAMPLER_ARG: '0.5', }); const env = getEnv(); assert.deepStrictEqual(env.OTEL_NO_PATCH_MODULES, ['a', 'b', 'c']); assert.strictEqual(env.OTEL_LOG_LEVEL, DiagLogLevel.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); @@ -117,20 +117,8 @@ describe('environment', () => { assert.strictEqual(env.OTEL_RESOURCE_ATTRIBUTES, ''); assert.strictEqual(env.OTEL_BSP_MAX_EXPORT_BATCH_SIZE, 40); assert.strictEqual(env.OTEL_BSP_SCHEDULE_DELAY, 50); - }); - - 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); + assert.strictEqual(env.OTEL_TRACES_SAMPLER, 'always_on'); + assert.strictEqual(env.OTEL_TRACES_SAMPLER_ARG, '0.5'); }); it('should parse OTEL_LOG_LEVEL despite casing', () => { @@ -158,12 +146,12 @@ describe('environment', () => { it('should remove a mock environment', () => { mockEnvironment({ OTEL_LOG_LEVEL: 'DEBUG', - OTEL_SAMPLING_PROBABILITY: 0.5, + OTEL_TRACES_SAMPLER: 'always_off', }); removeMockEnvironment(); const env = getEnv(); assert.strictEqual(env.OTEL_LOG_LEVEL, DiagLogLevel.INFO); - assert.strictEqual(env.OTEL_SAMPLING_PROBABILITY, 1); + assert.strictEqual(env.OTEL_TRACES_SAMPLER, 'parentbased_always_on'); }); }); }); diff --git a/packages/opentelemetry-tracing/README.md b/packages/opentelemetry-tracing/README.md index 46bc2557f5..476ece8e7c 100644 --- a/packages/opentelemetry-tracing/README.md +++ b/packages/opentelemetry-tracing/README.md @@ -46,8 +46,7 @@ span.end(); Tracing configuration is a merge of user supplied configuration with both the default configuration as specified in [config.ts](./src/config.ts) and an -environmentally configurable (via `OTEL_SAMPLING_PROBABILITY`) probability -sampler delegate of a [ParentBased](https://github.com/open-telemetry/opentelemetry-specification/blob/master/specification/trace/sdk.md#parentbased) sampler. +environmentally configurable sampling (via `OTEL_TRACES_SAMPLER` and `OTEL_TRACES_SAMPLER_ARG`). ## Example diff --git a/packages/opentelemetry-tracing/src/config.ts b/packages/opentelemetry-tracing/src/config.ts index a079458cd4..f6cceba863 100644 --- a/packages/opentelemetry-tracing/src/config.ts +++ b/packages/opentelemetry-tracing/src/config.ts @@ -14,7 +14,17 @@ * limitations under the License. */ -import { AlwaysOnSampler, getEnv } from '@opentelemetry/core'; +import { diag, Sampler } from '@opentelemetry/api'; +import { + AlwaysOffSampler, + AlwaysOnSampler, + getEnv, + ParentBasedSampler, + TraceIdRatioBasedSampler, +} from '@opentelemetry/core'; +import { ENVIRONMENT } from '@opentelemetry/core/src/utils/environment'; + +const env = getEnv(); /** * Default configuration. For fields with primitive values, any user-provided @@ -23,10 +33,74 @@ import { AlwaysOnSampler, getEnv } from '@opentelemetry/core'; * used to extend the default value. */ export const DEFAULT_CONFIG = { - sampler: new AlwaysOnSampler(), + sampler: buildSamplerFromEnv(env), traceParams: { numberOfAttributesPerSpan: getEnv().OTEL_SPAN_ATTRIBUTE_COUNT_LIMIT, numberOfLinksPerSpan: getEnv().OTEL_SPAN_LINK_COUNT_LIMIT, numberOfEventsPerSpan: getEnv().OTEL_SPAN_EVENT_COUNT_LIMIT, }, }; + +/** + * Based on environment, builds a sampler, complies with specification. + * @param env optional, by default uses getEnv(), but allows passing a value to reuse parsed environment + */ +export function buildSamplerFromEnv( + env: Required = getEnv() +): Sampler { + switch (env.OTEL_TRACES_SAMPLER) { + case 'always_on': + return new AlwaysOnSampler(); + case 'always_off': + return new AlwaysOffSampler(); + case 'parentbased_always_on': + return new ParentBasedSampler({ + root: new AlwaysOnSampler(), + }); + case 'parentbased_always_off': + return new ParentBasedSampler({ + root: new AlwaysOffSampler(), + }); + case 'traceidratio': + return new TraceIdRatioBasedSampler(getSamplerProbabilityFromEnv(env)); + case 'parentbased_traceidratio': + return new ParentBasedSampler({ + root: new TraceIdRatioBasedSampler(getSamplerProbabilityFromEnv(env)), + }); + default: + diag.error( + `OTEL_TRACES_SAMPLER value "${env.OTEL_TRACES_SAMPLER} invalid, defaulting to always_on".` + ); + return new AlwaysOnSampler(); + } +} + +function getSamplerProbabilityFromEnv( + env: Required +): number | undefined { + if ( + env.OTEL_TRACES_SAMPLER_ARG === undefined || + env.OTEL_TRACES_SAMPLER_ARG === '' + ) { + diag.error('"OTEL_TRACES_SAMPLER_ARG is empty, defaulting to 1.'); + return 1; + } + + const probability = Number(env.OTEL_TRACES_SAMPLER_ARG); + + if (isNaN(probability)) { + diag.error( + `"${env.OTEL_TRACES_SAMPLER_ARG}" was given as OTEL_TRACES_SAMPLER_ARG, but is invalid.` + ); + return undefined; + } + + if (probability < 0 || probability > 1) { + diag.error( + `OTEL_TRACES_SAMPLER_ARG value "${env.OTEL_TRACES_SAMPLER_ARG}" out of range ([0..1]).` + ); + return undefined; + } + + return probability; +} diff --git a/packages/opentelemetry-tracing/src/utility.ts b/packages/opentelemetry-tracing/src/utility.ts index 0150623865..615d4d57f3 100644 --- a/packages/opentelemetry-tracing/src/utility.ts +++ b/packages/opentelemetry-tracing/src/utility.ts @@ -14,32 +14,22 @@ * limitations under the License. */ -import { DEFAULT_CONFIG } from './config'; +import { buildSamplerFromEnv, DEFAULT_CONFIG } from './config'; import { TracerConfig } from './types'; -import { - ParentBasedSampler, - TraceIdRatioBasedSampler, - getEnv, -} from '@opentelemetry/core'; /** * Function to merge Default configuration (as specified in './config') with * user provided configurations. */ export function mergeConfig(userConfig: TracerConfig) { - const otelSamplingProbability = getEnv().OTEL_SAMPLING_PROBABILITY; + const perInstanceDefaults: Partial = { + sampler: buildSamplerFromEnv(), + }; const target = Object.assign( {}, DEFAULT_CONFIG, - // use default AlwaysOnSampler if otelSamplingProbability is 1 - otelSamplingProbability !== undefined && otelSamplingProbability < 1 - ? { - sampler: new ParentBasedSampler({ - root: new TraceIdRatioBasedSampler(otelSamplingProbability), - }), - } - : {}, + perInstanceDefaults, userConfig ); diff --git a/packages/opentelemetry-tracing/test/BasicTracerProvider.test.ts b/packages/opentelemetry-tracing/test/BasicTracerProvider.test.ts index 0f64753a24..04c30a38a2 100644 --- a/packages/opentelemetry-tracing/test/BasicTracerProvider.test.ts +++ b/packages/opentelemetry-tracing/test/BasicTracerProvider.test.ts @@ -315,7 +315,9 @@ describe('BasicTracerProvider', () => { }); it('should start a span with name and with invalid parent span', () => { - const tracer = new BasicTracerProvider().getTracer('default'); + const tracer = new BasicTracerProvider({ + sampler: new AlwaysOnSampler(), + }).getTracer('default'); const span = tracer.startSpan( 'my-span', {}, diff --git a/packages/opentelemetry-tracing/test/Tracer.test.ts b/packages/opentelemetry-tracing/test/Tracer.test.ts index 883d9b2791..9359b983ec 100644 --- a/packages/opentelemetry-tracing/test/Tracer.test.ts +++ b/packages/opentelemetry-tracing/test/Tracer.test.ts @@ -34,6 +34,9 @@ import { describe('Tracer', () => { const tracerProvider = new BasicTracerProvider(); + const envSource = (typeof window !== 'undefined' + ? window + : process.env) as any; class TestSampler implements Sampler { shouldSample() { @@ -47,9 +50,8 @@ describe('Tracer', () => { } afterEach(() => { - if (typeof process !== 'undefined' && process.release.name === 'node') { - delete process.env.OTEL_SAMPLING_PROBABILITY; - } + delete envSource.OTEL_TRACES_SAMPLER; + delete envSource.OTEL_TRACES_SAMPLER_ARG; }); it('should create a Tracer instance', () => { @@ -61,13 +63,16 @@ describe('Tracer', () => { assert.ok(tracer instanceof Tracer); }); - it('should use an AlwaysOnSampler by default', () => { + it('should use an ParentBasedSampler by default', () => { const tracer = new Tracer( { name: 'default', version: '0.0.1' }, {}, tracerProvider ); - assert.strictEqual(tracer['_sampler'].toString(), 'AlwaysOnSampler'); + assert.strictEqual( + tracer['_sampler'].toString(), + 'ParentBased{root=AlwaysOnSampler, remoteParentSampled=AlwaysOnSampler, remoteParentNotSampled=AlwaysOffSampler, localParentSampled=AlwaysOnSampler, localParentNotSampled=AlwaysOffSampler}' + ); }); it('should respect NO_RECORD sampling result', () => { @@ -174,63 +179,45 @@ describe('Tracer', () => { assert.strictEqual((span as Span).parentSpanId, undefined); }); - if (typeof process !== 'undefined' && process.release.name === 'node') { - it('should sample a trace when OTEL_SAMPLING_PROBABILITY is invalid', () => { - process.env.OTEL_SAMPLING_PROBABILITY = 'invalid value'; - const tracer = new Tracer( - { name: 'default', version: '0.0.1' }, - {}, - tracerProvider - ); - const span = tracer.startSpan('my-span'); - const context = span.context(); - assert.strictEqual(context.traceFlags, TraceFlags.SAMPLED); - span.end(); - }); - } - - if (typeof process !== 'undefined' && process.release.name === 'node') { - it('should sample a trace when OTEL_SAMPLING_PROBABILITY is greater than 1', () => { - process.env.OTEL_SAMPLING_PROBABILITY = '2'; - const tracer = new Tracer( - { name: 'default', version: '0.0.1' }, - {}, - tracerProvider - ); - const span = tracer.startSpan('my-span'); - const context = span.context(); - assert.strictEqual(context.traceFlags, TraceFlags.SAMPLED); - span.end(); - }); - } + it('should sample a trace when OTEL_TRACES_SAMPLER_ARG is unset', () => { + envSource.OTEL_TRACES_SAMPLER = 'traceidratio'; + envSource.OTEL_TRACES_SAMPLER_ARG = ''; + const tracer = new Tracer( + { name: 'default', version: '0.0.1' }, + {}, + tracerProvider + ); + const span = tracer.startSpan('my-span'); + const context = span.context(); + assert.strictEqual(context.traceFlags, TraceFlags.SAMPLED); + span.end(); + }); - if (typeof process !== 'undefined' && process.release.name === 'node') { - it('should not sample a trace when OTEL_SAMPLING_PROBABILITY is 0', () => { - process.env.OTEL_SAMPLING_PROBABILITY = '0'; - const tracer = new Tracer( - { name: 'default', version: '0.0.1' }, - {}, - tracerProvider - ); - const span = tracer.startSpan('my-span'); - const context = span.context(); - assert.strictEqual(context.traceFlags, TraceFlags.NONE); - span.end(); - }); - } + it('should not sample a trace when OTEL_TRACES_SAMPLER_ARG is out of range', () => { + envSource.OTEL_TRACES_SAMPLER = 'traceidratio'; + envSource.OTEL_TRACES_SAMPLER_ARG = '2'; + const tracer = new Tracer( + { name: 'default', version: '0.0.1' }, + {}, + tracerProvider + ); + const span = tracer.startSpan('my-span'); + const context = span.context(); + assert.strictEqual(context.traceFlags, TraceFlags.NONE); + span.end(); + }); - if (typeof process !== 'undefined' && process.release.name === 'node') { - it('should not sample a trace when OTEL_SAMPLING_PROBABILITY is less than 0', () => { - process.env.OTEL_SAMPLING_PROBABILITY = '-1'; - const tracer = new Tracer( - { name: 'default', version: '0.0.1' }, - {}, - tracerProvider - ); - const span = tracer.startSpan('my-span'); - const context = span.context(); - assert.strictEqual(context.traceFlags, TraceFlags.NONE); - span.end(); - }); - } + it('should not sample a trace when OTEL_TRACES_SAMPLER_ARG is 0', () => { + envSource.OTEL_TRACES_SAMPLER = 'traceidratio'; + envSource.OTEL_TRACES_SAMPLER_ARG = '0'; + const tracer = new Tracer( + { name: 'default', version: '0.0.1' }, + {}, + tracerProvider + ); + const span = tracer.startSpan('my-span'); + const context = span.context(); + assert.strictEqual(context.traceFlags, TraceFlags.NONE); + span.end(); + }); }); diff --git a/packages/opentelemetry-tracing/test/config.test.ts b/packages/opentelemetry-tracing/test/config.test.ts new file mode 100644 index 0000000000..7f81183c13 --- /dev/null +++ b/packages/opentelemetry-tracing/test/config.test.ts @@ -0,0 +1,102 @@ +/* + * 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 { + AlwaysOffSampler, + AlwaysOnSampler, + ParentBasedSampler, + TraceIdRatioBasedSampler, +} from '@opentelemetry/core'; +import * as assert from 'assert'; +import { buildSamplerFromEnv } from '../src/config'; + +describe('config', () => { + const envSource = (typeof window !== 'undefined' + ? window + : process.env) as any; + + describe('buildSamplerFromEnv()', () => { + afterEach(() => { + delete envSource.OTEL_TRACES_SAMPLER; + delete envSource.OTEL_TRACES_SAMPLER_ARG; + }); + + it('should handle always_on case', () => { + envSource.OTEL_TRACES_SAMPLER = 'always_on'; + assert.ok(buildSamplerFromEnv() instanceof AlwaysOnSampler); + assert.strictEqual(buildSamplerFromEnv().toString(), 'AlwaysOnSampler'); + }); + + it('should handle always_off case', () => { + envSource.OTEL_TRACES_SAMPLER = 'always_off'; + assert.ok(buildSamplerFromEnv() instanceof AlwaysOffSampler); + assert.strictEqual(buildSamplerFromEnv().toString(), 'AlwaysOffSampler'); + }); + + it('should handle traceidratio case', () => { + envSource.OTEL_TRACES_SAMPLER = 'traceidratio'; + envSource.OTEL_TRACES_SAMPLER_ARG = '0.1'; + assert.ok(buildSamplerFromEnv() instanceof TraceIdRatioBasedSampler); + assert.strictEqual( + buildSamplerFromEnv().toString(), + 'TraceIdRatioBased{0.1}' + ); + }); + + it('should handle parentbased_always_on case', () => { + envSource.OTEL_TRACES_SAMPLER = 'parentbased_always_on'; + assert.ok(buildSamplerFromEnv() instanceof ParentBasedSampler); + assert.strictEqual( + buildSamplerFromEnv().toString(), + 'ParentBased{root=AlwaysOnSampler, remoteParentSampled=AlwaysOnSampler, remoteParentNotSampled=AlwaysOffSampler, localParentSampled=AlwaysOnSampler, localParentNotSampled=AlwaysOffSampler}' + ); + }); + + it('should handle parentbased_always_off case', () => { + envSource.OTEL_TRACES_SAMPLER = 'parentbased_always_off'; + assert.ok(buildSamplerFromEnv() instanceof ParentBasedSampler); + assert.strictEqual( + buildSamplerFromEnv().toString(), + 'ParentBased{root=AlwaysOffSampler, remoteParentSampled=AlwaysOnSampler, remoteParentNotSampled=AlwaysOffSampler, localParentSampled=AlwaysOnSampler, localParentNotSampled=AlwaysOffSampler}' + ); + }); + + it('should handle parentbased_traceidratio case', () => { + envSource.OTEL_TRACES_SAMPLER = 'parentbased_traceidratio'; + envSource.OTEL_TRACES_SAMPLER_ARG = '0.2'; + assert.ok(buildSamplerFromEnv() instanceof ParentBasedSampler); + assert.strictEqual( + buildSamplerFromEnv().toString(), + 'ParentBased{root=TraceIdRatioBased{0.2}, remoteParentSampled=AlwaysOnSampler, remoteParentNotSampled=AlwaysOffSampler, localParentSampled=AlwaysOnSampler, localParentNotSampled=AlwaysOffSampler}' + ); + }); + + it('should handle default case with probability 1', () => { + assert.ok(buildSamplerFromEnv() instanceof ParentBasedSampler); + assert.strictEqual( + buildSamplerFromEnv().toString(), + 'ParentBased{root=AlwaysOnSampler, remoteParentSampled=AlwaysOnSampler, remoteParentNotSampled=AlwaysOffSampler, localParentSampled=AlwaysOnSampler, localParentNotSampled=AlwaysOffSampler}' + ); + }); + + it('should handle default case with probability lower than 1', () => { + assert.ok(buildSamplerFromEnv() instanceof ParentBasedSampler); + assert.strictEqual( + buildSamplerFromEnv().toString(), + 'ParentBased{root=AlwaysOnSampler, remoteParentSampled=AlwaysOnSampler, remoteParentNotSampled=AlwaysOffSampler, localParentSampled=AlwaysOnSampler, localParentNotSampled=AlwaysOffSampler}' + ); + }); + }); +}); diff --git a/packages/opentelemetry-tracing/test/export/ConsoleSpanExporter.test.ts b/packages/opentelemetry-tracing/test/export/ConsoleSpanExporter.test.ts index 7efac48ea6..1370bdbec3 100644 --- a/packages/opentelemetry-tracing/test/export/ConsoleSpanExporter.test.ts +++ b/packages/opentelemetry-tracing/test/export/ConsoleSpanExporter.test.ts @@ -14,6 +14,7 @@ * limitations under the License. */ +import { AlwaysOnSampler } from '@opentelemetry/core'; import * as assert from 'assert'; import * as sinon from 'sinon'; import { @@ -40,7 +41,9 @@ describe('ConsoleSpanExporter', () => { describe('.export()', () => { it('should export information about span', () => { assert.doesNotThrow(() => { - const basicTracerProvider = new BasicTracerProvider(); + const basicTracerProvider = new BasicTracerProvider({ + sampler: new AlwaysOnSampler(), + }); consoleExporter = new ConsoleSpanExporter(); const spyConsole = sinon.spy(console, 'log'); From adf6624999ce641b2df338618d20b94f1ba5cc37 Mon Sep 17 00:00:00 2001 From: Jakub Malinowski Date: Thu, 15 Apr 2021 21:57:19 +0200 Subject: [PATCH 2/3] fix: feedback in the PR --- README.md | 4 +++- packages/opentelemetry-tracing/src/config.ts | 16 ++++++++++------ .../opentelemetry-tracing/test/Tracer.test.ts | 2 +- 3 files changed, 14 insertions(+), 8 deletions(-) diff --git a/README.md b/README.md index 08ef023783..606d112dca 100644 --- a/README.md +++ b/README.md @@ -248,9 +248,11 @@ To request automatic tracing support for a module not on this list, please [file ### 0.18.x to 0.19.0 - All plugins have been removed in favor of instrumentations. - + - The `@opentelemetry/propagator-b3` package previously exported three propagators: `B3Propagator`,`B3SinglePropagator`, and `B3MultiPropagator`, but now only exports the `B3Propagator`. It extracts b3 context in single and multi-header encodings, and injects context using the single-header encoding by default, but can be configured to inject context using the multi-header endcoding during construction: `new B3Propagator({ injectEncoding: B3InjectEncoding.MULTI_HEADER })`. If you were previously using the `B3SinglePropagator` or `B3MultiPropagator` directly, you should update your code to use the `B3Propagator` with the appropriate configuration. See the [readme](./packages/opentelemetry-propagator-b3/readme.md) for full details and usage. +- Sampling configuration via environment variable has changed. If you were using `OTEL_SAMPLING_PROBABILITY` then you should replace it with `OTEL_TRACES_SAMPLER=parentbased_traceidratio` and `OTEL_TRACES_SAMPLER_ARG=` where `` is a number in the [0..1] range, e.g. "0.25". Default is 1.0 if unset. + ### 0.17.0 to 0.18.0 - `diag.setLogLevel` is removed and LogLevel can be set by an optional second parameter to `setLogger` diff --git a/packages/opentelemetry-tracing/src/config.ts b/packages/opentelemetry-tracing/src/config.ts index f6cceba863..087179e7d1 100644 --- a/packages/opentelemetry-tracing/src/config.ts +++ b/packages/opentelemetry-tracing/src/config.ts @@ -75,6 +75,8 @@ export function buildSamplerFromEnv( } } +const DEFAULT_RATIO = 1; + function getSamplerProbabilityFromEnv( env: Required ): number | undefined { @@ -82,24 +84,26 @@ function getSamplerProbabilityFromEnv( env.OTEL_TRACES_SAMPLER_ARG === undefined || env.OTEL_TRACES_SAMPLER_ARG === '' ) { - diag.error('"OTEL_TRACES_SAMPLER_ARG is empty, defaulting to 1.'); - return 1; + diag.error( + `OTEL_TRACES_SAMPLER_ARG is blank, defaulting to ${DEFAULT_RATIO}.` + ); + return DEFAULT_RATIO; } const probability = Number(env.OTEL_TRACES_SAMPLER_ARG); if (isNaN(probability)) { diag.error( - `"${env.OTEL_TRACES_SAMPLER_ARG}" was given as OTEL_TRACES_SAMPLER_ARG, but is invalid.` + `OTEL_TRACES_SAMPLER_ARG=${env.OTEL_TRACES_SAMPLER_ARG} was given, but it is invalid, defaulting to ${DEFAULT_RATIO}.` ); - return undefined; + return DEFAULT_RATIO; } if (probability < 0 || probability > 1) { diag.error( - `OTEL_TRACES_SAMPLER_ARG value "${env.OTEL_TRACES_SAMPLER_ARG}" out of range ([0..1]).` + `OTEL_TRACES_SAMPLER_ARG=${env.OTEL_TRACES_SAMPLER_ARG} was given, but it is out of range ([0..1]), defaulting to ${DEFAULT_RATIO}.` ); - return undefined; + return DEFAULT_RATIO; } return probability; diff --git a/packages/opentelemetry-tracing/test/Tracer.test.ts b/packages/opentelemetry-tracing/test/Tracer.test.ts index 9359b983ec..cde378b4eb 100644 --- a/packages/opentelemetry-tracing/test/Tracer.test.ts +++ b/packages/opentelemetry-tracing/test/Tracer.test.ts @@ -203,7 +203,7 @@ describe('Tracer', () => { ); const span = tracer.startSpan('my-span'); const context = span.context(); - assert.strictEqual(context.traceFlags, TraceFlags.NONE); + assert.strictEqual(context.traceFlags, TraceFlags.SAMPLED); span.end(); }); From c0497040ee5d7d181117efa701332229f65c984c Mon Sep 17 00:00:00 2001 From: Jakub Malinowski Date: Tue, 20 Apr 2021 14:57:16 +0200 Subject: [PATCH 3/3] refactor: extract OTEL_TRACES_SAMPLER values to enum --- packages/opentelemetry-core/src/index.ts | 1 + .../src/utils/environment.ts | 3 ++- .../opentelemetry-core/src/utils/sampling.ts | 24 +++++++++++++++++++ .../test/utils/environment.test.ts | 8 +++++-- packages/opentelemetry-tracing/src/config.ts | 17 +++++++------ 5 files changed, 43 insertions(+), 10 deletions(-) create mode 100644 packages/opentelemetry-core/src/utils/sampling.ts diff --git a/packages/opentelemetry-core/src/index.ts b/packages/opentelemetry-core/src/index.ts index 9245f0148a..871f02dc85 100644 --- a/packages/opentelemetry-core/src/index.ts +++ b/packages/opentelemetry-core/src/index.ts @@ -34,3 +34,4 @@ export * from './trace/TraceState'; export * from './trace/IdGenerator'; export * from './utils/url'; export * from './utils/wrap'; +export * from './utils/sampling'; diff --git a/packages/opentelemetry-core/src/utils/environment.ts b/packages/opentelemetry-core/src/utils/environment.ts index 8786cd3e1c..9554fe64a5 100644 --- a/packages/opentelemetry-core/src/utils/environment.ts +++ b/packages/opentelemetry-core/src/utils/environment.ts @@ -15,6 +15,7 @@ */ import { DiagLogLevel } from '@opentelemetry/api'; +import { TracesSamplerValues } from './sampling'; const DEFAULT_LIST_SEPARATOR = ','; @@ -104,8 +105,8 @@ export const DEFAULT_ENVIRONMENT: Required = { OTEL_SPAN_ATTRIBUTE_COUNT_LIMIT: 128, OTEL_SPAN_EVENT_COUNT_LIMIT: 128, OTEL_SPAN_LINK_COUNT_LIMIT: 128, + OTEL_TRACES_SAMPLER: TracesSamplerValues.ParentBasedAlwaysOn, OTEL_TRACES_SAMPLER_ARG: '', - OTEL_TRACES_SAMPLER: 'parentbased_always_on', }; /** diff --git a/packages/opentelemetry-core/src/utils/sampling.ts b/packages/opentelemetry-core/src/utils/sampling.ts new file mode 100644 index 0000000000..d18f59b6fc --- /dev/null +++ b/packages/opentelemetry-core/src/utils/sampling.ts @@ -0,0 +1,24 @@ +/* + * 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. + */ + +export enum TracesSamplerValues { + AlwaysOff = 'always_off', + AlwaysOn = 'always_on', + ParentBasedAlwaysOff = 'parentbased_always_off', + ParentBasedAlwaysOn = 'parentbased_always_on', + ParentBasedTraceIdRatio = 'parentbased_traceidratio', + TraceIdRatio = 'traceidratio', +} diff --git a/packages/opentelemetry-core/test/utils/environment.test.ts b/packages/opentelemetry-core/test/utils/environment.test.ts index d2112e69df..a993f10083 100644 --- a/packages/opentelemetry-core/test/utils/environment.test.ts +++ b/packages/opentelemetry-core/test/utils/environment.test.ts @@ -23,6 +23,7 @@ import { import * as assert from 'assert'; import * as sinon from 'sinon'; import { DiagLogLevel } from '@opentelemetry/api'; +import { TracesSamplerValues } from '../../src'; let lastMock: RAW_ENVIRONMENT = {}; @@ -146,12 +147,15 @@ describe('environment', () => { it('should remove a mock environment', () => { mockEnvironment({ OTEL_LOG_LEVEL: 'DEBUG', - OTEL_TRACES_SAMPLER: 'always_off', + OTEL_TRACES_SAMPLER: TracesSamplerValues.AlwaysOff, }); removeMockEnvironment(); const env = getEnv(); assert.strictEqual(env.OTEL_LOG_LEVEL, DiagLogLevel.INFO); - assert.strictEqual(env.OTEL_TRACES_SAMPLER, 'parentbased_always_on'); + assert.strictEqual( + env.OTEL_TRACES_SAMPLER, + TracesSamplerValues.ParentBasedAlwaysOn + ); }); }); }); diff --git a/packages/opentelemetry-tracing/src/config.ts b/packages/opentelemetry-tracing/src/config.ts index 087179e7d1..afc4db73df 100644 --- a/packages/opentelemetry-tracing/src/config.ts +++ b/packages/opentelemetry-tracing/src/config.ts @@ -19,6 +19,7 @@ import { AlwaysOffSampler, AlwaysOnSampler, getEnv, + TracesSamplerValues, ParentBasedSampler, TraceIdRatioBasedSampler, } from '@opentelemetry/core'; @@ -41,6 +42,8 @@ export const DEFAULT_CONFIG = { }, }; +const FALLBACK_OTEL_TRACES_SAMPLER = TracesSamplerValues.AlwaysOn; + /** * Based on environment, builds a sampler, complies with specification. * @param env optional, by default uses getEnv(), but allows passing a value to reuse parsed environment @@ -49,27 +52,27 @@ export function buildSamplerFromEnv( env: Required = getEnv() ): Sampler { switch (env.OTEL_TRACES_SAMPLER) { - case 'always_on': + case TracesSamplerValues.AlwaysOn: return new AlwaysOnSampler(); - case 'always_off': + case TracesSamplerValues.AlwaysOff: return new AlwaysOffSampler(); - case 'parentbased_always_on': + case TracesSamplerValues.ParentBasedAlwaysOn: return new ParentBasedSampler({ root: new AlwaysOnSampler(), }); - case 'parentbased_always_off': + case TracesSamplerValues.ParentBasedAlwaysOff: return new ParentBasedSampler({ root: new AlwaysOffSampler(), }); - case 'traceidratio': + case TracesSamplerValues.TraceIdRatio: return new TraceIdRatioBasedSampler(getSamplerProbabilityFromEnv(env)); - case 'parentbased_traceidratio': + case TracesSamplerValues.ParentBasedTraceIdRatio: return new ParentBasedSampler({ root: new TraceIdRatioBasedSampler(getSamplerProbabilityFromEnv(env)), }); default: diag.error( - `OTEL_TRACES_SAMPLER value "${env.OTEL_TRACES_SAMPLER} invalid, defaulting to always_on".` + `OTEL_TRACES_SAMPLER value "${env.OTEL_TRACES_SAMPLER} invalid, defaulting to ${FALLBACK_OTEL_TRACES_SAMPLER}".` ); return new AlwaysOnSampler(); }