From 206ac291d0199b145079cfaa8207812dd64c5f7f Mon Sep 17 00:00:00 2001 From: David Ashpole Date: Mon, 16 Mar 2026 09:30:51 -0400 Subject: [PATCH] Fix race in the lastvalue aggregation where 0 could be observed (#8056) This initializes the value of the gauge with the correct value before it is stored in the map. We end up storing the same thing twice on the first call, but that isn't a big deal performance-wise. Discovered during https://github.com/open-telemetry/opentelemetry-go/pull/8021 --- CHANGELOG.md | 1 + sdk/metric/internal/aggregate/lastvalue.go | 4 +++- sdk/metric/internal/aggregate/lastvalue_test.go | 3 --- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 322ff2130..69a31b74a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -28,6 +28,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm - 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) diff --git a/sdk/metric/internal/aggregate/lastvalue.go b/sdk/metric/internal/aggregate/lastvalue.go index 4924d732c..66d1f074a 100644 --- a/sdk/metric/internal/aggregate/lastvalue.go +++ b/sdk/metric/internal/aggregate/lastvalue.go @@ -31,10 +31,12 @@ func (s *lastValueMap[N]) measure( droppedAttr []attribute.KeyValue, ) { lv := s.values.LoadOrStoreAttr(fltrAttr, func(attr attribute.Set) any { - return &lastValuePoint[N]{ + p := &lastValuePoint[N]{ res: s.newRes(attr), attrs: attr, } + p.value.Store(value) + return p }).(*lastValuePoint[N]) lv.value.Store(value) diff --git a/sdk/metric/internal/aggregate/lastvalue_test.go b/sdk/metric/internal/aggregate/lastvalue_test.go index 9bc2851b5..ff6bd7ef4 100644 --- a/sdk/metric/internal/aggregate/lastvalue_test.go +++ b/sdk/metric/internal/aggregate/lastvalue_test.go @@ -490,9 +490,6 @@ func validateGauge[N int64 | float64](t *testing.T, aggs []metricdata.Aggregatio for _, v := range getConcurrentVals[N]() { valid[v] = true } - // TODO(dashpole): Fix a concurrency bug where a gauge can be collected with - // the value zero even when no zero-value measurements have been recorded. - valid[0] = true for _, agg := range aggs { s, ok := agg.(metricdata.Gauge[N])