From e69bd5301619627f2bc347ae976376a0deddee58 Mon Sep 17 00:00:00 2001 From: Daniel Dyla Date: Wed, 7 Oct 2020 12:01:35 -0400 Subject: [PATCH 1/9] feat: implement parent based sampler --- packages/opentelemetry-core/README.md | 33 +++++++- packages/opentelemetry-core/src/index.ts | 2 +- .../src/trace/sampler/ParentBasedSampler.ts | 82 +++++++++++++++++++ .../src/trace/sampler/ParentOrElseSampler.ts | 65 --------------- ...ler.test.ts => ParentBasedSampler.test.ts} | 24 +++--- packages/opentelemetry-tracing/README.md | 2 +- packages/opentelemetry-tracing/src/utility.ts | 8 +- 7 files changed, 129 insertions(+), 87 deletions(-) create mode 100644 packages/opentelemetry-core/src/trace/sampler/ParentBasedSampler.ts delete mode 100644 packages/opentelemetry-core/src/trace/sampler/ParentOrElseSampler.ts rename packages/opentelemetry-core/test/trace/{ParentOrElseSampler.test.ts => ParentBasedSampler.test.ts} (68%) diff --git a/packages/opentelemetry-core/README.md b/packages/opentelemetry-core/README.md index bd534ec0118..3f8027e8493 100644 --- a/packages/opentelemetry-core/README.md +++ b/packages/opentelemetry-core/README.md @@ -121,16 +121,41 @@ const tracerProvider = new NodeTracerProvider({ }); ``` -#### ParentOrElse +#### ParentBasedSampler -A composite sampler that either respects the parent span's sampling decision or delegates to `delegateSampler` for root spans. +* This is a composite sampler. `ParentBased` helps distinguished between the +following cases: + * No parent (root span). + * Remote parent with `sampled` flag `true` + * Remote parent with `sampled` flag `false` + * Local parent with `sampled` flag `true` + * Local parent with `sampled` flag `false` + +Required parameters: + +* `root(Sampler)` - Sampler called for spans with no parent (root spans) + +Optional parameters: + +* `remoteParentSampled(Sampler)` (default: `AlwaysOn`) +* `remoteParentNotSampled(Sampler)` (default: `AlwaysOff`) +* `localParentSampled(Sampler)` (default: `AlwaysOn`) +* `localParentNotSampled(Sampler)` (default: `AlwaysOff`) + +|Parent| parent.isRemote() | parent.isSampled()| Invoke sampler| +|--|--|--|--| +|absent| n/a | n/a |`root()`| +|present|true|true|`remoteParentSampled()`| +|present|true|false|`remoteParentNotSampled()`| +|present|false|true|`localParentSampled()`| +|present|false|false|`localParentNotSampled()`| ```js const { NodeTracerProvider } = require("@opentelemetry/node"); -const { ParentOrElseSampler, AlwaysOffSampler } = require("@opentelemetry/core"); +const { ParentBasedSampler, AlwaysOffSampler, TraceIdRatioBasedSampler } = require("@opentelemetry/core"); const tracerProvider = new NodeTracerProvider({ - sampler: new ParentOrElseSampler(new AlwaysOffSampler()) + sampler: new ParentBasedSampler({ root: new TraceIdRatioBasedSampler(0.5) }) }); ``` diff --git a/packages/opentelemetry-core/src/index.ts b/packages/opentelemetry-core/src/index.ts index b4053a5e07d..98d2586820b 100644 --- a/packages/opentelemetry-core/src/index.ts +++ b/packages/opentelemetry-core/src/index.ts @@ -31,7 +31,7 @@ export * from './platform'; export * from './trace/NoRecordingSpan'; export * from './trace/sampler/AlwaysOffSampler'; export * from './trace/sampler/AlwaysOnSampler'; -export * from './trace/sampler/ParentOrElseSampler'; +export * from './trace/sampler/ParentBasedSampler'; export * from './trace/sampler/TraceIdRatioBasedSampler'; export * from './trace/TraceState'; export * from './trace/IdGenerator'; diff --git a/packages/opentelemetry-core/src/trace/sampler/ParentBasedSampler.ts b/packages/opentelemetry-core/src/trace/sampler/ParentBasedSampler.ts new file mode 100644 index 00000000000..e24bb1ca893 --- /dev/null +++ b/packages/opentelemetry-core/src/trace/sampler/ParentBasedSampler.ts @@ -0,0 +1,82 @@ +/* + * 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 { Attributes, Link, Sampler, SamplingResult, SpanContext, SpanKind, TraceFlags } from '@opentelemetry/api'; +import { AlwaysOffSampler } from './AlwaysOffSampler'; +import { AlwaysOnSampler } from './AlwaysOnSampler'; + +/** + * A composite sampler that either respects the parent span's sampling decision + * or delegates to `delegateSampler` for root spans. + */ +export class ParentBasedSampler implements Sampler { + private root: Sampler; + private remoteParentSampled: Sampler; + private remoteParentNotSampled: Sampler; + private localParentSampled: Sampler; + private localParentNotSampled: Sampler; + + constructor(config: ParentBasedSamplerConfig) { + this.root = config.root; + this.remoteParentSampled = config.remoteParentSampled ?? new AlwaysOnSampler(); + this.remoteParentNotSampled = config.remoteParentNotSampled ?? new AlwaysOffSampler(); + this.localParentSampled = config.localParentSampled ?? new AlwaysOnSampler(); + this.localParentNotSampled = config.localParentNotSampled ?? new AlwaysOffSampler(); + } + + shouldSample( + parentContext: SpanContext | undefined, + traceId: string, + spanName: string, + spanKind: SpanKind, + attributes: Attributes, + links: Link[] + ): SamplingResult { + if (!parentContext) { + return this.root.shouldSample(parentContext, traceId, spanName, spanKind, attributes, links); + } + + if (parentContext.isRemote) { + if (parentContext.traceFlags & TraceFlags.SAMPLED) { + return this.remoteParentSampled.shouldSample(parentContext, traceId, spanName, spanKind, attributes, links) + } + return this.remoteParentNotSampled.shouldSample(parentContext, traceId, spanName, spanKind, attributes, links); + } + + if (parentContext.traceFlags & TraceFlags.SAMPLED) { + return this.localParentSampled.shouldSample(parentContext, traceId, spanName, spanKind, attributes, links); + } + + return this.localParentNotSampled.shouldSample(parentContext, traceId, spanName, spanKind, attributes, links); + } + + toString(): string { + return `ParentBased{root=${this.root.toString()}, remoteParentSampled=${this.remoteParentSampled.toString()}, remoteParentNotSampled=${this.remoteParentNotSampled.toString()}, localParentSampled=${this.localParentSampled.toString()}, localParentNotSampled=${this.localParentNotSampled.toString()}}`; + } +} + +interface ParentBasedSamplerConfig { + /** Sampler called for spans with no parent */ + root: Sampler; + /** Sampler called for spans with a remote parent which was sampled. Default AlwaysOn */ + remoteParentSampled?: Sampler; + /** Sampler called for spans with a remote parent which was not sampled. Default AlwaysOff */ + remoteParentNotSampled?: Sampler; + /** Sampler called for spans with a local parent which was sampled. Default AlwaysOn */ + localParentSampled?: Sampler; + /** Sampler called for spans with a local parent which was not sampled. Default AlwaysOff */ + localParentNotSampled?: Sampler; +} diff --git a/packages/opentelemetry-core/src/trace/sampler/ParentOrElseSampler.ts b/packages/opentelemetry-core/src/trace/sampler/ParentOrElseSampler.ts deleted file mode 100644 index 71495a3c706..00000000000 --- a/packages/opentelemetry-core/src/trace/sampler/ParentOrElseSampler.ts +++ /dev/null @@ -1,65 +0,0 @@ -/* - * 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 { - Sampler, - SpanContext, - TraceFlags, - SamplingDecision, - SpanKind, - Attributes, - Link, - SamplingResult, -} from '@opentelemetry/api'; - -/** - * A composite sampler that either respects the parent span's sampling decision - * or delegates to `delegateSampler` for root spans. - */ -export class ParentOrElseSampler implements Sampler { - constructor(private _delegateSampler: Sampler) {} - - shouldSample( - parentContext: SpanContext | undefined, - traceId: string, - spanName: string, - spanKind: SpanKind, - attributes: Attributes, - links: Link[] - ): SamplingResult { - // Respect the parent sampling decision if there is one - if (parentContext) { - return { - decision: - (TraceFlags.SAMPLED & parentContext.traceFlags) === TraceFlags.SAMPLED - ? SamplingDecision.RECORD_AND_SAMPLED - : SamplingDecision.NOT_RECORD, - }; - } - return this._delegateSampler.shouldSample( - parentContext, - traceId, - spanName, - spanKind, - attributes, - links - ); - } - - toString(): string { - return `ParentOrElse{${this._delegateSampler.toString()}}`; - } -} diff --git a/packages/opentelemetry-core/test/trace/ParentOrElseSampler.test.ts b/packages/opentelemetry-core/test/trace/ParentBasedSampler.test.ts similarity index 68% rename from packages/opentelemetry-core/test/trace/ParentOrElseSampler.test.ts rename to packages/opentelemetry-core/test/trace/ParentBasedSampler.test.ts index 915804fcd62..42346ea4679 100644 --- a/packages/opentelemetry-core/test/trace/ParentOrElseSampler.test.ts +++ b/packages/opentelemetry-core/test/trace/ParentBasedSampler.test.ts @@ -16,7 +16,7 @@ import * as assert from 'assert'; import * as api from '@opentelemetry/api'; import { AlwaysOnSampler } from '../../src/trace/sampler/AlwaysOnSampler'; -import { ParentOrElseSampler } from '../../src/trace/sampler/ParentOrElseSampler'; +import { ParentBasedSampler } from '../../src/trace/sampler/ParentBasedSampler'; import { TraceFlags, SpanKind } from '@opentelemetry/api'; import { AlwaysOffSampler } from '../../src/trace/sampler/AlwaysOffSampler'; import { TraceIdRatioBasedSampler } from '../../src'; @@ -25,23 +25,23 @@ const traceId = 'd4cda95b652f4a1592b449d5929fda1b'; const spanId = '6e0c63257de34c92'; const spanName = 'foobar'; -describe('ParentOrElseSampler', () => { +describe('ParentBasedSampler', () => { it('should reflect sampler name with delegate sampler', () => { - let sampler = new ParentOrElseSampler(new AlwaysOnSampler()); - assert.strictEqual(sampler.toString(), 'ParentOrElse{AlwaysOnSampler}'); + let sampler = new ParentBasedSampler({ root: new AlwaysOnSampler() }); + assert.strictEqual(sampler.toString(), 'ParentBased{root=AlwaysOnSampler, remoteParentSampled=AlwaysOnSampler, remoteParentNotSampled=AlwaysOffSampler, localParentSampled=AlwaysOnSampler, localParentNotSampled=AlwaysOffSampler}'); - sampler = new ParentOrElseSampler(new AlwaysOnSampler()); - assert.strictEqual(sampler.toString(), 'ParentOrElse{AlwaysOnSampler}'); + sampler = new ParentBasedSampler({ root: new AlwaysOffSampler() }); + assert.strictEqual(sampler.toString(), 'ParentBased{root=AlwaysOffSampler, remoteParentSampled=AlwaysOnSampler, remoteParentNotSampled=AlwaysOffSampler, localParentSampled=AlwaysOnSampler, localParentNotSampled=AlwaysOffSampler}'); - sampler = new ParentOrElseSampler(new TraceIdRatioBasedSampler(0.5)); + sampler = new ParentBasedSampler({ root: new TraceIdRatioBasedSampler(0.5) }); assert.strictEqual( sampler.toString(), - 'ParentOrElse{TraceIdRatioBased{0.5}}' + 'ParentBased{root=TraceIdRatioBased{0.5}, remoteParentSampled=AlwaysOnSampler, remoteParentNotSampled=AlwaysOffSampler, localParentSampled=AlwaysOnSampler, localParentNotSampled=AlwaysOffSampler}' ); }); it('should return api.SamplingDecision.NOT_RECORD for not sampled parent while composited with AlwaysOnSampler', () => { - const sampler = new ParentOrElseSampler(new AlwaysOnSampler()); + const sampler = new ParentBasedSampler({ root: new AlwaysOnSampler() }); const spanContext = { traceId, @@ -64,7 +64,7 @@ describe('ParentOrElseSampler', () => { }); it('should return api.SamplingDecision.RECORD_AND_SAMPLED while composited with AlwaysOnSampler', () => { - const sampler = new ParentOrElseSampler(new AlwaysOnSampler()); + const sampler = new ParentBasedSampler({ root: new AlwaysOnSampler() }); assert.deepStrictEqual( sampler.shouldSample( @@ -82,7 +82,7 @@ describe('ParentOrElseSampler', () => { }); it('should return api.SamplingDecision.RECORD_AND_SAMPLED for sampled parent while composited with AlwaysOffSampler', () => { - const sampler = new ParentOrElseSampler(new AlwaysOffSampler()); + const sampler = new ParentBasedSampler({ root: new AlwaysOffSampler() }); const spanContext = { traceId, @@ -105,7 +105,7 @@ describe('ParentOrElseSampler', () => { }); it('should return api.SamplingDecision.RECORD_AND_SAMPLED while composited with AlwaysOffSampler', () => { - const sampler = new ParentOrElseSampler(new AlwaysOffSampler()); + const sampler = new ParentBasedSampler({ root: new AlwaysOffSampler() }); assert.deepStrictEqual( sampler.shouldSample( diff --git a/packages/opentelemetry-tracing/README.md b/packages/opentelemetry-tracing/README.md index 1de732e1273..c43a28810dc 100644 --- a/packages/opentelemetry-tracing/README.md +++ b/packages/opentelemetry-tracing/README.md @@ -48,7 +48,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 [ParentOrElse](/~https://github.com/open-telemetry/opentelemetry-specification/blob/master/specification/trace/sdk.md#parentorelse) sampler. +sampler delegate of a [ParentBased](/~https://github.com/open-telemetry/opentelemetry-specification/blob/master/specification/trace/sdk.md#parentbased) sampler. ## Example diff --git a/packages/opentelemetry-tracing/src/utility.ts b/packages/opentelemetry-tracing/src/utility.ts index 15f5c832f5c..78f2d9b36b6 100644 --- a/packages/opentelemetry-tracing/src/utility.ts +++ b/packages/opentelemetry-tracing/src/utility.ts @@ -22,7 +22,7 @@ import { } from './config'; import { TracerConfig } from './types'; import { - ParentOrElseSampler, + ParentBasedSampler, TraceIdRatioBasedSampler, getEnv, } from '@opentelemetry/core'; @@ -40,9 +40,9 @@ export function mergeConfig(userConfig: TracerConfig) { // use default AlwaysOnSampler if otelSamplingProbability is 1 otelSamplingProbability !== undefined && otelSamplingProbability < 1 ? { - sampler: new ParentOrElseSampler( - new TraceIdRatioBasedSampler(otelSamplingProbability) - ), + sampler: new ParentBasedSampler({ + root: new TraceIdRatioBasedSampler(otelSamplingProbability) + }), } : {}, userConfig From 9a8e24a6669a4a297ee73e20d1855dadbafd7db1 Mon Sep 17 00:00:00 2001 From: Daniel Dyla Date: Wed, 7 Oct 2020 14:04:15 -0400 Subject: [PATCH 2/9] chore: lint --- .../src/trace/sampler/ParentBasedSampler.ts | 67 ++++++++++++++++--- .../test/trace/ParentBasedSampler.test.ts | 14 +++- packages/opentelemetry-tracing/src/utility.ts | 2 +- 3 files changed, 69 insertions(+), 14 deletions(-) diff --git a/packages/opentelemetry-core/src/trace/sampler/ParentBasedSampler.ts b/packages/opentelemetry-core/src/trace/sampler/ParentBasedSampler.ts index e24bb1ca893..4490049e1b3 100644 --- a/packages/opentelemetry-core/src/trace/sampler/ParentBasedSampler.ts +++ b/packages/opentelemetry-core/src/trace/sampler/ParentBasedSampler.ts @@ -14,7 +14,15 @@ * limitations under the License. */ -import { Attributes, Link, Sampler, SamplingResult, SpanContext, SpanKind, TraceFlags } from '@opentelemetry/api'; +import { + Attributes, + Link, + Sampler, + SamplingResult, + SpanContext, + SpanKind, + TraceFlags, +} from '@opentelemetry/api'; import { AlwaysOffSampler } from './AlwaysOffSampler'; import { AlwaysOnSampler } from './AlwaysOnSampler'; @@ -31,10 +39,14 @@ export class ParentBasedSampler implements Sampler { constructor(config: ParentBasedSamplerConfig) { this.root = config.root; - this.remoteParentSampled = config.remoteParentSampled ?? new AlwaysOnSampler(); - this.remoteParentNotSampled = config.remoteParentNotSampled ?? new AlwaysOffSampler(); - this.localParentSampled = config.localParentSampled ?? new AlwaysOnSampler(); - this.localParentNotSampled = config.localParentNotSampled ?? new AlwaysOffSampler(); + this.remoteParentSampled = + config.remoteParentSampled ?? new AlwaysOnSampler(); + this.remoteParentNotSampled = + config.remoteParentNotSampled ?? new AlwaysOffSampler(); + this.localParentSampled = + config.localParentSampled ?? new AlwaysOnSampler(); + this.localParentNotSampled = + config.localParentNotSampled ?? new AlwaysOffSampler(); } shouldSample( @@ -46,21 +58,56 @@ export class ParentBasedSampler implements Sampler { links: Link[] ): SamplingResult { if (!parentContext) { - return this.root.shouldSample(parentContext, traceId, spanName, spanKind, attributes, links); + return this.root.shouldSample( + parentContext, + traceId, + spanName, + spanKind, + attributes, + links + ); } if (parentContext.isRemote) { if (parentContext.traceFlags & TraceFlags.SAMPLED) { - return this.remoteParentSampled.shouldSample(parentContext, traceId, spanName, spanKind, attributes, links) + return this.remoteParentSampled.shouldSample( + parentContext, + traceId, + spanName, + spanKind, + attributes, + links + ); } - return this.remoteParentNotSampled.shouldSample(parentContext, traceId, spanName, spanKind, attributes, links); + return this.remoteParentNotSampled.shouldSample( + parentContext, + traceId, + spanName, + spanKind, + attributes, + links + ); } if (parentContext.traceFlags & TraceFlags.SAMPLED) { - return this.localParentSampled.shouldSample(parentContext, traceId, spanName, spanKind, attributes, links); + return this.localParentSampled.shouldSample( + parentContext, + traceId, + spanName, + spanKind, + attributes, + links + ); } - return this.localParentNotSampled.shouldSample(parentContext, traceId, spanName, spanKind, attributes, links); + return this.localParentNotSampled.shouldSample( + parentContext, + traceId, + spanName, + spanKind, + attributes, + links + ); } toString(): string { diff --git a/packages/opentelemetry-core/test/trace/ParentBasedSampler.test.ts b/packages/opentelemetry-core/test/trace/ParentBasedSampler.test.ts index 42346ea4679..ef43ce70bf1 100644 --- a/packages/opentelemetry-core/test/trace/ParentBasedSampler.test.ts +++ b/packages/opentelemetry-core/test/trace/ParentBasedSampler.test.ts @@ -28,12 +28,20 @@ const spanName = 'foobar'; describe('ParentBasedSampler', () => { it('should reflect sampler name with delegate sampler', () => { let sampler = new ParentBasedSampler({ root: new AlwaysOnSampler() }); - assert.strictEqual(sampler.toString(), 'ParentBased{root=AlwaysOnSampler, remoteParentSampled=AlwaysOnSampler, remoteParentNotSampled=AlwaysOffSampler, localParentSampled=AlwaysOnSampler, localParentNotSampled=AlwaysOffSampler}'); + assert.strictEqual( + sampler.toString(), + 'ParentBased{root=AlwaysOnSampler, remoteParentSampled=AlwaysOnSampler, remoteParentNotSampled=AlwaysOffSampler, localParentSampled=AlwaysOnSampler, localParentNotSampled=AlwaysOffSampler}' + ); sampler = new ParentBasedSampler({ root: new AlwaysOffSampler() }); - assert.strictEqual(sampler.toString(), 'ParentBased{root=AlwaysOffSampler, remoteParentSampled=AlwaysOnSampler, remoteParentNotSampled=AlwaysOffSampler, localParentSampled=AlwaysOnSampler, localParentNotSampled=AlwaysOffSampler}'); + assert.strictEqual( + sampler.toString(), + 'ParentBased{root=AlwaysOffSampler, remoteParentSampled=AlwaysOnSampler, remoteParentNotSampled=AlwaysOffSampler, localParentSampled=AlwaysOnSampler, localParentNotSampled=AlwaysOffSampler}' + ); - sampler = new ParentBasedSampler({ root: new TraceIdRatioBasedSampler(0.5) }); + sampler = new ParentBasedSampler({ + root: new TraceIdRatioBasedSampler(0.5), + }); assert.strictEqual( sampler.toString(), 'ParentBased{root=TraceIdRatioBased{0.5}, remoteParentSampled=AlwaysOnSampler, remoteParentNotSampled=AlwaysOffSampler, localParentSampled=AlwaysOnSampler, localParentNotSampled=AlwaysOffSampler}' diff --git a/packages/opentelemetry-tracing/src/utility.ts b/packages/opentelemetry-tracing/src/utility.ts index 78f2d9b36b6..98fe6fac552 100644 --- a/packages/opentelemetry-tracing/src/utility.ts +++ b/packages/opentelemetry-tracing/src/utility.ts @@ -41,7 +41,7 @@ export function mergeConfig(userConfig: TracerConfig) { otelSamplingProbability !== undefined && otelSamplingProbability < 1 ? { sampler: new ParentBasedSampler({ - root: new TraceIdRatioBasedSampler(otelSamplingProbability) + root: new TraceIdRatioBasedSampler(otelSamplingProbability), }), } : {}, From 25bb11b6c64191383a65614c3421638836621f9a Mon Sep 17 00:00:00 2001 From: Daniel Dyla Date: Wed, 7 Oct 2020 14:41:34 -0400 Subject: [PATCH 3/9] chore: markdown lint --- packages/opentelemetry-core/README.md | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/packages/opentelemetry-core/README.md b/packages/opentelemetry-core/README.md index 3f8027e8493..ffb5eccb1b1 100644 --- a/packages/opentelemetry-core/README.md +++ b/packages/opentelemetry-core/README.md @@ -123,24 +123,24 @@ const tracerProvider = new NodeTracerProvider({ #### ParentBasedSampler -* This is a composite sampler. `ParentBased` helps distinguished between the +- This is a composite sampler. `ParentBased` helps distinguished between the following cases: - * No parent (root span). - * Remote parent with `sampled` flag `true` - * Remote parent with `sampled` flag `false` - * Local parent with `sampled` flag `true` - * Local parent with `sampled` flag `false` + - No parent (root span). + - Remote parent with `sampled` flag `true` + - Remote parent with `sampled` flag `false` + - Local parent with `sampled` flag `true` + - Local parent with `sampled` flag `false` Required parameters: -* `root(Sampler)` - Sampler called for spans with no parent (root spans) +- `root(Sampler)` - Sampler called for spans with no parent (root spans) Optional parameters: -* `remoteParentSampled(Sampler)` (default: `AlwaysOn`) -* `remoteParentNotSampled(Sampler)` (default: `AlwaysOff`) -* `localParentSampled(Sampler)` (default: `AlwaysOn`) -* `localParentNotSampled(Sampler)` (default: `AlwaysOff`) +- `remoteParentSampled(Sampler)` (default: `AlwaysOn`) +- `remoteParentNotSampled(Sampler)` (default: `AlwaysOff`) +- `localParentSampled(Sampler)` (default: `AlwaysOn`) +- `localParentNotSampled(Sampler)` (default: `AlwaysOff`) |Parent| parent.isRemote() | parent.isSampled()| Invoke sampler| |--|--|--|--| From 728bdd6eb0d039c690f86bfdd6bcf379f306fab4 Mon Sep 17 00:00:00 2001 From: Daniel Dyla Date: Wed, 7 Oct 2020 15:49:54 -0400 Subject: [PATCH 4/9] chore: emit error when parent sampler configured with no root --- .../src/trace/sampler/ParentBasedSampler.ts | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/packages/opentelemetry-core/src/trace/sampler/ParentBasedSampler.ts b/packages/opentelemetry-core/src/trace/sampler/ParentBasedSampler.ts index 4490049e1b3..ea3035c4df5 100644 --- a/packages/opentelemetry-core/src/trace/sampler/ParentBasedSampler.ts +++ b/packages/opentelemetry-core/src/trace/sampler/ParentBasedSampler.ts @@ -25,6 +25,7 @@ import { } from '@opentelemetry/api'; import { AlwaysOffSampler } from './AlwaysOffSampler'; import { AlwaysOnSampler } from './AlwaysOnSampler'; +import { globalErrorHandler } from '../../common/global-error-handler'; /** * A composite sampler that either respects the parent span's sampling decision @@ -39,6 +40,11 @@ export class ParentBasedSampler implements Sampler { constructor(config: ParentBasedSamplerConfig) { this.root = config.root; + + if (!this.root) { + globalErrorHandler(new Error("ParentBasedSampler must have a root sampler configured")) + } + this.remoteParentSampled = config.remoteParentSampled ?? new AlwaysOnSampler(); this.remoteParentNotSampled = From 73b83c1f38a6e036ed22214747c80d905b28d1db Mon Sep 17 00:00:00 2001 From: Daniel Dyla Date: Fri, 9 Oct 2020 09:35:35 -0400 Subject: [PATCH 5/9] chore: add documentation to sampler readme --- packages/opentelemetry-core/README.md | 26 ++++++++++++++++++++++---- 1 file changed, 22 insertions(+), 4 deletions(-) diff --git a/packages/opentelemetry-core/README.md b/packages/opentelemetry-core/README.md index ffb5eccb1b1..90bdbbf98fd 100644 --- a/packages/opentelemetry-core/README.md +++ b/packages/opentelemetry-core/README.md @@ -108,16 +108,25 @@ const tracerProvider = new NodeTracerProvider({ }); ``` -#### Probability +#### TraceIdRatioBased -Samples a configurable percentage of traces, and additionally samples any trace that was sampled upstream. +Samples some percentage of traces, calculated deterministically using the trace ID. +Any trace that would be sampled at a given percentage will also be sampled at any higher percentage. + +The `TraceIDRatioSampler` may be used with the `ParentBasedSampler` to respect the sampled flag of an incoming trace. ```js const { NodeTracerProvider } = require("@opentelemetry/node"); const { TraceIdRatioBasedSampler } = require("@opentelemetry/core"); const tracerProvider = new NodeTracerProvider({ - sampler: new TraceIdRatioBasedSampler(0.5) + // See details of ParentBasedSampler below + sampler: new ParentBasedSampler({ + // Trace ID Ratio Sampler accepts a positional argument + // which represents the percentage of traces which should + // be sampled. + root: new TraceIdRatioBasedSampler(0.5) + }); }); ``` @@ -155,7 +164,16 @@ const { NodeTracerProvider } = require("@opentelemetry/node"); const { ParentBasedSampler, AlwaysOffSampler, TraceIdRatioBasedSampler } = require("@opentelemetry/core"); const tracerProvider = new NodeTracerProvider({ - sampler: new ParentBasedSampler({ root: new TraceIdRatioBasedSampler(0.5) }) + sampler: new ParentBasedSampler({ + // By default, the ParentBasedSampler will respect the parent span's sampling + // decision. This is configurable by providing a different sampler to use + // based on the situation. See configuration details above. + // + // This will delegate the sampling decision of all root traces (no parent) + // to the TraceIdRatioBasedSampler. + // See details of TraceIdRatioBasedSampler above. + root: new TraceIdRatioBasedSampler(0.5) + }) }); ``` From 6c6d4d7be9b3ede9139b70fcef17bb93c2678ef4 Mon Sep 17 00:00:00 2001 From: Daniel Dyla Date: Fri, 9 Oct 2020 09:36:22 -0400 Subject: [PATCH 6/9] chore: all private members prefix with _ --- .../src/trace/sampler/ParentBasedSampler.ts | 34 +++++++++---------- 1 file changed, 17 insertions(+), 17 deletions(-) diff --git a/packages/opentelemetry-core/src/trace/sampler/ParentBasedSampler.ts b/packages/opentelemetry-core/src/trace/sampler/ParentBasedSampler.ts index ea3035c4df5..77144ef00f4 100644 --- a/packages/opentelemetry-core/src/trace/sampler/ParentBasedSampler.ts +++ b/packages/opentelemetry-core/src/trace/sampler/ParentBasedSampler.ts @@ -32,26 +32,26 @@ import { globalErrorHandler } from '../../common/global-error-handler'; * or delegates to `delegateSampler` for root spans. */ export class ParentBasedSampler implements Sampler { - private root: Sampler; - private remoteParentSampled: Sampler; - private remoteParentNotSampled: Sampler; - private localParentSampled: Sampler; - private localParentNotSampled: Sampler; + private _root: Sampler; + private _remoteParentSampled: Sampler; + private _remoteParentNotSampled: Sampler; + private _localParentSampled: Sampler; + private _localParentNotSampled: Sampler; constructor(config: ParentBasedSamplerConfig) { - this.root = config.root; + this._root = config.root; - if (!this.root) { + if (!this._root) { globalErrorHandler(new Error("ParentBasedSampler must have a root sampler configured")) } - this.remoteParentSampled = + this._remoteParentSampled = config.remoteParentSampled ?? new AlwaysOnSampler(); - this.remoteParentNotSampled = + this._remoteParentNotSampled = config.remoteParentNotSampled ?? new AlwaysOffSampler(); - this.localParentSampled = + this._localParentSampled = config.localParentSampled ?? new AlwaysOnSampler(); - this.localParentNotSampled = + this._localParentNotSampled = config.localParentNotSampled ?? new AlwaysOffSampler(); } @@ -64,7 +64,7 @@ export class ParentBasedSampler implements Sampler { links: Link[] ): SamplingResult { if (!parentContext) { - return this.root.shouldSample( + return this._root.shouldSample( parentContext, traceId, spanName, @@ -76,7 +76,7 @@ export class ParentBasedSampler implements Sampler { if (parentContext.isRemote) { if (parentContext.traceFlags & TraceFlags.SAMPLED) { - return this.remoteParentSampled.shouldSample( + return this._remoteParentSampled.shouldSample( parentContext, traceId, spanName, @@ -85,7 +85,7 @@ export class ParentBasedSampler implements Sampler { links ); } - return this.remoteParentNotSampled.shouldSample( + return this._remoteParentNotSampled.shouldSample( parentContext, traceId, spanName, @@ -96,7 +96,7 @@ export class ParentBasedSampler implements Sampler { } if (parentContext.traceFlags & TraceFlags.SAMPLED) { - return this.localParentSampled.shouldSample( + return this._localParentSampled.shouldSample( parentContext, traceId, spanName, @@ -106,7 +106,7 @@ export class ParentBasedSampler implements Sampler { ); } - return this.localParentNotSampled.shouldSample( + return this._localParentNotSampled.shouldSample( parentContext, traceId, spanName, @@ -117,7 +117,7 @@ export class ParentBasedSampler implements Sampler { } toString(): string { - return `ParentBased{root=${this.root.toString()}, remoteParentSampled=${this.remoteParentSampled.toString()}, remoteParentNotSampled=${this.remoteParentNotSampled.toString()}, localParentSampled=${this.localParentSampled.toString()}, localParentNotSampled=${this.localParentNotSampled.toString()}}`; + return `ParentBased{root=${this._root.toString()}, remoteParentSampled=${this._remoteParentSampled.toString()}, remoteParentNotSampled=${this._remoteParentNotSampled.toString()}, localParentSampled=${this._localParentSampled.toString()}, localParentNotSampled=${this._localParentNotSampled.toString()}}`; } } From ea7dd9a9c35e0f75988c1b8d335b6458844dc28f Mon Sep 17 00:00:00 2001 From: Daniel Dyla Date: Fri, 9 Oct 2020 09:37:38 -0400 Subject: [PATCH 7/9] chore: add fallback sampler if root is unconfigured --- .../opentelemetry-core/src/trace/sampler/ParentBasedSampler.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/opentelemetry-core/src/trace/sampler/ParentBasedSampler.ts b/packages/opentelemetry-core/src/trace/sampler/ParentBasedSampler.ts index 77144ef00f4..c33a6e2d4d5 100644 --- a/packages/opentelemetry-core/src/trace/sampler/ParentBasedSampler.ts +++ b/packages/opentelemetry-core/src/trace/sampler/ParentBasedSampler.ts @@ -43,6 +43,7 @@ export class ParentBasedSampler implements Sampler { if (!this._root) { globalErrorHandler(new Error("ParentBasedSampler must have a root sampler configured")) + this._root = new AlwaysOnSampler(); } this._remoteParentSampled = From 025aa5e063743cff8043b9576a83fbcbf0162180 Mon Sep 17 00:00:00 2001 From: Daniel Dyla Date: Fri, 9 Oct 2020 09:42:46 -0400 Subject: [PATCH 8/9] chore: lint --- .../src/trace/sampler/ParentBasedSampler.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/packages/opentelemetry-core/src/trace/sampler/ParentBasedSampler.ts b/packages/opentelemetry-core/src/trace/sampler/ParentBasedSampler.ts index c33a6e2d4d5..98224171c4b 100644 --- a/packages/opentelemetry-core/src/trace/sampler/ParentBasedSampler.ts +++ b/packages/opentelemetry-core/src/trace/sampler/ParentBasedSampler.ts @@ -42,7 +42,9 @@ export class ParentBasedSampler implements Sampler { this._root = config.root; if (!this._root) { - globalErrorHandler(new Error("ParentBasedSampler must have a root sampler configured")) + globalErrorHandler( + new Error('ParentBasedSampler must have a root sampler configured') + ); this._root = new AlwaysOnSampler(); } From a518dd67c1532735f7cdb98ad1b4bf62b2119608 Mon Sep 17 00:00:00 2001 From: Daniel Dyla Date: Fri, 9 Oct 2020 11:29:26 -0400 Subject: [PATCH 9/9] chore: remove trailing space --- packages/opentelemetry-core/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/opentelemetry-core/README.md b/packages/opentelemetry-core/README.md index 90bdbbf98fd..5f60e179476 100644 --- a/packages/opentelemetry-core/README.md +++ b/packages/opentelemetry-core/README.md @@ -168,7 +168,7 @@ const tracerProvider = new NodeTracerProvider({ // By default, the ParentBasedSampler will respect the parent span's sampling // decision. This is configurable by providing a different sampler to use // based on the situation. See configuration details above. - // + // // This will delegate the sampling decision of all root traces (no parent) // to the TraceIdRatioBasedSampler. // See details of TraceIdRatioBasedSampler above.