Skip to content

Clarify in Composite Samplers OTEP the unreliable threshold case#4569

Merged
carlosalberto merged 5 commits intoopen-telemetry:mainfrom
PeterF778:4566_clarifications_for_Composite_Samplers_proposal
Aug 14, 2025
Merged

Clarify in Composite Samplers OTEP the unreliable threshold case#4569
carlosalberto merged 5 commits intoopen-telemetry:mainfrom
PeterF778:4566_clarifications_for_Composite_Samplers_proposal

Conversation

@PeterF778
Copy link
Copy Markdown
Contributor

Addressing issue #4566.

Fixes #4566

Changes

Clarifying the meaning of IsAjustedCountReliable, providing guidance for selecting the Randomness value R for sampling decision making.

@PeterF778 PeterF778 requested review from a team June 24, 2025 20:46
@carlosalberto
Copy link
Copy Markdown
Contributor

We usually don't update OTEPs, and instead clarify or refine things in the actual Spec contents. Probably worth checking this clarification exists in the Spec (too)? Wdyt @jmacd ?

@jmacd
Copy link
Copy Markdown
Contributor

jmacd commented Jul 1, 2025

I would feel comfortable adjusting this in the specification directly. It seems like a very small change to write a new OTEP.

@github-actions
Copy link
Copy Markdown

github-actions Bot commented Jul 9, 2025

This PR was marked stale due to lack of activity. It will be closed in 7 days.

@github-actions github-actions Bot added the Stale label Jul 9, 2025
@github-actions
Copy link
Copy Markdown

Closed as inactive. Feel free to reopen if this PR is still being worked on.

@github-actions github-actions Bot closed this Jul 16, 2025
@carlosalberto carlosalberto reopened this Jul 17, 2025
Comment thread oteps/trace/0250-Composite_Samplers.md Outdated
@github-actions github-actions Bot removed the Stale label Jul 18, 2025
@carlosalberto
Copy link
Copy Markdown
Contributor

@kentquirk @jmacd @oertl ping ;)

Copy link
Copy Markdown
Contributor

@jmacd jmacd left a comment

Choose a reason for hiding this comment

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

This is excellent, thank you @PeterF778. I will recount that over 4 OTEPs on the topic of sampling and as many years, we've repeatedly searched for how to represent something I have ill-described as "zero adjusted count". The idea that we let spans through a probabilistic sampler that are "not sampled" is a repeated theme, and we hadn't quite uncovered the relationship between "not sampled" and "is_threshold_reliable".

This PR solves the problem! There is still no such thing as "not sampled", instead we have "sampled with unknown randomness".💯

Comment thread oteps/trace/0250-Composite_Samplers.md Outdated
Co-authored-by: Kent Quirk <kentquirk@gmail.com>
@github-actions
Copy link
Copy Markdown

github-actions Bot commented Aug 8, 2025

This PR was marked stale due to lack of activity. It will be closed in 7 days.

@github-actions github-actions Bot added the Stale label Aug 8, 2025
@jmacd jmacd removed the Stale label Aug 14, 2025
@jmacd
Copy link
Copy Markdown
Contributor

jmacd commented Aug 14, 2025

d189076

@carlosalberto carlosalberto changed the title 4566 Clarifications for Composite Samplers proposal Clarify in Composite Samplers OTEP the unreliable threshold case Aug 14, 2025
Comment thread oteps/trace/0250-Composite_Samplers.md
@carlosalberto carlosalberto added this pull request to the merge queue Aug 14, 2025
Merged via the queue into open-telemetry:main with commit d43e95b Aug 14, 2025
6 checks passed
@carlosalberto carlosalberto mentioned this pull request Sep 9, 2025
github-merge-queue Bot pushed a commit that referenced this pull request Sep 16, 2025
### Entities

- Specify entity information via an environment variable.

([#4594](#4594))

### Common

- OTLP Exporters may allow devs to prepend a product identifier in
`User-Agent` header.

([#4560](#4560))
- ⚠️ **IMPORTANT**: Extending the set of standard attribute value types
is no longer a breaking change.

([#4614](#4614))

### OTEPs

- Clarify in Composite Samplers OTEP the unreliable threshold case.

([#4569](#4569))

---------

Co-authored-by: Armin Ruech <7052238+arminru@users.noreply.github.com>
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.

Composite Samplers Proposal needs clarifications

5 participants