[Extensions] Add consistent probability sampler - #4629
Conversation
Add an implementation of a consistent probability sampler. Contributes to open-telemetry#3678.
- Add fuzz tests for `ConsistentProbability`. - Fix StyleCop warning.
github-actions
Bot
added
the
comp:extensions
There was a problem hiding this comment.
Pull request overview
Adds a new consistent probability-based sampler to OpenTelemetry.Extensions, implementing the OpenTelemetry tracestate probability-sampling spec (including encoding/decoding thresholds and propagating ot=th/ot=rv), along with unit and property-based tests plus documentation and API surface tracking.
Changes:
- Added
ConsistentProbabilitySampler(public) with supporting internal codec utilities andtracestate(ot) parsing/serialization. - Added unit tests for the codec, tracestate handling, and sampler behavior; added fuzz/property tests to validate invariants across randomized inputs.
- Updated
OpenTelemetry.Extensionsdocs/changelog and public API declarations; added the new fuzz test project to the solution.
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| test/OpenTelemetry.Extensions.Tests/Trace/OtelTraceStateTests.cs | New unit tests for parsing/serializing ot tracestate (th/rv) while preserving other members/subkeys. |
| test/OpenTelemetry.Extensions.Tests/Trace/ConsistentProbabilityTests.cs | New unit tests validating threshold encoding/decoding against spec tables and invariants. |
| test/OpenTelemetry.Extensions.Tests/Trace/ConsistentProbabilitySamplerTests.cs | New unit tests validating sampler constructor validation, decisions, and tracestate propagation semantics. |
| test/OpenTelemetry.Extensions.FuzzTests/OpenTelemetry.Extensions.FuzzTests.csproj | New fuzz test project for randomized/property-based validation. |
| test/OpenTelemetry.Extensions.FuzzTests/ConsistentProbabilityFuzzTests.cs | Property-based tests (FsCheck) for codec/tracestate/sampler invariants. |
| src/OpenTelemetry.Extensions/Trace/ConsistentProbabilitySampler.cs | New sampler implementation that resolves randomness from rv, TraceId random flag, or generated randomness, and sets/clears th. |
| src/OpenTelemetry.Extensions/Internal/OtelTraceState.cs | New internal parser/serializer for ot tracestate entry supporting th/rv. |
| src/OpenTelemetry.Extensions/Internal/ConsistentProbability.cs | New internal codec for converting probability ↔ threshold and parsing hex56 values. |
| src/OpenTelemetry.Extensions/README.md | Documentation for ConsistentProbabilitySampler with usage snippet. |
| src/OpenTelemetry.Extensions/CHANGELOG.md | Added Unreleased entry for the new sampler. |
| src/OpenTelemetry.Extensions/.publicApi/PublicAPI.Unshipped.txt | Added new public API entries for ConsistentProbabilitySampler. |
| src/OpenTelemetry.Extensions/OpenTelemetry.Extensions.csproj | Added InternalsVisibleTo for the new fuzz test assembly. |
| opentelemetry-dotnet-contrib.slnx | Added the fuzz test project to the solution. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Add benchmarks for the sampler.
Add PR number.
Add more test coverage.
- Fix incorrect threshold clamping. - Prevent too-small thresholds from being specified. - Add missing test coverage. - Fix typo in code sample.
Fix flaky test on .NET Framework by using a Stopwatch instead of `DateTime.UtcNow`.
There was a problem hiding this comment.
Given that the package is beta and that the new public API exposed here is effectively an implementation of a stable abstract class and otherwise only accepts a double value to set the probability for sampling, this is OK to not be experimental?
If the spec were to change that would just be internal implementation details and we'd just ship a new version right?
There was a problem hiding this comment.
Yes, since the package is not stable, we do not need to do the experimental API dance.
Rename `injectedRandom` to `random`.
Wrap the ranges in `c` tags.
Use `Math.ILogB()` instead of manual code on .NET 8+.
Add an end-to-end test that demonstrates usage across multiple processes as if in a real distributed system.
martincostello
added
the
keep-open
Move entry after merge with main.
Pull request dashboard statusMerged · refreshed 2026-08-13 12:24 UTC Status above doesn't look right?
|
Kielek
left a comment
Kielek
left a comment
There was a problem hiding this comment.
I pushed some feedback directly to this PR. With this LGTM.
Please double check before merge.
Add polyfill for modern .NET char methods for ASCII characters and use in `OtelTraceState`.
LGTM. I pulled through the I'll look to see if there's other places in this repo that can benefit from the new polyfills separately. |
Fixes #3678
Changes
Add an implementation of a consistent probability sampler (spec).
Benchmarks
Summarised by Claude 🐙
Analysis
All four scenarios measure the per-span cost of a single
ShouldSamplecall with the sampler configured at 25%. The operations are tens-to-hundreds of nanoseconds and allocate only small, bounded amounts; the allocation figures are the most stable and meaningful signal (the net10.0 timings have wide error bars because the machine was noisier during that run).By scenario:
RandomTraceIdis the cheapest path on both runtimes. Norvis generated or written and no other tracestate members need preserving, so the outgoing tracestate is justot=th:<x>. On net462 it allocates 0 B — the trace-id randomness is read via a span slice, the threshold is formatted without an intermediate string, and there are no lists to allocate. On net10.0 it still shows 168 B solely becauseActivityTraceId.ToHexString()allocates a 32-char string (there is no public byte-level accessor to avoid it).RootSpan(baseline) andExplicitRandomValueare dominated by building the outgoingot=th:<x>;rv:<y>string — theStringBuilderplus the final serialized string, which is unavoidable since it is the return value.ExplicitRandomValueis slightly cheaper/leaner than the root case because it reuses the inheritedrvinstead of generating one.ExplicitRandomValueWithOtherMembersis the most expensive (~1.3× time and ~2× allocations vs. baseline). The two extra vendor members must be parsed into preserved strings and re-emitted. This is exactly the lazy-list code path that allocates only when extras are present — the common cases above pay nothing for it.Runtime differences:
#if NETpaths (Random.Shared.NextInt64,long.TryFormatinto stack buffers,stackallocspans), giving lower allocations on the string-building paths than net462, which falls back toNextBytes+ToString.RandomTraceIdpath holds on the fallback code.Takeaways:
TryFormatwork pays off: the hot paths allocate one small string (the required output) or nothing, and extra-member preservation is pay-for-what-you-use.RootSpan/ExplicitRandomValueallocations are the serialized tracestate string itself (intrinsic to the sampler's contract) plus, on net10.0, theTraceId.ToHexString()string on the trace-id path.Expand to view
ConsistentProbabilitySampler.ShouldSample(sampler configured at 25%)net10.0
net462
Merge requirement checklist
CHANGELOG.mdfiles updated for non-trivial changes