Add table summarizing sampler properties and processing #871
+20
−10
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -24,18 +24,18 @@ latest sampling could happen on the Collector which is out of process. | |
|
|
||
| The OpenTelemetry API has two properties responsible for the data collection: | ||
|
|
||
| * `IsRecording` field of a `Span`. If `false` the current `Span` discards all | ||
| tracing data (attributes, events, status, etc.). Users can use this property | ||
| to determine if collecting expensive trace data can be avoided. [Span | ||
| Processor](#span-processor) MUST receive only those spans which have this | ||
| field set to `true`. However, [Span Exporter](#span-exporter) SHOULD NOT | ||
| receive them unless the `Sampled` flag was also set. | ||
| * `Sampled` flag in `TraceFlags` on `SpanContext`. This flag is propagated via | ||
| the `SpanContext` to child Spans. For more details see the [W3C Trace Context | ||
| specification](https://www.w3.org/TR/trace-context/#sampled-flag). This flag indicates that the `Span` has been | ||
| `sampled` and will be exported. [Span Exporters](#span-exporter) MUST | ||
| receive those spans which have `Sampled` flag set to true and they SHOULD NOT receive the ones | ||
| that do not. | ||
|
|
||
| The flag combination `SampledFlag == false` and `IsRecording == true` | ||
| means that the current `Span` does record information, but most likely the child | ||
|
|
@@ -45,6 +45,16 @@ The flag combination `SampledFlag == true` and `IsRecording == false` | |
| could cause gaps in the distributed trace, and because of this OpenTelemetry API | ||
| MUST NOT allow this combination. | ||
|
|
||
| The following table summarizes the expected behavior for each combination of | ||
| `IsRecording` and `SampledFlag`. | ||
|
|
||
| | `IsRecording` | `Sampled` Flag | Span Processor receives Span? | Span Exporter receives Span? | | ||
|
There was a problem hiding this comment. Actually the third column is a change / additional specification, but I think it's good. @cijothomas Can you add "Fixes #782" to the PR description? |
||
| | ------------- | -------------- | ----------------------------- | ---------------------------- | | ||
| | true | true | true | true | | ||
| | true | false | true | false | | ||
| | false | true | Not allowed | Not allowed | | ||
cijothomas marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| | false | false | false | false | | ||
|
|
||
| The SDK defines the interface [`Sampler`](#sampler) as well as a set of | ||
| [built-in samplers](#built-in-samplers). | ||
|
|
||
|
|
||
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since the text here is "SHOULD NOT" - it means the recommendation is to not call Exporters for Spans without
Sampledflag set. Processors could still do it, and spec is not prohibiting it.As we used "SHOULD NOT" here, do we still require additional explicit clarification here, or in the table that "Depending on the Processor, Exporter may still get span without
Sampledflag set? @carlosalberto @Oberon00There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think it's necessary but it would be nice. If you want, you can add a sentence to the built in span processors section that they MUST NOT call exporters for unsampled spans (unless explicitly requested).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let me address that as a follow up PR. I fear adding it might delay this PRs progress.
(Meanwhile I'll to figure out under what scenarios a user want to sent unsampled spans to exporter).