Composite Head Samplers #4321
Composite Head Samplers #4321
Conversation
|
This PR was marked stale due to lack of activity. It will be closed in 7 days. |
|
Added |
PeterF778
force-pushed
the
composite_samplers
branch
from
59612f8 to
db169b3
Compare
|
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. |
|
|
||
| - The THRESHOLD value represented as a 14-character hexadecimal string, with value of `null` representing non-probabilistic `DROP` decision (implementations MAY use different representation, if it appears more performant or convenient), | ||
| - A function (`IsAdjustedCountReliable`) that provides a `boolean` value indicating that the adjusted count (calculated as reciprocal of the sampling probability) can be faithfully used to estimate span metrics, | ||
| - A function (`GetAttributes`) that provides a set of `Attributes` to be added to the `Span` in case of positive final sampling decision, |
There was a problem hiding this comment.
Nit: "a positive final sampling decision".
| - The THRESHOLD value represented as a 14-character hexadecimal string, with value of `null` representing non-probabilistic `DROP` decision (implementations MAY use different representation, if it appears more performant or convenient), | ||
| - A function (`IsAdjustedCountReliable`) that provides a `boolean` value indicating that the adjusted count (calculated as reciprocal of the sampling probability) can be faithfully used to estimate span metrics, | ||
| - A function (`GetAttributes`) that provides a set of `Attributes` to be added to the `Span` in case of positive final sampling decision, | ||
| - A function (`UpdateTraceState`) that given an input `Tracestate` and sampling Decision provides a `Tracestate` to be associated with the `Span`. The samplers SHOULD NOT add or modify the `th` value for the `ot` key within these functions. |
There was a problem hiding this comment.
Nit: add comma before "provides".
| - The THRESHOLD value represented as a 14-character hexadecimal string, with value of `null` representing non-probabilistic `DROP` decision (implementations MAY use different representation, if it appears more performant or convenient), | ||
| - A function (`IsAdjustedCountReliable`) that provides a `boolean` value indicating that the adjusted count (calculated as reciprocal of the sampling probability) can be faithfully used to estimate span metrics, | ||
| - A function (`GetAttributes`) that provides a set of `Attributes` to be added to the `Span` in case of positive final sampling decision, | ||
| - A function (`UpdateTraceState`) that given an input `Tracestate` and sampling Decision provides a `Tracestate` to be associated with the `Span`. The samplers SHOULD NOT add or modify the `th` value for the `ot` key within these functions. |
There was a problem hiding this comment.
I wasn't able to get this UpdateTraceState function at first, in particular, why the sampler cannot change the th value. Reading further and thinking more, it seems to me that this function is intended for the sampler implementation to add some vendor-specific info? The sampler can change the th value as part of its processing of ShouldSample.
If my guess is right, can we state the intended use of this function? That will help the implementors of the SDK or the samplers.
If my guess is not right, can you explain?
|
|
||
| The Predicates represent logical expressions which can access `Span` `Attributes` (or anything else available when the sampling decision is to be made), and perform tests on the accessible values. | ||
| For example, one can test if the target URL for a SERVER span matches a given pattern. | ||
| `Predicate` interface allows users to create custom categories based on information that is available at the time of making the sampling decision. |
There was a problem hiding this comment.
I think it's worth to state that the predicates should be independent on the sampling decisions.
Changes
Migrating from open-telemetry/oteps#250.
This is another approach for introducing Composite (Head) Samplers. The previous one (open-telemetry/oteps#240) proved too large, with some controversial elements.
This OTEP is a split-off from that one, focusing just on one area - new Composite Samplers.
Two prototypes exist for this functionality. We are seeking a third prototype.
In the Java-contrib repo: https://github.com/open-telemetry/opentelemetry-java-contrib/blob/main/consistent-sampling/README.md
In @jmacd's https://github.com/jmacd/go-sampler/blob/main/README.md.