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

Add table summarizing sampler properties and processing #871

Merged
merged 22 commits into from
Aug 26, 2020
Merged

Add table summarizing sampler properties and processing #871

Changes from 3 commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
ddcc2d7
Add table summarizing sampler properties and processing
cijothomas Aug 25, 2020
03d6d70
markdownlink
cijothomas Aug 25, 2020
29a4a91
Merge branch 'master' into cijothomas/sdksamper1
cijothomas Aug 25, 2020
66d1620
event -> data to avoid confusion with Event
cijothomas Aug 25, 2020
62aed50
events -> data
cijothomas Aug 25, 2020
3de6f46
plural
cijothomas Aug 25, 2020
0b625c8
use MUST
cijothomas Aug 25, 2020
7c7f265
Update specification/trace/sdk.md
cijothomas Aug 25, 2020
36b83c5
Update specification/trace/sdk.md
cijothomas Aug 25, 2020
686969e
reword
cijothomas Aug 25, 2020
9452678
spacing
cijothomas Aug 25, 2020
bbad7aa
more links fix
cijothomas Aug 25, 2020
f77ffdd
markdownl
cijothomas Aug 25, 2020
fbfe97d
Merge branch 'master' into cijothomas/sdksamper1
cijothomas Aug 25, 2020
5d92a46
Update specification/trace/sdk.md
cijothomas Aug 25, 2020
6ce78c8
Merge branch 'master' into cijothomas/sdksamper1
cijothomas Aug 25, 2020
8dbcc78
Merge branch 'master' into cijothomas/sdksamper1
cijothomas Aug 25, 2020
94e9bb4
Merge branch 'master' into cijothomas/sdksamper1
yurishkuro Aug 26, 2020
455b600
Merge branch 'master' into cijothomas/sdksamper1
cijothomas Aug 26, 2020
29fab81
Merge branch 'master' into cijothomas/sdksamper1
carlosalberto Aug 26, 2020
e9ca916
Merge branch 'master' into cijothomas/sdksamper1
cijothomas Aug 26, 2020
c241b1d
Merge branch 'master' into cijothomas/sdksamper1
yurishkuro Aug 26, 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
28 changes: 19 additions & 9 deletions specification/trace/sdk.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 `true` the current `Span` records
tracing events (attributes, events, status, etc.), otherwise all tracing
events are dropped. Users can use this property to determine if expensive
trace events can be avoided. [Span Processors](#span-processor) will receive
all spans with this flag set. However, [Span Exporter](#span-exporter) will
not receive them unless the `Sampled` flag was set.
* `IsRecording` field of a `Span`. If `true` the current `Span` records tracing
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
* `IsRecording` field of a `Span`. If `true` the current `Span` records tracing
* `IsRecording` field of a `Span`. If `true` the current `Span` records in memory tracing

Copy link
Member

@Oberon00 Oberon00 Aug 25, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

What is "in memory data"? I think the current wording (== the old wording) is more clear here. What would be more clear IMHO would be:

Suggested change
* `IsRecording` field of a `Span`. If `true` the current `Span` records tracing
* `IsRecording` field of a `Span`. If `false` the current `Span` discards all tracing

and remove the "otherwise ...".

events (attributes, events, status, etc.), otherwise all tracing events are
cijothomas marked this conversation as resolved.
Show resolved Hide resolved
dropped. Users can use this property to determine if expensive trace events
cijothomas marked this conversation as resolved.
Show resolved Hide resolved
can be avoided. [Span Processors](#span-processor) will receive only those
cijothomas marked this conversation as resolved.
Show resolved Hide resolved
spans which has this field set to `true`. However, [Span
cijothomas marked this conversation as resolved.
Show resolved Hide resolved
Exporter](#span-exporter) will not receive them unless the `Sampled` flag was
cijothomas marked this conversation as resolved.
Show resolved Hide resolved
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][trace-flags]. This flag indicates that the `Span` has been
`sampled` and will be exported. [Span Processor](#span-processor) and [Span
Exporter](#span-exporter) will receive spans with the `Sampled` flag set for
processing.
`sampled` and will be exported. [Span Exporters](#span-exporter) will receive
cijothomas marked this conversation as resolved.
Show resolved Hide resolved
only those spans with the `Sampled` flag set.
Oberon00 marked this conversation as resolved.
Show resolved Hide resolved

The flag combination `SampledFlag == false` and `IsRecording == true`
means that the current `Span` does record information, but most likely the child
Expand All @@ -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? |
Oberon00 marked this conversation as resolved.
Show resolved Hide resolved
| ----------- | ------------ | ----------------------------- | ---------------------------- |
| true | true | true | true |
| true | false | true | false |
| false | true | Not allowed | Not allowed |
| false | false | false | false |

The SDK defines the interface [`Sampler`](#sampler) as well as a set of
[built-in samplers](#built-in-samplers).

Expand Down