From 3c338a8480332e18813300772bfaeba32e7a87dd Mon Sep 17 00:00:00 2001 From: legendecas Date: Fri, 8 May 2020 11:35:52 +0800 Subject: [PATCH] feat: spec compliant sampling result support --- packages/opentelemetry-api/src/index.ts | 1 + .../opentelemetry-api/src/trace/Sampler.ts | 15 ++++- .../src/trace/SamplingResult.ts | 28 ++++++++ .../src/trace/sampler/ProbabilitySampler.ts | 36 +++++++--- .../test/trace/ProbabilitySampler.test.ts | 49 ++++++++++---- packages/opentelemetry-tracing/src/Tracer.ts | 38 +++++++---- .../opentelemetry-tracing/test/Tracer.test.ts | 67 +++++++++++++++++++ 7 files changed, 199 insertions(+), 35 deletions(-) create mode 100644 packages/opentelemetry-api/src/trace/SamplingResult.ts create mode 100644 packages/opentelemetry-tracing/test/Tracer.test.ts diff --git a/packages/opentelemetry-api/src/index.ts b/packages/opentelemetry-api/src/index.ts index a4a7406e716..6b77e9dd159 100644 --- a/packages/opentelemetry-api/src/index.ts +++ b/packages/opentelemetry-api/src/index.ts @@ -39,6 +39,7 @@ export * from './trace/NoopSpan'; export * from './trace/NoopTracer'; export * from './trace/NoopTracerProvider'; export * from './trace/Sampler'; +export * from './trace/SamplingResult'; export * from './trace/span_context'; export * from './trace/span_kind'; export * from './trace/span'; diff --git a/packages/opentelemetry-api/src/trace/Sampler.ts b/packages/opentelemetry-api/src/trace/Sampler.ts index 798b5450f75..d96e936e008 100644 --- a/packages/opentelemetry-api/src/trace/Sampler.ts +++ b/packages/opentelemetry-api/src/trace/Sampler.ts @@ -15,6 +15,10 @@ */ import { SpanContext } from './span_context'; +import { SpanKind } from './span_kind'; +import { Attributes } from './attributes'; +import { Link } from './link'; +import { SamplingResult } from './SamplingResult'; /** * This interface represent a sampler. Sampling is a mechanism to control the @@ -25,12 +29,19 @@ export interface Sampler { /** * Checks whether span needs to be created and tracked. * - * TODO: Consider to add required arguments https://github.com/open-telemetry/opentelemetry-specification/blob/master/specification/sampling-api.md#shouldsample * @param [parentContext] Parent span context. Typically taken from the wire. * Can be null. * @returns whether span should be sampled or not. */ - shouldSample(parentContext?: SpanContext): boolean; + shouldSample( + parentContext: SpanContext | undefined, + traceId: string, + spanId: string, + spanName: string, + spanKind: SpanKind, + attributes: Attributes, + links: Link[] + ): SamplingResult; /** Returns the sampler name or short description with the configuration. */ toString(): string; diff --git a/packages/opentelemetry-api/src/trace/SamplingResult.ts b/packages/opentelemetry-api/src/trace/SamplingResult.ts new file mode 100644 index 00000000000..9cd4a93c9f2 --- /dev/null +++ b/packages/opentelemetry-api/src/trace/SamplingResult.ts @@ -0,0 +1,28 @@ +/*! + * Copyright 2020, 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 { Attributes } from './attributes'; + +export enum SamplingDecision { + NOT_RECORD = 0, + RECORD = 0b01, + RECORD_AND_SAMPLED = 0b11, +} + +export interface SamplingResult { + decision: SamplingDecision; + attributes?: Readonly; +} diff --git a/packages/opentelemetry-core/src/trace/sampler/ProbabilitySampler.ts b/packages/opentelemetry-core/src/trace/sampler/ProbabilitySampler.ts index 398ebb58df6..4f880baa604 100644 --- a/packages/opentelemetry-core/src/trace/sampler/ProbabilitySampler.ts +++ b/packages/opentelemetry-core/src/trace/sampler/ProbabilitySampler.ts @@ -14,7 +14,12 @@ * limitations under the License. */ -import { Sampler, SpanContext, TraceFlags } from '@opentelemetry/api'; +import { + Sampler, + SpanContext, + TraceFlags, + SamplingDecision, +} from '@opentelemetry/api'; /** Sampler that samples a given fraction of traces. */ export class ProbabilitySampler implements Sampler { @@ -22,16 +27,31 @@ export class ProbabilitySampler implements Sampler { this._probability = this._normalize(_probability); } - shouldSample(parentContext?: SpanContext) { + shouldSample(parentContext: SpanContext | null = null) { // Respect the parent sampling decision if there is one if (parentContext && typeof parentContext.traceFlags !== 'undefined') { - return ( - (TraceFlags.SAMPLED & parentContext.traceFlags) === TraceFlags.SAMPLED - ); + return { + decision: + (TraceFlags.SAMPLED & parentContext.traceFlags) === TraceFlags.SAMPLED + ? SamplingDecision.RECORD_AND_SAMPLED + : SamplingDecision.NOT_RECORD, + }; } - if (this._probability >= 1.0) return true; - else if (this._probability <= 0) return false; - return Math.random() < this._probability; + if (this._probability >= 1.0) { + return { + decision: SamplingDecision.RECORD_AND_SAMPLED, + }; + } else if (this._probability <= 0) { + return { + decision: SamplingDecision.NOT_RECORD, + }; + } + return { + decision: + Math.random() < this._probability + ? SamplingDecision.RECORD_AND_SAMPLED + : SamplingDecision.NOT_RECORD, + }; } toString(): string { diff --git a/packages/opentelemetry-core/test/trace/ProbabilitySampler.test.ts b/packages/opentelemetry-core/test/trace/ProbabilitySampler.test.ts index b0426899a27..c6d7baed45a 100644 --- a/packages/opentelemetry-core/test/trace/ProbabilitySampler.test.ts +++ b/packages/opentelemetry-core/test/trace/ProbabilitySampler.test.ts @@ -15,6 +15,7 @@ */ import * as assert from 'assert'; +import * as api from '@opentelemetry/api'; import { ProbabilitySampler, ALWAYS_SAMPLER, @@ -24,60 +25,82 @@ import { describe('ProbabilitySampler', () => { it('should return a always sampler for 1', () => { const sampler = new ProbabilitySampler(1); - assert.strictEqual(sampler.shouldSample(), true); + assert.deepStrictEqual(sampler.shouldSample(), { + decision: api.SamplingDecision.RECORD_AND_SAMPLED, + }); }); it('should return a always sampler for >1', () => { const sampler = new ProbabilitySampler(100); - assert.strictEqual(sampler.shouldSample(), true); + assert.deepStrictEqual(sampler.shouldSample(), { + decision: api.SamplingDecision.RECORD_AND_SAMPLED, + }); assert.strictEqual(sampler.toString(), 'ProbabilitySampler{1}'); }); it('should return a never sampler for 0', () => { const sampler = new ProbabilitySampler(0); - assert.strictEqual(sampler.shouldSample(), false); + assert.deepStrictEqual(sampler.shouldSample(), { + decision: api.SamplingDecision.NOT_RECORD, + }); }); it('should return a never sampler for <0', () => { const sampler = new ProbabilitySampler(-1); - assert.strictEqual(sampler.shouldSample(), false); + assert.deepStrictEqual(sampler.shouldSample(), { + decision: api.SamplingDecision.NOT_RECORD, + }); }); it('should sample according to the probability', () => { Math.random = () => 1 / 10; const sampler = new ProbabilitySampler(0.2); - assert.strictEqual(sampler.shouldSample(), true); + assert.deepStrictEqual(sampler.shouldSample(), { + decision: api.SamplingDecision.RECORD_AND_SAMPLED, + }); assert.strictEqual(sampler.toString(), 'ProbabilitySampler{0.2}'); Math.random = () => 5 / 10; - assert.strictEqual(sampler.shouldSample(), false); + assert.deepStrictEqual(sampler.shouldSample(), { + decision: api.SamplingDecision.NOT_RECORD, + }); }); - it('should return true for ALWAYS_SAMPLER', () => { - assert.strictEqual(ALWAYS_SAMPLER.shouldSample(), true); + it('should return api.SamplingDecision.RECORD_AND_SAMPLED for ALWAYS_SAMPLER', () => { + assert.deepStrictEqual(ALWAYS_SAMPLER.shouldSample(), { + decision: api.SamplingDecision.RECORD_AND_SAMPLED, + }); assert.strictEqual(ALWAYS_SAMPLER.toString(), 'ProbabilitySampler{1}'); }); - it('should return false for NEVER_SAMPLER', () => { - assert.strictEqual(NEVER_SAMPLER.shouldSample(), false); + it('should return decision: api.SamplingDecision.NOT_RECORD for NEVER_SAMPLER', () => { + assert.deepStrictEqual(NEVER_SAMPLER.shouldSample(), { + decision: api.SamplingDecision.NOT_RECORD, + }); assert.strictEqual(NEVER_SAMPLER.toString(), 'ProbabilitySampler{0}'); }); it('should handle NaN', () => { const sampler = new ProbabilitySampler(NaN); - assert.strictEqual(sampler.shouldSample(), false); + assert.deepStrictEqual(sampler.shouldSample(), { + decision: api.SamplingDecision.NOT_RECORD, + }); assert.strictEqual(sampler.toString(), 'ProbabilitySampler{0}'); }); it('should handle -NaN', () => { const sampler = new ProbabilitySampler(-NaN); - assert.strictEqual(sampler.shouldSample(), false); + assert.deepStrictEqual(sampler.shouldSample(), { + decision: api.SamplingDecision.NOT_RECORD, + }); assert.strictEqual(sampler.toString(), 'ProbabilitySampler{0}'); }); it('should handle undefined', () => { const sampler = new ProbabilitySampler(undefined); - assert.strictEqual(sampler.shouldSample(), false); + assert.deepStrictEqual(sampler.shouldSample(), { + decision: api.SamplingDecision.NOT_RECORD, + }); assert.strictEqual(sampler.toString(), 'ProbabilitySampler{0}'); }); }); diff --git a/packages/opentelemetry-tracing/src/Tracer.ts b/packages/opentelemetry-tracing/src/Tracer.ts index 24408d80e48..03a6652ea73 100644 --- a/packages/opentelemetry-tracing/src/Tracer.ts +++ b/packages/opentelemetry-tracing/src/Tracer.ts @@ -66,8 +66,6 @@ export class Tracer implements api.Tracer { context = api.context.active() ): api.Span { const parentContext = getParent(options, context); - // make sampling decision - const samplingDecision = this._sampler.shouldSample(parentContext); const spanId = randomSpanId(); let traceId; let traceState; @@ -79,12 +77,30 @@ export class Tracer implements api.Tracer { traceId = parentContext.traceId; traceState = parentContext.traceState; } - const traceFlags = samplingDecision - ? api.TraceFlags.SAMPLED - : api.TraceFlags.NONE; + const spanKind = options.kind ?? api.SpanKind.INTERNAL; + const links = options.links ?? []; + const attributes = { ...this._defaultAttributes, ...options.attributes }; + // make sampling decision + const samplingResult = this._sampler.shouldSample( + parentContext, + traceId, + spanId, + name, + spanKind, + attributes, + links + ); + + const traceFlags = + samplingResult.decision === api.SamplingDecision.RECORD_AND_SAMPLED + ? api.TraceFlags.SAMPLED + : api.TraceFlags.NONE; const spanContext = { traceId, spanId, traceFlags, traceState }; - if (!samplingDecision) { - this.logger.debug('Sampling is off, starting no recording span'); + if ( + (samplingResult.decision & api.SamplingDecision.RECORD) !== + api.SamplingDecision.RECORD + ) { + this.logger.debug('Recording is off, starting no recording span'); return new NoRecordingSpan(spanContext); } @@ -92,15 +108,13 @@ export class Tracer implements api.Tracer { this, name, spanContext, - options.kind || api.SpanKind.INTERNAL, + spanKind, parentContext ? parentContext.spanId : undefined, - options.links || [], + links, options.startTime ); // Set default attributes - span.setAttributes( - Object.assign({}, this._defaultAttributes, options.attributes) - ); + span.setAttributes(Object.assign(attributes, samplingResult.attributes)); return span; } diff --git a/packages/opentelemetry-tracing/test/Tracer.test.ts b/packages/opentelemetry-tracing/test/Tracer.test.ts new file mode 100644 index 00000000000..78b8cbedfda --- /dev/null +++ b/packages/opentelemetry-tracing/test/Tracer.test.ts @@ -0,0 +1,67 @@ +/*! + * Copyright 2020, 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 * as assert from 'assert'; +import { + NoopSpan, + Sampler, + SamplingDecision, +} from '@opentelemetry/api'; +import { BasicTracerProvider, Tracer, Span } from '../src'; +import { NoopLogger, ALWAYS_SAMPLER, NEVER_SAMPLER } from '@opentelemetry/core'; + +describe('Tracer', () => { + const tracerProvider = new BasicTracerProvider({ + logger: new NoopLogger(), + }); + + class TestSampler implements Sampler { + shouldSample() { + return { + decision: SamplingDecision.RECORD_AND_SAMPLED, + attributes: { + testAttribute: 'foobar', + }, + }; + } + } + + it('should create a Tracer instance', () => { + const tracer = new Tracer({}, tracerProvider); + assert.ok(tracer instanceof Tracer); + }); + + it('should respect NO_RECORD sampling result', () => { + const tracer = new Tracer({ sampler: NEVER_SAMPLER }, tracerProvider); + const span = tracer.startSpan('span1'); + assert.ok(span instanceof NoopSpan); + span.end(); + }); + + it('should respect RECORD_AND_SAMPLE sampling result', () => { + const tracer = new Tracer({ sampler: ALWAYS_SAMPLER }, tracerProvider); + const span = tracer.startSpan('span2'); + assert.ok(!(span instanceof NoopSpan)); + span.end(); + }); + + it('should start a span with attributes in sampling result', () => { + const tracer = new Tracer({ sampler: new TestSampler() }, tracerProvider); + const span = tracer.startSpan('span3'); + assert.strictEqual((span as Span).attributes.testAttribute, 'foobar'); + span.end(); + }); +});