From 3f74c343c213be3e34051ddd954196d84690aed0 Mon Sep 17 00:00:00 2001 From: Yuri Shkuro Date: Thu, 29 Aug 2019 17:13:17 -0400 Subject: [PATCH 01/28] Support late sampling decisions Signed-off-by: Yuri Shkuro --- src/span.js | 4 + test/samplers/delayed_sampling.js | 170 ++++++++++++++++++++++++++++++ 2 files changed, 174 insertions(+) create mode 100644 test/samplers/delayed_sampling.js diff --git a/src/span.js b/src/span.js index e87d7b742..ff2d24d7b 100644 --- a/src/span.js +++ b/src/span.js @@ -56,6 +56,10 @@ export default class Span { return this._tracer._serviceName; } + getTags(): Array { + return this._tags; + } + static _getBaggageHeaderCache() { if (!Span._baggageHeaderCache) { Span._baggageHeaderCache = {}; diff --git a/test/samplers/delayed_sampling.js b/test/samplers/delayed_sampling.js new file mode 100644 index 000000000..2d1785e08 --- /dev/null +++ b/test/samplers/delayed_sampling.js @@ -0,0 +1,170 @@ +// @flow +// Copyright (c) 2019 Uber Technologies, Inc. +// +// 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 +// +// http://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 { assert } from 'chai'; +import { SAMPLER_API_V2 } from '../../src/samplers/constants'; +import Span from '../../src/span'; + +describe('delayed sampling', () => { + declare type Matcher = { + tagValue: string, + // can add more flags here, like firehose + }; + + class TagEqualsSampler implements Sampler { + apiVersion = SAMPLER_API_V2; + + _tagKey: string; + _matchers: { [string]: Matcher }; + _undecided: SamplingDecision; + + constructor(tagKey: string, matchers: Array) { + this._tagKey = tagKey; + this._matchers = {}; + matchers.forEach(m => { + this._matchers[m.tagValue] = m; + }); + this._undecided = { sample: false, retryable: true, tags: null }; + } + + _findTag(tags: Array): ?Tag { + for (let i = 0; i < tags.length; i++) { + if (tags[i].key === this._tagKey) { + return tags[i]; + } + } + return null; + } + + _createOutTags(tagValue: string): { [string]: string } { + return { + 'sampler.type': 'TagEqualsSampler', + 'sampler.param': tagValue, + }; + } + + _decide(tagValue: any): SamplingDecision { + const match: ?Matcher = this._matchers[tagValue]; + if (match) { + return { sample: true, retryable: false, tags: this._createOutTags(match.tagValue) }; + } + return this._undecided; + } + + onCreateSpan(span: Span): SamplingDecision { + const tag: ?Tag = this._findTag(span.getTags()); + if (tag) { + return this._decide(tag.value); + } + return this._undecided; + } + + onSetOperationName(span: Span, operationName: string): SamplingDecision { + return this.onCreateSpan(span); + } + + onSetTag(span: Span, key: string, value: any): SamplingDecision { + if (key === this._tagKey) { + return this._decide(value); + } + return this._undecided; + } + + toString(): string { + return 'TagEqualsSampler'; + } + + close(callback: ?Function): void { + if (callback) { + callback(); + } + } + } + + declare type PrioritySamplerState = { + samplerFired: Array, + }; + + /** + * PrioritySampler contains a list of samplers that it interrogates in order. + * Sampling methods return as soon as one of the samplers returns sample=true. + * The retryable state for each underlying sampler is stored in the extended context + * and once retryable=false is returned by one of the delegates it will never be + * called against. + * + * TODO: since sampling state is shared across all spans of the trace, the execution + * of the sampler should probably only happen on the local-root span. + */ + class PrioritySampler implements Sampler { + apiVersion = SAMPLER_API_V2; + + _delegates: Array; + + constructor(samplers: Array) { + this._delegates = samplers; + } + + _getState(span: Span): PrioritySamplerState { + const store = span.context()._samplingState.extendedState(); + const stateKey = 'DelegatingSampler'; // TODO ideally should be uniqueName() per BaseSamplerB2 + let state: ?PrioritySamplerState = store[stateKey]; + if (!state) { + state = { + samplerFired: Array(this._delegates.length).fill(false), + }; + store[stateKey] = state; + } + return state; + } + + onCreateSpan(span: Span): SamplingDecision { + const state = this._getState(span); + let retryable = false; + for (let i = 0; i < this._delegates.length; i++) { + if (state.samplerFired[i]) { + continue; + } + const d = this._delegates[i].onCreateSpan(span); + retryable = retryable || d.retryable; + if (!d.retryable) { + state.samplerFired[i] = true; + } + if (d.sample) { + return d; // TODO do we want to alter out tags? + } + } + return { sample: false, retryable: retryable, tags: null }; + } + + onSetOperationName(span: Span, operationName: string): SamplingDecision { + return this.onCreateSpan(span); + } + + onSetTag(span: Span, key: string, value: any): SamplingDecision { + if (key === this._tagKey) { + return this._decide(value); + } + return this._undecided; + } + + toString(): string { + return 'DelegatingSampler'; + } + + close(callback: ?Function): void { + this._lateBindingSampler.close(() => this._defaultEagerSampler.close(callback)); + } + } + + it('', () => {}); +}); From 683ca38a4827a7620eb82c71d65d8fdf0a938ad0 Mon Sep 17 00:00:00 2001 From: Yuri Shkuro Date: Thu, 29 Aug 2019 17:26:47 -0400 Subject: [PATCH 02/28] Extract countdown callback into util Signed-off-by: Yuri Shkuro --- src/reporters/composite_reporter.js | 19 +++---------------- src/util.js | 14 ++++++++++++++ 2 files changed, 17 insertions(+), 16 deletions(-) diff --git a/src/reporters/composite_reporter.js b/src/reporters/composite_reporter.js index f055d58e6..968f5f161 100644 --- a/src/reporters/composite_reporter.js +++ b/src/reporters/composite_reporter.js @@ -12,6 +12,7 @@ // the License. import Span from '../span.js'; +import Utils from '../util.js'; export default class CompositeReporter implements Reporter { _reporters: Array; @@ -30,23 +31,9 @@ export default class CompositeReporter implements Reporter { }); } - _compositeCallback(limit: number, callback: () => void): () => void { - let count = 0; - return () => { - count++; - if (count >= limit) { - callback(); - } - }; - } - close(callback?: () => void): void { - const modifiedCallback = callback - ? this._compositeCallback(this._reporters.length, callback) - : function() {}; - this._reporters.forEach(r => { - r.close(modifiedCallback); - }); + const countdownCallback = Utils.countdownCallback(this._reporters.length, callback); + this._reporters.forEach(r => r.close(countdownCallback)); } setProcess(serviceName: string, tags: Array): void { diff --git a/src/util.js b/src/util.js index a46594c17..a52425f1d 100644 --- a/src/util.js +++ b/src/util.js @@ -148,4 +148,18 @@ export default class Utils { error(err); }); } + + /** + * Creates a callback function that only delegates to passed callback + * after limit invocations. + */ + static countdownCallback(limit: number, callback: ?() => void): () => void { + let count = 0; + return () => { + count++; + if (count >= limit && callback) { + callback(); + } + }; + } } From 784c5f80948b3db5a3ad887cbd19fbc9210d4970 Mon Sep 17 00:00:00 2001 From: Yuri Shkuro Date: Thu, 29 Aug 2019 17:27:21 -0400 Subject: [PATCH 03/28] Add PrioritySampler Signed-off-by: Yuri Shkuro --- src/_flow/sampler.js | 2 +- test/samplers/delayed_sampling.js | 17 ++++++++++------- 2 files changed, 11 insertions(+), 8 deletions(-) diff --git a/src/_flow/sampler.js b/src/_flow/sampler.js index afef368f0..debf819d4 100644 --- a/src/_flow/sampler.js +++ b/src/_flow/sampler.js @@ -30,5 +30,5 @@ declare interface Sampler { onSetOperationName(span: Span, operationName: string): SamplingDecision; onSetTag(span: Span, key: string, value: any): SamplingDecision; - close(callback: ?Function): void; + close(callback: ?() => void): void; } diff --git a/test/samplers/delayed_sampling.js b/test/samplers/delayed_sampling.js index 2d1785e08..bfc300161 100644 --- a/test/samplers/delayed_sampling.js +++ b/test/samplers/delayed_sampling.js @@ -14,6 +14,7 @@ import { assert } from 'chai'; import { SAMPLER_API_V2 } from '../../src/samplers/constants'; import Span from '../../src/span'; +import Utils from '../../src/util'; describe('delayed sampling', () => { declare type Matcher = { @@ -147,24 +148,26 @@ describe('delayed sampling', () => { } onSetOperationName(span: Span, operationName: string): SamplingDecision { + // FIXME: return this.onCreateSpan(span); } onSetTag(span: Span, key: string, value: any): SamplingDecision { - if (key === this._tagKey) { - return this._decide(value); - } - return this._undecided; + // FIXME: + return this.onCreateSpan(span); } toString(): string { return 'DelegatingSampler'; } - close(callback: ?Function): void { - this._lateBindingSampler.close(() => this._defaultEagerSampler.close(callback)); + close(callback: ?() => void): void { + const countdownCallback = Utils.countdownCallback(this._delegates.length, callback); + this._delegates.forEach(r => r.close(countdownCallback)); } } - it('', () => {}); + it('', () => { + // TODO: tet me + }); }); From eb17f0cc5792070519597efdbc89467d60a50291 Mon Sep 17 00:00:00 2001 From: Yuri Shkuro Date: Fri, 30 Aug 2019 14:15:51 -0400 Subject: [PATCH 04/28] Remove eager finalizer Signed-off-by: Yuri Shkuro --- src/samplers/_adapt_sampler.js | 2 +- src/span.js | 1 - src/span_context.js | 9 + src/tracer.js | 7 +- test/samplers/delayed_sampling.js | 55 ++++- test/span.js | 355 +++++++++++++++--------------- 6 files changed, 246 insertions(+), 183 deletions(-) diff --git a/src/samplers/_adapt_sampler.js b/src/samplers/_adapt_sampler.js index 8234ab601..d2b27f6a7 100644 --- a/src/samplers/_adapt_sampler.js +++ b/src/samplers/_adapt_sampler.js @@ -61,7 +61,7 @@ class LegacySamplerV1Adapter implements Sampler { onCreateSpan(span: Span): SamplingDecision { const outTags = {}; const isSampled = this._delegate.isSampled(span.operationName, outTags); - return { sample: isSampled, retryable: true, tags: outTags }; + return { sample: isSampled, retryable: false, tags: outTags }; } onSetOperationName(span: Span, operationName: string): SamplingDecision { diff --git a/src/span.js b/src/span.js index ff2d24d7b..a7259c25f 100644 --- a/src/span.js +++ b/src/span.js @@ -196,7 +196,6 @@ export default class Span { return; } - this._spanContext.finalizeSampling(); if (this._spanContext.isSampled()) { let endTime = finishTime || this._tracer.now(); this._duration = endTime - this._startTime; diff --git a/src/span_context.js b/src/span_context.js index f49304e3c..d57dd4f37 100644 --- a/src/span_context.js +++ b/src/span_context.js @@ -26,6 +26,7 @@ export default class SpanContext { _baggage: any; _debugId: ?string; _samplingState: SamplingState; + _remote: boolean; // indicates that span context represents a remote parent constructor( traceId: any, @@ -138,6 +139,10 @@ export default class SpanContext { this._samplingState.setFirehose(value); } + _setRemote(value: boolean) { + this._remote - value; + } + set baggage(baggage: any): void { this._baggage = baggage; } @@ -154,6 +159,10 @@ export default class SpanContext { return this._samplingState.setFinal(true); } + isRemote(): boolean { + return this._remote; + } + isDebugIDContainerOnly(): boolean { return !this.isValid && Boolean(this._debugId); } diff --git a/src/tracer.js b/src/tracer.js index 673224e24..0a135929b 100644 --- a/src/tracer.js +++ b/src/tracer.js @@ -246,7 +246,9 @@ export default class Tracer { hasValidParent = true; let spanId = Utils.getRandom64(); ctx = parent._makeChildContext(spanId); - ctx.finalizeSampling(); // will finalize sampling for all spans sharing this traceId + if (ctx.isRemote()) { + ctx.finalizeSampling(); // will finalize sampling for all spans sharing this traceId + } } const isRpcServer = Boolean(userTags && userTags[otTags.SPAN_KIND] === otTags.SPAN_KIND_RPC_SERVER); @@ -282,7 +284,6 @@ export default class Tracer { throw new Error(`Unsupported format: ${format}`); } const _context = spanContext instanceof Span ? spanContext.context() : spanContext; - _context.finalizeSampling(); injector.inject(_context, carrier); } @@ -303,7 +304,7 @@ export default class Tracer { } const spanContext = extractor.extract(carrier); if (spanContext) { - spanContext.finalizeSampling(); + spanContext._setRemote(true); } return spanContext; } diff --git a/test/samplers/delayed_sampling.js b/test/samplers/delayed_sampling.js index bfc300161..b358a7a9e 100644 --- a/test/samplers/delayed_sampling.js +++ b/test/samplers/delayed_sampling.js @@ -12,9 +12,14 @@ // the License. import { assert } from 'chai'; +import * as opentracing from 'opentracing'; import { SAMPLER_API_V2 } from '../../src/samplers/constants'; import Span from '../../src/span'; import Utils from '../../src/util'; +import ConstSampler from '../../src/samplers/const_sampler'; +import { adaptSamplerOrThrow } from '../../src/samplers/_adapt_sampler'; +import InMemoryReporter from '../../src/reporters/in_memory_reporter'; +import Tracer from '../../src/tracer'; describe('delayed sampling', () => { declare type Matcher = { @@ -112,12 +117,12 @@ describe('delayed sampling', () => { _delegates: Array; constructor(samplers: Array) { - this._delegates = samplers; + this._delegates = samplers.map(s => adaptSamplerOrThrow(s)); } _getState(span: Span): PrioritySamplerState { const store = span.context()._samplingState.extendedState(); - const stateKey = 'DelegatingSampler'; // TODO ideally should be uniqueName() per BaseSamplerB2 + const stateKey = 'PrioritySampler'; // TODO ideally should be uniqueName() per BaseSamplerB2 let state: ?PrioritySamplerState = store[stateKey]; if (!state) { state = { @@ -140,6 +145,7 @@ describe('delayed sampling', () => { if (!d.retryable) { state.samplerFired[i] = true; } + console.log(`retryable=${retryable}, sample=${d.sample}`); if (d.sample) { return d; // TODO do we want to alter out tags? } @@ -167,7 +173,48 @@ describe('delayed sampling', () => { } } - it('', () => { - // TODO: tet me + describe('with PrioritySampler', () => { + const tagSampler = new TagEqualsSampler('theWho', [{ tagValue: 'Bender' }, { tagValue: 'Leela' }]); + const constSampler = new ConstSampler(false); + const priSampler = new PrioritySampler([tagSampler, constSampler]); + const reporter = new InMemoryReporter(); + const tracer = new Tracer('test-service-name', reporter, priSampler); + + beforeEach(() => {}); + + it('should not sample or finalize new span without tags', () => { + let span = tracer.startSpan('opName'); + assert.isFalse(span._spanContext.isSampled(), 'sampled'); + assert.isFalse(span._spanContext.samplingFinalized, 'finalized'); + }); + + it('should sample and finalize created span with tag', () => { + let span = tracer.startSpan('opName', { tags: { theWho: 'Bender' } }); + assert.isTrue(span._spanContext.isSampled(), 'sampled'); + assert.isTrue(span._spanContext.samplingFinalized, 'finalized'); + }); + + it('should sample and finalize span after setTag', () => { + let span = tracer.startSpan('opName'); + assert.isFalse(span._spanContext.isSampled(), 'sampled'); + assert.isFalse(span._spanContext.samplingFinalized, 'finalized'); + span.setTag('theWho', 'Leela'); + }); + + it('should not sample or finalize span after starting a child span', () => { + let span = tracer.startSpan('opName'); + let span2 = tracer.startSpan('opName2', { childOf: span.context() }); + assert.isFalse(span._spanContext.isSampled(), 'sampled'); + assert.isFalse(span._spanContext.samplingFinalized, 'finalized'); + }); + + it('should not sample or finalize span after serializing context', () => { + let span = tracer.startSpan('opName'); + let carrier = {}; + tracer.inject(span.context(), opentracing.FORMAT_TEXT_MAP, carrier); + assert.isOk(carrier); + assert.isFalse(span._spanContext.isSampled(), 'sampled'); + assert.isFalse(span._spanContext.samplingFinalized, 'finalized'); + }); }); }); diff --git a/test/span.js b/test/span.js index 6cc729534..86c5f0e7e 100644 --- a/test/span.js +++ b/test/span.js @@ -26,22 +26,30 @@ import sinon from 'sinon'; import Tracer from '../src/tracer'; import Utils from '../src/util'; import DefaultThrottler from '../src/throttler/default_throttler'; +import BaseSamplerV2 from '../src/samplers/v2/base'; -describe('span should', () => { +function _prepareObjects() { let reporter = new InMemoryReporter(); - let tracer, span, spanContext; + let tracer = new Tracer('test-service-name', reporter, new ConstSampler(true), { + logger: new MockLogger(), + }); - beforeEach(() => { - tracer = new Tracer('test-service-name', reporter, new ConstSampler(true), { logger: new MockLogger() }); + let spanContext = SpanContext.withBinaryIds( + Utils.encodeInt64(1), + Utils.encodeInt64(2), + Utils.encodeInt64(3), + constants.SAMPLED_MASK + ); - spanContext = SpanContext.withBinaryIds( - Utils.encodeInt64(1), - Utils.encodeInt64(2), - Utils.encodeInt64(3), - constants.SAMPLED_MASK - ); + let span = new Span(tracer, 'op-name', spanContext, tracer.now()); + return { reporter, tracer, span, spanContext }; +} - span = new Span(tracer, 'op-name', spanContext, tracer.now()); +describe('span should', () => { + var reporter, tracer, span, spanContext; + + beforeEach(() => { + ({ reporter, tracer, span, spanContext } = _prepareObjects()); }); it('return span context when context() is called', () => { @@ -85,9 +93,9 @@ describe('span should', () => { it('set debug and sampling flags through sampling priority via setTag', () => { span.setTag(opentracing.Tags.SAMPLING_PRIORITY, 3); - assert.isOk(span.context().isDebug()); - assert.isOk(span.context().isSampled()); - assert.isOk( + assert.isTrue(span.context().isDebug()); + assert.isTrue(span.context().isSampled()); + assert.isTrue( JaegerTestUtils.hasTags(span, { 'sampling.priority': 3, }) @@ -99,9 +107,9 @@ describe('span should', () => { tags[opentracing.Tags.SAMPLING_PRIORITY] = 3; span.addTags(tags); - assert.isOk(span.context().isDebug()); - assert.isOk(span.context().isSampled()); - assert.isOk( + assert.isTrue(span.context().isDebug()); + assert.isTrue(span.context().isSampled()); + assert.isTrue( JaegerTestUtils.hasTags(span, { 'sampling.priority': 3, }) @@ -111,7 +119,7 @@ describe('span should', () => { it('unset sampling on span via sampling priority', () => { span.setTag(opentracing.Tags.SAMPLING_PRIORITY, 0); - assert.isNotOk(span.context().isSampled()); + assert.isFalse(span.context().isSampled()); }); it('add tags', () => { @@ -132,7 +140,7 @@ describe('span should', () => { } } - assert.isOk(span._tags.length, 4); + assert.equal(span._tags.length, 4); assert.equal(count, 2); }); @@ -192,7 +200,7 @@ describe('span should', () => { let key = span._normalizeBaggageKey(unnormalizedKey); assert.equal(key, 'some-key'); - assert.isOk(unnormalizedKey in Span._getBaggageHeaderCache()); + assert.isTrue(unnormalizedKey in Span._getBaggageHeaderCache()); }); it('not be set to debug via setTag if throttled', () => { @@ -201,8 +209,8 @@ describe('span should', () => { const prevTagLength = span._tags.length; span.setTag(opentracing.Tags.SAMPLING_PRIORITY, 1); - assert.isOk(span.context().samplingFinalized); - assert.isNotOk(span.context().isDebug()); + assert.isTrue(span.context().samplingFinalized); + assert.isFalse(span.context().isDebug()); assert.equal( prevTagLength, span._tags.length, @@ -218,8 +226,8 @@ describe('span should', () => { const tags = {}; tags[opentracing.Tags.SAMPLING_PRIORITY] = 1; span.addTags(tags); - assert.isOk(span.context().samplingFinalized); - assert.isNotOk(span.context().isDebug()); + assert.isTrue(span.context().samplingFinalized); + assert.isFalse(span.context().isDebug()); assert.equal( prevTagLength, span._tags.length, @@ -233,8 +241,8 @@ describe('span should', () => { span = new Span(tracer, 'op-name', spanContext, tracer.now()); span.setTag(opentracing.Tags.SAMPLING_PRIORITY, 1); - assert.isOk(span.context().samplingFinalized); - assert.isOk(span.context().isDebug()); + assert.isTrue(span.context().samplingFinalized); + assert.isTrue(span.context().isDebug()); assert.deepEqual(span._tags[span._tags.length - 1], { key: 'sampling.priority', value: 1 }); const prevTagLength = span._tags.length; @@ -244,166 +252,165 @@ describe('span should', () => { assert.equal(prevTagLength, span._tags.length, 'The sampling.priority tag should only be set once'); }); - describe('adaptive sampling tests for span', () => { - let options = [ - { desc: 'sampled span: ', sampling: true, reportedSpans: 1 }, - { desc: 'unsampled span: ', sampling: false, reportedSpans: 0 }, - ]; - _.each(options, o => { - it(o.desc + 'should save tags, and logs on an unsampled span incase it later becomes sampled', () => { - let reporter = new InMemoryReporter(); - let tracer = new Tracer('test-service-name', reporter, new ConstSampler(false), { - logger: new MockLogger(), - }); - let span = tracer.startSpan('initially-unsampled-span'); - assert.ok(span._isWriteable(), 'span is writeable when created'); - assert.equal(false, span.context().samplingFinalized, 'span is not finalized when created'); - assert.ok(span._isWriteable(), 'span is writeable when created'); - span.setTag('tagKeyOne', 'tagValueOne'); - span.addTags({ - tagKeyTwo: 'tagValueTwo', - }); - assert.ok(span._isWriteable(), 'span is writeable after setting tags'); - span.log({ logkeyOne: 'logValueOne' }); - - tracer._sampler = adaptSamplerOrThrow(new ConstSampler(o.sampling)); - span.setOperationName('sampled-span'); - span.finish(); - - assert.deepEqual(span._tags[0], { key: 'tagKeyOne', value: 'tagValueOne' }); - assert.deepEqual(span._tags[1], { key: 'tagKeyTwo', value: 'tagValueTwo' }); - assert.deepEqual(span._logs[0].fields[0], { key: 'logkeyOne', value: 'logValueOne' }); - assert.equal(reporter.spans.length, o.reportedSpans); - }); + describe('setTag', () => { + it('should set a tag, and return a span', () => { + let newSpan = span.setTag('key', 'value'); + assert.isTrue(newSpan instanceof Span); + assert.isTrue(JaegerTestUtils.hasTags(span, { key: 'value' })); }); + }); - describe('span sampling finalizer', () => { - it('should trigger when it inherits a sampling decision', () => { - assert.equal(span.context().samplingFinalized, false, 'Span created in before each is not finalized'); - - let childSpan = tracer.startSpan('child-span', { childOf: span }); - assert.isOk(span.context().samplingFinalized); - assert.isOk(childSpan.context().samplingFinalized); - }); - - it('should trigger when it sets the sampling priority', () => { - // Span created in before each is not finalized. - assert.equal(span.context().samplingFinalized, false); - - span.setTag(opentracing.Tags.SAMPLING_PRIORITY, 1); - assert.isOk(span.context().samplingFinalized); - assert.deepEqual(span._tags[span._tags.length - 1], { key: 'sampling.priority', value: 1 }); - - const unsampledSpan = tracer.startSpan('unsampled-span'); - const prevTagLength = span._tags.length; - unsampledSpan.setTag(opentracing.Tags.SAMPLING_PRIORITY, -1); - assert.isOk(unsampledSpan.context().samplingFinalized); - assert.equal( - prevTagLength, - span._tags.length, - 'The sampling.priority tag should not be set if span is finalized and not sampled' - ); - }); - - it('should trigger on a finish()-ed span', () => { - // Span created in before each is not finalized. - assert.equal(span.context().samplingFinalized, false); - - span.finish(); - assert.isOk(span.context().samplingFinalized); - }); - - it('should trigger after calling setOperationName', () => { - // Span created in before each is not finalized. - assert.equal(span.context().samplingFinalized, false); - - span.setOperationName('fry'); - assert.isOk(span.context().samplingFinalized); - }); - - it('should trigger when its context is injected into headers', () => { - // Span created in before each is not finalized. - assert.equal(span.context().samplingFinalized, false); - - let headers = {}; - tracer.inject(span.context(), opentracing.FORMAT_HTTP_HEADERS, headers); - - assert.isOk(span.context().samplingFinalized); - }); - }); + // TODO(oibe) need tests for standard tags, and handlers +}); - it('isWriteable returns true if not finalized, or the span is sampled', () => { - tracer = new Tracer('test-service-name', new InMemoryReporter(), new ConstSampler(false), { - logger: new MockLogger(), - }); - let unFinalizedSpan = tracer.startSpan('unFinalizedSpan'); - assert.equal(false, unFinalizedSpan.context().samplingFinalized); - assert.equal(true, unFinalizedSpan._isWriteable()); - - tracer._sampler = adaptSamplerOrThrow(new ConstSampler(true)); - let sampledSpan = tracer.startSpan('sampled-span'); - assert.equal(true, sampledSpan.context().isSampled()); - sampledSpan.finish(); // finalizes the span - assert.equal(true, sampledSpan.context().samplingFinalized); - assert.equal(true, sampledSpan._isWriteable()); - }); +describe('sampling finalizer', () => { + var reporter, tracer, span, spanContext; + + beforeEach(() => { + ({ reporter, tracer, span, spanContext } = _prepareObjects()); + }); - it('2nd setOperationName should add sampler tags to span, and change operationName', () => { - tracer = new Tracer('test-service-name', new InMemoryReporter(), new ConstSampler(true), { - logger: new MockLogger(), - }); - let span = tracer.startSpan('fry'); - - assert.equal(span.operationName, 'fry'); - assert.isOk( - JaegerTestUtils.hasTags(span, { - 'sampler.type': 'const', - 'sampler.param': true, - }) - ); - tracer._sampler = adaptSamplerOrThrow(new ProbabilisticSampler(1.0)); - span.setOperationName('re-sampled-span'); - - assert.equal(span.operationName, 're-sampled-span'); - assert.isOk( - JaegerTestUtils.hasTags(span, { - 'sampler.type': 'probabilistic', - 'sampler.param': 1, - }) - ); + class RetryableSampler extends BaseSamplerV2 { + _decision: boolean; + constructor(decision: boolean) { + super('RetryableSampler'); + this._decision = decision; + } + _tags(): {} { + return { + 'sampler.type': 'const', + 'sampler.param': this._decision, + }; + } + onCreateSpan(span: Span): SamplingDecision { + return { sample: this._decision, retryable: true, tags: this._tags() }; + } + onSetOperationName(span: Span, operationName: string): SamplingDecision { + return { sample: this._decision, retryable: false, tags: this._tags() }; + } + onSetTag(span: Span, key: string, value: any): SamplingDecision { + return { sample: this._decision, retryable: true, tags: this._tags() }; + } + } + + it('should keep the span writeable', () => { + let tracer = new Tracer('test-service-name', reporter, new RetryableSampler(false)); + let span = tracer.startSpan('initially-unsampled-span'); + assert.isTrue(span._isWriteable(), 'span is writeable when created'); + assert.isFalse(span.context().samplingFinalized, 'span is not finalized when created'); + span.setTag('tagKeyOne', 'tagValueOne'); + span.addTags({ + tagKeyTwo: 'tagValueTwo', }); + span.log({ logkeyOne: 'logValueOne' }); + assert.isTrue(span._isWriteable(), 'span is writeable after setting tags'); + assert.isTrue( + JaegerTestUtils.hasTags( + span, + { + tagKeyOne: 'tagValueOne', + tagKeyTwo: 'tagValueTwo', + }, + 'matching tags' + ) + ); + assert.deepEqual(span._logs[0].fields[0], { key: 'logkeyOne', value: 'logValueOne' }); + }); - it('2nd setOperationName should not change the sampling tags, but should change the operationName', () => { - let span = tracer.startSpan('fry'); + it('should make span non-writeable when sampler returns retryable=false', () => { + let tracer = new Tracer('test-service-name', reporter, new RetryableSampler(false)); + let span = tracer.startSpan('initially-unsampled-span'); + assert.isTrue(span._isWriteable(), 'span is writeable when created'); + assert.isFalse(span.context().samplingFinalized, 'span is not finalized when created'); + // note: RetryableSampler returns retryable=false from onSetOperation() + span.setOperationName('replace-op-name'); + assert.isFalse(span._isWriteable(), 'span is writeable after setting tags'); + assert.isTrue(span.context().samplingFinalized, 'span is not finalized when created'); + }); - span.setOperationName('new-span-one'); - assert.equal(span.operationName, 'new-span-one'); + it('should share sampling state with children spans', () => { + let tracer = new Tracer('test-service-name', reporter, new RetryableSampler(false)); + let span = tracer.startSpan('initially-unsampled-span'); + assert.equal(span.context().samplingFinalized, false, 'new unsampled span is not finalized'); - // update sampler to something will always sample - tracer._sampler = adaptSamplerOrThrow(new ProbabilisticSampler(1.0)); + let childSpan = tracer.startSpan('child-span', { childOf: span }); + assert.isFalse(span.context().samplingFinalized); + assert.isFalse(childSpan.context().samplingFinalized); + }); - // The second cal lshould rename the operation name, but - // not re-sample the span. This is because finalize was set - // in the first 'setOperationName' call. - span.setOperationName('new-span-two'); + it('should trigger when it sets the sampling priority', () => { + assert.isFalse(span.context().samplingFinalized, 'manual span is not finalized'); - assert.equal(span.operationName, 'new-span-two'); - assert.isOk( - JaegerTestUtils.hasTags(span, { - 'sampler.type': 'const', - 'sampler.param': true, - }) - ); - }); + span.setTag(opentracing.Tags.SAMPLING_PRIORITY, 1); + assert.isTrue(span.context().samplingFinalized); + assert.deepEqual(span._tags[span._tags.length - 1], { key: 'sampling.priority', value: 1 }); + + const unsampledSpan = tracer.startSpan('unsampled-span'); + const prevTagLength = span._tags.length; + unsampledSpan.setTag(opentracing.Tags.SAMPLING_PRIORITY, -1); + assert.isTrue(unsampledSpan.context().samplingFinalized); + assert.equal( + prevTagLength, + span._tags.length, + 'The sampling.priority tag should not be set if span is finalized and not sampled' + ); }); - describe('setTag', () => { - it('should set a tag, and return a span', () => { - let newSpan = span.setTag('key', 'value'); - assert.isOk(newSpan instanceof Span); - assert.isOk(_.isEqual(span._tags[0], { key: 'key', value: 'value' })); - }); + it('should finalize the span sampled with V1 sampler', () => { + let span = tracer.startSpan('test'); + assert.isTrue(span.context().samplingFinalized); }); - // TODO(oibe) need tests for standard tags, and handlers + it('should not trigger on a finish()-ed span', () => { + assert.isFalse(span.context().samplingFinalized, 'manual span is not finalized'); + span.finish(); + assert.isFalse(span.context().samplingFinalized, 'finished span may remain unfinalized'); + }); + + it('should trigger after calling setOperationName with V1 sampler', () => { + assert.isFalse(span.context().samplingFinalized, 'manual span is not finalized'); + span.setOperationName('fry'); + assert.isTrue(span.context().samplingFinalized, 'finalized by V1 sampler'); + }); + + it('should not trigger when its context is injected into headers', () => { + assert.isFalse(span.context().samplingFinalized, 'manual span is not finalized'); + + let headers = {}; + tracer.inject(span.context(), opentracing.FORMAT_HTTP_HEADERS, headers); + + assert.isFalse(span.context().samplingFinalized, 'remains unfinalized after inject()'); + }); + + it('should keep isWriteable=true if span is sampled or not finalized', () => { + let tracer = new Tracer('test-service-name', reporter, new RetryableSampler(false)); + let span = tracer.startSpan('initially-unsampled-span'); + assert.isFalse(span.context().samplingFinalized, 'not finalized'); + assert.isFalse(span.context().isSampled(), 'not sampled'); + assert.isTrue(span._isWriteable()); + + tracer._sampler = adaptSamplerOrThrow(new ConstSampler(true)); + let sampledSpan = tracer.startSpan('sampled-span'); + assert.isTrue(sampledSpan.context().isSampled(), 'sampled'); + assert.isTrue(sampledSpan.context().samplingFinalized, 'finalized'); + assert.isTrue(sampledSpan._isWriteable(), 'writeable'); + }); + + it('should allow 2nd setOperationName to change operationName, but not to affect sampling', () => { + let span = tracer.startSpan('fry'); + assert.equal(span.operationName, 'fry'); + assert.isTrue(span._spanContext.isSampled()); + assert.isTrue(span._spanContext.samplingFinalized); + assert.isTrue( + JaegerTestUtils.hasTags(span, { + 'sampler.type': 'const', + 'sampler.param': true, + }) + ); + tracer._sampler = adaptSamplerOrThrow(new ProbabilisticSampler(1.0)); + span._tags = []; // since we don't de-dupe tags, JaegerTestUtils.hasTags() below fails + span.setOperationName('re-sampled-span'); + assert.equal(span.operationName, 're-sampled-span'); + assert.equal(0, span._tags.length); + }); }); From c4cafd17d2b7ca5500b9aa367382a5bf27f05394 Mon Sep 17 00:00:00 2001 From: Yuri Shkuro Date: Fri, 30 Aug 2019 14:24:28 -0400 Subject: [PATCH 05/28] Clean up flwo errors Signed-off-by: Yuri Shkuro --- src/span_context.js | 2 +- test/samplers/delayed_sampling.js | 3 +-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/src/span_context.js b/src/span_context.js index d57dd4f37..b10642aaf 100644 --- a/src/span_context.js +++ b/src/span_context.js @@ -140,7 +140,7 @@ export default class SpanContext { } _setRemote(value: boolean) { - this._remote - value; + this._remote = value; } set baggage(baggage: any): void { diff --git a/test/samplers/delayed_sampling.js b/test/samplers/delayed_sampling.js index b358a7a9e..08cd362b7 100644 --- a/test/samplers/delayed_sampling.js +++ b/test/samplers/delayed_sampling.js @@ -116,7 +116,7 @@ describe('delayed sampling', () => { _delegates: Array; - constructor(samplers: Array) { + constructor(samplers: Array) { this._delegates = samplers.map(s => adaptSamplerOrThrow(s)); } @@ -145,7 +145,6 @@ describe('delayed sampling', () => { if (!d.retryable) { state.samplerFired[i] = true; } - console.log(`retryable=${retryable}, sample=${d.sample}`); if (d.sample) { return d; // TODO do we want to alter out tags? } From 706d57c4e9877e1db5d12cc6ff63d69d8955bd7f Mon Sep 17 00:00:00 2001 From: Yuri Shkuro Date: Fri, 30 Aug 2019 14:36:19 -0400 Subject: [PATCH 06/28] Do not use array.fill() Signed-off-by: Yuri Shkuro --- test/samplers/delayed_sampling.js | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/test/samplers/delayed_sampling.js b/test/samplers/delayed_sampling.js index 08cd362b7..6337aa3b9 100644 --- a/test/samplers/delayed_sampling.js +++ b/test/samplers/delayed_sampling.js @@ -126,8 +126,12 @@ describe('delayed sampling', () => { let state: ?PrioritySamplerState = store[stateKey]; if (!state) { state = { - samplerFired: Array(this._delegates.length).fill(false), + samplerFired: Array(this._delegates.length), }; + // cannot use array.fill() in Node 0.10 + for (let i = 0; i < this._delegates.length; i++) { + state.samplerFired[i] = false; + } store[stateKey] = state; } return state; From 76be657071b8eef7610736eefa8da0955f0d2176 Mon Sep 17 00:00:00 2001 From: Yuri Shkuro Date: Fri, 30 Aug 2019 14:56:36 -0400 Subject: [PATCH 07/28] Add tests for remote parent Signed-off-by: Yuri Shkuro --- src/span_context.js | 1 + src/tracer.js | 2 +- test/span.js | 15 +++++++++++++++ 3 files changed, 17 insertions(+), 1 deletion(-) diff --git a/src/span_context.js b/src/span_context.js index b10642aaf..a5f5a1018 100644 --- a/src/span_context.js +++ b/src/span_context.js @@ -48,6 +48,7 @@ export default class SpanContext { this._baggage = baggage; this._debugId = debugId; this._samplingState = samplingState || new SamplingState(this.spanIdStr); + this._remote = false; } get traceId(): any { diff --git a/src/tracer.js b/src/tracer.js index 0a135929b..ef1e9d8c4 100644 --- a/src/tracer.js +++ b/src/tracer.js @@ -246,7 +246,7 @@ export default class Tracer { hasValidParent = true; let spanId = Utils.getRandom64(); ctx = parent._makeChildContext(spanId); - if (ctx.isRemote()) { + if (parent.isRemote()) { ctx.finalizeSampling(); // will finalize sampling for all spans sharing this traceId } } diff --git a/test/span.js b/test/span.js index 86c5f0e7e..0e1e00243 100644 --- a/test/span.js +++ b/test/span.js @@ -382,6 +382,21 @@ describe('sampling finalizer', () => { assert.isFalse(span.context().samplingFinalized, 'remains unfinalized after inject()'); }); + it('should finalize the child span created with remote parent', () => { + let tracer = new Tracer('test-service-name', reporter, new RetryableSampler(false)); + let span = tracer.startSpan('test'); + assert.isFalse(span.context().samplingFinalized, 'new root span not finalized'); + let span2 = tracer.startSpan('test2', { childOf: span.context() }); + assert.isFalse(span2.context().samplingFinalized, 'child span not finalized'); + let carrier = {}; + tracer.inject(span2.context(), opentracing.FORMAT_HTTP_HEADERS, carrier); + let ctx = tracer.extract(opentracing.FORMAT_HTTP_HEADERS, carrier); + console.log(ctx); + assert.isTrue(ctx.isRemote(), 'extracted context is "remote"'); + let span3 = tracer.startSpan('test2', { childOf: ctx }); + assert.isTrue(span3.context().samplingFinalized, 'child span of remote parent is finalized'); + }); + it('should keep isWriteable=true if span is sampled or not finalized', () => { let tracer = new Tracer('test-service-name', reporter, new RetryableSampler(false)); let span = tracer.startSpan('initially-unsampled-span'); From 79e3190d52c4f732e29a74bc2763a16d6f74ce6f Mon Sep 17 00:00:00 2001 From: Yuri Shkuro Date: Fri, 30 Aug 2019 17:39:45 -0400 Subject: [PATCH 08/28] rename ratelimiting_sampler.js -> rate_limiting_sampler.js Signed-off-by: Yuri Shkuro --- src/configuration.js | 2 +- src/index.js | 2 +- src/samplers/guaranteed_throughput_sampler.js | 6 +++--- .../{ratelimiting_sampler.js => rate_limiting_sampler.js} | 0 src/samplers/remote_sampler.js | 7 ++++--- test/init_tracer.js | 2 +- test/samplers/all_samplers.js | 2 +- ...telimiting_sampler_test.js => rate_limiting_sampler.js} | 2 +- test/samplers/remote_sampler.js | 2 +- test/span.js | 4 ++-- 10 files changed, 15 insertions(+), 14 deletions(-) rename src/samplers/{ratelimiting_sampler.js => rate_limiting_sampler.js} (100%) rename test/samplers/{ratelimiting_sampler_test.js => rate_limiting_sampler.js} (97%) diff --git a/src/configuration.js b/src/configuration.js index e435cfe3c..12a5c9a16 100644 --- a/src/configuration.js +++ b/src/configuration.js @@ -12,7 +12,7 @@ import ConstSampler from './samplers/const_sampler'; import ProbabilisticSampler from './samplers/probabilistic_sampler'; -import RateLimitingSampler from './samplers/ratelimiting_sampler'; +import RateLimitingSampler from './samplers/rate_limiting_sampler'; import RemoteReporter from './reporters/remote_reporter'; import CompositeReporter from './reporters/composite_reporter'; import LoggingReporter from './reporters/logging_reporter'; diff --git a/src/index.js b/src/index.js index 473378fd8..2019d2bc0 100644 --- a/src/index.js +++ b/src/index.js @@ -19,7 +19,7 @@ import Tracer from './tracer'; import ConstSampler from './samplers/const_sampler'; import ProbabilisticSampler from './samplers/probabilistic_sampler'; -import RateLimitingSampler from './samplers/ratelimiting_sampler'; +import RateLimitingSampler from './samplers/rate_limiting_sampler'; import RemoteSampler from './samplers/remote_sampler'; import CompositeReporter from './reporters/composite_reporter'; diff --git a/src/samplers/guaranteed_throughput_sampler.js b/src/samplers/guaranteed_throughput_sampler.js index 67c43d7a3..f1e5d7c29 100644 --- a/src/samplers/guaranteed_throughput_sampler.js +++ b/src/samplers/guaranteed_throughput_sampler.js @@ -11,9 +11,9 @@ // or implied. See the License for the specific language governing permissions and limitations under // the License. -import * as constants from '../constants.js'; -import ProbabilisticSampler from './probabilistic_sampler.js'; -import RateLimitingSampler from './ratelimiting_sampler.js'; +import * as constants from '../constants'; +import ProbabilisticSampler from './probabilistic_sampler'; +import RateLimitingSampler from './rate_limiting_sampler'; // GuaranteedThroughputProbabilisticSampler is a sampler that leverages both probabilisticSampler and // rateLimitingSampler. The rateLimitingSampler is used as a guaranteed lower bound sampler such that diff --git a/src/samplers/ratelimiting_sampler.js b/src/samplers/rate_limiting_sampler.js similarity index 100% rename from src/samplers/ratelimiting_sampler.js rename to src/samplers/rate_limiting_sampler.js diff --git a/src/samplers/remote_sampler.js b/src/samplers/remote_sampler.js index 2c60bcaf2..b0fcc45df 100644 --- a/src/samplers/remote_sampler.js +++ b/src/samplers/remote_sampler.js @@ -13,7 +13,7 @@ import url from 'url'; import ProbabilisticSampler from './probabilistic_sampler.js'; -import RateLimitingSampler from './ratelimiting_sampler.js'; +import RateLimitingSampler from './rate_limiting_sampler.js'; import PerOperationSampler from './per_operation_sampler.js'; import Metrics from '../metrics/metrics.js'; import NullLogger from '../logger.js'; @@ -26,7 +26,8 @@ const DEFAULT_MAX_OPERATIONS = 2000; const DEFAULT_SAMPLING_HOST = '0.0.0.0'; const DEFAULT_SAMPLING_PORT = 5778; const PROBABILISTIC_STRATEGY_TYPE = 'PROBABILISTIC'; -const RATELIMITING_STRATEGY_TYPE = 'RATE_LIMITING'; +const RATE_LIMITING_STRATEGY_TYPE = 'RATE_LIMITING'; +const PER_OPERATION_STRATEGY_TYPE = 'PER_OPERATION'; export default class RemoteControlledSampler implements LegacySamplerV1 { _serviceName: string; @@ -156,7 +157,7 @@ export default class RemoteControlledSampler implements LegacySamplerV1 { if (response.strategyType === PROBABILISTIC_STRATEGY_TYPE && response.probabilisticSampling) { let samplingRate = response.probabilisticSampling.samplingRate; newSampler = new ProbabilisticSampler(samplingRate); - } else if (response.strategyType === RATELIMITING_STRATEGY_TYPE && response.rateLimitingSampling) { + } else if (response.strategyType === RATE_LIMITING_STRATEGY_TYPE && response.rateLimitingSampling) { let maxTracesPerSecond = response.rateLimitingSampling.maxTracesPerSecond; if (this._sampler instanceof RateLimitingSampler) { let sampler: RateLimitingSampler = this._sampler; diff --git a/test/init_tracer.js b/test/init_tracer.js index 32343f8e9..654bf6621 100644 --- a/test/init_tracer.js +++ b/test/init_tracer.js @@ -18,7 +18,7 @@ import RemoteReporter from '../src/reporters/remote_reporter'; import ConstSampler from '../src/samplers/const_sampler'; import ProbabilisticSampler from '../src/samplers/probabilistic_sampler'; import RemoteSampler from '../src/samplers/remote_sampler'; -import RateLimitingSampler from '../src/samplers/ratelimiting_sampler'; +import RateLimitingSampler from '../src/samplers/rate_limiting_sampler'; import { initTracer, initTracerFromEnv } from '../src/index.js'; import opentracing from 'opentracing'; import RemoteThrottler from '../src/throttler/remote_throttler'; diff --git a/test/samplers/all_samplers.js b/test/samplers/all_samplers.js index e8c63d84a..9fc5e1a58 100644 --- a/test/samplers/all_samplers.js +++ b/test/samplers/all_samplers.js @@ -16,7 +16,7 @@ import sinon from 'sinon'; import * as constants from '../../src/constants.js'; import ConstSampler from '../../src/samplers/const_sampler.js'; import ProbabilisticSampler from '../../src/samplers/probabilistic_sampler.js'; -import RateLimitingSampler from '../../src/samplers/ratelimiting_sampler.js'; +import RateLimitingSampler from '../../src/samplers/rate_limiting_sampler.js'; import GuaranteedThroughputSampler from '../../src/samplers/guaranteed_throughput_sampler.js'; import PerOperationSampler from '../../src/samplers/per_operation_sampler.js'; import RemoteSampler from '../../src/samplers/remote_sampler.js'; diff --git a/test/samplers/ratelimiting_sampler_test.js b/test/samplers/rate_limiting_sampler.js similarity index 97% rename from test/samplers/ratelimiting_sampler_test.js rename to test/samplers/rate_limiting_sampler.js index 611396645..8004d2941 100644 --- a/test/samplers/ratelimiting_sampler_test.js +++ b/test/samplers/rate_limiting_sampler.js @@ -13,7 +13,7 @@ import { assert, expect } from 'chai'; import ProbabilisticSampler from '../../src/samplers/probabilistic_sampler.js'; -import RateLimitingSampler from '../../src/samplers/ratelimiting_sampler.js'; +import RateLimitingSampler from '../../src/samplers/rate_limiting_sampler.js'; import sinon from 'sinon'; describe('RateLimitingSampler should', () => { diff --git a/test/samplers/remote_sampler.js b/test/samplers/remote_sampler.js index 8bca31b5e..5f6a91307 100644 --- a/test/samplers/remote_sampler.js +++ b/test/samplers/remote_sampler.js @@ -13,7 +13,7 @@ import { assert } from 'chai'; import sinon from 'sinon'; import Metrics from '../../src/metrics/metrics.js'; -import RateLimitingSampler from '../../src/samplers/ratelimiting_sampler'; +import RateLimitingSampler from '../../src/samplers/rate_limiting_sampler'; import ProbabilisticSampler from '../../src/samplers/probabilistic_sampler.js'; import PerOperationSampler from '../../src/samplers/per_operation_sampler'; import RemoteSampler from '../../src/samplers/remote_sampler'; diff --git a/test/span.js b/test/span.js index 0e1e00243..5fe5beac2 100644 --- a/test/span.js +++ b/test/span.js @@ -358,7 +358,7 @@ describe('sampling finalizer', () => { it('should finalize the span sampled with V1 sampler', () => { let span = tracer.startSpan('test'); - assert.isTrue(span.context().samplingFinalized); + assert.isTrue(span.context().samplingFinalized, 'finalized'); }); it('should not trigger on a finish()-ed span', () => { @@ -423,7 +423,7 @@ describe('sampling finalizer', () => { }) ); tracer._sampler = adaptSamplerOrThrow(new ProbabilisticSampler(1.0)); - span._tags = []; // since we don't de-dupe tags, JaegerTestUtils.hasTags() below fails + span._tags = []; // JaegerTestUtils.hasTags() below doesn't work with dupes span.setOperationName('re-sampled-span'); assert.equal(span.operationName, 're-sampled-span'); assert.equal(0, span._tags.length); From f74eae161269336c8d23a02b36d186216e296a06 Mon Sep 17 00:00:00 2001 From: Yuri Shkuro Date: Fri, 30 Aug 2019 18:35:04 -0400 Subject: [PATCH 09/28] Upgrade probabilistic and rate limiting samplers to v2 Signed-off-by: Yuri Shkuro --- src/samplers/_adapt_sampler.js | 60 ++++++++++++++++++++------- src/samplers/probabilistic_sampler.js | 12 +++--- src/samplers/rate_limiting_sampler.js | 12 +++--- test/init_tracer.js | 14 ++++--- 4 files changed, 63 insertions(+), 35 deletions(-) diff --git a/src/samplers/_adapt_sampler.js b/src/samplers/_adapt_sampler.js index d2b27f6a7..fa9a18538 100644 --- a/src/samplers/_adapt_sampler.js +++ b/src/samplers/_adapt_sampler.js @@ -13,7 +13,6 @@ import { SAMPLER_API_V2 } from './constants'; import Span from '../span'; -import BaseSamplerV2 from './v2/base'; function adaptSampler(sampler: any): ?Sampler { if (!sampler) { @@ -40,6 +39,46 @@ export function adaptSamplerOrThrow(sampler: any): Sampler { export default adaptSampler; +export class LegacySamplerV1Base implements Sampler { + apiVersion = SAMPLER_API_V2; + + constructor() {} + + isSampled(operationName: string, outTags: {}): boolean { + throw new Error('Subclass must override isSampled()'); + } + + // equal(other: LegacySamplerV1): boolean { + // throw new Error('Subclass must override equal()'); + // } + + // name(): string { + // throw new Error('Subclass must override name()'); + // } + + onCreateSpan(span: Span): SamplingDecision { + const outTags = {}; + const isSampled = this.isSampled(span.operationName, outTags); + return { sample: isSampled, retryable: false, tags: outTags }; + } + + onSetOperationName(span: Span, operationName: string): SamplingDecision { + const outTags = {}; + const isSampled = this.isSampled(span.operationName, outTags); + return { sample: isSampled, retryable: false, tags: outTags }; + } + + onSetTag(span: Span, key: string, value: any): SamplingDecision { + return { sample: false, retryable: true, tags: null }; + } + + close(callback: ?Function) { + if (callback) { + callback(); + } + } +} + /** * Transforms legacy v1 sampler into V2. * Primarily intended for simple samplers that are not sensitive to @@ -50,28 +89,17 @@ export default adaptSampler; * where as onSetOperation() returns retryable=false, since that is what the tracer * used to do. */ -class LegacySamplerV1Adapter implements Sampler { +class LegacySamplerV1Adapter extends LegacySamplerV1Base { apiVersion = SAMPLER_API_V2; _delegate: LegacySamplerV1; constructor(delegate: LegacySamplerV1) { + super(); this._delegate = delegate; } - onCreateSpan(span: Span): SamplingDecision { - const outTags = {}; - const isSampled = this._delegate.isSampled(span.operationName, outTags); - return { sample: isSampled, retryable: false, tags: outTags }; - } - - onSetOperationName(span: Span, operationName: string): SamplingDecision { - const outTags = {}; - const isSampled = this._delegate.isSampled(span.operationName, outTags); - return { sample: isSampled, retryable: false, tags: outTags }; - } - - onSetTag(span: Span, key: string, value: any): SamplingDecision { - return { sample: false, retryable: true, tags: null }; + isSampled(operationName: string, outTags: {}) { + return this._delegate.isSampled(operationName, outTags); } toString(): string { diff --git a/src/samplers/probabilistic_sampler.js b/src/samplers/probabilistic_sampler.js index e1c5186db..3efe2c197 100644 --- a/src/samplers/probabilistic_sampler.js +++ b/src/samplers/probabilistic_sampler.js @@ -12,11 +12,15 @@ // the License. import * as constants from '../constants.js'; +import { SAMPLER_API_V2 } from './constants'; +import { LegacySamplerV1Base } from './_adapt_sampler'; -export default class ProbabilisticSampler implements LegacySamplerV1 { +export default class ProbabilisticSampler extends LegacySamplerV1Base implements LegacySamplerV1 { + apiVersion = SAMPLER_API_V2; _samplingRate: number; constructor(samplingRate: number) { + super(); if (samplingRate < 0.0 || samplingRate > 1.0) { throw new Error( `The sampling rate must be less than 0.0 and greater than 1.0. Received ${samplingRate}` @@ -58,10 +62,4 @@ export default class ProbabilisticSampler implements LegacySamplerV1 { return this.samplingRate === other.samplingRate; } - - close(callback: ?Function): void { - if (callback) { - callback(); - } - } } diff --git a/src/samplers/rate_limiting_sampler.js b/src/samplers/rate_limiting_sampler.js index 2397819ef..846140251 100644 --- a/src/samplers/rate_limiting_sampler.js +++ b/src/samplers/rate_limiting_sampler.js @@ -13,12 +13,16 @@ import * as constants from '../constants.js'; import RateLimiter from '../rate_limiter.js'; +import { SAMPLER_API_V2 } from './constants'; +import { LegacySamplerV1Base } from './_adapt_sampler'; -export default class RateLimitingSampler implements LegacySamplerV1 { +export default class RateLimitingSampler extends LegacySamplerV1Base implements LegacySamplerV1 { + apiVersion = SAMPLER_API_V2; _rateLimiter: RateLimiter; _maxTracesPerSecond: number; constructor(maxTracesPerSecond: number, initBalance: ?number) { + super(); this._init(maxTracesPerSecond, initBalance); } @@ -70,10 +74,4 @@ export default class RateLimitingSampler implements LegacySamplerV1 { return this.maxTracesPerSecond === other.maxTracesPerSecond; } - - close(callback: ?Function): void { - if (callback) { - callback(); - } - } } diff --git a/test/init_tracer.js b/test/init_tracer.js index 654bf6621..930f058bd 100644 --- a/test/init_tracer.js +++ b/test/init_tracer.js @@ -96,7 +96,11 @@ describe('initTracer', () => { config.sampler = samplerConfig; let tracer = initTracer(config); - expect(tracer._sampler._delegate).to.be.an.instanceof(expectedType); + if (tracer._sampler._delegate) { + expect(tracer._sampler._delegate).to.be.an.instanceof(expectedType); + } else { + expect(tracer._sampler).to.be.an.instanceof(expectedType); + } tracer.close(); // TODO(oibe:head) test utils for expectedParam here? }); @@ -378,8 +382,8 @@ describe('initTracerFromENV', () => { process.env.JAEGER_SAMPLER_TYPE = 'probabilistic'; process.env.JAEGER_SAMPLER_PARAM = 0.5; let tracer = initTracerFromEnv(); - expect(tracer._sampler._delegate).to.be.an.instanceof(ProbabilisticSampler); - assert.equal(tracer._sampler._delegate._samplingRate, 0.5); + expect(tracer._sampler).to.be.an.instanceof(ProbabilisticSampler); + assert.equal(tracer._sampler._samplingRate, 0.5); tracer.close(); process.env.JAEGER_SAMPLER_TYPE = 'remote'; @@ -399,8 +403,8 @@ describe('initTracerFromENV', () => { process.env.JAEGER_SAMPLER_TYPE = 'probabilistic'; process.env.JAEGER_SAMPLER_PARAM = 0.5; let tracer = initTracerFromEnv(); - expect(tracer._sampler._delegate).to.be.an.instanceof(ProbabilisticSampler); - assert.equal(tracer._sampler._delegate._samplingRate, 0.5); + expect(tracer._sampler).to.be.an.instanceof(ProbabilisticSampler); + assert.equal(tracer._sampler._samplingRate, 0.5); tracer.close(); process.env.JAEGER_SAMPLER_TYPE = 'remote'; From f2ee493c9b13fee5bc8de100e3fd2b788763a666 Mon Sep 17 00:00:00 2001 From: Yuri Shkuro Date: Sat, 31 Aug 2019 19:27:14 -0400 Subject: [PATCH 10/28] Upgrade most samplers to V2 Signed-off-by: Yuri Shkuro --- src/samplers/_adapt_sampler.js | 33 ++++++----------- src/samplers/const_sampler.js | 14 ++++---- src/samplers/per_operation_sampler.js | 27 +++++++++++--- src/samplers/probabilistic_sampler.js | 14 ++++++-- src/samplers/rate_limiting_sampler.js | 8 ++--- src/samplers/remote_sampler.js | 51 +++++++++++++++------------ test/init_tracer.js | 32 +++++++++-------- test/samplers/adapt_sampler.js | 26 ++++++++------ test/samplers/all_samplers.js | 13 +++++-- 9 files changed, 127 insertions(+), 91 deletions(-) diff --git a/src/samplers/_adapt_sampler.js b/src/samplers/_adapt_sampler.js index fa9a18538..599deb11c 100644 --- a/src/samplers/_adapt_sampler.js +++ b/src/samplers/_adapt_sampler.js @@ -13,8 +13,9 @@ import { SAMPLER_API_V2 } from './constants'; import Span from '../span'; +import BaseSamplerV2 from './v2/base'; -function adaptSampler(sampler: any): ?Sampler { +export function adaptSampler(sampler: any): ?Sampler { if (!sampler) { return null; } @@ -37,25 +38,19 @@ export function adaptSamplerOrThrow(sampler: any): Sampler { return s; } -export default adaptSampler; - -export class LegacySamplerV1Base implements Sampler { - apiVersion = SAMPLER_API_V2; - - constructor() {} +/** + * Convenience base class for simple samplers that implement isSampled() function + * that is not sensitive to its arguments. + */ +export default class LegacySamplerV1Base extends BaseSamplerV2 { + constructor(name: string) { + super(name); + } isSampled(operationName: string, outTags: {}): boolean { throw new Error('Subclass must override isSampled()'); } - // equal(other: LegacySamplerV1): boolean { - // throw new Error('Subclass must override equal()'); - // } - - // name(): string { - // throw new Error('Subclass must override name()'); - // } - onCreateSpan(span: Span): SamplingDecision { const outTags = {}; const isSampled = this.isSampled(span.operationName, outTags); @@ -71,12 +66,6 @@ export class LegacySamplerV1Base implements Sampler { onSetTag(span: Span, key: string, value: any): SamplingDecision { return { sample: false, retryable: true, tags: null }; } - - close(callback: ?Function) { - if (callback) { - callback(); - } - } } /** @@ -94,7 +83,7 @@ class LegacySamplerV1Adapter extends LegacySamplerV1Base { _delegate: LegacySamplerV1; constructor(delegate: LegacySamplerV1) { - super(); + super(delegate.name()); this._delegate = delegate; } diff --git a/src/samplers/const_sampler.js b/src/samplers/const_sampler.js index 00c149aae..26772ca2f 100644 --- a/src/samplers/const_sampler.js +++ b/src/samplers/const_sampler.js @@ -11,12 +11,16 @@ // or implied. See the License for the specific language governing permissions and limitations under // the License. -import * as constants from '../constants.js'; +import * as constants from '../constants'; +import { SAMPLER_API_V2 } from './constants'; +import LegacySamplerV1Base from './_adapt_sampler'; -export default class ConstSampler implements LegacySamplerV1 { +export default class ConstSampler extends LegacySamplerV1Base implements LegacySamplerV1 { + apiVersion = SAMPLER_API_V2; _decision: boolean; constructor(decision: boolean) { + super('ConstSampler'); this._decision = decision; } @@ -47,10 +51,4 @@ export default class ConstSampler implements LegacySamplerV1 { return this.decision === other.decision; } - - close(callback: ?Function): void { - if (callback) { - callback(); - } - } } diff --git a/src/samplers/per_operation_sampler.js b/src/samplers/per_operation_sampler.js index 1b851e9a3..867898762 100644 --- a/src/samplers/per_operation_sampler.js +++ b/src/samplers/per_operation_sampler.js @@ -12,9 +12,10 @@ // the License. import assert from 'assert'; -import * as constants from '../constants.js'; -import ProbabilisticSampler from './probabilistic_sampler.js'; -import GuaranteedThroughputSampler from './guaranteed_throughput_sampler.js'; +import * as constants from '../constants'; +import { SAMPLER_API_V2 } from './constants'; +import ProbabilisticSampler from './probabilistic_sampler'; +import GuaranteedThroughputSampler from './guaranteed_throughput_sampler'; type SamplersByOperation = { [key: string]: GuaranteedThroughputSampler, __proto__: null }; @@ -23,7 +24,8 @@ type SamplersByOperation = { [key: string]: GuaranteedThroughputSampler, __proto // that all endpoints are represented in the sampled traces. If the number // of distinct operation names exceeds maxOperations, all other names are // sampled with a default probabilistic sampler. -export default class PerOperationSampler implements LegacySamplerV1 { +export default class PerOperationSampler implements Sampler { + apiVersion = SAMPLER_API_V2; _maxOperations: number; _samplersByOperation: SamplersByOperation; _defaultSampler: ProbabilisticSampler; @@ -89,6 +91,23 @@ export default class PerOperationSampler implements LegacySamplerV1 { return sampler.isSampled(operation, tags); } + onCreateSpan(span: Span): SamplingDecision { + const outTags = {}; + const isSampled = this.isSampled(span.operationName, outTags); + // NB: return retryable=true here since we can change decision after setOperationName(). + return { sample: isSampled, retryable: true, tags: outTags }; + } + + onSetOperationName(span: Span, operationName: string): SamplingDecision { + const outTags = {}; + const isSampled = this.isSampled(span.operationName, outTags); + return { sample: isSampled, retryable: false, tags: outTags }; + } + + onSetTag(span: Span, key: string, value: any): SamplingDecision { + return { sample: false, retryable: true, tags: null }; + } + equal(other: LegacySamplerV1): boolean { return false; // TODO equal should be removed } diff --git a/src/samplers/probabilistic_sampler.js b/src/samplers/probabilistic_sampler.js index 3efe2c197..c205fecb2 100644 --- a/src/samplers/probabilistic_sampler.js +++ b/src/samplers/probabilistic_sampler.js @@ -11,16 +11,16 @@ // or implied. See the License for the specific language governing permissions and limitations under // the License. -import * as constants from '../constants.js'; +import * as constants from '../constants'; import { SAMPLER_API_V2 } from './constants'; -import { LegacySamplerV1Base } from './_adapt_sampler'; +import LegacySamplerV1Base from './_adapt_sampler'; export default class ProbabilisticSampler extends LegacySamplerV1Base implements LegacySamplerV1 { apiVersion = SAMPLER_API_V2; _samplingRate: number; constructor(samplingRate: number) { - super(); + super('ProbabilisticSampler'); if (samplingRate < 0.0 || samplingRate > 1.0) { throw new Error( `The sampling rate must be less than 0.0 and greater than 1.0. Received ${samplingRate}` @@ -30,6 +30,14 @@ export default class ProbabilisticSampler extends LegacySamplerV1Base implements this._samplingRate = samplingRate; } + update(samplingRate: number): boolean { + if (this._samplingRate == samplingRate) { + return false; + } + this._samplingRate = samplingRate; + return true; + } + name(): string { return 'ProbabilisticSampler'; } diff --git a/src/samplers/rate_limiting_sampler.js b/src/samplers/rate_limiting_sampler.js index 846140251..e87022477 100644 --- a/src/samplers/rate_limiting_sampler.js +++ b/src/samplers/rate_limiting_sampler.js @@ -11,10 +11,10 @@ // or implied. See the License for the specific language governing permissions and limitations under // the License. -import * as constants from '../constants.js'; -import RateLimiter from '../rate_limiter.js'; +import * as constants from '../constants'; import { SAMPLER_API_V2 } from './constants'; -import { LegacySamplerV1Base } from './_adapt_sampler'; +import LegacySamplerV1Base from './_adapt_sampler'; +import RateLimiter from '../rate_limiter'; export default class RateLimitingSampler extends LegacySamplerV1Base implements LegacySamplerV1 { apiVersion = SAMPLER_API_V2; @@ -22,7 +22,7 @@ export default class RateLimitingSampler extends LegacySamplerV1Base implements _maxTracesPerSecond: number; constructor(maxTracesPerSecond: number, initBalance: ?number) { - super(); + super('RateLimitingSampler'); this._init(maxTracesPerSecond, initBalance); } diff --git a/src/samplers/remote_sampler.js b/src/samplers/remote_sampler.js index b0fcc45df..5de1cce24 100644 --- a/src/samplers/remote_sampler.js +++ b/src/samplers/remote_sampler.js @@ -12,11 +12,13 @@ // the License. import url from 'url'; -import ProbabilisticSampler from './probabilistic_sampler.js'; -import RateLimitingSampler from './rate_limiting_sampler.js'; -import PerOperationSampler from './per_operation_sampler.js'; -import Metrics from '../metrics/metrics.js'; -import NullLogger from '../logger.js'; +import { SAMPLER_API_V2 } from './constants'; +import { adaptSamplerOrThrow } from './_adapt_sampler'; +import ProbabilisticSampler from './probabilistic_sampler'; +import RateLimitingSampler from './rate_limiting_sampler'; +import PerOperationSampler from './per_operation_sampler'; +import Metrics from '../metrics/metrics'; +import NullLogger from '../logger'; import NoopMetricFactory from '../metrics/noop/metric_factory'; import Utils from '../util'; @@ -29,9 +31,10 @@ const PROBABILISTIC_STRATEGY_TYPE = 'PROBABILISTIC'; const RATE_LIMITING_STRATEGY_TYPE = 'RATE_LIMITING'; const PER_OPERATION_STRATEGY_TYPE = 'PER_OPERATION'; -export default class RemoteControlledSampler implements LegacySamplerV1 { +export default class RemoteControlledSampler implements Sampler { + apiVersion = SAMPLER_API_V2; _serviceName: string; - _sampler: LegacySamplerV1; + _sampler: Sampler; _logger: Logger; _metrics: Metrics; @@ -62,7 +65,9 @@ export default class RemoteControlledSampler implements LegacySamplerV1 { */ constructor(serviceName: string, options: any = {}) { this._serviceName = serviceName; - this._sampler = options.sampler || new ProbabilisticSampler(DEFAULT_INITIAL_SAMPLING_RATE); + this._sampler = adaptSamplerOrThrow( + options.sampler || new ProbabilisticSampler(DEFAULT_INITIAL_SAMPLING_RATE) + ); this._logger = options.logger || new NullLogger(); this._metrics = options.metrics || new Metrics(new NoopMetricFactory()); this._refreshInterval = options.refreshInterval || DEFAULT_REFRESH_INTERVAL; @@ -153,11 +158,15 @@ export default class RemoteControlledSampler implements LegacySamplerV1 { this._sampler = new PerOperationSampler(response.operationSampling, this._maxOperations); return true; } - let newSampler: LegacySamplerV1; if (response.strategyType === PROBABILISTIC_STRATEGY_TYPE && response.probabilisticSampling) { let samplingRate = response.probabilisticSampling.samplingRate; - newSampler = new ProbabilisticSampler(samplingRate); - } else if (response.strategyType === RATE_LIMITING_STRATEGY_TYPE && response.rateLimitingSampling) { + if (this._sampler instanceof ProbabilisticSampler) { + return this._sampler.update(samplingRate); + } + this._sampler = new ProbabilisticSampler(samplingRate); + return true; + } + if (response.strategyType === RATE_LIMITING_STRATEGY_TYPE && response.rateLimitingSampling) { let maxTracesPerSecond = response.rateLimitingSampling.maxTracesPerSecond; if (this._sampler instanceof RateLimitingSampler) { let sampler: RateLimitingSampler = this._sampler; @@ -165,23 +174,21 @@ export default class RemoteControlledSampler implements LegacySamplerV1 { } this._sampler = new RateLimitingSampler(maxTracesPerSecond); return true; - } else { - throw 'Malformed response: ' + JSON.stringify(response); } - if (this._sampler.equal(newSampler)) { - return false; - } - this._sampler = newSampler; - return true; + throw 'Malformed response: ' + JSON.stringify(response); + } + + onCreateSpan(span: Span): SamplingDecision { + return this._sampler.onCreateSpan(span); } - isSampled(operation: string, tags: any): boolean { - return this._sampler.isSampled(operation, tags); + onSetOperationName(span: Span, operationName: string): SamplingDecision { + return this._sampler.onSetOperationName(span, operationName); } - equal(other: LegacySamplerV1): boolean { - return false; + onSetTag(span: Span, key: string, value: any): SamplingDecision { + return this._sampler.onSetTag(span, key, value); } close(callback: ?Function): void { diff --git a/test/init_tracer.js b/test/init_tracer.js index 930f058bd..52d67d814 100644 --- a/test/init_tracer.js +++ b/test/init_tracer.js @@ -71,7 +71,7 @@ describe('initTracer', () => { }; let tracer = initTracer(config); - expect(tracer._sampler._delegate).to.be.an.instanceof(RemoteSampler); + expect(tracer._sampler).to.be.an.instanceof(RemoteSampler); expect(tracer._reporter).to.be.an.instanceof(RemoteReporter); tracer.close(done); }); @@ -257,10 +257,12 @@ describe('initTracer', () => { metrics: metrics, } ); + expect(tracer._reporter).to.be.an.instanceof(RemoteReporter); assert.equal(tracer._reporter._metrics._factory, metrics); assert.equal(tracer._reporter._logger, logger); - assert.equal(tracer._sampler._delegate._metrics._factory, metrics); - assert.equal(tracer._sampler._delegate._logger, logger); + expect(tracer._sampler).to.be.an.instanceof(RemoteSampler); + assert.equal(tracer._sampler._metrics._factory, metrics); + assert.equal(tracer._sampler._logger, logger); tracer.close(done); }); @@ -390,10 +392,10 @@ describe('initTracerFromENV', () => { process.env.JAEGER_SAMPLER_MANAGER_HOST_PORT = 'localhost:8080'; process.env.JAEGER_SAMPLER_REFRESH_INTERVAL = 100; tracer = initTracerFromEnv(); - expect(tracer._sampler._delegate).to.be.an.instanceof(RemoteSampler); - assert.equal(tracer._sampler._delegate._host, 'localhost'); - assert.equal(tracer._sampler._delegate._port, 8080); - assert.equal(tracer._sampler._delegate._refreshInterval, 100); + expect(tracer._sampler).to.be.an.instanceof(RemoteSampler); + assert.equal(tracer._sampler._host, 'localhost'); + assert.equal(tracer._sampler._port, 8080); + assert.equal(tracer._sampler._refreshInterval, 100); tracer.close(); }); @@ -412,10 +414,10 @@ describe('initTracerFromENV', () => { process.env.JAEGER_SAMPLER_PORT = 8080; process.env.JAEGER_SAMPLER_REFRESH_INTERVAL = 100; tracer = initTracerFromEnv(); - expect(tracer._sampler._delegate).to.be.an.instanceof(RemoteSampler); - assert.equal(tracer._sampler._delegate._host, 'localhost'); - assert.equal(tracer._sampler._delegate._port, 8080); - assert.equal(tracer._sampler._delegate._refreshInterval, 100); + expect(tracer._sampler).to.be.an.instanceof(RemoteSampler); + assert.equal(tracer._sampler._host, 'localhost'); + assert.equal(tracer._sampler._port, 8080); + assert.equal(tracer._sampler._refreshInterval, 100); tracer.close(); }); @@ -530,10 +532,10 @@ describe('initTracerFromENV', () => { }; let tracer = initTracerFromEnv(config, options); assert.equal(tracer._serviceName, 'test-service-arg'); - expect(tracer._sampler._delegate).to.be.an.instanceof(RemoteSampler); - assert.equal(tracer._sampler._delegate._host, 'localhost'); - assert.equal(tracer._sampler._delegate._port, 8080); - assert.equal(tracer._sampler._delegate._refreshInterval, 100); + expect(tracer._sampler).to.be.an.instanceof(RemoteSampler); + assert.equal(tracer._sampler._host, 'localhost'); + assert.equal(tracer._sampler._port, 8080); + assert.equal(tracer._sampler._refreshInterval, 100); assert.equal(tracer._tags['KEY2'], 'VALUE2'); tracer.close(done); }); diff --git a/test/samplers/adapt_sampler.js b/test/samplers/adapt_sampler.js index 3628db6e4..edcb509ab 100644 --- a/test/samplers/adapt_sampler.js +++ b/test/samplers/adapt_sampler.js @@ -12,33 +12,37 @@ // the License. import { assert } from 'chai'; -import adaptSampler from '../../src/samplers/_adapt_sampler'; -import { adaptSamplerOrThrow } from '../../src/samplers/_adapt_sampler'; -import BaseSamplerV2 from '../../src/samplers/v2/base'; +// Import Tracer here to work around a weird bug that causes the error like this: +// TypeError: Super expression must either be null or a function, not undefined +// at _inherits (.../jaeger-client-node/src/samplers/const_sampler.js:27:113) +// The error seems to be related to a recursive import _adapt_sampler -> Span -> Tracer -> _adapt_sampler. +import Tracer from '../../src/tracer'; +import * as adapter from '../../src/samplers/_adapt_sampler'; import ConstSampler from '../../src/samplers/const_sampler'; +import GuaranteedThroughputSampler from '../../src/samplers/guaranteed_throughput_sampler'; describe('adaptSampler', () => { it('should return null for null argument', () => { - assert.isNull(adaptSampler(null)); + assert.isNull(adapter.adaptSampler(null)); }); it('should return wrapper for v1 sampler', () => { - let s1 = new ConstSampler(false); - let s2: any = adaptSampler(s1); + let s1 = new GuaranteedThroughputSampler(0, 1.0); + let s2: any = adapter.adaptSampler(s1); assert.deepEqual(s1, s2._delegate); }); it('should return v2 sampler as is', () => { - let s1 = new BaseSamplerV2('name1'); - assert.equal(s1, adaptSampler(s1)); + let s1 = new ConstSampler('name1'); + assert.equal(s1, adapter.adaptSampler(s1)); }); it('should delegate toString', () => { - let s1 = new ConstSampler(false); - let s2: any = adaptSampler(s1); + let s1 = new GuaranteedThroughputSampler(0, 1.0); + let s2: any = adapter.adaptSampler(s1); assert.equal(s1.toString(), s2.toString()); }); }); describe('adaptSamplerOrThrow', () => { it('should throw on unrecognized sampler', () => { - assert.throws(() => adaptSamplerOrThrow(null), Error, 'Unrecognized sampler: null'); + assert.throws(() => adapter.adaptSamplerOrThrow(null), Error, 'Unrecognized sampler: null'); }); }); diff --git a/test/samplers/all_samplers.js b/test/samplers/all_samplers.js index 9fc5e1a58..68b21bdbb 100644 --- a/test/samplers/all_samplers.js +++ b/test/samplers/all_samplers.js @@ -85,7 +85,7 @@ describe('All samplers', () => { let sampler = samplerSetup['sampler']; it(sampler.toString(), () => { let expectedTags = {}; - let expectedDecision = !!samplerSetup['decision']; + let expectedDecision = Boolean(samplerSetup['decision']); let description = `${sampler.toString()}, param=${samplerSetup['param']}`; if (expectedDecision) { @@ -93,7 +93,16 @@ describe('All samplers', () => { expectedTags[constants.SAMPLER_PARAM_TAG_KEY] = samplerSetup['param']; } let actualTags = {}; - let decision = sampler.isSampled('operation', actualTags); + let decision: boolean; + if (typeof sampler.isSampled === 'function') { + decision = sampler.isSampled('operation', actualTags); + } else { + // assume V2 sampler + let s: Span = {}; + let d = sampler.onCreateSpan(s); + decision = d.sample; + actualTags = d.tags; + } assert.equal(decision, expectedDecision, description); assert.deepEqual(actualTags, expectedTags, description); }); From 6fe64c0248dcfbef6dcad39d1dd8b54870ec29ed Mon Sep 17 00:00:00 2001 From: Yuri Shkuro Date: Sat, 31 Aug 2019 20:11:06 -0400 Subject: [PATCH 11/28] Fix flow error Signed-off-by: Yuri Shkuro --- test/samplers/adapt_sampler.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/samplers/adapt_sampler.js b/test/samplers/adapt_sampler.js index edcb509ab..dbb76dc8f 100644 --- a/test/samplers/adapt_sampler.js +++ b/test/samplers/adapt_sampler.js @@ -31,7 +31,7 @@ describe('adaptSampler', () => { assert.deepEqual(s1, s2._delegate); }); it('should return v2 sampler as is', () => { - let s1 = new ConstSampler('name1'); + let s1 = new ConstSampler(true); assert.equal(s1, adapter.adaptSampler(s1)); }); it('should delegate toString', () => { From 5f0f5e152ca27eb1fb6dc9db088ecfd2e7f1115e Mon Sep 17 00:00:00 2001 From: Yuri Shkuro Date: Sat, 31 Aug 2019 23:19:02 -0400 Subject: [PATCH 12/28] Cleanup some stuff Signed-off-by: Yuri Shkuro --- src/samplers/const_sampler.js | 2 -- src/samplers/probabilistic_sampler.js | 2 -- src/samplers/rate_limiting_sampler.js | 2 -- src/samplers/remote_sampler.js | 1 - 4 files changed, 7 deletions(-) diff --git a/src/samplers/const_sampler.js b/src/samplers/const_sampler.js index 26772ca2f..1ab44aef2 100644 --- a/src/samplers/const_sampler.js +++ b/src/samplers/const_sampler.js @@ -12,11 +12,9 @@ // the License. import * as constants from '../constants'; -import { SAMPLER_API_V2 } from './constants'; import LegacySamplerV1Base from './_adapt_sampler'; export default class ConstSampler extends LegacySamplerV1Base implements LegacySamplerV1 { - apiVersion = SAMPLER_API_V2; _decision: boolean; constructor(decision: boolean) { diff --git a/src/samplers/probabilistic_sampler.js b/src/samplers/probabilistic_sampler.js index c205fecb2..60aa7b83f 100644 --- a/src/samplers/probabilistic_sampler.js +++ b/src/samplers/probabilistic_sampler.js @@ -12,11 +12,9 @@ // the License. import * as constants from '../constants'; -import { SAMPLER_API_V2 } from './constants'; import LegacySamplerV1Base from './_adapt_sampler'; export default class ProbabilisticSampler extends LegacySamplerV1Base implements LegacySamplerV1 { - apiVersion = SAMPLER_API_V2; _samplingRate: number; constructor(samplingRate: number) { diff --git a/src/samplers/rate_limiting_sampler.js b/src/samplers/rate_limiting_sampler.js index e87022477..2d3dcd1e0 100644 --- a/src/samplers/rate_limiting_sampler.js +++ b/src/samplers/rate_limiting_sampler.js @@ -12,12 +12,10 @@ // the License. import * as constants from '../constants'; -import { SAMPLER_API_V2 } from './constants'; import LegacySamplerV1Base from './_adapt_sampler'; import RateLimiter from '../rate_limiter'; export default class RateLimitingSampler extends LegacySamplerV1Base implements LegacySamplerV1 { - apiVersion = SAMPLER_API_V2; _rateLimiter: RateLimiter; _maxTracesPerSecond: number; diff --git a/src/samplers/remote_sampler.js b/src/samplers/remote_sampler.js index 5de1cce24..b24f6a725 100644 --- a/src/samplers/remote_sampler.js +++ b/src/samplers/remote_sampler.js @@ -29,7 +29,6 @@ const DEFAULT_SAMPLING_HOST = '0.0.0.0'; const DEFAULT_SAMPLING_PORT = 5778; const PROBABILISTIC_STRATEGY_TYPE = 'PROBABILISTIC'; const RATE_LIMITING_STRATEGY_TYPE = 'RATE_LIMITING'; -const PER_OPERATION_STRATEGY_TYPE = 'PER_OPERATION'; export default class RemoteControlledSampler implements Sampler { apiVersion = SAMPLER_API_V2; From d0775614f65c4886313dc988c8e3f94d035b13ef Mon Sep 17 00:00:00 2001 From: Yuri Shkuro Date: Sun, 1 Sep 2019 00:04:54 -0400 Subject: [PATCH 13/28] Test that per-operation-sampler still behaves as before Signed-off-by: Yuri Shkuro --- .vscode/launch.json | 2 +- .../samplers/guaranteed_throughput_sampler.js | 8 +++--- test/samplers/per_operation_sampler.js | 27 +++++++++++++++++++ 3 files changed, 32 insertions(+), 5 deletions(-) diff --git a/.vscode/launch.json b/.vscode/launch.json index b0732de36..c8bbe4919 100644 --- a/.vscode/launch.json +++ b/.vscode/launch.json @@ -4,7 +4,7 @@ { "type": "node", "request": "launch", - "name": "Mocha Current File", + "name": "Debug Current File", "program": "${workspaceFolder}/node_modules/mocha/bin/_mocha", "args": ["--timeout", "999999", "--compilers", "js:babel-core/register", "--colors", "${file}"], "console": "integratedTerminal", diff --git a/test/samplers/guaranteed_throughput_sampler.js b/test/samplers/guaranteed_throughput_sampler.js index d950a8dd8..13c3098b5 100644 --- a/test/samplers/guaranteed_throughput_sampler.js +++ b/test/samplers/guaranteed_throughput_sampler.js @@ -34,8 +34,8 @@ describe('GuaranteedThroughput sampler', () => { it('should equal itself', () => { let sampler = new GuaranteedThroughputSampler(2, 0); - assert.isOk(sampler.equal(sampler)); - assert.isOk(sampler.equal(new GuaranteedThroughputSampler(2, 0))); + assert.isTrue(sampler.equal(sampler)); + assert.isTrue(sampler.equal(new GuaranteedThroughputSampler(2, 0))); sampler.close(); }); @@ -65,10 +65,10 @@ describe('GuaranteedThroughput sampler', () => { sampler.close(); }); - let assertValues = function assertValues(sampler, lb, rate) { + function assertValues(sampler, lb, rate) { assert.equal(lb, sampler._lowerBoundSampler.maxTracesPerSecond); assert.equal(rate, sampler._probabilisticSampler.samplingRate); - }; + } it('should not change when update() called with the same values', () => { let sampler = new GuaranteedThroughputSampler(2, 1.0); diff --git a/test/samplers/per_operation_sampler.js b/test/samplers/per_operation_sampler.js index 92a1c495c..d3ea92b3e 100644 --- a/test/samplers/per_operation_sampler.js +++ b/test/samplers/per_operation_sampler.js @@ -13,7 +13,9 @@ import { assert } from 'chai'; import sinon from 'sinon'; +import NoopReporter from '../../src/reporters/noop_reporter'; import PerOperationSampler from '../../src/samplers/per_operation_sampler'; +import Tracer from '../../src/tracer'; describe('PerOperationSampler', () => { let strategies: PerOperationSamplingStrategies = { @@ -141,4 +143,29 @@ describe('PerOperationSampler', () => { assert.strictEqual(sampler._samplersByOperation['op1'], s1); assert.strictEqual(sampler._samplersByOperation['op2'], s2); }); + + it('should leave span non-finalized after first sampling, and finalize after setOperation', () => { + let strategies: PerOperationSamplingStrategies = { + defaultLowerBoundTracesPerSecond: 0, + defaultSamplingProbability: 1, + perOperationStrategies: [ + { + operation: 'op1', + probabilisticSampling: { samplingRate: 0 }, + }, + { + operation: 'op2', + probabilisticSampling: { samplingRate: 1 }, + }, + ], + }; + let sampler = new PerOperationSampler(strategies, 2); + let tracer = new Tracer('test', new NoopReporter(), sampler); + let span = tracer.startSpan('op1'); + assert.isFalse(span._spanContext.isSampled()); + assert.isFalse(span._spanContext.samplingFinalized); + span.setOperationName('op2'); + assert.isTrue(span._spanContext.isSampled()); + assert.isTrue(span._spanContext.samplingFinalized); + }); }); From a1cb4ae4d0a7103e7d49a1b9e63da28aa85574aa Mon Sep 17 00:00:00 2001 From: Yuri Shkuro Date: Tue, 3 Sep 2019 12:18:27 -0400 Subject: [PATCH 14/28] Address review comments Signed-off-by: Yuri Shkuro --- src/samplers/per_operation_sampler.js | 3 +- src/util.js | 4 +- test/samplers/delayed_sampling.js | 55 ++++++++++++++++++--------- 3 files changed, 40 insertions(+), 22 deletions(-) diff --git a/src/samplers/per_operation_sampler.js b/src/samplers/per_operation_sampler.js index 867898762..5cd5dc164 100644 --- a/src/samplers/per_operation_sampler.js +++ b/src/samplers/per_operation_sampler.js @@ -12,10 +12,9 @@ // the License. import assert from 'assert'; -import * as constants from '../constants'; import { SAMPLER_API_V2 } from './constants'; -import ProbabilisticSampler from './probabilistic_sampler'; import GuaranteedThroughputSampler from './guaranteed_throughput_sampler'; +import ProbabilisticSampler from './probabilistic_sampler'; type SamplersByOperation = { [key: string]: GuaranteedThroughputSampler, __proto__: null }; diff --git a/src/util.js b/src/util.js index a52425f1d..e8d61a3ec 100644 --- a/src/util.js +++ b/src/util.js @@ -151,7 +151,9 @@ export default class Utils { /** * Creates a callback function that only delegates to passed callback - * after limit invocations. + * after limit invocations. Useful in types like CompositeReporter that + * needs to invoke the top level callback only after all delegates' close() methods + * are called. */ static countdownCallback(limit: number, callback: ?() => void): () => void { let count = 0; diff --git a/test/samplers/delayed_sampling.js b/test/samplers/delayed_sampling.js index 6337aa3b9..bd08252d2 100644 --- a/test/samplers/delayed_sampling.js +++ b/test/samplers/delayed_sampling.js @@ -97,9 +97,22 @@ describe('delayed sampling', () => { } } - declare type PrioritySamplerState = { - samplerFired: Array, - }; + /** + * PrioritySamplerState keeps the state of all underlying samplers, specifically + * whether each of them has previously returned retryable=false, in which case + * those samplers are no longer invoked on future sampling calls. + */ + class PrioritySamplerState { + samplerFired: Array; + + constructor(numDelegateSamplers: number) { + this.samplerFired = Array(numDelegateSamplers); + // cannot use array.fill() in Node 0.10 + for (let i = 0; i < numDelegateSamplers; i++) { + this.samplerFired[i] = false; + } + } + } /** * PrioritySampler contains a list of samplers that it interrogates in order. @@ -120,31 +133,25 @@ describe('delayed sampling', () => { this._delegates = samplers.map(s => adaptSamplerOrThrow(s)); } - _getState(span: Span): PrioritySamplerState { + _getOrCreateState(span: Span): PrioritySamplerState { const store = span.context()._samplingState.extendedState(); const stateKey = 'PrioritySampler'; // TODO ideally should be uniqueName() per BaseSamplerB2 let state: ?PrioritySamplerState = store[stateKey]; if (!state) { - state = { - samplerFired: Array(this._delegates.length), - }; - // cannot use array.fill() in Node 0.10 - for (let i = 0; i < this._delegates.length; i++) { - state.samplerFired[i] = false; - } + state = new PrioritySamplerState(this._delegates.length); store[stateKey] = state; } return state; } - onCreateSpan(span: Span): SamplingDecision { - const state = this._getState(span); + _trySampling(span: Span, fn: Function): SamplingDecision { + const state = this._getOrCreateState(span); let retryable = false; for (let i = 0; i < this._delegates.length; i++) { if (state.samplerFired[i]) { continue; } - const d = this._delegates[i].onCreateSpan(span); + const d = fn(this._delegates[i]); retryable = retryable || d.retryable; if (!d.retryable) { state.samplerFired[i] = true; @@ -156,14 +163,22 @@ describe('delayed sampling', () => { return { sample: false, retryable: retryable, tags: null }; } + onCreateSpan(span: Span): SamplingDecision { + return this._trySampling(span, function(delegate: Sampler): SamplingDecision { + return delegate.onCreateSpan(span); + }); + } + onSetOperationName(span: Span, operationName: string): SamplingDecision { - // FIXME: - return this.onCreateSpan(span); + return this._trySampling(span, function(delegate: Sampler): SamplingDecision { + return delegate.onSetOperationName(span, operationName); + }); } onSetTag(span: Span, key: string, value: any): SamplingDecision { - // FIXME: - return this.onCreateSpan(span); + return this._trySampling(span, function(delegate: Sampler): SamplingDecision { + return delegate.onSetTag(span, key, value); + }); } toString(): string { @@ -176,7 +191,7 @@ describe('delayed sampling', () => { } } - describe('with PrioritySampler', () => { + describe('with PrioritySampler and TagSampler', () => { const tagSampler = new TagEqualsSampler('theWho', [{ tagValue: 'Bender' }, { tagValue: 'Leela' }]); const constSampler = new ConstSampler(false); const priSampler = new PrioritySampler([tagSampler, constSampler]); @@ -202,6 +217,8 @@ describe('delayed sampling', () => { assert.isFalse(span._spanContext.isSampled(), 'sampled'); assert.isFalse(span._spanContext.samplingFinalized, 'finalized'); span.setTag('theWho', 'Leela'); + assert.isTrue(span._spanContext.isSampled(), 'sampled'); + assert.isTrue(span._spanContext.samplingFinalized, 'finalized'); }); it('should not sample or finalize span after starting a child span', () => { From 689652b180799b36fb18dad5ca3ae442c480cfed Mon Sep 17 00:00:00 2001 From: Yuri Shkuro Date: Tue, 3 Sep 2019 12:59:37 -0400 Subject: [PATCH 15/28] Simplify with inheritance Signed-off-by: Yuri Shkuro --- test/samplers/delayed_sampling.js | 28 ++++++---------------------- 1 file changed, 6 insertions(+), 22 deletions(-) diff --git a/test/samplers/delayed_sampling.js b/test/samplers/delayed_sampling.js index bd08252d2..84f7c0590 100644 --- a/test/samplers/delayed_sampling.js +++ b/test/samplers/delayed_sampling.js @@ -13,11 +13,11 @@ import { assert } from 'chai'; import * as opentracing from 'opentracing'; -import { SAMPLER_API_V2 } from '../../src/samplers/constants'; import Span from '../../src/span'; import Utils from '../../src/util'; import ConstSampler from '../../src/samplers/const_sampler'; import { adaptSamplerOrThrow } from '../../src/samplers/_adapt_sampler'; +import BaseSamplerV2 from '../../src/samplers/v2/base'; import InMemoryReporter from '../../src/reporters/in_memory_reporter'; import Tracer from '../../src/tracer'; @@ -27,14 +27,13 @@ describe('delayed sampling', () => { // can add more flags here, like firehose }; - class TagEqualsSampler implements Sampler { - apiVersion = SAMPLER_API_V2; - + class TagEqualsSampler extends BaseSamplerV2 { _tagKey: string; _matchers: { [string]: Matcher }; _undecided: SamplingDecision; constructor(tagKey: string, matchers: Array) { + super('TagEqualsSampler'); this._tagKey = tagKey; this._matchers = {}; matchers.forEach(m => { @@ -85,16 +84,6 @@ describe('delayed sampling', () => { } return this._undecided; } - - toString(): string { - return 'TagEqualsSampler'; - } - - close(callback: ?Function): void { - if (callback) { - callback(); - } - } } /** @@ -124,18 +113,17 @@ describe('delayed sampling', () => { * TODO: since sampling state is shared across all spans of the trace, the execution * of the sampler should probably only happen on the local-root span. */ - class PrioritySampler implements Sampler { - apiVersion = SAMPLER_API_V2; - + class PrioritySampler extends BaseSamplerV2 { _delegates: Array; constructor(samplers: Array) { + super('PrioritySampler'); this._delegates = samplers.map(s => adaptSamplerOrThrow(s)); } _getOrCreateState(span: Span): PrioritySamplerState { const store = span.context()._samplingState.extendedState(); - const stateKey = 'PrioritySampler'; // TODO ideally should be uniqueName() per BaseSamplerB2 + const stateKey = this.uniqueName(); let state: ?PrioritySamplerState = store[stateKey]; if (!state) { state = new PrioritySamplerState(this._delegates.length); @@ -181,10 +169,6 @@ describe('delayed sampling', () => { }); } - toString(): string { - return 'DelegatingSampler'; - } - close(callback: ?() => void): void { const countdownCallback = Utils.countdownCallback(this._delegates.length, callback); this._delegates.forEach(r => r.close(countdownCallback)); From 714f55c5e950b47450b7b6227a62f5ea84f81ca4 Mon Sep 17 00:00:00 2001 From: Yuri Shkuro Date: Tue, 3 Sep 2019 15:20:27 -0400 Subject: [PATCH 16/28] Move new samplers to /experimental/ Signed-off-by: Yuri Shkuro --- src/samplers/experimental/priority_sampler.js | 103 ++++++++ .../experimental/tag_equals_sampler.js | 80 +++++++ test/samplers/delayed_sampling.js | 224 ------------------ test/samplers/exp_delayed_sampling.js | 74 ++++++ 4 files changed, 257 insertions(+), 224 deletions(-) create mode 100644 src/samplers/experimental/priority_sampler.js create mode 100644 src/samplers/experimental/tag_equals_sampler.js delete mode 100644 test/samplers/delayed_sampling.js create mode 100644 test/samplers/exp_delayed_sampling.js diff --git a/src/samplers/experimental/priority_sampler.js b/src/samplers/experimental/priority_sampler.js new file mode 100644 index 000000000..018b010ff --- /dev/null +++ b/src/samplers/experimental/priority_sampler.js @@ -0,0 +1,103 @@ +// @flow +// Copyright (c) 2019 Uber Technologies, Inc. +// +// 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 +// +// http://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 { adaptSamplerOrThrow } from '../_adapt_sampler'; +import BaseSamplerV2 from '../v2/base'; +import Span from '../../span'; +import Utils from '../../util'; + +/** + * PrioritySamplerState keeps the state of all underlying samplers, specifically + * whether each of them has previously returned retryable=false, in which case + * those samplers are no longer invoked on future sampling calls. + */ +export class PrioritySamplerState { + samplerFired: Array; + + constructor(numDelegateSamplers: number) { + this.samplerFired = Array(numDelegateSamplers); + // TODO: for some reason Babel does not translate array.fill() to polyfil that works w/ Node 0.10 + for (let i = 0; i < numDelegateSamplers; i++) { + this.samplerFired[i] = false; + } + } +} + +/** + * PrioritySampler contains a list of samplers that it interrogates in order. + * Sampling methods return as soon as one of the samplers returns sample=true. + * The retryable state for each underlying sampler is stored in the extended context + * and once retryable=false is returned by one of the delegates it will never be + * called against. + */ +export default class PrioritySampler extends BaseSamplerV2 { + _delegates: Array; + + constructor(samplers: Array) { + super('PrioritySampler'); + this._delegates = samplers.map(s => adaptSamplerOrThrow(s)); + } + + _getOrCreateState(span: Span): PrioritySamplerState { + const store = span.context()._samplingState.extendedState(); + const stateKey = this.uniqueName(); + let state: ?PrioritySamplerState = store[stateKey]; + if (!state) { + state = new PrioritySamplerState(this._delegates.length); + store[stateKey] = state; + } + return state; + } + + _trySampling(span: Span, fn: Function): SamplingDecision { + const state = this._getOrCreateState(span); + let retryable = false; + for (let i = 0; i < this._delegates.length; i++) { + if (state.samplerFired[i]) { + continue; + } + const d = fn(this._delegates[i]); + retryable = retryable || d.retryable; + if (!d.retryable) { + state.samplerFired[i] = true; + } + if (d.sample) { + return d; // TODO do we want to alter out tags? + } + } + return { sample: false, retryable: retryable, tags: null }; + } + + onCreateSpan(span: Span): SamplingDecision { + return this._trySampling(span, function(delegate: Sampler): SamplingDecision { + return delegate.onCreateSpan(span); + }); + } + + onSetOperationName(span: Span, operationName: string): SamplingDecision { + return this._trySampling(span, function(delegate: Sampler): SamplingDecision { + return delegate.onSetOperationName(span, operationName); + }); + } + + onSetTag(span: Span, key: string, value: any): SamplingDecision { + return this._trySampling(span, function(delegate: Sampler): SamplingDecision { + return delegate.onSetTag(span, key, value); + }); + } + + close(callback: ?() => void): void { + const countdownCallback = Utils.countdownCallback(this._delegates.length, callback); + this._delegates.forEach(r => r.close(countdownCallback)); + } +} diff --git a/src/samplers/experimental/tag_equals_sampler.js b/src/samplers/experimental/tag_equals_sampler.js new file mode 100644 index 000000000..c5ec059e1 --- /dev/null +++ b/src/samplers/experimental/tag_equals_sampler.js @@ -0,0 +1,80 @@ +// @flow +// Copyright (c) 2019 Uber Technologies, Inc. +// +// 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 +// +// http://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 { adaptSamplerOrThrow } from '../_adapt_sampler'; +import BaseSamplerV2 from '../v2/base'; +import Span from '../../span'; + +declare type Matcher = { + tagValue: string, + firehose: boolean, +}; + +export default class TagEqualsSampler extends BaseSamplerV2 { + _tagKey: string; + _matchers: { [string]: Matcher }; + _undecided: SamplingDecision; + + constructor(tagKey: string, matchers: Array) { + super('TagEqualsSampler'); + this._tagKey = tagKey; + this._matchers = {}; + matchers.forEach(m => { + this._matchers[m.tagValue] = m; + }); + this._undecided = { sample: false, retryable: true, tags: null }; + } + + _findTag(tags: Array): ?Tag { + for (let i = 0; i < tags.length; i++) { + if (tags[i].key === this._tagKey) { + return tags[i]; + } + } + return null; + } + + _createOutTags(tagValue: string): { [string]: string } { + return { + 'sampler.type': 'TagEqualsSampler', + 'sampler.param': tagValue, + }; + } + + _decide(tagValue: any): SamplingDecision { + const match: ?Matcher = this._matchers[tagValue]; + if (match) { + return { sample: true, retryable: false, tags: this._createOutTags(match.tagValue) }; + } + return this._undecided; + } + + onCreateSpan(span: Span): SamplingDecision { + const tag: ?Tag = this._findTag(span.getTags()); + if (tag) { + return this._decide(tag.value); + } + return this._undecided; + } + + onSetOperationName(span: Span, operationName: string): SamplingDecision { + return this.onCreateSpan(span); + } + + onSetTag(span: Span, key: string, value: any): SamplingDecision { + if (key === this._tagKey) { + return this._decide(value); + } + return this._undecided; + } +} diff --git a/test/samplers/delayed_sampling.js b/test/samplers/delayed_sampling.js deleted file mode 100644 index 84f7c0590..000000000 --- a/test/samplers/delayed_sampling.js +++ /dev/null @@ -1,224 +0,0 @@ -// @flow -// Copyright (c) 2019 Uber Technologies, Inc. -// -// 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 -// -// http://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 { assert } from 'chai'; -import * as opentracing from 'opentracing'; -import Span from '../../src/span'; -import Utils from '../../src/util'; -import ConstSampler from '../../src/samplers/const_sampler'; -import { adaptSamplerOrThrow } from '../../src/samplers/_adapt_sampler'; -import BaseSamplerV2 from '../../src/samplers/v2/base'; -import InMemoryReporter from '../../src/reporters/in_memory_reporter'; -import Tracer from '../../src/tracer'; - -describe('delayed sampling', () => { - declare type Matcher = { - tagValue: string, - // can add more flags here, like firehose - }; - - class TagEqualsSampler extends BaseSamplerV2 { - _tagKey: string; - _matchers: { [string]: Matcher }; - _undecided: SamplingDecision; - - constructor(tagKey: string, matchers: Array) { - super('TagEqualsSampler'); - this._tagKey = tagKey; - this._matchers = {}; - matchers.forEach(m => { - this._matchers[m.tagValue] = m; - }); - this._undecided = { sample: false, retryable: true, tags: null }; - } - - _findTag(tags: Array): ?Tag { - for (let i = 0; i < tags.length; i++) { - if (tags[i].key === this._tagKey) { - return tags[i]; - } - } - return null; - } - - _createOutTags(tagValue: string): { [string]: string } { - return { - 'sampler.type': 'TagEqualsSampler', - 'sampler.param': tagValue, - }; - } - - _decide(tagValue: any): SamplingDecision { - const match: ?Matcher = this._matchers[tagValue]; - if (match) { - return { sample: true, retryable: false, tags: this._createOutTags(match.tagValue) }; - } - return this._undecided; - } - - onCreateSpan(span: Span): SamplingDecision { - const tag: ?Tag = this._findTag(span.getTags()); - if (tag) { - return this._decide(tag.value); - } - return this._undecided; - } - - onSetOperationName(span: Span, operationName: string): SamplingDecision { - return this.onCreateSpan(span); - } - - onSetTag(span: Span, key: string, value: any): SamplingDecision { - if (key === this._tagKey) { - return this._decide(value); - } - return this._undecided; - } - } - - /** - * PrioritySamplerState keeps the state of all underlying samplers, specifically - * whether each of them has previously returned retryable=false, in which case - * those samplers are no longer invoked on future sampling calls. - */ - class PrioritySamplerState { - samplerFired: Array; - - constructor(numDelegateSamplers: number) { - this.samplerFired = Array(numDelegateSamplers); - // cannot use array.fill() in Node 0.10 - for (let i = 0; i < numDelegateSamplers; i++) { - this.samplerFired[i] = false; - } - } - } - - /** - * PrioritySampler contains a list of samplers that it interrogates in order. - * Sampling methods return as soon as one of the samplers returns sample=true. - * The retryable state for each underlying sampler is stored in the extended context - * and once retryable=false is returned by one of the delegates it will never be - * called against. - * - * TODO: since sampling state is shared across all spans of the trace, the execution - * of the sampler should probably only happen on the local-root span. - */ - class PrioritySampler extends BaseSamplerV2 { - _delegates: Array; - - constructor(samplers: Array) { - super('PrioritySampler'); - this._delegates = samplers.map(s => adaptSamplerOrThrow(s)); - } - - _getOrCreateState(span: Span): PrioritySamplerState { - const store = span.context()._samplingState.extendedState(); - const stateKey = this.uniqueName(); - let state: ?PrioritySamplerState = store[stateKey]; - if (!state) { - state = new PrioritySamplerState(this._delegates.length); - store[stateKey] = state; - } - return state; - } - - _trySampling(span: Span, fn: Function): SamplingDecision { - const state = this._getOrCreateState(span); - let retryable = false; - for (let i = 0; i < this._delegates.length; i++) { - if (state.samplerFired[i]) { - continue; - } - const d = fn(this._delegates[i]); - retryable = retryable || d.retryable; - if (!d.retryable) { - state.samplerFired[i] = true; - } - if (d.sample) { - return d; // TODO do we want to alter out tags? - } - } - return { sample: false, retryable: retryable, tags: null }; - } - - onCreateSpan(span: Span): SamplingDecision { - return this._trySampling(span, function(delegate: Sampler): SamplingDecision { - return delegate.onCreateSpan(span); - }); - } - - onSetOperationName(span: Span, operationName: string): SamplingDecision { - return this._trySampling(span, function(delegate: Sampler): SamplingDecision { - return delegate.onSetOperationName(span, operationName); - }); - } - - onSetTag(span: Span, key: string, value: any): SamplingDecision { - return this._trySampling(span, function(delegate: Sampler): SamplingDecision { - return delegate.onSetTag(span, key, value); - }); - } - - close(callback: ?() => void): void { - const countdownCallback = Utils.countdownCallback(this._delegates.length, callback); - this._delegates.forEach(r => r.close(countdownCallback)); - } - } - - describe('with PrioritySampler and TagSampler', () => { - const tagSampler = new TagEqualsSampler('theWho', [{ tagValue: 'Bender' }, { tagValue: 'Leela' }]); - const constSampler = new ConstSampler(false); - const priSampler = new PrioritySampler([tagSampler, constSampler]); - const reporter = new InMemoryReporter(); - const tracer = new Tracer('test-service-name', reporter, priSampler); - - beforeEach(() => {}); - - it('should not sample or finalize new span without tags', () => { - let span = tracer.startSpan('opName'); - assert.isFalse(span._spanContext.isSampled(), 'sampled'); - assert.isFalse(span._spanContext.samplingFinalized, 'finalized'); - }); - - it('should sample and finalize created span with tag', () => { - let span = tracer.startSpan('opName', { tags: { theWho: 'Bender' } }); - assert.isTrue(span._spanContext.isSampled(), 'sampled'); - assert.isTrue(span._spanContext.samplingFinalized, 'finalized'); - }); - - it('should sample and finalize span after setTag', () => { - let span = tracer.startSpan('opName'); - assert.isFalse(span._spanContext.isSampled(), 'sampled'); - assert.isFalse(span._spanContext.samplingFinalized, 'finalized'); - span.setTag('theWho', 'Leela'); - assert.isTrue(span._spanContext.isSampled(), 'sampled'); - assert.isTrue(span._spanContext.samplingFinalized, 'finalized'); - }); - - it('should not sample or finalize span after starting a child span', () => { - let span = tracer.startSpan('opName'); - let span2 = tracer.startSpan('opName2', { childOf: span.context() }); - assert.isFalse(span._spanContext.isSampled(), 'sampled'); - assert.isFalse(span._spanContext.samplingFinalized, 'finalized'); - }); - - it('should not sample or finalize span after serializing context', () => { - let span = tracer.startSpan('opName'); - let carrier = {}; - tracer.inject(span.context(), opentracing.FORMAT_TEXT_MAP, carrier); - assert.isOk(carrier); - assert.isFalse(span._spanContext.isSampled(), 'sampled'); - assert.isFalse(span._spanContext.samplingFinalized, 'finalized'); - }); - }); -}); diff --git a/test/samplers/exp_delayed_sampling.js b/test/samplers/exp_delayed_sampling.js new file mode 100644 index 000000000..b533a3d3b --- /dev/null +++ b/test/samplers/exp_delayed_sampling.js @@ -0,0 +1,74 @@ +// @flow +// Copyright (c) 2019 Uber Technologies, Inc. +// +// 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 +// +// http://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 { assert } from 'chai'; +import * as opentracing from 'opentracing'; +import Span from '../../src/span'; +import Utils from '../../src/util'; +import ConstSampler from '../../src/samplers/const_sampler'; +import TagEqualsSampler from '../../src/samplers/experimental/tag_equals_sampler'; +import PrioritySampler from '../../src/samplers/experimental/priority_sampler'; +import InMemoryReporter from '../../src/reporters/in_memory_reporter'; +import Tracer from '../../src/tracer'; + +describe('delayed sampling', () => { + describe('with PrioritySampler and TagSampler', () => { + const tagSampler = new TagEqualsSampler('theWho', [ + { tagValue: 'Bender', firehose: false }, + { tagValue: 'Leela', firehose: true }, + ]); + const constSampler = new ConstSampler(false); + const priSampler = new PrioritySampler([tagSampler, constSampler]); + const reporter = new InMemoryReporter(); + const tracer = new Tracer('test-service-name', reporter, priSampler); + + beforeEach(() => {}); + + it('should not sample or finalize new span without tags', () => { + let span = tracer.startSpan('opName'); + assert.isFalse(span._spanContext.isSampled(), 'sampled'); + assert.isFalse(span._spanContext.samplingFinalized, 'finalized'); + }); + + it('should sample and finalize created span with tag', () => { + let span = tracer.startSpan('opName', { tags: { theWho: 'Bender' } }); + assert.isTrue(span._spanContext.isSampled(), 'sampled'); + assert.isTrue(span._spanContext.samplingFinalized, 'finalized'); + }); + + it('should sample and finalize span after setTag', () => { + let span = tracer.startSpan('opName'); + assert.isFalse(span._spanContext.isSampled(), 'sampled'); + assert.isFalse(span._spanContext.samplingFinalized, 'finalized'); + span.setTag('theWho', 'Leela'); + assert.isTrue(span._spanContext.isSampled(), 'sampled'); + assert.isTrue(span._spanContext.samplingFinalized, 'finalized'); + }); + + it('should not sample or finalize span after starting a child span', () => { + let span = tracer.startSpan('opName'); + let span2 = tracer.startSpan('opName2', { childOf: span.context() }); + assert.isFalse(span._spanContext.isSampled(), 'sampled'); + assert.isFalse(span._spanContext.samplingFinalized, 'finalized'); + }); + + it('should not sample or finalize span after serializing context', () => { + let span = tracer.startSpan('opName'); + let carrier = {}; + tracer.inject(span.context(), opentracing.FORMAT_TEXT_MAP, carrier); + assert.isOk(carrier); + assert.isFalse(span._spanContext.isSampled(), 'sampled'); + assert.isFalse(span._spanContext.samplingFinalized, 'finalized'); + }); + }); +}); From 6bc8f20c15dbf05cfaa61c7f63ec89489eae1d43 Mon Sep 17 00:00:00 2001 From: Yuri Shkuro Date: Tue, 3 Sep 2019 15:25:53 -0400 Subject: [PATCH 17/28] Support firehose in tag sampler Signed-off-by: Yuri Shkuro --- src/samplers/experimental/tag_equals_sampler.js | 9 ++++++--- test/samplers/exp_delayed_sampling.js | 17 ++++++++++------- 2 files changed, 16 insertions(+), 10 deletions(-) diff --git a/src/samplers/experimental/tag_equals_sampler.js b/src/samplers/experimental/tag_equals_sampler.js index c5ec059e1..ffa998838 100644 --- a/src/samplers/experimental/tag_equals_sampler.js +++ b/src/samplers/experimental/tag_equals_sampler.js @@ -51,9 +51,12 @@ export default class TagEqualsSampler extends BaseSamplerV2 { }; } - _decide(tagValue: any): SamplingDecision { + _decide(span: Span, tagValue: any): SamplingDecision { const match: ?Matcher = this._matchers[tagValue]; if (match) { + if (match.firehose) { + span._spanContext._setFirehose(true); + } return { sample: true, retryable: false, tags: this._createOutTags(match.tagValue) }; } return this._undecided; @@ -62,7 +65,7 @@ export default class TagEqualsSampler extends BaseSamplerV2 { onCreateSpan(span: Span): SamplingDecision { const tag: ?Tag = this._findTag(span.getTags()); if (tag) { - return this._decide(tag.value); + return this._decide(span, tag.value); } return this._undecided; } @@ -73,7 +76,7 @@ export default class TagEqualsSampler extends BaseSamplerV2 { onSetTag(span: Span, key: string, value: any): SamplingDecision { if (key === this._tagKey) { - return this._decide(value); + return this._decide(span, value); } return this._undecided; } diff --git a/test/samplers/exp_delayed_sampling.js b/test/samplers/exp_delayed_sampling.js index b533a3d3b..9b7588a7b 100644 --- a/test/samplers/exp_delayed_sampling.js +++ b/test/samplers/exp_delayed_sampling.js @@ -46,13 +46,16 @@ describe('delayed sampling', () => { assert.isTrue(span._spanContext.samplingFinalized, 'finalized'); }); - it('should sample and finalize span after setTag', () => { - let span = tracer.startSpan('opName'); - assert.isFalse(span._spanContext.isSampled(), 'sampled'); - assert.isFalse(span._spanContext.samplingFinalized, 'finalized'); - span.setTag('theWho', 'Leela'); - assert.isTrue(span._spanContext.isSampled(), 'sampled'); - assert.isTrue(span._spanContext.samplingFinalized, 'finalized'); + [{ who: 'Bender', firehose: false }, { who: 'Leela', firehose: true }].forEach(t => { + it(`should sample and finalize span after setTag "${t.who}" and set firehose=${t.firehose}`, () => { + let span = tracer.startSpan('opName'); + assert.isFalse(span._spanContext.isSampled(), 'sampled'); + assert.isFalse(span._spanContext.samplingFinalized, 'finalized'); + span.setTag('theWho', t.who); + assert.isTrue(span._spanContext.isSampled(), 'sampled'); + assert.isTrue(span._spanContext.samplingFinalized, 'finalized'); + assert.equal(t.firehose, span._spanContext.isFirehose(), 'finalized'); + }); }); it('should not sample or finalize span after starting a child span', () => { From cfe6768edde3fae8162e9b160c774845113cb011 Mon Sep 17 00:00:00 2001 From: Yuri Shkuro Date: Tue, 3 Sep 2019 15:41:37 -0400 Subject: [PATCH 18/28] Fix flow issue Signed-off-by: Yuri Shkuro --- test/samplers/exp_delayed_sampling.js | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/test/samplers/exp_delayed_sampling.js b/test/samplers/exp_delayed_sampling.js index 9b7588a7b..44b43dac3 100644 --- a/test/samplers/exp_delayed_sampling.js +++ b/test/samplers/exp_delayed_sampling.js @@ -47,7 +47,10 @@ describe('delayed sampling', () => { }); [{ who: 'Bender', firehose: false }, { who: 'Leela', firehose: true }].forEach(t => { - it(`should sample and finalize span after setTag "${t.who}" and set firehose=${t.firehose}`, () => { + // have to coerce t.firehose to string, because flow complains about it otherwise. + it(`should sample and finalize span after setTag "${t.who}" and set firehose=${String( + t.firehose + )}`, () => { let span = tracer.startSpan('opName'); assert.isFalse(span._spanContext.isSampled(), 'sampled'); assert.isFalse(span._spanContext.samplingFinalized, 'finalized'); From ae7dc12cf3a3bd8cfe3e636a69424cb4a212a1d3 Mon Sep 17 00:00:00 2001 From: Yuri Shkuro Date: Tue, 3 Sep 2019 16:14:02 -0400 Subject: [PATCH 19/28] Expose loggers Signed-off-by: Yuri Shkuro --- src/index.js | 6 ++++++ src/logger.js | 9 +++++++++ 2 files changed, 15 insertions(+) diff --git a/src/index.js b/src/index.js index 2019d2bc0..ba9abf983 100644 --- a/src/index.js +++ b/src/index.js @@ -28,6 +28,9 @@ import LoggingReporter from './reporters/logging_reporter'; import NoopReporter from './reporters/noop_reporter'; import RemoteReporter from './reporters/remote_reporter'; +import NullLogger from './logger'; +import { ConsoleLogger } from './logger'; + import TextMapCodec from './propagators/text_map_codec'; import ZipkinB3TextMapCodec from './propagators/zipkin_b3_text_map_codec'; @@ -57,6 +60,9 @@ module.exports = { NoopReporter, RemoteReporter, + NullLogger, + ConsoleLogger, + TextMapCodec, ZipkinB3TextMapCodec, diff --git a/src/logger.js b/src/logger.js index edd9607b2..dd1e672dc 100644 --- a/src/logger.js +++ b/src/logger.js @@ -15,3 +15,12 @@ export default class NullLogger { info(msg: string): void {} error(msg: string): void {} } + +export class ConsoleLogger { + info(msg: string): void { + console.log('INFO ' + msg); + } + error(msg: string): void { + console.log('ERROR ' + msg); + } +} From 4cd7ce24ff7472e86e2fecadeb9ec7088e85f1bc Mon Sep 17 00:00:00 2001 From: Yuri Shkuro Date: Tue, 3 Sep 2019 18:11:48 -0400 Subject: [PATCH 20/28] Extend remote sampler in test Signed-off-by: Yuri Shkuro --- .../experimental/tag_equals_sampler.js | 12 ++ src/samplers/remote_sampler.js | 4 +- test/samplers/exp_delayed_sampling.js | 2 - test/samplers/exp_extend_remote_sampler.js | 109 ++++++++++++++++++ 4 files changed, 123 insertions(+), 4 deletions(-) create mode 100644 test/samplers/exp_extend_remote_sampler.js diff --git a/src/samplers/experimental/tag_equals_sampler.js b/src/samplers/experimental/tag_equals_sampler.js index ffa998838..fbd47a242 100644 --- a/src/samplers/experimental/tag_equals_sampler.js +++ b/src/samplers/experimental/tag_equals_sampler.js @@ -35,6 +35,18 @@ export default class TagEqualsSampler extends BaseSamplerV2 { this._undecided = { sample: false, retryable: true, tags: null }; } + static fromStrategy(strategy: any): TagEqualsSampler { + let key = strategy.key; + let matchers: Array = []; + Object.keys(strategy.values).forEach(v => { + matchers.push({ + tagValue: v, + firehose: Boolean(strategy.values[v].firehose), + }); + }); + return new TagEqualsSampler(key, matchers); + } + _findTag(tags: Array): ?Tag { for (let i = 0; i < tags.length; i++) { if (tags[i].key === this._tagKey) { diff --git a/src/samplers/remote_sampler.js b/src/samplers/remote_sampler.js index b24f6a725..c6a4bb66f 100644 --- a/src/samplers/remote_sampler.js +++ b/src/samplers/remote_sampler.js @@ -112,7 +112,7 @@ export default class RemoteControlledSampler implements Sampler { _refreshSamplingStrategy() { let serviceName: string = encodeURIComponent(this._serviceName); const success: Function = body => { - this._parseSamplingServerResponse(body); + this._handleSamplingServerResponse(body); }; const error: Function = err => { this._logger.error(`Error in fetching sampling strategy: ${err}.`); @@ -121,7 +121,7 @@ export default class RemoteControlledSampler implements Sampler { Utils.httpGet(this._host, this._port, `/sampling?service=${serviceName}`, success, error); } - _parseSamplingServerResponse(body: string) { + _handleSamplingServerResponse(body: string) { this._metrics.samplerRetrieved.increment(1); let strategy; try { diff --git a/test/samplers/exp_delayed_sampling.js b/test/samplers/exp_delayed_sampling.js index 44b43dac3..58824d560 100644 --- a/test/samplers/exp_delayed_sampling.js +++ b/test/samplers/exp_delayed_sampling.js @@ -32,8 +32,6 @@ describe('delayed sampling', () => { const reporter = new InMemoryReporter(); const tracer = new Tracer('test-service-name', reporter, priSampler); - beforeEach(() => {}); - it('should not sample or finalize new span without tags', () => { let span = tracer.startSpan('opName'); assert.isFalse(span._spanContext.isSampled(), 'sampled'); diff --git a/test/samplers/exp_extend_remote_sampler.js b/test/samplers/exp_extend_remote_sampler.js new file mode 100644 index 000000000..d540a2ae2 --- /dev/null +++ b/test/samplers/exp_extend_remote_sampler.js @@ -0,0 +1,109 @@ +// @flow +// Copyright (c) 2019 Uber Technologies, Inc. +// +// 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 +// +// http://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 { assert } from 'chai'; +import ConfigServer from '../lib/config_server'; +import InMemoryReporter from '../../src/reporters/in_memory_reporter'; +import LocalBackend from '../lib/metrics/local/backend.js'; +import LocalMetricFactory from '../lib/metrics/local/metric_factory.js'; +import Metrics from '../../src/metrics/metrics.js'; +import MockLogger from '../lib/mock_logger'; +import PrioritySampler from '../../src/samplers/experimental/priority_sampler'; +import RemoteSampler from '../../src/samplers/remote_sampler'; +import Span from '../../src/span'; +import TagEqualsSampler from '../../src/samplers/experimental/tag_equals_sampler'; +import Tracer from '../../src/tracer'; +import Utils from '../../src/util'; + +describe('extended remote sampler', () => { + class ExtendedRemoteSampler extends RemoteSampler { + constructor(serviceName: string, options: any = {}) { + super(serviceName, options); + } + + _updateSampler(strategy: any): boolean { + if (strategy.strategyType == 'extended' && strategy.tagEqualsStrategy) { + let tagSampler = TagEqualsSampler.fromStrategy(strategy.tagEqualsStrategy); + if (this._sampler instanceof PrioritySampler) { + this._sampler = this._sampler._delegates[1]; + } + super._updateSampler(strategy); + this._sampler = new PrioritySampler([tagSampler, this._sampler]); + return true; + } + return super._updateSampler(strategy); + } + } + + let server: ConfigServer; + let logger: MockLogger; + let metrics: Metrics; + let remoteSampler: ExtendedRemoteSampler; + + before(() => { + server = new ConfigServer().start(); + }); + + after(() => { + server.close(); + }); + + beforeEach(() => { + server.clearConfigs(); + logger = new MockLogger(); + metrics = new Metrics(new LocalMetricFactory()); + remoteSampler = new ExtendedRemoteSampler('service1', { + refreshInterval: 0, + metrics: metrics, + logger: logger, + }); + }); + + afterEach(() => { + remoteSampler.close(); + }); + + it('should parse extended strategy response', function(done) { + server.addStrategy('service1', { + strategyType: 'extended', + tagEqualsStrategy: { + key: 'theTag', + values: { + value1: { + firehose: true, + }, + value2: { + firehose: false, + }, + }, + }, + operationSampling: { + defaultLowerBoundTracesPerSecond: 0, + defaultSamplingProbability: 0, + perOperationStrategies: [ + { + operation: 'op1', + probabilisticSampling: { + samplingRate: 0, + }, + }, + ], + }, + }); + remoteSampler._onSamplerUpdate = s => { + assert.instanceOf(s, PrioritySampler); + done(); + }; + remoteSampler._refreshSamplingStrategy(); + }); +}); From 88436f75285d042f32997162d9e3dfdc3714f1dd Mon Sep 17 00:00:00 2001 From: Yuri Shkuro Date: Tue, 3 Sep 2019 18:55:16 -0400 Subject: [PATCH 21/28] Increase code coverage Signed-off-by: Yuri Shkuro --- src/index.js | 6 ---- src/logger.js | 9 ----- test/samplers/per_operation_sampler.js | 7 ++++ test/samplers/remote_sampler.js | 50 ++++++++++++++++++++++++++ 4 files changed, 57 insertions(+), 15 deletions(-) diff --git a/src/index.js b/src/index.js index ba9abf983..2019d2bc0 100644 --- a/src/index.js +++ b/src/index.js @@ -28,9 +28,6 @@ import LoggingReporter from './reporters/logging_reporter'; import NoopReporter from './reporters/noop_reporter'; import RemoteReporter from './reporters/remote_reporter'; -import NullLogger from './logger'; -import { ConsoleLogger } from './logger'; - import TextMapCodec from './propagators/text_map_codec'; import ZipkinB3TextMapCodec from './propagators/zipkin_b3_text_map_codec'; @@ -60,9 +57,6 @@ module.exports = { NoopReporter, RemoteReporter, - NullLogger, - ConsoleLogger, - TextMapCodec, ZipkinB3TextMapCodec, diff --git a/src/logger.js b/src/logger.js index dd1e672dc..edd9607b2 100644 --- a/src/logger.js +++ b/src/logger.js @@ -15,12 +15,3 @@ export default class NullLogger { info(msg: string): void {} error(msg: string): void {} } - -export class ConsoleLogger { - info(msg: string): void { - console.log('INFO ' + msg); - } - error(msg: string): void { - console.log('ERROR ' + msg); - } -} diff --git a/test/samplers/per_operation_sampler.js b/test/samplers/per_operation_sampler.js index d3ea92b3e..4fd9a28f0 100644 --- a/test/samplers/per_operation_sampler.js +++ b/test/samplers/per_operation_sampler.js @@ -168,4 +168,11 @@ describe('PerOperationSampler', () => { assert.isTrue(span._spanContext.isSampled()); assert.isTrue(span._spanContext.samplingFinalized); }); + + it('should have onSetTags', () => { + let sampler = new PerOperationSampler(strategies, 0); + let span: Span = {}; + let decision = sampler.onSetTag(span, 'pi', 3.1415); + assert.deepEqual(decision, { sample: false, retryable: true, tags: null }); + }); }); diff --git a/test/samplers/remote_sampler.js b/test/samplers/remote_sampler.js index 5f6a91307..70f4ae546 100644 --- a/test/samplers/remote_sampler.js +++ b/test/samplers/remote_sampler.js @@ -147,6 +147,22 @@ describe('RemoteSampler', () => { remoteSampler._refreshSamplingStrategy(); }); + it('should reset probabilistic sampler', done => { + remoteSampler._sampler = new RateLimitingSampler(10); + assert.instanceOf(remoteSampler._sampler, RateLimitingSampler); + remoteSampler._onSamplerUpdate = s => { + assert.instanceOf(remoteSampler._sampler, ProbabilisticSampler); + done(); + }; + server.addStrategy('service1', { + strategyType: 'PROBABILISTIC', + probabilisticSampling: { + samplingRate: 1.0, + }, + }); + remoteSampler._refreshSamplingStrategy(); + }); + it('should set per-operation sampler', done => { server.addStrategy('service1', { strategyType: 'PROBABILISTIC', @@ -205,4 +221,38 @@ describe('RemoteSampler', () => { clock.tick(20); }); + + it('should delegate all sampling calls', () => { + const decision: SamplingDecision = { + sample: false, + retryable: true, + tags: null, + fake: 'fake', + }; + const mockSampler: Sampler = { + onCreateSpan: function onCreateSpan(span: Span): SamplingDecision { + this._onCreateSpan = [span]; + return decision; + }, + onSetOperationName: function onSetOperationName(span: Span, operationName: string): SamplingDecision { + this._onSetOperationName = [span, operationName]; + return decision; + }, + onSetTag: function onSetOperationName(span: Span, key: string, value: any): SamplingDecision { + this._onSetTag = [span, key, value]; + return decision; + }, + }; + remoteSampler._sampler = mockSampler; + const span: Span = { fake: 'fake' }; + + assert.deepEqual(decision, remoteSampler.onCreateSpan(span)); + assert.deepEqual([span], mockSampler._onCreateSpan); + + assert.deepEqual(decision, remoteSampler.onSetOperationName(span, 'op1')); + assert.deepEqual([span, 'op1'], mockSampler._onSetOperationName); + + assert.deepEqual(decision, remoteSampler.onSetTag(span, 'pi', 3.1415)); + assert.deepEqual([span, 'pi', 3.1415], mockSampler._onSetTag); + }); }); From 79cdc3d8ceb9992c074ded3cea47e5cf3f74152f Mon Sep 17 00:00:00 2001 From: Yuri Shkuro Date: Tue, 3 Sep 2019 19:19:11 -0400 Subject: [PATCH 22/28] Add more tests Signed-off-by: Yuri Shkuro --- test/samplers/adapt_sampler.js | 8 +++ test/samplers/exp_delayed_sampling.js | 78 -------------------- test/samplers/exp_priority_sampler.js | 100 ++++++++++++++++++++++++++ 3 files changed, 108 insertions(+), 78 deletions(-) delete mode 100644 test/samplers/exp_delayed_sampling.js create mode 100644 test/samplers/exp_priority_sampler.js diff --git a/test/samplers/adapt_sampler.js b/test/samplers/adapt_sampler.js index dbb76dc8f..260f5b403 100644 --- a/test/samplers/adapt_sampler.js +++ b/test/samplers/adapt_sampler.js @@ -18,6 +18,7 @@ import { assert } from 'chai'; // The error seems to be related to a recursive import _adapt_sampler -> Span -> Tracer -> _adapt_sampler. import Tracer from '../../src/tracer'; import * as adapter from '../../src/samplers/_adapt_sampler'; +import LegacySamplerV1Base from '../../src/samplers/_adapt_sampler'; import ConstSampler from '../../src/samplers/const_sampler'; import GuaranteedThroughputSampler from '../../src/samplers/guaranteed_throughput_sampler'; @@ -46,3 +47,10 @@ describe('adaptSamplerOrThrow', () => { assert.throws(() => adapter.adaptSamplerOrThrow(null), Error, 'Unrecognized sampler: null'); }); }); + +describe('LegacySamplerV1Base', () => { + it('should throw in isSampled', () => { + let c = new LegacySamplerV1Base('test'); + assert.throws(() => c.isSampled('op1', {}), Error, 'Subclass must override isSampled()'); + }); +}); diff --git a/test/samplers/exp_delayed_sampling.js b/test/samplers/exp_delayed_sampling.js deleted file mode 100644 index 58824d560..000000000 --- a/test/samplers/exp_delayed_sampling.js +++ /dev/null @@ -1,78 +0,0 @@ -// @flow -// Copyright (c) 2019 Uber Technologies, Inc. -// -// 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 -// -// http://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 { assert } from 'chai'; -import * as opentracing from 'opentracing'; -import Span from '../../src/span'; -import Utils from '../../src/util'; -import ConstSampler from '../../src/samplers/const_sampler'; -import TagEqualsSampler from '../../src/samplers/experimental/tag_equals_sampler'; -import PrioritySampler from '../../src/samplers/experimental/priority_sampler'; -import InMemoryReporter from '../../src/reporters/in_memory_reporter'; -import Tracer from '../../src/tracer'; - -describe('delayed sampling', () => { - describe('with PrioritySampler and TagSampler', () => { - const tagSampler = new TagEqualsSampler('theWho', [ - { tagValue: 'Bender', firehose: false }, - { tagValue: 'Leela', firehose: true }, - ]); - const constSampler = new ConstSampler(false); - const priSampler = new PrioritySampler([tagSampler, constSampler]); - const reporter = new InMemoryReporter(); - const tracer = new Tracer('test-service-name', reporter, priSampler); - - it('should not sample or finalize new span without tags', () => { - let span = tracer.startSpan('opName'); - assert.isFalse(span._spanContext.isSampled(), 'sampled'); - assert.isFalse(span._spanContext.samplingFinalized, 'finalized'); - }); - - it('should sample and finalize created span with tag', () => { - let span = tracer.startSpan('opName', { tags: { theWho: 'Bender' } }); - assert.isTrue(span._spanContext.isSampled(), 'sampled'); - assert.isTrue(span._spanContext.samplingFinalized, 'finalized'); - }); - - [{ who: 'Bender', firehose: false }, { who: 'Leela', firehose: true }].forEach(t => { - // have to coerce t.firehose to string, because flow complains about it otherwise. - it(`should sample and finalize span after setTag "${t.who}" and set firehose=${String( - t.firehose - )}`, () => { - let span = tracer.startSpan('opName'); - assert.isFalse(span._spanContext.isSampled(), 'sampled'); - assert.isFalse(span._spanContext.samplingFinalized, 'finalized'); - span.setTag('theWho', t.who); - assert.isTrue(span._spanContext.isSampled(), 'sampled'); - assert.isTrue(span._spanContext.samplingFinalized, 'finalized'); - assert.equal(t.firehose, span._spanContext.isFirehose(), 'finalized'); - }); - }); - - it('should not sample or finalize span after starting a child span', () => { - let span = tracer.startSpan('opName'); - let span2 = tracer.startSpan('opName2', { childOf: span.context() }); - assert.isFalse(span._spanContext.isSampled(), 'sampled'); - assert.isFalse(span._spanContext.samplingFinalized, 'finalized'); - }); - - it('should not sample or finalize span after serializing context', () => { - let span = tracer.startSpan('opName'); - let carrier = {}; - tracer.inject(span.context(), opentracing.FORMAT_TEXT_MAP, carrier); - assert.isOk(carrier); - assert.isFalse(span._spanContext.isSampled(), 'sampled'); - assert.isFalse(span._spanContext.samplingFinalized, 'finalized'); - }); - }); -}); diff --git a/test/samplers/exp_priority_sampler.js b/test/samplers/exp_priority_sampler.js new file mode 100644 index 000000000..d71719681 --- /dev/null +++ b/test/samplers/exp_priority_sampler.js @@ -0,0 +1,100 @@ +// @flow +// Copyright (c) 2019 Uber Technologies, Inc. +// +// 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 +// +// http://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 { assert } from 'chai'; +import sinon from 'sinon'; +import * as opentracing from 'opentracing'; +import Span from '../../src/span'; +import Utils from '../../src/util'; +import ConstSampler from '../../src/samplers/const_sampler'; +import TagEqualsSampler from '../../src/samplers/experimental/tag_equals_sampler'; +import PrioritySampler from '../../src/samplers/experimental/priority_sampler'; +import InMemoryReporter from '../../src/reporters/in_memory_reporter'; +import Tracer from '../../src/tracer'; +import BaseSamplerV2 from '../../src/samplers/v2/base'; + +describe('PrioritySampler with TagSampler', () => { + const tagSampler = new TagEqualsSampler('theWho', [ + { tagValue: 'Bender', firehose: false }, + { tagValue: 'Leela', firehose: true }, + ]); + const constSampler = new ConstSampler(false); + const priSampler = new PrioritySampler([tagSampler, constSampler]); + const reporter = new InMemoryReporter(); + const tracer = new Tracer('test-service-name', reporter, priSampler); + + it('should not sample or finalize new span without tags and after setOperation', () => { + let span = tracer.startSpan('opName'); + assert.isFalse(span._spanContext.isSampled(), 'sampled'); + assert.isFalse(span._spanContext.samplingFinalized, 'finalized'); + span.setOperationName('opName2'); + assert.isFalse(span._spanContext.isSampled(), 'sampled'); + assert.isFalse(span._spanContext.samplingFinalized, 'finalized'); + }); + + it('should sample and finalize created span with tag', () => { + let span = tracer.startSpan('opName', { tags: { theWho: 'Bender' } }); + assert.isTrue(span._spanContext.isSampled(), 'sampled'); + assert.isTrue(span._spanContext.samplingFinalized, 'finalized'); + }); + + [{ who: 'Bender', firehose: false }, { who: 'Leela', firehose: true }].forEach(t => { + // have to coerce t.firehose to string, because flow complains about it otherwise. + it(`should sample and finalize span after setTag "${t.who}" and set firehose=${String( + t.firehose + )}`, () => { + let span = tracer.startSpan('opName'); + assert.isFalse(span._spanContext.isSampled(), 'sampled'); + assert.isFalse(span._spanContext.samplingFinalized, 'finalized'); + span.setTag('theWho', t.who); + assert.isTrue(span._spanContext.isSampled(), 'sampled'); + assert.isTrue(span._spanContext.samplingFinalized, 'finalized'); + assert.equal(t.firehose, span._spanContext.isFirehose(), 'finalized'); + }); + }); + + it('should not sample or finalize span after starting a child span', () => { + let span = tracer.startSpan('opName'); + let span2 = tracer.startSpan('opName2', { childOf: span.context() }); + assert.isFalse(span._spanContext.isSampled(), 'sampled'); + assert.isFalse(span._spanContext.samplingFinalized, 'finalized'); + }); + + it('should not sample or finalize span after serializing context', () => { + let span = tracer.startSpan('opName'); + let carrier = {}; + tracer.inject(span.context(), opentracing.FORMAT_TEXT_MAP, carrier); + assert.isOk(carrier); + assert.isFalse(span._spanContext.isSampled(), 'sampled'); + assert.isFalse(span._spanContext.samplingFinalized, 'finalized'); + }); + + it('should delegate calls to close() and invoke a callback', () => { + let s1: Sampler = new BaseSamplerV2('s1'); + let s2: Sampler = new BaseSamplerV2('s2'); + s1.close = c => { + s1._closed = true; + c(); + }; + s2.close = c => { + s2._closed = true; + c(); + }; + let callback = sinon.spy(); + let priSampler = new PrioritySampler([s1, s2]); + priSampler.close(callback); + assert.isTrue(s1._closed); + assert.isTrue(s1._closed); + assert.isTrue(callback.calledOnce); + }); +}); From 352dbcc08ce329f905cf42686f25bd20dcd1ade0 Mon Sep 17 00:00:00 2001 From: Yuri Shkuro Date: Tue, 3 Sep 2019 19:20:03 -0400 Subject: [PATCH 23/28] Fix flow error Signed-off-by: Yuri Shkuro --- test/samplers/exp_priority_sampler.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/samplers/exp_priority_sampler.js b/test/samplers/exp_priority_sampler.js index d71719681..d7f4728db 100644 --- a/test/samplers/exp_priority_sampler.js +++ b/test/samplers/exp_priority_sampler.js @@ -80,8 +80,8 @@ describe('PrioritySampler with TagSampler', () => { }); it('should delegate calls to close() and invoke a callback', () => { - let s1: Sampler = new BaseSamplerV2('s1'); - let s2: Sampler = new BaseSamplerV2('s2'); + let s1: any = new BaseSamplerV2('s1'); + let s2: any = new BaseSamplerV2('s2'); s1.close = c => { s1._closed = true; c(); From 6241aae0a817eaac766383383877457a34ae7566 Mon Sep 17 00:00:00 2001 From: Yuri Shkuro Date: Tue, 3 Sep 2019 22:12:45 -0400 Subject: [PATCH 24/28] Add tests Signed-off-by: Yuri Shkuro --- test/samplers/adapt_sampler.js | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/test/samplers/adapt_sampler.js b/test/samplers/adapt_sampler.js index 260f5b403..0522160b7 100644 --- a/test/samplers/adapt_sampler.js +++ b/test/samplers/adapt_sampler.js @@ -40,6 +40,21 @@ describe('adaptSampler', () => { let s2: any = adapter.adaptSampler(s1); assert.equal(s1.toString(), s2.toString()); }); + it('should delegate sampling methods to isSampled', () => { + let s1: any = new ConstSampler(true); + s1.apiVersion = ''; // break V2 compatibility + let s2: any = adapter.adaptSampler(s1); + assert.deepEqual(s1, s2._delegate); + s1._called = 0; + s1.isSampled = (operationName: string, outTags: {}) => { + s1._called++; + }; + let span: Span = {}; + s1.onCreateSpan(span); + s1.onSetOperationName(span, 'op1'); + s1.onSetTag(span, 'pi', 3.1415); + assert.equal(2, s1._called); + }); }); describe('adaptSamplerOrThrow', () => { From 31cdc22fc27e0af587b003558b0bdcaf6ecf3786 Mon Sep 17 00:00:00 2001 From: Yuri Shkuro Date: Tue, 3 Sep 2019 22:34:23 -0400 Subject: [PATCH 25/28] More tests Signed-off-by: Yuri Shkuro --- .../experimental/tag_equals_sampler.js | 20 ++++++++++- test/samplers/adapt_sampler.js | 36 ++++++++++++++----- 2 files changed, 46 insertions(+), 10 deletions(-) diff --git a/src/samplers/experimental/tag_equals_sampler.js b/src/samplers/experimental/tag_equals_sampler.js index fbd47a242..c7312517b 100644 --- a/src/samplers/experimental/tag_equals_sampler.js +++ b/src/samplers/experimental/tag_equals_sampler.js @@ -35,6 +35,23 @@ export default class TagEqualsSampler extends BaseSamplerV2 { this._undecided = { sample: false, retryable: true, tags: null }; } + /** + * Creates the sampler from a JSON configuration of the following form: + * + * { + * key: 'taKey', + * values: { + * 'tagValue1': { + * firehose: true, + * }, + * 'tagValue1: { + * firehose: false, + * }, + * }, + * } + * + * @param {JSON} strategy + */ static fromStrategy(strategy: any): TagEqualsSampler { let key = strategy.key; let matchers: Array = []; @@ -83,7 +100,8 @@ export default class TagEqualsSampler extends BaseSamplerV2 { } onSetOperationName(span: Span, operationName: string): SamplingDecision { - return this.onCreateSpan(span); + // this sampler is not sensitive to operationName, so no reason to re-evaluate the tags. + return this._undecided; } onSetTag(span: Span, key: string, value: any): SamplingDecision { diff --git a/test/samplers/adapt_sampler.js b/test/samplers/adapt_sampler.js index 0522160b7..848161843 100644 --- a/test/samplers/adapt_sampler.js +++ b/test/samplers/adapt_sampler.js @@ -12,6 +12,7 @@ // the License. import { assert } from 'chai'; +import sinon from 'sinon'; // Import Tracer here to work around a weird bug that causes the error like this: // TypeError: Super expression must either be null or a function, not undefined // at _inherits (.../jaeger-client-node/src/samplers/const_sampler.js:27:113) @@ -26,6 +27,9 @@ describe('adaptSampler', () => { it('should return null for null argument', () => { assert.isNull(adapter.adaptSampler(null)); }); + it('should return null for malformed argument', () => { + assert.isNull(adapter.adaptSampler({ fake: 'fake', apiVersion: 'v1' })); + }); it('should return wrapper for v1 sampler', () => { let s1 = new GuaranteedThroughputSampler(0, 1.0); let s2: any = adapter.adaptSampler(s1); @@ -40,6 +44,15 @@ describe('adaptSampler', () => { let s2: any = adapter.adaptSampler(s1); assert.equal(s1.toString(), s2.toString()); }); +}); + +describe('adaptSamplerOrThrow', () => { + it('should throw on unrecognized sampler', () => { + assert.throws(() => adapter.adaptSamplerOrThrow(null), Error, 'Unrecognized sampler: null'); + }); +}); + +describe('LegacySamplerV1Adapter', () => { it('should delegate sampling methods to isSampled', () => { let s1: any = new ConstSampler(true); s1.apiVersion = ''; // break V2 compatibility @@ -50,16 +63,21 @@ describe('adaptSampler', () => { s1._called++; }; let span: Span = {}; - s1.onCreateSpan(span); - s1.onSetOperationName(span, 'op1'); - s1.onSetTag(span, 'pi', 3.1415); - assert.equal(2, s1._called); + s2.onCreateSpan(span); + s2.onSetOperationName(span, 'op1'); + s2.onSetTag(span, 'pi', 3.1415); // this one is no-op, so does not increment the counter + s2.isSampled('op1', {}); + assert.equal(3, s1._called); }); -}); - -describe('adaptSamplerOrThrow', () => { - it('should throw on unrecognized sampler', () => { - assert.throws(() => adapter.adaptSamplerOrThrow(null), Error, 'Unrecognized sampler: null'); + it('should delegate close()', () => { + let s1: any = new ConstSampler(true); + s1.apiVersion = ''; // break V2 compatibility + let s2: any = adapter.adaptSampler(s1); + assert.deepEqual(s1, s2._delegate); + let span: Span = {}; + let callback = sinon.spy(); + s2.close(callback); + assert.isTrue(callback.called); }); }); From e10dbdc3dec2c44cf7cdb26088af87f14fea42a7 Mon Sep 17 00:00:00 2001 From: Yuri Shkuro Date: Tue, 3 Sep 2019 23:11:54 -0400 Subject: [PATCH 26/28] More tests Signed-off-by: Yuri Shkuro --- .../experimental/tag_equals_sampler.js | 16 +--- test/samplers/exp_priority_sampler.js | 2 +- test/samplers/exp_tag_sampler.js | 73 +++++++++++++++++++ 3 files changed, 76 insertions(+), 15 deletions(-) create mode 100644 test/samplers/exp_tag_sampler.js diff --git a/src/samplers/experimental/tag_equals_sampler.js b/src/samplers/experimental/tag_equals_sampler.js index c7312517b..893a40b35 100644 --- a/src/samplers/experimental/tag_equals_sampler.js +++ b/src/samplers/experimental/tag_equals_sampler.js @@ -64,15 +64,6 @@ export default class TagEqualsSampler extends BaseSamplerV2 { return new TagEqualsSampler(key, matchers); } - _findTag(tags: Array): ?Tag { - for (let i = 0; i < tags.length; i++) { - if (tags[i].key === this._tagKey) { - return tags[i]; - } - } - return null; - } - _createOutTags(tagValue: string): { [string]: string } { return { 'sampler.type': 'TagEqualsSampler', @@ -92,15 +83,12 @@ export default class TagEqualsSampler extends BaseSamplerV2 { } onCreateSpan(span: Span): SamplingDecision { - const tag: ?Tag = this._findTag(span.getTags()); - if (tag) { - return this._decide(span, tag.value); - } + // onCreateSpan is called on a brand new span that has no tags yet, so nothing to do here. return this._undecided; } onSetOperationName(span: Span, operationName: string): SamplingDecision { - // this sampler is not sensitive to operationName, so no reason to re-evaluate the tags. + // this sampler is not sensitive to operationName, so nothing to do here. return this._undecided; } diff --git a/test/samplers/exp_priority_sampler.js b/test/samplers/exp_priority_sampler.js index d7f4728db..695961899 100644 --- a/test/samplers/exp_priority_sampler.js +++ b/test/samplers/exp_priority_sampler.js @@ -34,7 +34,7 @@ describe('PrioritySampler with TagSampler', () => { const tracer = new Tracer('test-service-name', reporter, priSampler); it('should not sample or finalize new span without tags and after setOperation', () => { - let span = tracer.startSpan('opName'); + let span = tracer.startSpan('opName', { tags: { theWho: 'Fry' } }); // NB: wrong tag value used assert.isFalse(span._spanContext.isSampled(), 'sampled'); assert.isFalse(span._spanContext.samplingFinalized, 'finalized'); span.setOperationName('opName2'); diff --git a/test/samplers/exp_tag_sampler.js b/test/samplers/exp_tag_sampler.js new file mode 100644 index 000000000..684d82ddf --- /dev/null +++ b/test/samplers/exp_tag_sampler.js @@ -0,0 +1,73 @@ +// @flow +// Copyright (c) 2019 Uber Technologies, Inc. +// +// 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 +// +// http://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 { assert } from 'chai'; +import sinon from 'sinon'; +import * as opentracing from 'opentracing'; +import Span from '../../src/span'; +import Utils from '../../src/util'; +import TagEqualsSampler from '../../src/samplers/experimental/tag_equals_sampler'; +import InMemoryReporter from '../../src/reporters/in_memory_reporter'; +import Tracer from '../../src/tracer'; + +describe('TagSampler', () => { + const tagSampler = new TagEqualsSampler('theWho', [ + { tagValue: 'Bender', firehose: false }, + { tagValue: 'Leela', firehose: true }, + ]); + const reporter = new InMemoryReporter(); + const tracer = new Tracer('test-service-name', reporter, tagSampler); + + it('should not sample or finalize new span without tags and after setOperation', () => { + let span = tracer.startSpan('opName', { tags: { theWho: 'Fry' } }); // wrong tag value + assert.isFalse(span._spanContext.isSampled(), 'sampled'); + assert.isFalse(span._spanContext.samplingFinalized, 'finalized'); + span.setOperationName('opName2'); + assert.isFalse(span._spanContext.isSampled(), 'sampled'); + assert.isFalse(span._spanContext.samplingFinalized, 'finalized'); + }); + + it('should sample and finalize created span with tag', () => { + let span = tracer.startSpan('opName', { tags: { theWho: 'Bender' } }); + console.log('span has tags'); + console.log(span.getTags()); + assert.isTrue(span._spanContext.isSampled(), 'sampled'); + assert.isTrue(span._spanContext.samplingFinalized, 'finalized'); + }); + + [{ who: 'Bender', firehose: false }, { who: 'Leela', firehose: true }].forEach(t => { + // have to coerce t.firehose to string, because flow complains about it otherwise. + it(`should sample and finalize span after setTag "${t.who}" and set firehose=${String( + t.firehose + )}`, () => { + let span = tracer.startSpan('opName'); + assert.isFalse(span._spanContext.isSampled(), 'sampled'); + assert.isFalse(span._spanContext.samplingFinalized, 'finalized'); + span.setTag('theTag', 'Fry'); // wrong tag key + span.setTag('theWho', 'Fry'); // wrong tag value + assert.isFalse(span._spanContext.isSampled(), 'sampled'); + assert.isFalse(span._spanContext.samplingFinalized, 'finalized'); + span.setTag('theWho', t.who); + assert.isTrue(span._spanContext.isSampled(), 'sampled'); + assert.isTrue(span._spanContext.samplingFinalized, 'finalized'); + assert.equal(t.firehose, span._spanContext.isFirehose(), 'finalized'); + }); + }); + + it('should not sample or finalize span after starting a child span', () => { + let span = tracer.startSpan('opName'); + let span2 = tracer.startSpan('opName2', { childOf: span.context() }); + assert.isFalse(span._spanContext.isSampled(), 'sampled'); + assert.isFalse(span._spanContext.samplingFinalized, 'finalized'); + }); +}); From 465599c9cc9e02c5b494800d16adb0c315875bdb Mon Sep 17 00:00:00 2001 From: Yuri Shkuro Date: Wed, 4 Sep 2019 15:22:40 -0400 Subject: [PATCH 27/28] Use 'classicStrategy' property Signed-off-by: Yuri Shkuro --- test/samplers/exp_extend_remote_sampler.js | 29 +++++++++++----------- 1 file changed, 15 insertions(+), 14 deletions(-) diff --git a/test/samplers/exp_extend_remote_sampler.js b/test/samplers/exp_extend_remote_sampler.js index d540a2ae2..2552a5205 100644 --- a/test/samplers/exp_extend_remote_sampler.js +++ b/test/samplers/exp_extend_remote_sampler.js @@ -32,16 +32,16 @@ describe('extended remote sampler', () => { } _updateSampler(strategy: any): boolean { - if (strategy.strategyType == 'extended' && strategy.tagEqualsStrategy) { + if (strategy.tagEqualsStrategy) { let tagSampler = TagEqualsSampler.fromStrategy(strategy.tagEqualsStrategy); if (this._sampler instanceof PrioritySampler) { this._sampler = this._sampler._delegates[1]; } - super._updateSampler(strategy); + super._updateSampler(strategy.classicStrategy); this._sampler = new PrioritySampler([tagSampler, this._sampler]); return true; } - return super._updateSampler(strategy); + return super._updateSampler(strategy.classicStrategy); } } @@ -75,7 +75,6 @@ describe('extended remote sampler', () => { it('should parse extended strategy response', function(done) { server.addStrategy('service1', { - strategyType: 'extended', tagEqualsStrategy: { key: 'theTag', values: { @@ -87,17 +86,19 @@ describe('extended remote sampler', () => { }, }, }, - operationSampling: { - defaultLowerBoundTracesPerSecond: 0, - defaultSamplingProbability: 0, - perOperationStrategies: [ - { - operation: 'op1', - probabilisticSampling: { - samplingRate: 0, + classicStrategy: { + operationSampling: { + defaultLowerBoundTracesPerSecond: 0, + defaultSamplingProbability: 0, + perOperationStrategies: [ + { + operation: 'op1', + probabilisticSampling: { + samplingRate: 0, + }, }, - }, - ], + ], + }, }, }); remoteSampler._onSamplerUpdate = s => { From 0454958f98acee848b4d4c2754b50a330b291f0f Mon Sep 17 00:00:00 2001 From: Yuri Shkuro Date: Wed, 4 Sep 2019 16:05:11 -0400 Subject: [PATCH 28/28] Fix flow error Signed-off-by: Yuri Shkuro --- test/samplers/exp_extend_remote_sampler.js | 1 + 1 file changed, 1 insertion(+) diff --git a/test/samplers/exp_extend_remote_sampler.js b/test/samplers/exp_extend_remote_sampler.js index 2552a5205..79d403536 100644 --- a/test/samplers/exp_extend_remote_sampler.js +++ b/test/samplers/exp_extend_remote_sampler.js @@ -75,6 +75,7 @@ describe('extended remote sampler', () => { it('should parse extended strategy response', function(done) { server.addStrategy('service1', { + strategyType: '', // this is needed yo satisfy server.addStrategy type tagEqualsStrategy: { key: 'theTag', values: {