Skip to content

fix(sdk/metric): Fix data race with initialization of NewPeriodicReader - #9028

Merged
dmathieu merged 12 commits into
open-telemetry:mainfrom
CTrando:fix-periodic-reader-inst-race
Oct 1, 2026
Merged

dmathieu merged 12 commits into
open-telemetry:mainfrom
CTrando:fix-periodic-reader-inst-race

Conversation

@CTrando

@CTrando CTrando commented Sep 24, 2026

Copy link
Copy Markdown
Contributor

Fixes #8769

Just have to move the initialization of a field before the goroutine starts up. Test works by creating a bunch of workers with a small timeout on the goroutine to replicate the data race.

You have to run the test with the -race flag if you want to reproduce the data race test failure.

@linux-foundation-easycla

linux-foundation-easycla Bot commented Sep 24, 2026 •

Copy link
Copy Markdown

CLA Signed
The committers listed above are authorized under a signed CLA.

  • ✅ login: CTrando / name: Cameron Trando (81fadb8)

@codecov

codecov Bot commented Sep 25, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 89.1%. Comparing base (16b1798) to head (31d7f64).

Additional details and impacted files

Impacted file tree graph

@@          Coverage Diff          @@
##            main   #9028   +/-   ##
=====================================
  Coverage   89.1%   89.1%           
=====================================
  Files        338     338           
  Lines      22274   22274           
=====================================
+ Hits       19854   19855    +1     
+ Misses      2420    2419    -1     
Files with missing lines Coverage Δ
sdk/metric/periodic_reader.go 87.4% <100.0%> (ø)

... and 1 file with indirect coverage changes

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

Comment thread sdk/metric/periodic_reader_test.go Outdated

@ps-mir ps-mir left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The fix looks ok. But test is not a reliable regression guard, specially when cpu contention is high.

Is there a way observ.NewInstrumentation can be held long enough. TestPeriodicReaderInstrumentationError could be of some use.

Comment thread sdk/metric/periodic_reader_test.go Outdated
@CTrando
CTrando requested a review from ps-mir September 28, 2026 17:03

@ps-mir ps-mir left a comment •

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Much better, This test is deterministic in producing the Reader non registration and fails 60/60 without fix.

A efficiency nit for reducing time.After.

Comment thread sdk/metric/periodic_reader_test.go Outdated
@CTrando
CTrando requested a review from MrAlias September 30, 2026 17:44
@CTrando

CTrando commented Sep 30, 2026

Copy link
Copy Markdown
Contributor Author

Looks like I need another approval, then I can merge this 🙇

@ps-mir

ps-mir commented Oct 1, 2026

Copy link
Copy Markdown
Member

Looks like I need another approval, then I can merge this 🙇

PRs need two approvals for merge. Also only maintainers can do the merge.

@dmathieu
dmathieu merged commit d760665 into open-telemetry:main Oct 1, 2026
51 checks passed
pellared added a commit that referenced this pull request Oct 2, 2026
This release contains the first stable release of the OpenTelemetry Go
Logs API and SDK.
Our project stability guarantees now apply to the following modules:

- `go.opentelemetry.io/otel/log`
- `go.opentelemetry.io/otel/sdk/log`

See our [versioning
policy](https://github.com/open-telemetry/opentelemetry-go/blob/main/VERSIONING.md)
for more information about these stability guarantees.

### Added

- Add `String` method for `KeyValue` type in
`go.opentelemetry.io/otel/attribute`. (#8205)
- Add `String` method for `Set` type in
`go.opentelemetry.io/otel/attribute`. (#8347)
- Add `WithMaxExportBatchSize` to `go.opentelemetry.io/otel/sdk/metric`
to configure the maximum export batch size for `PeriodicReader`. (#8960)
- Add `WithAttributeValueDepthLimit`, `DefaultAttributeValueDepthLimit`,
and `SpanLimits.AttributeValueDepthLimit` in
`go.opentelemetry.io/otel/sdk/trace` to configure the maximum depth of
span, event, link, and instrumentation scope attribute values. (#8938)
- Add `WithAttributeValueDepthLimit` in
`go.opentelemetry.io/otel/sdk/log` to configure the maximum depth of log
record and instrumentation scope attribute values. (#8938)
- Add `WithMaxResponseSize` to
`go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracehttp`,
`go.opentelemetry.io/otel/exporters/otlp/otlpmetric/otlpmetrichttp`, and
  `go.opentelemetry.io/otel/exporters/otlp/otlplog/otlploghttp`. (#8941)
- Add experimental `ProbabilitySampler` in
`go.opentelemetry.io/otel/sdk/trace/x` that conforms to the
[OpenTelemetry specification's threshold-based sampling
algorithm](https://opentelemetry.io/docs/specs/otel/trace/sdk/#probabilitysampler).
(#8123)
- Add experimental `*Binder` extension interfaces in
`go.opentelemetry.io/otel/metric/x` for instruments that support binding
attributes ahead of time. (#8760)
- Add the experimental `Finisher` synchronous metric instrument
extension interface to `go.opentelemetry.io/otel/metric/x`. (#8906)

### Changed

- Use the [OpenTelemetry Attribute Collection representation for
non-OTLP
protocols](https://opentelemetry.io/docs/specs/otel/common/#attribute-collection-representation-for-non-otlp)
for metric data diffs in
`go.opentelemetry.io/otel/sdk/metric/metricdata/metricdatatest`. (#8993)
- Apply a maximum depth of 64 by default to span, event, link, log
record, and instrumentation scope attribute values in
`go.opentelemetry.io/otel/sdk/trace` and
`go.opentelemetry.io/otel/sdk/log`. (#8938)
- Reduce allocations when applying attribute value length limits in
`go.opentelemetry.io/otel/sdk/trace` and
`go.opentelemetry.io/otel/sdk/log` by rebuilding composite values only
when truncation is needed. (#8912)
- Decode `traceparent` using a hex lookup table in
`go.opentelemetry.io/otel/propagation`, which rejects the
specification-disallowed upper-case characters without a separate scan
of the header. (#8739)
- Decode all characters at constant indices in `TraceIDFromHex` and
`SpanIDFromHex` in `go.opentelemetry.io/otel/trace` to eliminate bounds
checks. (#8740)

### Fixed

- Count valid U+FFFD replacement characters toward attribute value
length limits in `go.opentelemetry.io/otel/trace`,
`go.opentelemetry.io/otel/sdk/trace`, and
`go.opentelemetry.io/otel/sdk/log`.
- Truncate string attribute values to empty strings when the configured
length limit is zero, including malformed UTF-8, in
`go.opentelemetry.io/otel/trace`, `go.opentelemetry.io/otel/sdk/trace`,
and `go.opentelemetry.io/otel/sdk/log`. (#9054)
- Fix a data race in `NewPeriodicReader` where `r.inst` was assigned
after the background goroutine was launched in
`go.opentelemetry.io/otel/sdk/metric`. (#9028)
- Prevent precision loss for large `int64` values in `Sum`, `Histogram`,
and `ExponentialHistogram` aggregations in
`go.opentelemetry.io/otel/sdk/metric`. (#8981)
- Ensure `grpc.DialOption` values passed via `WithDialOption` take
precedence over conflicting internally-computed defaults in
  `go.opentelemetry.io/otel/exporters/otlp/otlplog/otlploggrpc`,
`go.opentelemetry.io/otel/exporters/otlp/otlpmetric/otlpmetricgrpc`, and
`go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc`.
(#8805, #8834, #8836)
- Treat overflowing `OTEL_METRIC_EXPORT_INTERVAL` and
`OTEL_METRIC_EXPORT_TIMEOUT` values as invalid in
`go.opentelemetry.io/otel/sdk/metric`. (#8800)
- Prevent a panic in `NewFixedSizeReservoir` and
`FixedSizeReservoirProvider` when given a negative size in
`go.opentelemetry.io/otel/sdk/metric/exemplar`; negative sizes are now
clamped to zero, consistent with a size of zero. (#8832)
- Preserve exponential histogram `ZeroThreshold` during OTLP metric
export in `go.opentelemetry.io/otel/exporters/otlp/otlpmetric`. (#8801)
- Ensure metric helpers in `go.opentelemetry.io/otel/semconv/v1.32.0`,
`go.opentelemetry.io/otel/semconv/v1.33.0`, and
`go.opentelemetry.io/otel/semconv/v1.34.0` record measurements only once
when no attributes are provided. (#8849)
- Treat empty `OTEL_TRACES_SAMPLER` and `OTEL_TRACES_SAMPLER_ARG` values
as unset in `go.opentelemetry.io/otel/sdk/trace`. (#8873)
- Normalize global `OTEL_EXPORTER_OTLP_ENDPOINT` path joining in
`go.opentelemetry.io/otel/exporters/otlp/otlplog/otlploghttp` so
trailing-slash base URLs do not produce double-slash log export paths.
(#8864)
- Prevent a deadlock when formatting an error passed to `RecordError`
calls back into the span in `go.opentelemetry.io/otel/sdk/trace`.
(#8815)
- Prevent `SimpleSpanProcessor.Shutdown` from panicking when constructed
with a nil exporter in `go.opentelemetry.io/otel/sdk/trace`. (#8844)
- Propagate invalid exponential histogram scale errors to Prometheus
exporter self-observability metrics in
`go.opentelemetry.io/otel/exporters/prometheus`. (#8839)
- Retain instrument advisory attributes configured with
`go.opentelemetry.io/otel/metric/x.WithDefaultAttributes` when a
matching View in `go.opentelemetry.io/otel/sdk/metric` does not specify
an attribute filter. (#8859)
- Ignore HTTP URL paths when building OTLP/gRPC metric exporter targets
from `OTEL_EXPORTER_OTLP_ENDPOINT` and
`OTEL_EXPORTER_OTLP_METRICS_ENDPOINT`, while preserving `unix://` and
`unix-abstract://` targets; treat `unix-abstract://` endpoints as
insecure in
`go.opentelemetry.io/otel/exporters/otlp/otlpmetric/otlpmetricgrpc` and
`go.opentelemetry.io/otel/exporters/otlp/otlpmetric/otlpmetrichttp`.
(#8862)
- Ignore HTTP(S) paths when deriving gRPC trace exporter endpoints from
`OTEL_EXPORTER_OTLP_ENDPOINT` and `OTEL_EXPORTER_OTLP_TRACES_ENDPOINT`
in `go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc`.
(#8852)
- Bound memory used while parsing dense W3C property metadata by storing
it compactly and materializing `Property` values on demand in
`go.opentelemetry.io/otel/baggage` and
`go.opentelemetry.io/otel/propagation`.

### Removed

- Remove the experimental `OTEL_GO_X_METRIC_EXPORT_BATCH_SIZE`
environment variable in `go.opentelemetry.io/otel/sdk/metric`.
  Use the `WithMaxExportBatchSize` option instead. (#8960)
@pellared pellared added this to the v1.47.0 milestone Oct 2, 2026
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.

Data race when initializing PeriodicReader with short interval

5 participants