[PeterF778]

The Composite Samplers based on Consistent Probability Sampling are described at https://github.com/open-telemetry/opentelemetry-specification/blob/v1.57.0/oteps/trace/0250-Composite_Samplers.md

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 82.45%. Comparing base (79608eb) to head (f8588d2).
⚠️ Report is 75 commits behind head on main.

❌ Your project check has failed because the head coverage (82.30%) is below the target coverage (89.00%). You can increase the head coverage or adjust the target coverage.

Additional details and impacted files

@@             Coverage Diff              @@
##               main    #8450      +/-   ##
============================================
- Coverage     91.02%   82.45%   -8.57%     
- Complexity     7817    10465    +2648     
============================================
  Files           893     1062     +169     
  Lines         23719    32133    +8414     
  Branches       2364     4418    +2054     
============================================
+ Hits          21590    26496    +4906     
- Misses         1408     4810    +3402     
- Partials        721      827     +106     

☔ View full report in Codecov by Harness.
📢 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.

[zeitlinger]

I agree with removing traceId from GetSamplingIntent and with the adjusted-count naming, but the OTEP still lists TraceId as a required Predicate argument. Is that part of the OTEP stale/inconsistent? If so, could you link the spec issue/PR or clarify the intended interpretation here?

[PeterF778]

the OTEP still lists TraceId as a required Predicate argument. Is that part of the OTEP stale/inconsistent? If so, could you link the spec issue/PR or clarify the intended interpretation here?

Good catch! Yes, the OTEP (https://github.com/open-telemetry/opentelemetry-specification/blob/v1.57.0/oteps/trace/0250-Composite_Samplers.md#predicate) lists TraceId as a parameter, but this is a mistake. Predicates and GetSamplingIntent must not depend on trace id for the same reasons.
The prototype implementation (https://github.com/open-telemetry/opentelemetry-java-contrib/blob/main/consistent-sampling/src/main/java/io/opentelemetry/contrib/sampler/consistent56/Predicate.java) does not have this parameter.

[jack-berg]

@PeterF778 this looks good but for the sampling SIG - OTEPs are just the original proposal but stop being the source of truth once they are codified in spec text. In this case, the source of truth is now: https://github.com/open-telemetry/opentelemetry-specification/blob/v1.57.0/specification/trace/sdk.md#getsamplingintent

Can someone from the sampling SIG go and update https://github.com/open-telemetry/opentelemetry-specification/blame/efceef79720efc238645666139a7cee1b0145df1/specification/trace/sdk.md#L661 to reflect your update here? Thanks.

[jack-berg]

Suggested change

	* @return true iff the threshold can be reliably used for adjusted count calculation
	* @return true if the threshold can be reliably used for adjusted count calculation

[jkwatson]

Is this intended to mean "If and only if"? if so, maybe better to spell it out in case someone isn't familiar with the abbreviation.

[PeterF778]

Yes, indeed. Thanks, I'll fix it later today.

[PeterF778]

Can someone from the sampling SIG go and update https://github.com/open-telemetry/opentelemetry-specification/blame/efceef79720efc238645666139a7cee1b0145df1/specification/trace/sdk.md#L661 to reflect your update here? Thanks.

@jack-berg Does this change require an Issue?

[jack-berg]

Yes, but just as a formality

[linux-foundation-easycla]

The committers listed above are authorized under a signed CLA.

• ✅ login: PeterF778 / name: Peter Findeisen (b837e4c)

[PeterF778]

I messed up the branch with my first commit this morning, so I restored it to the previous state.

[PeterF778]

@PeterF778 this looks good but for the sampling SIG - OTEPs are just the original proposal but stop being the source of truth once they are codified in spec text. In this case, the source of truth is now: https://github.com/open-telemetry/opentelemetry-specification/blob/v1.57.0/specification/trace/sdk.md#getsamplingintent

Can someone from the sampling SIG go and update https://github.com/open-telemetry/opentelemetry-specification/blame/efceef79720efc238645666139a7cee1b0145df1/specification/trace/sdk.md#L661 to reflect your update here? Thanks.

The updates to the specs are merged now (open-telemetry/opentelemetry-specification#5161). Can we proceed with this PR?