Skip to content

Conversation

jmacd
Copy link
Contributor

@jmacd jmacd commented Jul 29, 2024

Fixes #1413.

Changes

Updates Trace SDK and TraceState handling specifications with OTEP 235 sampling thresholds. This PR depends on #4162 to introduce the concept of Trace Randomness. This PR is the second part of two, it focuses on thresholds.

  • Revise TraceIdRatioBased algorithm section. The existing TODO implies this is not a breaking change.
  • Change text about TraceIdRatioBased construction
  • Move text about TraceIdRatioBased description (leave unmodified).

The content of OTEP 235 was revised for clarity by @kalyanaj in open-telemetry/oteps#261. I've heavily copied from the final text in that still-unmerged OTEP. I introduced new content explaining how to compute thresholds from probabilities with use of variable precision, referring to the OTel Collector-Contrib pkg/sampling reference implementation. The new (Golang) demonstration code is validated here, https://go.dev/play/p/7eLM6FkuoA5.

A proof of concept for this specification along with #4162 can be found in open-telemetry/opentelemetry-go#5645.

Part of #3602.

Product of the Sampling SIG members @kentquirk @kalyanaj @oertl @PeterF778 and myself.

@jmacd
Copy link
Contributor Author

jmacd commented Jul 30, 2024

Feedback from the OTel Spec SIG meeting discussion cc/ @jsuereth:

  • Please add a migration guide to explain how transitioning samplers will work; in particular, it's not safe to begin using non-root independent sampling until TraceIdRatioBased samplers are replaced everywhere in a trace. Until then, only safe to continue using ParentBased sampling w/ root TraceIdRatioBased decision.

Update: 68fa270

Copy link

github-actions bot commented Aug 7, 2024

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

@github-actions github-actions bot removed the Stale label Aug 8, 2024
jmacd added a commit that referenced this pull request Aug 15, 2024
This reduces the number of lines of diff in PR 4166, which replaces the
entire `tracestate-probability-sampling.md` file with new contents.

Part of #4166.

## Changes

Move a file, place a link to it and explain that a change is in
progress.
@jmacd
Copy link
Contributor Author

jmacd commented Aug 15, 2024

@kalyanaj @PeterF778 @oertl @kentquirk Please take another look at this PR, especially the file tracestate-probability-sampling.md which now reads as a new file, not as a major rewrite. The contents are derived from open-telemetry/oteps#261.

@jmacd
Copy link
Contributor Author

jmacd commented Aug 15, 2024

@open-telemetry/specs-trace-approvers @open-telemetry/specs-approvers @open-telemetry/technical-committee this PR has reached consensus in the Sampling SIG, we have multiple prototypes implemented, and we are looking for final approvals.

Copy link

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 Mar 26, 2025
@jmacd
Copy link
Contributor Author

jmacd commented Mar 28, 2025

There had been an example written in Golang for computing the threshold with precision, and (a) I found a bug, (b) I rewrote it so that all the examples are now in Python. I fixed a few other inconsistencies. This PR is up-to-date but should be held until a corresponding draft for OTEP-0250 (#4321) is available.

@jmacd jmacd mentioned this pull request Mar 28, 2025
5 tasks
@jmacd
Copy link
Contributor Author

jmacd commented Mar 28, 2025

With some great luck, I have opened #4466 with the follow-on work (thats +300 PRs).
I believe this is ready to merge.

@jmacd jmacd moved this to In review in Sampling SIG Mar 28, 2025
@reyang
Copy link
Member

reyang commented Apr 2, 2025

markdown-link-check is failing due to HTTP 429 (OpenTelemetry trying to DoS itself? 👀)

image

@reyang reyang enabled auto-merge (squash) April 2, 2025 18:02
@reyang reyang merged commit 26e99d4 into open-telemetry:main Apr 2, 2025
5 of 6 checks passed
@github-project-automation github-project-automation bot moved this from In review to Done in Sampling SIG Apr 2, 2025
@carlosalberto carlosalberto mentioned this pull request Apr 8, 2025
carlosalberto added a commit that referenced this pull request Apr 15, 2025
### Context

- Add context propagation through Environment Variables specification.

([#4454](#4454))
- On Propagators API, stabilize `GetAll` on the `TextMap` Extract.

([#4472](#4472))

### Traces

- Define sampling threshold field in OpenTelemetry TraceState; define
the behavior
of TraceIdRatioBased sampler in terms of W3C Trace Context Level 2
randomness.

([#4166](#4166))

### Metrics

- Clarify SDK behavior for Instrument Advisory Parameter.

([#4389](#4389))

### Logs

- Add `Enabled` opt-in operation to the `LogRecordProcessor`.

([#4439](#4439))
- Stabilize `Logger.Enabled`.

([#4463](#4463))
- Stabilize `EventName`.

([#4475](#4475))

### Baggage

- Add context (baggage) propagation through Environment Variables
specification.

([#4454](#4454))

### Resource

- Add Datamodel for Entities.

([#4442](#4442))

### SDK Configuration

- Convert declarative config env var substitution syntax to ABNF.

([#4448](#4448))
- List declarative config supported SDK extension plugin interfaces.

([#4452](#4452))

---------

Co-authored-by: Armin Ruech <[email protected]>
@carlosalberto carlosalberto mentioned this pull request Jul 10, 2025
github-merge-queue bot pushed a commit that referenced this pull request Jul 18, 2025
July 2025 release.

### Context

- Add Supplementary Guidelines for environment variables as context
carrier
  specification.

([#4548](#4548))

### Traces

- Define sampling threshold field in OpenTelemetry TraceState; define
the behavior
of TraceIdRatioBased sampler in terms of W3C Trace Context Level 2
randomness.

([#4166](#4166))
- Define CompositeSampler implementation and built-in ComposableSampler
interfaces.

([#4466](#4466))
- Define how SDK implements `Tracer.Enabled`.

([#4537](#4537))

### Logs

- Stabilize `Event Name` parameter of `Logger.Enabled`.

([#4534](#4534))
- Stabilize SDK and No-Op `Logger.Enabled`.

([#4536](#4536))
- `SeverityNumber=0` MAY be used to represent an unspecified value.

([#4535](#4535))

### Baggage

- Add Supplementary Guidelines for environment variables as context
carrier
  specification.

([#4548](#4548))

### Compatibility

- Clarify expectations about Prometheus content negotiation for metric
names.

([#4543](#4543))

---------

Co-authored-by: Armin Ruech <[email protected]>
Co-authored-by: Trask Stalnaker <[email protected]>
github-merge-queue bot pushed a commit that referenced this pull request Sep 30, 2025
Part of #1413 
Fixes #4601 

## Changes

Restores TraceIdRatioBased specification from before
#4166.

* [x] Related issues #1413
* [x] Related [OTEP
235](https://github.com/open-telemetry/opentelemetry-specification/blob/main/oteps/trace/0235-sampling-threshold-in-trace-state.md)
* [x]
[`CHANGELOG.md`](https://github.com/open-telemetry/opentelemetry-specification/blob/main/CHANGELOG.md)
file updated for non-trivial changes

---------

Co-authored-by: Otmar Ertl <[email protected]>
Co-authored-by: Trask Stalnaker <[email protected]>
Co-authored-by: Carlos Alberto Cortez <[email protected]>
github-merge-queue bot pushed a commit that referenced this pull request Oct 10, 2025
Goal is to improve our public documentation, which has two documents
with the same title today, the second one being correct.

Finishes the deprecation process started in #4168.
When #4166 added the new sampling specification, the old one was
renamed.
Now that the new specification is complete in the specification, the old
one is a distraction.

## Changes

This moves the old document into the `oteps/` repository where it will
remain a permanent record of this effort.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

Review approach & specify algorithm for TraceIdRatioBasedSampler (ProbabilitySampler)