From 402a0bd6f40d41df496d1206237efeae17a16c4a Mon Sep 17 00:00:00 2001 From: Ehsan Saei <71217171+esigo@users.noreply.github.com> Date: Thu, 11 Aug 2022 19:23:16 +0200 Subject: [PATCH] fix metrics race condition (#1552) --- .../sdk/metrics/aggregation/histogram_aggregation.h | 2 +- .../sdk/metrics/aggregation/lastvalue_aggregation.h | 2 +- .../opentelemetry/sdk/metrics/aggregation/sum_aggregation.h | 2 +- sdk/src/metrics/aggregation/histogram_aggregation.cc | 2 ++ sdk/src/metrics/aggregation/lastvalue_aggregation.cc | 2 ++ sdk/src/metrics/aggregation/sum_aggregation.cc | 1 + 6 files changed, 8 insertions(+), 3 deletions(-) diff --git a/sdk/include/opentelemetry/sdk/metrics/aggregation/histogram_aggregation.h b/sdk/include/opentelemetry/sdk/metrics/aggregation/histogram_aggregation.h index b39f442849..9e53bd45f0 100644 --- a/sdk/include/opentelemetry/sdk/metrics/aggregation/histogram_aggregation.h +++ b/sdk/include/opentelemetry/sdk/metrics/aggregation/histogram_aggregation.h @@ -40,7 +40,7 @@ class LongHistogramAggregation : public Aggregation PointType ToPoint() const noexcept override; private: - opentelemetry::common::SpinLockMutex lock_; + mutable opentelemetry::common::SpinLockMutex lock_; HistogramPointData point_data_; bool record_min_max_ = true; }; diff --git a/sdk/include/opentelemetry/sdk/metrics/aggregation/lastvalue_aggregation.h b/sdk/include/opentelemetry/sdk/metrics/aggregation/lastvalue_aggregation.h index 3b2c08f8ce..b5439b7969 100644 --- a/sdk/include/opentelemetry/sdk/metrics/aggregation/lastvalue_aggregation.h +++ b/sdk/include/opentelemetry/sdk/metrics/aggregation/lastvalue_aggregation.h @@ -31,7 +31,7 @@ class LongLastValueAggregation : public Aggregation PointType ToPoint() const noexcept override; private: - opentelemetry::common::SpinLockMutex lock_; + mutable opentelemetry::common::SpinLockMutex lock_; LastValuePointData point_data_; }; diff --git a/sdk/include/opentelemetry/sdk/metrics/aggregation/sum_aggregation.h b/sdk/include/opentelemetry/sdk/metrics/aggregation/sum_aggregation.h index 14f13bd727..0d08cc35ec 100644 --- a/sdk/include/opentelemetry/sdk/metrics/aggregation/sum_aggregation.h +++ b/sdk/include/opentelemetry/sdk/metrics/aggregation/sum_aggregation.h @@ -32,7 +32,7 @@ class LongSumAggregation : public Aggregation PointType ToPoint() const noexcept override; private: - opentelemetry::common::SpinLockMutex lock_; + mutable opentelemetry::common::SpinLockMutex lock_; SumPointData point_data_; }; diff --git a/sdk/src/metrics/aggregation/histogram_aggregation.cc b/sdk/src/metrics/aggregation/histogram_aggregation.cc index 9a81c0ac82..ba25b634ad 100644 --- a/sdk/src/metrics/aggregation/histogram_aggregation.cc +++ b/sdk/src/metrics/aggregation/histogram_aggregation.cc @@ -93,6 +93,7 @@ std::unique_ptr LongHistogramAggregation::Diff(const Aggregation &n PointType LongHistogramAggregation::ToPoint() const noexcept { + const std::lock_guard locked(lock_); return point_data_; } @@ -176,6 +177,7 @@ std::unique_ptr DoubleHistogramAggregation::Diff( PointType DoubleHistogramAggregation::ToPoint() const noexcept { + const std::lock_guard locked(lock_); return point_data_; } diff --git a/sdk/src/metrics/aggregation/lastvalue_aggregation.cc b/sdk/src/metrics/aggregation/lastvalue_aggregation.cc index a125005335..c5bb684f05 100644 --- a/sdk/src/metrics/aggregation/lastvalue_aggregation.cc +++ b/sdk/src/metrics/aggregation/lastvalue_aggregation.cc @@ -68,6 +68,7 @@ std::unique_ptr LongLastValueAggregation::Diff(const Aggregation &n PointType LongLastValueAggregation::ToPoint() const noexcept { + const std::lock_guard locked(lock_); return point_data_; } @@ -126,6 +127,7 @@ std::unique_ptr DoubleLastValueAggregation::Diff( PointType DoubleLastValueAggregation::ToPoint() const noexcept { + const std::lock_guard locked(lock_); return point_data_; } } // namespace metrics diff --git a/sdk/src/metrics/aggregation/sum_aggregation.cc b/sdk/src/metrics/aggregation/sum_aggregation.cc index 5ca786496e..cb265733b1 100644 --- a/sdk/src/metrics/aggregation/sum_aggregation.cc +++ b/sdk/src/metrics/aggregation/sum_aggregation.cc @@ -56,6 +56,7 @@ std::unique_ptr LongSumAggregation::Diff(const Aggregation &next) c PointType LongSumAggregation::ToPoint() const noexcept { + const std::lock_guard locked(lock_); return point_data_; }