Skip to content

Commit

Permalink
Remove SamplingHint from the Sampling otep (open-telemetry#79)
Browse files Browse the repository at this point in the history
* Remove SamplingHint from the Sampling otep

Signed-off-by: Bogdan Drutu <bogdandrutu@gmail.com>

* Update text/0006-sampling.md

Co-Authored-By: John Watson <jkwatson@gmail.com>

* Update text/0006-sampling.md

Co-Authored-By: John Watson <jkwatson@gmail.com>

Co-authored-by: John Watson <jkwatson@gmail.com>
  • Loading branch information
bogdandrutu and jkwatson authored Jan 28, 2020
1 parent 322da45 commit cdb654b
Showing 1 changed file with 7 additions and 28 deletions.
35 changes: 7 additions & 28 deletions text/0006-sampling.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
This section tries to summarize all the changes proposed in this RFC:
1. Move the `Sampler` interface from the API to SDK package. Apply some minor changes to the
`Sampler` API.
1. Add a new `SamplerHint` concept to the API package.
1. Add capability to record `Attributes` that can be used for sampling decision during the `Span`
creation time.
1. Remove `addLink` APIs from the `Span` interface, and allow recording links only during the span
Expand Down Expand Up @@ -68,9 +67,10 @@ Examples: gRPC, Express, Django developers.

**Solution:**

* On the start Span operation, the OpenTelemetry API will allow marking a span with one of three
choices for the [SamplingHint](#samplinghint).

* For the moment, the OpenTelemetry API will not offer any `SamplingHint` functionality for the last use case.
This is intentional to avoid premature optimizations, and it is based on the fact that changing an API is
backwards incompatible compared to adding a new API.

### Infrastructure package/binary developer
Examples: HBase, Envoy developers.

Expand Down Expand Up @@ -170,26 +170,11 @@ distributed trace, and because of this OpenTelemetry API should NOT allow this c
It is safe to assume that users of the API should only access the `RecordEvents` property when
instrumenting code and never access `SampledFlag` unless used in context propagators.

### SamplingHint
This is a new concept added in the OpenTelemetry API that allows to suggest sampling hints to the
implementation of the API:
* `NOT_RECORD`
* Suggest to not set (`RecordEvents = false`) and not propagate (`SampledFlag = false`).
* `RECORD`
* Suggest to set (`RecordEvents = true`) but not propagate (`SampledFlag = false`).
* `RECORD_AND_PROPAGATE`
* Suggest to set (`RecordEvents = true`) and propagate (`SampledFlag = true`).

The default option for the span creation is to not have any suggestion (or suggestion is not
specified). This can be implemented by using `null` as the default option or any language specific
mechanism to achieve the same result.

### Sampler interface
The interface for the Sampler class that is available only in the OpenTelemetry SDK:
* `TraceID`
* `SpanID`
* Parent `SpanContext` if any
* `SamplerHint`
* `Links`
* Span name
* `SpanKind`
Expand All @@ -204,19 +189,13 @@ It produces an output called `SamplingResult` that includes:
### Default Samplers
These are the default samplers implemented in the OpenTelemetry SDK:
* ALWAYS_ON
* Ignores all values in SamplingHint.
* ALWAYS_OFF
* Ignores all values in SamplingHint.
* ALWAYS_PARENT
* Ignores all values in SamplingHint.
* Trust parent sampling decision (trusting and propagating parent `SampledFlag`).
* For root Spans (no parent available) returns `NOT_RECORD`.
* Probability
* Allows users to configure to ignore or not the SamplingHint for every value different than
`UNSPECIFIED`.
* Default is to NOT ignore `NOT_RECORD` and `RECORD_AND_PROPAGATE` but ignores `RECORD`.
* Allows users to configure to ignore the parent `SampledFlag`.
* Allows users to configure if probability applies only for "root spans", "root spans and remote
* Allows users to configure if probability applies only for "root spans", "root spans and remote
parent", or "all spans".
* Default is to apply only for "root spans and remote parent".
* Remote parent property should be added to the SpanContext see specs [PR/216][specs-pr-216]
Expand All @@ -229,7 +208,7 @@ These are the default samplers implemented in the OpenTelemetry SDK:
|ALWAYS_ON|`True`|`True`|
|ALWAYS_OFF|`False`|`False`|
|ALWAYS_PARENT|`False`|`False`|
|Probability|`SamplingHint==RECORD OR SampledFlag`|`SamplingHint==RECORD_AND_PROPAGATE OR Probability`|
|Probability|`Same as SampledFlag`|`Probability`|

**Child Span Decision:**

Expand All @@ -238,7 +217,7 @@ These are the default samplers implemented in the OpenTelemetry SDK:
|ALWAYS_ON|`True`|`True`|
|ALWAYS_OFF|`False`|`False`|
|ALWAYS_PARENT|`ParentSampledFlag`|`ParentSampledFlag`|
|Probability|`SamplingHint==RECORD OR SampledFlag`|`ParentSampledFlag OR SamplingHint==RECORD_AND_PROPAGATE OR Probability`|
|Probability|`Same as SampledFlag`|`ParentSampledFlag OR Probability`|

### Links
This RFC proposes that Links will be recorded only during the start `Span` operation, because:
Expand Down

0 comments on commit cdb654b

Please sign in to comment.