Skip to content

fix(sdk/trace): return spec-compliant TraceIdRatioBased description#8027

Merged
dmathieu merged 3 commits into
open-telemetry:mainfrom
ash2k:never-sample
Mar 12, 2026
Merged

fix(sdk/trace): return spec-compliant TraceIdRatioBased description#8027
dmathieu merged 3 commits into
open-telemetry:mainfrom
ash2k:never-sample

Conversation

@ash2k
Copy link
Copy Markdown
Member

@ash2k ash2k commented Mar 9, 2026

Optimize performance when sampling is disabled - no need to calculate fraction based on trace id (in traceIDRatioSampler) if fraction is zero.

@codecov
Copy link
Copy Markdown

codecov Bot commented Mar 9, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 81.5%. Comparing base (f4da59e) to head (9ce33a4).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@          Coverage Diff          @@
##            main   #8027   +/-   ##
=====================================
  Coverage   81.5%   81.5%           
=====================================
  Files        304     304           
  Lines      23438   23448   +10     
=====================================
+ Hits       19124   19133    +9     
  Misses      3927    3927           
- Partials     387     388    +1     
Files with missing lines Coverage Δ
sdk/trace/sampling.go 100.0% <100.0%> (ø)

... and 2 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@ash2k
Copy link
Copy Markdown
Member Author

ash2k commented Mar 9, 2026

Some strange unrelated failures in tests.

Comment thread sdk/trace/sampling.go Outdated
@flc1125

This comment was marked as outdated.

@ash2k ash2k force-pushed the never-sample branch 2 times, most recently from 84f93c4 to d3b9252 Compare March 10, 2026 06:20
@dashpole
Copy link
Copy Markdown
Collaborator

dashpole commented Mar 10, 2026

@ash2k Can you add a test to make sure the descriptions of each are correct?

FYI @open-telemetry/go-maintainers this changes the string result of calling Description() on TraceIDRatioBased(1) from "AlwaysOnSampler" to "TraceIDRatioBased{1}". This is required by the Spec:

Description MUST return a string of the form "TraceIdRatioBased{RATIO}" with RATIO replaced with the Sampler instance’s trace sampling ratio represented as a decimal number.

I think this is unlikely to break users, and is something we should do to make sure we comply with the specification.

@ash2k can you make sure that the changed Description return value is called out in the changelog in the Changed section?

Comment thread sdk/trace/sampling.go
Comment thread sdk/trace/sampling.go Outdated
Comment thread sdk/trace/sampling.go Outdated
@ash2k ash2k changed the title chore(sdk/trace): use NeverSample() when sampling is at or below zero fix(sdk/trace): return spec-compliant TraceIdRatioBased description Mar 11, 2026
@ash2k
Copy link
Copy Markdown
Member Author

ash2k commented Mar 11, 2026

I've made the requested changes, PTAL.

Comment thread CHANGELOG.md Outdated
Co-authored-by: David Ashpole <dashpole@google.com>
@dmathieu dmathieu merged commit ddd2b0e into open-telemetry:main Mar 12, 2026
34 checks passed
@ash2k ash2k deleted the never-sample branch March 12, 2026 09:18
@pellared pellared added this to the v1.43.0 milestone Apr 1, 2026
dmathieu added a commit that referenced this pull request Apr 3, 2026
Release issue:
#8127

## Added

- Add `IsRandom` and `WithRandom` on `TraceFlags`, and `IsRandom` on
`SpanContext` in `go.opentelemetry.io/otel/trace`
for [W3C Trace Context Level 2 Random Trace ID
Flag](https://www.w3.org/TR/trace-context-2/#random-trace-id-flag)
support. (#8012)
- Add service detection with `WithService` in
`go.opentelemetry.io/otel/sdk/resource`. (#7642)
- Add `DefaultWithContext` and `EnvironmentWithContext` in
`go.opentelemetry.io/otel/sdk/resource` to support plumbing
`context.Context` through default and environment detectors. (#8051)
- Support attributes with empty value (`attribute.EMPTY`) in
`go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc`.
(#8038)
- Support attributes with empty value (`attribute.EMPTY`) in
`go.opentelemetry.io/otel/exporters/otlp/otlpmetric/otlpmetricgrpc`.
(#8038)
- Support attributes with empty value (`attribute.EMPTY`) in
`go.opentelemetry.io/otel/exporters/otlp/otlplog/otlploggrpc`. (#8038)
- Support attributes with empty value (`attribute.EMPTY`) in
`go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracehttp`.
(#8038)
- Support attributes with empty value (`attribute.EMPTY`) in
`go.opentelemetry.io/otel/exporters/otlp/otlpmetric/otlpmetrichttp`.
(#8038)
- Support attributes with empty value (`attribute.EMPTY`) in
`go.opentelemetry.io/otel/exporters/otlp/otlplog/otlploghttp`. (#8038)
- Support attributes with empty value (`attribute.EMPTY`) in
`go.opentelemetry.io/otel/sdk/metric/metricdata/metricdatatest`. (#8038)
- Add support for per-series start time tracking for cumulative metrics
in `go.opentelemetry.io/otel/sdk/metric`.
  Set `OTEL_GO_X_PER_SERIES_START_TIMESTAMPS=true` to enable. (#8060)
- Add `WithCardinalityLimitSelector` for metric reader for configuring
cardinality limits specific to the instrument kind. (#7855)

## Changed

- Introduce the `EMPTY` Type in `go.opentelemetry.io/otel/attribute` to
reflect that an empty value is now a valid value, with `INVALID`
remaining as a deprecated alias of `EMPTY`. (#8038)
- Refactor slice handling in `go.opentelemetry.io/otel/attribute` to
optimize short slice values with fixed-size fast paths. (#8039)
- Improve performance of span metric recording in
`go.opentelemetry.io/otel/sdk/trace` by returning early if
self-observability is not enabled. (#8067)
- Improve formatting of metric data diffs in
`go.opentelemetry.io/otel/sdk/metric/metricdata/metricdatatest`. (#8073)

## Deprecated

- Deprecate `INVALID` in `go.opentelemetry.io/otel/attribute`. Use
`EMPTY` instead. (#8038)

## Fixed

- Return spec-compliant `TraceIdRatioBased` description. This is a
breaking behavioral change, but it is necessary to
make the implementation
[spec-compliant](https://opentelemetry.io/docs/specs/otel/trace/sdk/#traceidratiobased).
(#8027)
- Fix a race condition in `go.opentelemetry.io/otel/sdk/metric` where
the lastvalue aggregation could collect the value 0 even when no
zero-value measurements were recorded. (#8056)
- Limit HTTP response body to 4 MiB in
`go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracehttp` to
mitigate excessive memory usage caused by a misconfigured or malicious
server.
Responses exceeding the limit are treated as non-retryable errors.
(#8108)
- Limit HTTP response body to 4 MiB in
`go.opentelemetry.io/otel/exporters/otlp/otlpmetric/otlpmetrichttp` to
mitigate excessive memory usage caused by a misconfigured or malicious
server.
Responses exceeding the limit are treated as non-retryable errors.
(#8108)
- Limit HTTP response body to 4 MiB in
`go.opentelemetry.io/otel/exporters/otlp/otlplog/otlploghttp` to
mitigate excessive memory usage caused by a misconfigured or malicious
server.
Responses exceeding the limit are treated as non-retryable errors.
(#8108)
- `WithHostID` detector in `go.opentelemetry.io/otel/sdk/resource` to
use full path for `kenv` command on BSD. (#8113)
- Fix missing `request.GetBody` in
`go.opentelemetry.io/otel/exporters/otlp/otlplog/otlploghttp` to
correctly handle HTTP2 GOAWAY frame. (#8096)

---------

Co-authored-by: Tyler Yahn <MrAlias@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.

5 participants