Skip to content

Conversation

@t089
Copy link
Contributor

@t089 t089 commented Aug 12, 2024

This adds a OTelTraceIdRatioBasedSampler (see Spec).

The sampler takes the last 8 bytes of the TraceID and interprets them as a single UInt64 value which is then used for the sampling decision. As long as those bytes are randomly distributed, this gives a deterministic, but random sampling decision.

Copy link
Member

@slashmo slashmo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR 🙏 It's a good addition to the package, thanks for also adding benchmarks 👍 I'll do a final review pass once we get the breaking change in W3C Trace Context merged. My comments so far have mostly been around formatting.

@slashmo slashmo marked this pull request as ready for review September 29, 2024 10:34
@slashmo slashmo marked this pull request as draft September 29, 2024 10:34
@codecov
Copy link

codecov bot commented Oct 1, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 99.01%. Comparing base (90d63b4) to head (efb467d).

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #131      +/-   ##
==========================================
+ Coverage   99.00%   99.01%   +0.01%     
==========================================
  Files          65       68       +3     
  Lines        1800     1831      +31     
==========================================
+ Hits         1782     1813      +31     
  Misses         18       18              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@t089 t089 changed the title Draft: Adds OTelTraceIdRatioBasedSampler Adds OTelTraceIdRatioBasedSampler Oct 2, 2024
@t089 t089 marked this pull request as ready for review October 2, 2024 08:05
@t089 t089 requested a review from slashmo October 2, 2024 08:05
slashmo
slashmo previously approved these changes Nov 30, 2024
Copy link
Member

@slashmo slashmo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks for contributing this sampler and especially for your patience waiting on my review 🙏

@slashmo slashmo enabled auto-merge (squash) November 30, 2024 11:13
@slashmo slashmo changed the title Adds OTelTraceIdRatioBasedSampler Adds OTelTraceIDRatioBasedSampler Nov 30, 2024
@slashmo slashmo disabled auto-merge November 30, 2024 11:13
@slashmo slashmo enabled auto-merge (squash) November 30, 2024 11:14
@slashmo slashmo merged commit 0df9e80 into swift-otel:main Nov 30, 2024
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add trace-id ratio based sampler (probabilistic)

2 participants