Prototype for W3C Trace Context Level 2 support in TraceIDRatioBased sampler #5645
Conversation
…o jmacd/randomness
…o jmacd/experimental_new_sampler_with_otep_250
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #5645 +/- ##
======================================
Coverage 84.4% 84.5%
======================================
Files 272 272
Lines 22506 22679 +173
======================================
+ Hits 19017 19166 +149
- Misses 3153 3172 +19
- Partials 336 341 +5 |
| // 0x1.ffffffffffffe0p-01 | ||
| // 0x0.fffffffffffff0p+00 | ||
| // math.Nextafter(1.0, 0.0) | ||
| maxSupportedProbability float64 = 1 - 0x1p-52 |
There was a problem hiding this comment.
The name maxSupportedProbability could be misleading here, as consistent sampling naturally also supports sampling with a probability of 1.
|
Do we have some benchmarks that would demonstrate the overhead of the added randomness support? Can we add |
|
|
||
| // The update is expected to be a single key-value of the form | ||
| // `XX:value` for with two-character key. | ||
| upkey := updated[:colonOffset] |
There was a problem hiding this comment.
I see that this is guaranteed to have a th: prefix in the current code, but do you think it would be worth having a sanity check to future-proof it?
| // in `otts` which is the OTel tracestate value (i.e., the top-level "ot" value). | ||
| func tracestateHasRandomness(otts string) (randomness uint64, hasRandom bool) { | ||
| var low int | ||
| if has := strings.HasPrefix(otts, "rv:"); has { |
There was a problem hiding this comment.
| if has := strings.HasPrefix(otts, "rv:"); has { | |
| if strings.HasPrefix(otts, "rv:") { |
| if incoming == "" { | ||
| return updated | ||
| } | ||
| var out strings.Builder |
There was a problem hiding this comment.
this might be clearer later, but can the final size be determined at this point already?
|
The sampling SIG is pursuing a new "optional" sampler API that will improve the performance of this feature. |
This is a prototype implementation for changes to the TraceIDRatioBased sampler based on W3C Trace Context Level 2 which includes support for trace randomness.
This is a combined prototype for the SDK-relevant portions of open-telemetry/opentelemetry-specification#4162 and open-telemetry/opentelemetry-specification#4166.