Skip to content
This repository has been archived by the owner on Sep 13, 2022. It is now read-only.

All spans of a trace share sampling state #377

Merged
merged 33 commits into from
Aug 29, 2019
Merged
Show file tree
Hide file tree
Changes from 19 commits
Commits
Show all changes
33 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions .vscode/launch.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,17 @@
{
"version": "0.2.0",
"configurations": [
{
"type": "node",
"request": "launch",
"name": "Mocha Current File",
"program": "${workspaceFolder}/node_modules/mocha/bin/_mocha",
"args": ["--timeout", "999999", "--compilers", "js:babel-core/register", "--colors", "${file}"],
"console": "integratedTerminal",
"internalConsoleOptions": "neverOpen",
// "runtimeVersion": "10",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: invalid JSON

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's fine, actually. It's not a real JSON file. If you open VSCode settings as JSON, it has other comments, like

// Place your settings in this file to overwrite the default settings

"sourceMaps": true
},
{
"name": "Debug Mocha tests",
"type": "node",
Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@
"precommit": "lint-staged",
"test": "make test",
"test-dist": "mocha dist/test/ dist/test/baggage/ dist/test/throttler/ dist/test/samplers/ dist/test/metrics/",
"test-all": "npm run test-core && npm run test-samplers && npm run test-crossdock && npm run test-baggage && npm run test-throttler && npm run test-prom-metrics",
"test-all": "npm run test-core && npm run test-samplers && npm run test-baggage && npm run test-throttler && npm run test-prom-metrics && npm run test-crossdock",
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

moved crossdock test in the last position

"test-core": "mocha --compilers js:babel-core/register test",
"test-samplers": "mocha --compilers js:babel-core/register test/samplers",
"test-baggage": "mocha --compilers js:babel-core/register test/baggage",
Expand Down
34 changes: 34 additions & 0 deletions src/_flow/sampler.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
// @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 Span from '../span';
import type { SamplerApiVersion } from '../samplers/constants';

/**
* Sampler methods return SamplingDecision struct.
*/
declare type SamplingDecision = {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps we should call this SamplingResult because it contains information in addition to the decision (0/1) ?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think "decision" is a more accurate term. It does not mean that it's a primitive yes/no value (cf. Supreme Court decision - lots of additional details), yet it's a decision that is not yet implemented, whereas "result" would be the consequence of implementing that decision.

sample: boolean,
retryable: boolean,
tags: ?{},
};

declare interface Sampler {
apiVersion: SamplerApiVersion;

onCreateSpan(span: Span): SamplingDecision;
onSetOperationName(span: Span, operationName: string): SamplingDecision;
onSetTag(span: Span, key: string, value: any): SamplingDecision;

close(callback: ?Function): void;
}
2 changes: 1 addition & 1 deletion src/configuration.js
Original file line number Diff line number Diff line change
Expand Up @@ -224,7 +224,7 @@ export default class Configuration {
}

if (options.logger) {
options.logger.info(`Initializing Jaeger Tracer with ${reporter.name()} and ${sampler.name()}`);
options.logger.info(`Initializing Jaeger Tracer with ${reporter} and ${sampler}`);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

don't need to rely on name() function, toString should work.

}

return new Tracer(config.serviceName, reporter, sampler, {
Expand Down
77 changes: 77 additions & 0 deletions src/samplers/_adapt_sampler.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
// @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 { SAMPLER_API_V2 } from './constants';
import Span from '../span';
import BaseSamplerV2 from './v2/base';

function adaptSampler(sampler: any): ?Sampler {
if (!sampler) {
return null;
}
if (sampler.apiVersion === SAMPLER_API_V2) {
// already v2 API compatible
return sampler;
}
if (!sampler.apiVersion) {
// v1 legacy sampler
return new LegacySamplerV1Adapter(sampler);
}
return null;
}

export function adaptSamplerOrThrow(sampler: any): Sampler {
const s = adaptSampler(sampler);
if (!s) {
throw new Error(`Unrecognized sampler: ${sampler}`);
}
return s;
}

export default adaptSampler;

/**
* Transforms legacy v1 sampler into V2.
* Primarily intended for simple samplers that are not sensitive to
* things like operation names or tags and make a decision only once.
* As such, this always returns retryable=false.
*/
class LegacySamplerV1Adapter extends BaseSamplerV2 {
_delegate: LegacySamplerV1;

constructor(wrapped: LegacySamplerV1) {
super(`SamplerV1Adapter(${wrapped.name()})`);
this._delegate = wrapped;
}

onCreateSpan(span: Span): SamplingDecision {
const outTags = {};
const isSampled = this._delegate.isSampled(span.operationName, outTags);
// TODO not sure if retryable: false is correct here; depends on the sampler
return { sample: isSampled, retryable: true, 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 };
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think a comment with some reasoning would be useful when we look at this later

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have them in the class comment.

}

close(callback: ?Function) {
this._delegate.close(callback);
}
}
16 changes: 16 additions & 0 deletions src/samplers/constants.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
// @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.

export const SAMPLER_API_V2 = 'SAMPLER_API_V2';

export type SamplerApiVersion = typeof SAMPLER_API_V2;
60 changes: 60 additions & 0 deletions src/samplers/v2/base.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
// @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 { SAMPLER_API_V2 } from '../constants.js';
import Span from '../../span';

let _instanceId = 0;

export default class BaseSamplerV2 implements Sampler {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

at this point it's not used, until we resolve the babel issue with inheritance. I could remove it, it will still be in the PR history.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should remove it if we are not using it

apiVersion = SAMPLER_API_V2;
_name: string;
_uniqueName: string;
_cachedDecision: SamplingDecision;

constructor(name: string) {
this._name = name;
this._uniqueName = BaseSamplerV2._getInstanceId(name);
this._cachedDecision = { sample: false, retryable: false, tags: null };
}

static _getInstanceId(name: string) {
return `${name}[${_instanceId++}]`;
}

name() {
return this._name;
}

uniqueName() {
return this._uniqueName;
}

onCreateSpan(span: Span): SamplingDecision {
throw new Error(`${this.name()} does not implement onCreateSpan`);
}

onSetOperationName(span: Span, operationName: string): SamplingDecision {
return this._cachedDecision;
}

onSetTag(span: Span, key: string, value: any): SamplingDecision {
return this._cachedDecision;
}

close(callback: ?Function): void {
if (callback) {
callback();
}
}
}
48 changes: 48 additions & 0 deletions src/samplers/v2/const_sampler.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
// @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 * as constants from '../../constants.js';
import Span from '../../span';
import BaseSamplerV2 from './base';

export default class ConstSamplerV2 extends BaseSamplerV2 {
_cachedDecision: SamplingDecision;

constructor(decision: boolean) {
super('ConstSampler');
const tags = {};
tags[constants.SAMPLER_TYPE_TAG_KEY] = constants.SAMPLER_TYPE_CONST;
tags[constants.SAMPLER_PARAM_TAG_KEY] = Boolean(decision);
this._cachedDecision = {
sample: Boolean(decision),
retryable: false,
tags: tags,
};
}

toString() {
return `${this.name()}(version=2, ${this._cachedDecision ? 'always' : 'never'})`;
}

get decision() {
return this._cachedDecision.sample;
}

onCreateSpan(span: Span): SamplingDecision {
return this._cachedDecision;
}

onSetOperationName(span: Span, operationName: string): SamplingDecision {
return this._cachedDecision;
}
}
112 changes: 112 additions & 0 deletions src/samplers/v2/sampling_state.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,112 @@
// @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 { DEBUG_MASK, FIREHOSE_MASK, SAMPLED_MASK } from '../../constants';
import SpanContext from '../../span_context';

export default class SamplingState {
// samplers may store their individual states in this map
_extendedState: { [string]: any } = {};

// shared Flags from span context
_flags: number = 0;

/**
* When state is not final, sampling will be retried on other write operations,
* and spans will remain writable.
*
* This field exists to help distinguish between when a span can have a properly
* correlated operation name -> sampling rate mapping, and when it cannot.
* Adaptive sampling uses the operation name of a span to correlate it with
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should the part about adaptive sampling live elsewhere?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it will, remember this is a change that aims to keep status quo.

* a sampling rate. If an operation name is set on a span after the span's creation
* then adaptive sampling cannot associate the operation name with the proper sampling rate.
* In order to correct this we allow a span to be written to, so that we can re-sample
* it in the case that an operation name is set after span creation. Situations
* where a span context's sampling decision is finalized include:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are all of these situations still accurate?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

as of this diff, yes

* - it has inherited the sampling decision from its parent
* - its debug flag is set via the sampling.priority tag
* - it is finish()-ed
* - setOperationName is called
* - it is used as a parent for another span
* - its context is serialized using injectors
*/
_final: boolean = false;

// Span ID of the local root span, i.e. the first span in this process for this trace.
_localRootSpanIdStr: ?string;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we rename this to firstInProcessSpanIdStr? (go client has a var called firstInProcess...)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think 'local root' is a good name, and I've seen it used in other places (zipkin, OC).


constructor(localRootSpanIdStr: ?string) {
this._localRootSpanIdStr = localRootSpanIdStr;
}

// checks if another span context has the same Span ID as the local root span
isLocalRootSpan(context: SpanContext) {
return this._localRootSpanIdStr === context.spanIdStr;
}

localRootSpanId(): ?string {
return this._localRootSpanIdStr;
}

extendedState() {
return this._extendedState;
}

isFinal() {
return this._final;
}

setFinal(value: boolean) {
this._final = value;
}

isSampled() {
return Boolean(this._flags & SAMPLED_MASK);
}

setSampled(enable: boolean) {
this._toggleFlag(SAMPLED_MASK, enable);
}

isDebug() {
return Boolean(this._flags & DEBUG_MASK);
}

setDebug(enable: boolean) {
this._toggleFlag(DEBUG_MASK, enable);
}

isFirehose() {
return Boolean(this._flags & FIREHOSE_MASK);
}

setFirehose(enable: boolean) {
this._toggleFlag(FIREHOSE_MASK, enable);
}

flags() {
return this._flags;
}

setFlags(flags: number) {
this._flags = flags;
}

_toggleFlag(mask: number, enable: boolean) {
if (enable) {
this._flags |= mask;
} else {
this._flags &= ~mask;
}
}
}
Loading