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

dmathieu merged 3 commits into
open-telemetry:mainfrom
ash2k:never-sample

Conversation

@ash2k

@ash2k ash2k commented Mar 9, 2026 •

Copy link
Copy Markdown
Member

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

@codecov

codecov Bot commented Mar 9, 2026 •

Copy link
Copy Markdown

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

ash2k commented Mar 9, 2026

Copy link
Copy Markdown
Member Author

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

dashpole commented Mar 10, 2026 •

Copy link
Copy Markdown
Contributor

@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

ash2k commented Mar 11, 2026

Copy link
Copy Markdown
Member Author

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