Skip to content
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<opentelemetry::common::SpinLockMutex> guard(hashmap_lock_);
std::lock_guard<std::mutex> guard(hashmap_lock_);
#ifdef ENABLE_METRICS_EXEMPLAR_PREVIEW
const bool offer_exemplars =
ExemplarFilterEnabled(exemplar_filter_type_, opentelemetry::context::Context{});
Expand Down Expand Up @@ -130,7 +130,7 @@ class AsyncMetricStorage : public MetricStorage, public AsyncWritableMetricStora

std::shared_ptr<AttributesHashMap> delta_metrics = nullptr;
{
std::lock_guard<opentelemetry::common::SpinLockMutex> guard(hashmap_lock_);
std::lock_guard<std::mutex> guard(hashmap_lock_);
delta_metrics = std::move(delta_hash_map_);
delta_hash_map_ =
std::make_unique<AttributesHashMap>(aggregation_config_->cardinality_limit_);
Expand All @@ -148,7 +148,7 @@ class AsyncMetricStorage : public MetricStorage, public AsyncWritableMetricStora
const AggregationConfig *aggregation_config_;
std::unique_ptr<AttributesHashMap> cumulative_hash_map_;
std::unique_ptr<AttributesHashMap> delta_hash_map_;
opentelemetry::common::SpinLockMutex hashmap_lock_;
std::mutex hashmap_lock_;
#ifdef ENABLE_METRICS_EXEMPLAR_PREVIEW
ExemplarFilterType exemplar_filter_type_;
nostd::shared_ptr<ExemplarReservoir> exemplar_reservoir_;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@
#include <unordered_map>

#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"
Expand Down Expand Up @@ -197,7 +196,7 @@ class SyncMetricStorage : public MetricStorage, public SyncWritableMetricStorage
std::shared_ptr<BoundSyncWritableMetricStorage> 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
Expand Down Expand Up @@ -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<Aggregation> current_;
bool dirty_ = false;
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,9 @@

#include <list>
#include <memory>
#include <mutex>
#include <unordered_map>

#include "opentelemetry/common/spin_lock_mutex.h"
#include "opentelemetry/common/timestamp.h"
#include "opentelemetry/nostd/function_ref.h"
#include "opentelemetry/nostd/span.h"
Expand Down Expand Up @@ -54,7 +54,7 @@ class TemporalMetricStorage
std::unordered_map<CollectorHandle *, LastReportedMetrics> 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;
Expand Down
15 changes: 7 additions & 8 deletions sdk/src/metrics/state/sync_metric_storage.cc
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@
# include <unordered_set>
# include <vector>

# 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"
Expand All @@ -47,7 +46,7 @@ bool SyncMetricStorage::Collect(CollectorHandle *collector,
std::shared_ptr<AttributesHashMap> 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<std::shared_ptr<BoundEntry>> entry_snapshot;
#endif
{
Expand All @@ -66,7 +65,7 @@ bool SyncMetricStorage::Collect(CollectorHandle *collector,
bool can_erase = false;
if (it->second.use_count() == 1)
{
std::lock_guard<opentelemetry::common::SpinLockMutex> g(it->second->lock_);
std::lock_guard<std::mutex> g(it->second->lock_);
can_erase = !it->second->dirty_;
}
if (can_erase)
Expand Down Expand Up @@ -95,15 +94,15 @@ 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)
{
std::unique_ptr<Aggregation> rotated;
MetricAttributes attrs_copy;
{
std::lock_guard<opentelemetry::common::SpinLockMutex> g(entry->lock_);
std::lock_guard<std::mutex> g(entry->lock_);
Comment thread
dbarker marked this conversation as resolved.
if (!entry->dirty_)
{
continue;
Expand Down Expand Up @@ -160,7 +159,7 @@ bool SyncMetricStorage::Collect(CollectorHandle *collector,
// satisfy the documented invariant on dirty_.
bool entry_dirty = false;
{
std::lock_guard<opentelemetry::common::SpinLockMutex> g(it->second->lock_);
std::lock_guard<std::mutex> g(it->second->lock_);
entry_dirty = it->second->dirty_;
}
if (entry_dirty)
Expand Down Expand Up @@ -228,7 +227,7 @@ void SyncMetricStorage::BoundEntry::RecordLong(int64_t value) noexcept
"is not long");
return;
}
std::lock_guard<opentelemetry::common::SpinLockMutex> guard(lock_);
std::lock_guard<std::mutex> guard(lock_);
current_->Aggregate(value);
dirty_ = true;
}
Expand All @@ -242,7 +241,7 @@ void SyncMetricStorage::BoundEntry::RecordDouble(double value) noexcept
"is not double");
return;
}
std::lock_guard<opentelemetry::common::SpinLockMutex> guard(lock_);
std::lock_guard<std::mutex> guard(lock_);
current_->Aggregate(value);
dirty_ = true;
}
Expand Down
3 changes: 1 addition & 2 deletions sdk/src/metrics/state/temporal_metric_storage.cc
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@
#include <utility>
#include <vector>

#include "opentelemetry/common/spin_lock_mutex.h"
#include "opentelemetry/common/timestamp.h"
#include "opentelemetry/nostd/function_ref.h"
#include "opentelemetry/nostd/span.h"
Expand Down Expand Up @@ -46,7 +45,7 @@ bool TemporalMetricStorage::buildMetrics(CollectorHandle *collector,
const std::shared_ptr<AttributesHashMap> &delta_metrics,
nostd::function_ref<bool(MetricData)> callback) noexcept
{
std::lock_guard<opentelemetry::common::SpinLockMutex> guard(lock_);
std::lock_guard<std::mutex> guard(lock_);
AggregationTemporality aggregation_temporarily =
collector->GetAggregationTemporality(instrument_descriptor_.type_);
// Per OTel spec (issue #4062): the start_ts for the first delta collection
Expand Down
Loading