fix metrics race condition (#1552)

open-telemetry · Aug 11, 2022 · 8844771 · 8844771 · github-actions · Aug 11, 2022
1 parent 4cf41c4
commit 8844771
Show file tree

Hide file tree

Showing 6 changed files with 8 additions and 3 deletions.
diff --git 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
@@ -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
@@ -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
@@ -93,6 +93,7 @@ std::unique_ptr<Aggregation> LongHistogramAggregation::Diff(const Aggregation &n
 
 PointType LongHistogramAggregation::ToPoint() const noexcept
 {
+  const std::lock_guard<opentelemetry::common::SpinLockMutex> locked(lock_);
   return point_data_;
 }
 
@@ -176,6 +177,7 @@ std::unique_ptr<Aggregation> DoubleHistogramAggregation::Diff(
 
 PointType DoubleHistogramAggregation::ToPoint() const noexcept
 {
+  const std::lock_guard<opentelemetry::common::SpinLockMutex> locked(lock_);
   return point_data_;
 }
 

diff --git a/sdk/src/metrics/aggregation/lastvalue_aggregation.cc b/sdk/src/metrics/aggregation/lastvalue_aggregation.cc
@@ -68,6 +68,7 @@ std::unique_ptr<Aggregation> LongLastValueAggregation::Diff(const Aggregation &n
 
 PointType LongLastValueAggregation::ToPoint() const noexcept
 {
+  const std::lock_guard<opentelemetry::common::SpinLockMutex> locked(lock_);
   return point_data_;
 }
 
@@ -126,6 +127,7 @@ std::unique_ptr<Aggregation> DoubleLastValueAggregation::Diff(
 
 PointType DoubleLastValueAggregation::ToPoint() const noexcept
 {
+  const std::lock_guard<opentelemetry::common::SpinLockMutex> 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
@@ -56,6 +56,7 @@ std::unique_ptr<Aggregation> LongSumAggregation::Diff(const Aggregation &next) c
 
 PointType LongSumAggregation::ToPoint() const noexcept
 {
+  const std::lock_guard<opentelemetry::common::SpinLockMutex> locked(lock_);
   return point_data_;
 }
-Original file line number
+Diff line change
@@ Expand Up @@
     PointType LongSumAggregation::ToPoint() const noexcept
     {
+      const std::lock_guard<opentelemetry::common::SpinLockMutex> locked(lock_);
       return point_data_;
     }
@@ Expand Down @@
Benchmark suite	Current: `8844771`	Previous: `4cf41c4`	Ratio
`BM_ExtractBaggageHavingTenEntries`	`3.733669064067311` ns/iter	`1.6299645764651691` ns/iter	`2.29`
`BM_ExtractBaggageWith180Entries`	`3.9675012570236956` ns/iter	`1.6148549386566466` ns/iter	`2.46`
`BM_NaiveSpinLockThrashing/1/process_time/real_time`	`0.2528039345610737` ms/iter	`0.10703374411313589` ms/iter	`2.36`