diff --git a/CHANGELOG.md b/CHANGELOG.md index 462236afa6..97730e4059 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -56,6 +56,9 @@ Increment the: (programmatic and from yaml) ([#4366](https://github.com/open-telemetry/opentelemetry-cpp/pull/4366)) +* [SDK] Replace SpinLockMutex with std::mutex in the metrics storage clases + [#4416](https://github.com/open-telemetry/opentelemetry-cpp/pull/4416) + * [CONFIGURATION/BUILD] Add resource detector targets and README [#4430](https://github.com/open-telemetry/opentelemetry-cpp/pull/4430) diff --git a/sdk/include/opentelemetry/sdk/metrics/state/async_metric_storage.h b/sdk/include/opentelemetry/sdk/metrics/state/async_metric_storage.h index 674863428b..b1c31cf324 100644 --- a/sdk/include/opentelemetry/sdk/metrics/state/async_metric_storage.h +++ b/sdk/include/opentelemetry/sdk/metrics/state/async_metric_storage.h @@ -63,7 +63,7 @@ class AsyncMetricStorage : public MetricStorage, public AsyncWritableMetricStora // Async counter always record monotonically increasing values, and the // exporter/reader can request either for delta or cumulative value. // So we convert the async counter value to delta before passing it to temporal storage. - std::lock_guard guard(hashmap_lock_); + std::lock_guard guard(hashmap_lock_); #ifdef ENABLE_METRICS_EXEMPLAR_PREVIEW const bool offer_exemplars = ExemplarFilterEnabled(exemplar_filter_type_, opentelemetry::context::Context{}); @@ -130,7 +130,7 @@ class AsyncMetricStorage : public MetricStorage, public AsyncWritableMetricStora std::shared_ptr delta_metrics = nullptr; { - std::lock_guard guard(hashmap_lock_); + std::lock_guard guard(hashmap_lock_); delta_metrics = std::move(delta_hash_map_); delta_hash_map_ = std::make_unique(aggregation_config_->cardinality_limit_); @@ -148,7 +148,7 @@ class AsyncMetricStorage : public MetricStorage, public AsyncWritableMetricStora const AggregationConfig *aggregation_config_; std::unique_ptr cumulative_hash_map_; std::unique_ptr delta_hash_map_; - opentelemetry::common::SpinLockMutex hashmap_lock_; + std::mutex hashmap_lock_; #ifdef ENABLE_METRICS_EXEMPLAR_PREVIEW ExemplarFilterType exemplar_filter_type_; nostd::shared_ptr exemplar_reservoir_; diff --git a/sdk/include/opentelemetry/sdk/metrics/state/sync_metric_storage.h b/sdk/include/opentelemetry/sdk/metrics/state/sync_metric_storage.h index c8d58bb7b2..16289d9d73 100644 --- a/sdk/include/opentelemetry/sdk/metrics/state/sync_metric_storage.h +++ b/sdk/include/opentelemetry/sdk/metrics/state/sync_metric_storage.h @@ -11,7 +11,6 @@ #include #include "opentelemetry/common/key_value_iterable.h" -#include "opentelemetry/common/spin_lock_mutex.h" #include "opentelemetry/common/timestamp.h" #include "opentelemetry/context/context.h" #include "opentelemetry/nostd/function_ref.h" @@ -197,7 +196,7 @@ class SyncMetricStorage : public MetricStorage, public SyncWritableMetricStorage std::shared_ptr Bind( const opentelemetry::common::KeyValueIterable &attributes) noexcept override; - // Internal: stable bound entry. Self-contained: owns its own spinlock and + // Internal: stable bound entry. Self-contained: owns its own mutex and // aggregation so the user-held handle stays safe to call even if the parent // SyncMetricStorage is destroyed first (writes simply have no observer). // Collect() rotates current_ when dirty so bound + unbound writes for the @@ -226,7 +225,7 @@ class SyncMetricStorage : public MetricStorage, public SyncWritableMetricStorage InstrumentValueType value_type_; MetricAttributes attributes_; // Protected by lock_. - opentelemetry::common::SpinLockMutex lock_; + std::mutex lock_; std::unique_ptr current_; bool dirty_ = false; }; diff --git a/sdk/include/opentelemetry/sdk/metrics/state/temporal_metric_storage.h b/sdk/include/opentelemetry/sdk/metrics/state/temporal_metric_storage.h index d86093c376..50fa1a2240 100644 --- a/sdk/include/opentelemetry/sdk/metrics/state/temporal_metric_storage.h +++ b/sdk/include/opentelemetry/sdk/metrics/state/temporal_metric_storage.h @@ -5,9 +5,9 @@ #include #include +#include #include -#include "opentelemetry/common/spin_lock_mutex.h" #include "opentelemetry/common/timestamp.h" #include "opentelemetry/nostd/function_ref.h" #include "opentelemetry/nostd/span.h" @@ -54,7 +54,7 @@ class TemporalMetricStorage std::unordered_map last_reported_metrics_; // Lock while building metrics - mutable opentelemetry::common::SpinLockMutex lock_; + mutable std::mutex lock_; const AggregationConfig *aggregation_config_; opentelemetry::common::SystemTimestamp last_delta_collection_ts_; bool has_last_delta_collection_ts_ = false; diff --git a/sdk/src/metrics/state/sync_metric_storage.cc b/sdk/src/metrics/state/sync_metric_storage.cc index 245ef77454..92d1262e74 100644 --- a/sdk/src/metrics/state/sync_metric_storage.cc +++ b/sdk/src/metrics/state/sync_metric_storage.cc @@ -22,7 +22,6 @@ # include # include -# include "opentelemetry/common/spin_lock_mutex.h" # include "opentelemetry/sdk/common/global_log_handler.h" # include "opentelemetry/sdk/metrics/aggregation/aggregation.h" # include "opentelemetry/sdk/metrics/data/exemplar_data.h" @@ -47,7 +46,7 @@ bool SyncMetricStorage::Collect(CollectorHandle *collector, std::shared_ptr delta_metrics = nullptr; #ifdef OPENTELEMETRY_HAVE_METRICS_BOUND_INSTRUMENTS_PREVIEW // Snapshot of bound entries (under map lock) that we will rotate without - // holding the map lock. Each entry has its own spinlock for the swap. + // holding the map lock. Each entry has its own mutex for the swap. std::vector> entry_snapshot; #endif { @@ -66,7 +65,7 @@ bool SyncMetricStorage::Collect(CollectorHandle *collector, bool can_erase = false; if (it->second.use_count() == 1) { - std::lock_guard g(it->second->lock_); + std::lock_guard g(it->second->lock_); can_erase = !it->second->dirty_; } if (can_erase) @@ -95,7 +94,7 @@ bool SyncMetricStorage::Collect(CollectorHandle *collector, } #ifdef OPENTELEMETRY_HAVE_METRICS_BOUND_INSTRUMENTS_PREVIEW - // Rotate dirty bound entries: under each entry's own spinlock, swap out the + // Rotate dirty bound entries: under each entry's own mutex, swap out the // current aggregation and merge it into delta_metrics so bound + unbound // writes for the same post-filter attribute set produce one datapoint. for (auto &entry : entry_snapshot) @@ -103,7 +102,7 @@ bool SyncMetricStorage::Collect(CollectorHandle *collector, std::unique_ptr rotated; MetricAttributes attrs_copy; { - std::lock_guard g(entry->lock_); + std::lock_guard g(entry->lock_); if (!entry->dirty_) { continue; @@ -160,7 +159,7 @@ bool SyncMetricStorage::Collect(CollectorHandle *collector, // satisfy the documented invariant on dirty_. bool entry_dirty = false; { - std::lock_guard g(it->second->lock_); + std::lock_guard g(it->second->lock_); entry_dirty = it->second->dirty_; } if (entry_dirty) @@ -228,7 +227,7 @@ void SyncMetricStorage::BoundEntry::RecordLong(int64_t value) noexcept "is not long"); return; } - std::lock_guard guard(lock_); + std::lock_guard guard(lock_); current_->Aggregate(value); dirty_ = true; } @@ -242,7 +241,7 @@ void SyncMetricStorage::BoundEntry::RecordDouble(double value) noexcept "is not double"); return; } - std::lock_guard guard(lock_); + std::lock_guard guard(lock_); current_->Aggregate(value); dirty_ = true; } diff --git a/sdk/src/metrics/state/temporal_metric_storage.cc b/sdk/src/metrics/state/temporal_metric_storage.cc index 85a0a4b3c0..71af45ab0b 100644 --- a/sdk/src/metrics/state/temporal_metric_storage.cc +++ b/sdk/src/metrics/state/temporal_metric_storage.cc @@ -9,7 +9,6 @@ #include #include -#include "opentelemetry/common/spin_lock_mutex.h" #include "opentelemetry/common/timestamp.h" #include "opentelemetry/nostd/function_ref.h" #include "opentelemetry/nostd/span.h" @@ -46,7 +45,7 @@ bool TemporalMetricStorage::buildMetrics(CollectorHandle *collector, const std::shared_ptr &delta_metrics, nostd::function_ref callback) noexcept { - std::lock_guard guard(lock_); + std::lock_guard guard(lock_); AggregationTemporality aggregation_temporarily = collector->GetAggregationTemporality(instrument_descriptor_.type_); // Per OTel spec (issue #4062): the start_ts for the first delta collection