Add incubator implementation of composite sampling spec by anuraaga · Pull Request #7626 · open-telemetry/opentelemetry-java

anuraaga · 2025-08-29T07:29:26Z

This implements composite samplers as in the development spec

https://opentelemetry.io/docs/specs/otel/trace/sdk/#built-in-composablesamplers
https://opentelemetry.io/docs/specs/otel/trace/tracestate-probability-sampling/

The implementation is quite similar to the contrib one that was used to drive the OTEP, though the naming is updated to match what made it to the spec

https://github.com/open-telemetry/opentelemetry-java-contrib/tree/fc6507e5e24473fc4a131e3bda1d4b1156fe18c9/consistent-sampling/src/main/java/io/opentelemetry/contrib/sampler/consistent56

For reference, implementations for JS and Python

open-telemetry/opentelemetry-js#5839
open-telemetry/opentelemetry-python#4714

codecov · 2025-08-29T07:32:40Z

Codecov Report

❌ Patch coverage is 96.85864% with 6 lines in your changes missing coverage. Please review.
✅ Project coverage is 90.07%. Comparing base (63d4617) to head (c5ebbb6).
⚠️ Report is 34 commits behind head on main.

Files with missing lines	Patch %	Lines
...nsion/incubator/trace/samplers/OtelTraceState.java	94.44%	2 Missing and 3 partials ⚠️
...ion/incubator/trace/samplers/CompositeSampler.java	97.14%	1 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##               main    #7626      +/-   ##
============================================
+ Coverage     89.99%   90.07%   +0.08%     
- Complexity     7079     7172      +93     
============================================
  Files           803      812       +9     
  Lines         21412    21653     +241     
  Branches       2086     2123      +37     
============================================
+ Hits          19269    19504     +235     
- Misses         1479     1480       +1     
- Partials        664      669       +5

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

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

trask · 2025-09-04T16:09:57Z

cc @PeterF778 @oertl

jkwatson · 2025-09-08T19:44:51Z

+      Attributes attributes,
+      List<LinkData> parentLinks) {
+    TraceState traceState = Span.fromContext(parentContext).getSpanContext().getTraceState();
+    OtelTraceState otTraceState = OtelTraceState.parse(traceState);


teency nit: rename to otelTraceState so as to not confuse my poor brain between this and openTracing.

I thought by now we may have been past that but guess not ;)

jkwatson · 2025-09-08T19:45:57Z

+              otTraceState.getRandomValue(), INVALID_THRESHOLD, otTraceState.getRest());
+    }
+
+    String ot = otTraceState.serialize();


maybe rename to "seralizedState"?

jkwatson · 2025-09-08T20:02:01Z

Seems like this would be an excellent target for some property-based tests. I can't remember if we have any in here already, but it might be a good opportunity, since we can very clearly define the invariants, and generate random inputs, verifying the outputs match the expectations.

anuraaga · 2025-09-09T08:13:41Z

Thanks - we have fuzz tests already which I think are basically what you're getting at. Sounds good will add some

jkwatson · 2025-09-09T18:00:19Z

Thanks - we have fuzz tests already which I think are basically what you're getting at. Sounds good will add some

Fuzz testing is slightly different, in that the goal is usually just to make sure that things don't break, rather than testing invariants, but if you can use the same framework to accomplish property testing, that would be awesome.

PeterF778 · 2025-09-10T21:15:30Z

+    boolean sampled = false;
+    if (isValidThreshold(intent.getThreshold())) {
+      thresholdReliable = intent.isThresholdReliable();
+      long randomValue;


If the threshold is not reliable, the sampler must not use the random value from the trace state or from the trace-id. This is because we cannot guarantee that these "random" values have the required uniform distribution over the (MIN_THRESHOLD, MAX_THRESHOLD) interval.

I can't find anything in the spec on this. I noticed the caveat to deal with rate limiting sampler but since that's also not in the spec went with the simpler implementation. Is this something to change now or can we follow up after spec clarification?

The spec changes are pending, but you can see how this case is handled in https://github.com/open-telemetry/opentelemetry-java-contrib/blob/main/consistent-sampling/src/main/java/io/opentelemetry/contrib/sampler/consistent56/ConsistentSampler.java

Ok went ahead and switched to generating a random when unreliable

anuraaga · 2025-09-11T09:44:00Z

+    boolean sampled = false;
+    if (isValidThreshold(intent.getThreshold())) {
+      thresholdReliable = intent.isThresholdReliable();
+      long randomValue;


Ok went ahead and switched to generating a random when unreliable

anuraaga · 2025-09-11T09:44:50Z

+      Attributes attributes,
+      List<LinkData> parentLinks) {
+    TraceState traceState = Span.fromContext(parentContext).getSpanContext().getTraceState();
+    OtelTraceState otTraceState = OtelTraceState.parse(traceState);


I thought by now we may have been past that but guess not ;)

anuraaga · 2025-09-11T09:45:27Z

+      assertState(output, rv, th);
+    }
+
+    private static void assertState(OtelTraceState state, long rv, long th) {


@jkwatson This is what I came up with for testing properties based on fuzz. What do you think?

I can't say I grok all the assertions, but this looks good! Thanks!

anuraaga · 2025-09-18T00:59:27Z

@jkwatson Think we can proceed with this PR, or anything I should look at? Thanks

jkwatson · 2025-09-18T02:48:45Z

@jkwatson Think we can proceed with this PR, or anything I should look at? Thanks

If @PeterF778 can give it a 👍🏽 then I think we're good to go.

PeterF778

Looks nice and clean, thanks!

jkwatson

Thanks!

otelbot · 2025-09-18T16:47:27Z

Thank you for your contribution @anuraaga! 🎉 We would like to hear from you about your experience contributing to OpenTelemetry by taking a few minutes to fill out this survey.

…ry#7626)

Add incubator implementation of composite sampling spec

0b75ab4

anuraaga requested a review from a team as a code owner August 29, 2025 07:29

anuraaga added 2 commits August 29, 2025 16:55

More test coverage

a7bc4e2

More test coverage

8720246

anuraaga mentioned this pull request Sep 1, 2025

Add experimental composite samplers elastic/elastic-otel-java#777

Merged

jkwatson reviewed Sep 8, 2025

View reviewed changes

PeterF778 reviewed Sep 10, 2025

View reviewed changes

Cleanups

65fadc5

anuraaga commented Sep 11, 2025

View reviewed changes

anuraaga added 2 commits September 11, 2025 18:47

Comment

8faec3d

Fix regex

c5ebbb6

PeterF778 approved these changes Sep 18, 2025

View reviewed changes

jkwatson approved these changes Sep 18, 2025

View reviewed changes

jkwatson merged commit 74bc3bd into open-telemetry:main Sep 18, 2025
29 checks passed

the-clam pushed a commit to the-clam/opentelemetry-java that referenced this pull request Nov 6, 2025

Add incubator implementation of composite sampling spec (open-telemet…

7b70296

…ry#7626)

Conversation

anuraaga commented Aug 29, 2025

Uh oh!

codecov Bot commented Aug 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

trask commented Sep 4, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jkwatson commented Sep 8, 2025

Uh oh!

anuraaga commented Sep 9, 2025

Uh oh!

jkwatson commented Sep 9, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

anuraaga commented Sep 18, 2025

Uh oh!

jkwatson commented Sep 18, 2025

Uh oh!

PeterF778 left a comment

Choose a reason for hiding this comment

Uh oh!

jkwatson left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

otelbot Bot commented Sep 18, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

codecov Bot commented Aug 29, 2025 •

edited

Loading