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.

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

Update Sampler interface base on OTEP 0006-sampling #296

Merged
merged 7 commits into from
Oct 15, 2019
Merged
Changes from 2 commits
Commits
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
84 changes: 45 additions & 39 deletions specification/sdk-tracing.md
Original file line number Diff line number Diff line change
Expand Up @@ -29,29 +29,41 @@ as well as a set of [built-in samplers](#built-in-samplers).
### Sampler

`Sampler` interface allows to create custom samplers which will return a
sampling `Decision` based on information that is typically available just before
the `Span` was created.
sampling `SamplingResult` based on information that is typically available just
before the `Span` was created.

#### ShouldSample

Returns the sampling Decision for a `Span` to be created.

**Required arguments:**
Copy link
Member

Choose a reason for hiding this comment

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

Would it make sense to generalize this, in light of #31? In Jaeger we recently extended the API to allow sampling decisions to be made later in the span life cycle

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

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 these are moments when the Sampler is called from the SDK perspective. I do agree that this is a nice to have config. Filed #307


- `SpanContext` of a parent `Span`. Typically extracted from the wire. Can be
* `SpanContext` of a parent `Span`. Typically extracted from the wire. Can be
`null`.
- `TraceId` of the `Span` to be created. It can be different from the `TraceId`
* `TraceId` of the `Span` to be created. It can be different from the `TraceId`
in the `SpanContext`. Typically in situations when the `Span` to be created
starts a new Trace.
- `SpanId` of the `Span` to be created.
- Name of the `Span` to be created.
- Collection of links that will be associated with the `Span` to be created.
Typically useful for batch operations, see [Links Between
Spans](overview.md#links-between-spans).
* `SpanId` of the `Span` to be created.
* Name of the `Span` to be created.
* `SpanKind`
* Initial set of `Attributes` for the `Span` being constructed
* Collection of links that will be associated with the `Span` to be created.
Typically useful for batch operations, see
[Links Between Spans](overview.md#links-between-spans).

**Return value:**

Sampling `Decision` whether span should be sampled or not.
It produces an output called `SamplingResult` which contains:

* A `Decision` enum:
* `NOT_RECORD` - `IsRecording() == false` and all recorded trace events
will be dropped.
* `RECORD` - `IsRecording() == true` AND `SampledFlag` is not set.
* `RECORD_AND_SAMPLED` - `IsRecording() == true` AND `SampledFlag` is set.
* A set of span Attributes that will also be added to the `Span`.
* The list of attributes returned by `SamplingResult` MUST be immutable.
Caller may call this method any number of times and can safely cache the
returned value.

#### GetDescription

Expand All @@ -61,37 +73,31 @@ be displayed on debug pages or in the logs. Example:

Description MUST NOT change over time and caller can cache the returned value.

### Decision

`Decision` is an interface with two getters describing the sampling decision.

#### IsSampled

Return sampling decision whether span should be sampled or not. `True` value of
`IsSampled` flag means that Span information needs to be recorded.

#### GetAttributes

Return attributes to be attached to the `Span`. These attributes should be added
to the `Span` only for root span or when sampling decision `IsSampled` changes
from false to true.

Examples of attribute may be algorithm used to make a decision and sampling
priority. Another example may be recording the reason trace was marked as
"important" to sample in. For instance, when traces from specific user session
should be collected, session identifier can be added to attributes.

The list of attributes returned by `Decision` MUST be immutable. Caller may call
this method any number of times and can safely cache the returned value.

### Built-in samplers

API MUST provide a way to create the following built-in samplers:

- Always sample. `Sampler` returns `Decision` with `IsSampled=true` and empty
arguments collection. Description MUST be `AlwaysSampleSampler`.
- Never sample. `Sampler` returns `Decision` with `IsSampled=false` and empty
arguments collection. Description MUST be `NeverSampleSampler`.
These are the default samplers implemented in the OpenTelemetry SDK:

* ALWAYS_ON
Copy link
Contributor

Choose a reason for hiding this comment

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

IMO, this isn't a particularly clear name. Can we make it ALWAYS_RETAIN and the other be ALWAYS_DROP ?

Copy link
Contributor

Choose a reason for hiding this comment

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

I guess not?

Copy link
Member Author

Choose a reason for hiding this comment

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

Please file an issue to not forget about this.

Copy link
Contributor

Choose a reason for hiding this comment

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

#314 done!

* 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
there should be configuration to change this.
* 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

TODO: Add details about how the probability sampler is implemented as a function
of the `TraceID`.

## Tracer Creation

Expand Down