Add spec for AlwaysRecord sampler #4699
Conversation
jmacd
left a comment
jmacd
left a comment
There was a problem hiding this comment.
Looks good to me. As we discussed in Sampling SIG, it will help readers to be reminded somewhere in this text that Exporters are already expected to drop these spans, all we're doing is making it easy to setup non-sampling recorded spans by default.
| - Other root spans are sampled at 10% | ||
| - Child spans follow their parent's sampling decision | ||
|
|
||
| #### AlwaysRecord |
There was a problem hiding this comment.
#nit: I think we should put this above the CompositeSampler to avoid it being confused as a Built-In ComposableSampler.
Sampling SIG: does it make sense to add a Built-In ComposableSampler variation of this?
There was a problem hiding this comment.
Ping @majanjua-amzn
(also we need a CHANGELOG entry)
| **Status**: [Development](../document-status.md) | ||
|
|
||
| This is a sampler decorator. `AlwaysRecord` helps to have spans between always be processed by `SpanProcessor`s. It returns the following given a sampled flag from the root sampler: | ||
| * Root sampler returns `DROP` -> AlwaysRecord returns `RECORD_ONLY` |
There was a problem hiding this comment.
| * Root sampler returns `DROP` -> AlwaysRecord returns `RECORD_ONLY` | |
| * Root sampler returns `DROP` -> AlwaysRecord returns `RECORD_ONLY` |
|
|
||
| **Status**: [Development](../document-status.md) | ||
|
|
||
| This is a sampler decorator. `AlwaysRecord` helps to have spans between always be processed by `SpanProcessor`s. It returns the following given a sampled flag from the root sampler: |
There was a problem hiding this comment.
Please make the line length wrapping consistent with the reset of the document and the rest of the changes here.
| * Root sampler returns `RECORD_ONLY` -> AlwaysRecord returns `RECORD_ONLY` | ||
| * Root sampler returns `RECORD_AND_SAMPLE` -> AlwaysRecord returns `RECORD_AND_SAMPLE` | ||
|
|
||
| **Required parameters:** |
There was a problem hiding this comment.
Following formatting from the rest of this document.
| **Required parameters:** | |
| Required parameters: |
6 participants
NOTE: PR created to be discussed in next relevant SIG meeting
Fixes #4698
Changes
Adds the spec for a new AlwaysRecord sampler being proposed in #4698
Notes
Solutions to this problem in multiple supported languages can already be found below:
Checklist
CHANGELOG.mdfile updated for non-trivial changes