[SDK] Reduce lock contention in SyncMetricStorage for concurrent metric recording (#3959)

Author: perhapsmaple

sdk/include/opentelemetry/sdk/metrics/state/attributes_hashmap.h

@@ -10,13 +10,11 @@
 #include <unordered_map>
 #include <utility>
 
-#include "opentelemetry/common/key_value_iterable.h"
 #include "opentelemetry/nostd/function_ref.h"
 #include "opentelemetry/sdk/common/attribute_utils.h"
 #include "opentelemetry/sdk/common/attributemap_hash.h"
 #include "opentelemetry/sdk/metrics/aggregation/aggregation.h"
 #include "opentelemetry/sdk/metrics/state/filtered_ordered_attribute_map.h"
-#include "opentelemetry/sdk/metrics/view/attributes_processor.h"
 #include "opentelemetry/version.h"
 
 OPENTELEMETRY_BEGIN_NAMESPACE
@@ -80,30 +78,6 @@ class AttributesHashMapWithCustomHash
    * If not present, it uses the provided callback to generate
    * value and store in the hash
    */
-  Aggregation *GetOrSetDefault(
-      const opentelemetry::common::KeyValueIterable &attributes,
-      const AttributesProcessor *attributes_processor,
-      nostd::function_ref<std::unique_ptr<Aggregation>()> aggregation_callback)
-  {
-    // TODO: avoid constructing MetricAttributes from KeyValueIterable for
-    // hash_map_.find which is a heavy operation
-    MetricAttributes attr{attributes, attributes_processor};
-
-    auto it = hash_map_.find(attr);
-    if (it != hash_map_.end())
-    {
-      return it->second.get();
-    }
-
-    if (IsOverflowAttributes(attr))
-    {
-      return GetOrSetOveflowAttributes(aggregation_callback);
-    }
-
-    auto result = hash_map_.emplace(std::move(attr), aggregation_callback());
-    return result.first->second.get();
-  }
-
   Aggregation *GetOrSetDefault(
       const MetricAttributes &attributes,
       nostd::function_ref<std::unique_ptr<Aggregation>()> aggregation_callback)
@@ -144,13 +118,6 @@ class AttributesHashMapWithCustomHash
   /**
    * Set the value for given key, overwriting the value if already present
    */
-  void Set(const opentelemetry::common::KeyValueIterable &attributes,
-           const AttributesProcessor *attributes_processor,
-           std::unique_ptr<Aggregation> aggr)
-  {
-    Set(MetricAttributes{attributes, attributes_processor}, std::move(aggr));
-  }
-
   void Set(const MetricAttributes &attributes, std::unique_ptr<Aggregation> aggr)
   {
     auto it = hash_map_.find(attributes);

sdk/include/opentelemetry/sdk/metrics/state/sync_metric_storage.h

@@ -119,9 +119,10 @@ class SyncMetricStorage : public MetricStorage, public SyncWritableMetricStorage
     }
 #endif
 
+    MetricAttributes attr{attributes, attributes_processor_.get()};
     std::lock_guard<opentelemetry::common::SpinLockMutex> guard(attribute_hashmap_lock_);
-    attributes_hashmap_
-        ->GetOrSetDefault(attributes, attributes_processor_.get(), create_default_aggregation_)
+    // cppcheck-suppress accessMoved
+    attributes_hashmap_->GetOrSetDefault(std::move(attr), create_default_aggregation_)
         ->Aggregate(value);
   }
 
@@ -160,9 +161,10 @@ class SyncMetricStorage : public MetricStorage, public SyncWritableMetricStorage
                                             std::chrono::system_clock::now());
     }
 #endif
+    MetricAttributes attr{attributes, attributes_processor_.get()};
     std::lock_guard<opentelemetry::common::SpinLockMutex> guard(attribute_hashmap_lock_);
-    attributes_hashmap_
-        ->GetOrSetDefault(attributes, attributes_processor_.get(), create_default_aggregation_)
+    // cppcheck-suppress accessMoved
+    attributes_hashmap_->GetOrSetDefault(std::move(attr), create_default_aggregation_)
         ->Aggregate(value);
   }
 

sdk/test/metrics/async_metric_storage_test.cc

@@ -25,13 +25,11 @@
 #include "opentelemetry/sdk/metrics/state/attributes_hashmap.h"
 #include "opentelemetry/sdk/metrics/state/filtered_ordered_attribute_map.h"
 #include "opentelemetry/sdk/metrics/state/metric_collector.h"
+#include "opentelemetry/sdk/metrics/view/attributes_processor.h"
 
 #ifdef ENABLE_METRICS_EXEMPLAR_PREVIEW
-#  include "opentelemetry/sdk/metrics/data/exemplar_data.h"
 #  include "opentelemetry/sdk/metrics/exemplar/filter_type.h"
 #  include "opentelemetry/sdk/metrics/exemplar/reservoir.h"
-#else
-#  include "opentelemetry/sdk/metrics/view/attributes_processor.h"
 #endif
 
 using namespace opentelemetry::sdk::metrics;