Probability sampling specification #1899
Conversation
| Although sampling can be carried out in multiple stages, OpenTelemetry | ||
| specifies a dedicated field in the Span data model for representing | ||
| probability at the "head" of the distributed trace, where it describes the | ||
| probability the `Sampled` flag was set in the Span's initial sampling |
There was a problem hiding this comment.
probability the
Sampled
probability when the Sampled?
Co-authored-by: Yuri Shkuro <yurishkuro@users.noreply.github.com>
…specification into jmacd/sampling_spec
…ication into jmacd/sampling_spec
|
Reviewers, please consider another look at open-telemetry/oteps#168 which now covers a number of corner cases and is generally finished. Once that merges we can continue working on this PR. The questions that I propose we defer for "future debate" are summarized here in this comment open-telemetry/oteps#168 (comment) and I propose that we debate them here afterhttps://github.com/open-telemetry/oteps/pull/168 merges. |
|
FYI open-telemetry/oteps#175 adds a glossary of sampling terms used that would be used to flesh out a tracing |
There was a problem hiding this comment.
@jsuereth I thought that we don't immediately define specs for OTEPs, especially when we are in a new unknown area. Also this changes a "stable" document without having a prototype which we should not correct?
| base-2 logarithm in a small number of bits. | ||
|
|
||
| The OpenTelemetry Span field for encoding adjusted count is named | ||
| `log_head_adjusted_count`, with the default value zero representing the case |
There was a problem hiding this comment.
Where is this defined in the data-model? Why is this needed if the tracestate is already present?
There was a problem hiding this comment.
(Remember this PR is a draft until all the OTEPs merge.)
The field log_head_adjusted_count is described in https://github.com/open-telemetry/oteps/blob/main/text/trace/0170-sampling-probability.md.
You make a valid point about tracestate. On the other hand, it's less clearly part of the data model if the user is required to parse the tracestate (twice, as there are two syntaxes) to recover the information. Besides, tracestate is not an ideal solution in the long term. Our best long term solution is to modify the W3C traceparent to include a few extra bytes. Then, the rationale of storing log_head_adjusted_count as an independent field is that we may have other means of determining log_head_adjusted_count in the future.
For now, I would suggest that OTel erases its own tracestate record from the span data and specify fields for anything that's part of the data model. As it stands, the r value from OTEP 168 is not being recorded and as far as I know, no one has asked for that.
Co-authored-by: Reiley Yang <reyang@microsoft.com>
|
This PR was marked stale due to lack of activity. It will be closed in 7 days. |
|
Closed as inactive. Feel free to reopen if this PR is still being worked on. |
This proposes changes in the specification implied by OTEPs 168, 170, and 175:
open-telemetry/oteps#168
open-telemetry/oteps#170
open-telemetry/oteps#175
The justification for these changes is given in those two OTEPs. Our goal is to ensure that Span-to-Metrics pipelines can be built to statistically count spans after they have been sampled and before they have been assembled into traces.
The two OTEPs are not receiving enough attention. This PR is meant to consolidate the implied changes of those two OTEPs combined in a way that makes it easier to see what is being specified (WHAT) without all the related justification (WHY). If you approve these changes, please also review and approve those OTEPs first.
Fixes #1413.
Part of #1819.
This cannot be merged before OTEPs 168 and 170 are merged, but this is also not a draft, it is a complete proposal. This has been prototyped twice, once in Java and once in Go. See OTEP 170 for more detail about these prototypes.