Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Replace ALWAYS_PARENT with a composite ParentOrElse sampler #609

Merged
merged 4 commits into from
May 18, 2020
Merged

Replace ALWAYS_PARENT with a composite ParentOrElse sampler #609

Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
f3bcd45
Split ALWAYS_PARENT based on default behavior if no parent
bogdandrutu May 16, 2020
59626b5
Rename RootOnly to ParentOrElse
bogdandrutu May 18, 2020
7673214
Address feedback
bogdandrutu May 18, 2020
f865514
More feedback
bogdandrutu May 18, 2020
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
49 changes: 32 additions & 17 deletions specification/trace/sdk.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,29 +101,44 @@ Description MUST NOT change over time and caller can cache the returned value.

### Built-in samplers

These are the default samplers implemented in the OpenTelemetry SDK:

* ALWAYS_ON
* This will be used as a default.
* Description MUST be `AlwaysOnSampler`.
* ALWAYS_OFF
* Description MUST be `AlwaysOffSampler`.
* ALWAYS_PARENT
* `Returns RECORD_AND_SAMPLED` if `SampledFlag` is set to true on parent
SpanContext and `NOT_RECORD` otherwise.
* Description MUST be `AlwaysParentSampler`.
* Probability
* The default behavior should be to trust the parent `SampledFlag`. However
#### AlwaysOn

* This will be used as a default.
bogdandrutu marked this conversation as resolved.
Show resolved Hide resolved
* Returns `RECORD_AND_SAMPLED` always.
* Description MUST be `AlwaysOnSampler`.

#### AlwaysOff

* Returns `NOT_RECORD` always.
* Description MUST be `AlwaysOffSampler`.

#### Probability

* The default behavior should be to trust the parent `SampledFlag`. However
Copy link
Member

Choose a reason for hiding this comment

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

Should this have the "always" behavior instead of trusting parent, to be consistent? If trusting parent is needed, use Sampers.parentOrElse(Samplers.probabilistic(r)).

Copy link
Member

Choose a reason for hiding this comment

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

I see you have added TODO, so will be in the separate PR?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes.

there should be configuration to change this.
* The default behavior is to apply the sampling probability only for Spans
* The default behavior is to apply the sampling probability only for Spans
that are root spans (no parent) and Spans with remote parent. However there
should be configuration to change this to "root spans only", or "all spans".
* Description MUST be `ProbabilitySampler{0.000100}`.

#### Probability Sampler algorithm
* Description MUST be `ProbabilitySampler{0.000100}`.

TODO: Add details about how the probability sampler is implemented as a function
bogdandrutu marked this conversation as resolved.
Show resolved Hide resolved
of the `TraceID`.
TODO: Split out the parent handling.

#### ParentOrElse

* This is a composite sampler. `ParentOrElse(delegateSampler)` delegates to `delegateSampler` for root spans and otherwise respects the parent sampling decision.
bogdandrutu marked this conversation as resolved.
Show resolved Hide resolved
* If parent exists:
* If parent's `SampledFlag` is set to `true` returns `RECORD_AND_SAMPLED`
* If parent's `SampledFlag` is set to `false` returns `NOT_RECORD`
* If no parent (root span) exists returns the result of the `delegateSampler`.
* Description MUST be `ParentOrElse{delegateSampler.getDescription()}`.

|Parent|`ParentOrElse(delegateSampler)`
|--|--|
|Exists and `SampledFlag` is `true`|`RECORD_AND_SAMPLED`|
|Exists and `SampledFlag` is `false`|`NOT_RECORD`|
|No parent(root spans)|Result of `delegateSampler()`|

## Tracer Creation

Expand Down