From e98e336eaa5d8845d8367414a815c41b9fc8b1db Mon Sep 17 00:00:00 2001
From: Tom Tan <totan@microsoft.com>
Date: Mon, 24 Mar 2025 21:42:41 -0700
Subject: [PATCH 01/23] [METRICS SDK] Fix hash collision in MetricAttributes

---
 .../sdk/common/attributemap_hash.h            |   1 +
 .../sdk/metrics/state/async_metric_storage.h  |  11 +-
 .../sdk/metrics/state/attributes_hashmap.h    | 105 +++++++++---------
 .../state/filtered_ordered_attribute_map.h    |  28 ++++-
 .../sdk/metrics/state/sync_metric_storage.h   |   8 +-
 .../state/filtered_ordered_attribute_map.cc   |   9 ++
 .../metrics/state/temporal_metric_storage.cc  |  14 +--
 .../metrics/attributes_hashmap_benchmark.cc   |   2 +-
 sdk/test/metrics/attributes_hashmap_test.cc   |  20 ++--
 sdk/test/metrics/cardinality_limit_test.cc    |   2 +-
 10 files changed, 115 insertions(+), 85 deletions(-)

diff --git a/sdk/include/opentelemetry/sdk/common/attributemap_hash.h b/sdk/include/opentelemetry/sdk/common/attributemap_hash.h
index 39df29b278..8b88d3677d 100644
--- a/sdk/include/opentelemetry/sdk/common/attributemap_hash.h
+++ b/sdk/include/opentelemetry/sdk/common/attributemap_hash.h
@@ -75,6 +75,7 @@ inline size_t GetHashForAttributeMap(const OrderedAttributeMap &attribute_map)
 }
 
 // Calculate hash of keys and values of KeyValueIterable, filtered using callback.
+// TODO: remove this, this is not correct!
 inline size_t GetHashForAttributeMap(
     const opentelemetry::common::KeyValueIterable &attributes,
     nostd::function_ref<bool(nostd::string_view)> is_key_present_callback)
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 14e9a3fbfa..52f4d82aaf 100644
--- a/sdk/include/opentelemetry/sdk/metrics/state/async_metric_storage.h
+++ b/sdk/include/opentelemetry/sdk/metrics/state/async_metric_storage.h
@@ -71,15 +71,14 @@ class AsyncMetricStorage : public MetricStorage, public AsyncWritableMetricStora
 
       auto aggr = DefaultAggregation::CreateAggregation(aggregation_type_, instrument_descriptor_);
       aggr->Aggregate(measurement.second);
-      auto hash = opentelemetry::sdk::common::GetHashForAttributeMap(measurement.first);
-      auto prev = cumulative_hash_map_->Get(hash);
+      auto prev = cumulative_hash_map_->Get(measurement.first);
       if (prev)
       {
         auto delta = prev->Diff(*aggr);
         // store received value in cumulative map, and the diff in delta map (to pass it to temporal
         // storage)
-        cumulative_hash_map_->Set(measurement.first, std::move(aggr), hash);
-        delta_hash_map_->Set(measurement.first, std::move(delta), hash);
+        cumulative_hash_map_->Set(measurement.first, std::move(aggr), 0);
+        delta_hash_map_->Set(measurement.first, std::move(delta), 0);
       }
       else
       {
@@ -87,8 +86,8 @@ class AsyncMetricStorage : public MetricStorage, public AsyncWritableMetricStora
         cumulative_hash_map_->Set(
             measurement.first,
             DefaultAggregation::CloneAggregation(aggregation_type_, instrument_descriptor_, *aggr),
-            hash);
-        delta_hash_map_->Set(measurement.first, std::move(aggr), hash);
+            0);
+        delta_hash_map_->Set(measurement.first, std::move(aggr), 0);
       }
     }
   }
diff --git a/sdk/include/opentelemetry/sdk/metrics/state/attributes_hashmap.h b/sdk/include/opentelemetry/sdk/metrics/state/attributes_hashmap.h
index ddf2a8b206..1af4ae058f 100644
--- a/sdk/include/opentelemetry/sdk/metrics/state/attributes_hashmap.h
+++ b/sdk/include/opentelemetry/sdk/metrics/state/attributes_hashmap.h
@@ -15,6 +15,7 @@
 #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"
 
@@ -29,9 +30,8 @@ using opentelemetry::sdk::common::OrderedAttributeMap;
 constexpr size_t kAggregationCardinalityLimit = 2000;
 const std::string kAttributesLimitOverflowKey = "otel.metrics.overflow";
 const bool kAttributesLimitOverflowValue      = true;
-const size_t kOverflowAttributesHash          = opentelemetry::sdk::common::GetHashForAttributeMap(
-    {{kAttributesLimitOverflowKey,
-               kAttributesLimitOverflowValue}});  // precalculated for optimization
+const MetricAttributes kOverflowAttributes = {
+    {kAttributesLimitOverflowKey, kAttributesLimitOverflowValue}};  // precalculated for optimization
 
 class AttributeHashGenerator
 {
@@ -48,12 +48,12 @@ class AttributesHashMap
   AttributesHashMap(size_t attributes_limit = kAggregationCardinalityLimit)
       : attributes_limit_(attributes_limit)
   {}
-  Aggregation *Get(size_t hash) const
+  Aggregation *Get(MetricAttributes attributes) const
   {
-    auto it = hash_map_.find(hash);
+    auto it = hash_map_.find(attributes);
     if (it != hash_map_.end())
     {
-      return it->second.second.get();
+      return it->second.get();
     }
     return nullptr;
   }
@@ -62,7 +62,7 @@ class AttributesHashMap
    * @return check if key is present in hash
    *
    */
-  bool Has(size_t hash) const { return hash_map_.find(hash) != hash_map_.end(); }
+  bool Has(const MetricAttributes& attributes) const { return hash_map_.find(attributes) != hash_map_.end(); }
 
   /**
    * @return the pointer to value for given key if present.
@@ -72,32 +72,15 @@ class AttributesHashMap
   Aggregation *GetOrSetDefault(const opentelemetry::common::KeyValueIterable &attributes,
                                const AttributesProcessor *attributes_processor,
                                std::function<std::unique_ptr<Aggregation>()> aggregation_callback,
+                               // TODO: remove this, this is not correct!
                                size_t hash)
   {
-    auto it = hash_map_.find(hash);
-    if (it != hash_map_.end())
-    {
-      return it->second.second.get();
-    }
-
-    if (IsOverflowAttributes())
-    {
-      return GetOrSetOveflowAttributes(aggregation_callback);
-    }
-
     MetricAttributes attr{attributes, attributes_processor};
 
-    hash_map_[hash] = {attr, aggregation_callback()};
-    return hash_map_[hash].second.get();
-  }
-
-  Aggregation *GetOrSetDefault(std::function<std::unique_ptr<Aggregation>()> aggregation_callback,
-                               size_t hash)
-  {
-    auto it = hash_map_.find(hash);
+    auto it = hash_map_.find(attr);
     if (it != hash_map_.end())
     {
-      return it->second.second.get();
+      return it->second.get();
     }
 
     if (IsOverflowAttributes())
@@ -105,19 +88,37 @@ class AttributesHashMap
       return GetOrSetOveflowAttributes(aggregation_callback);
     }
 
-    MetricAttributes attr{};
-    hash_map_[hash] = {attr, aggregation_callback()};
-    return hash_map_[hash].second.get();
+    hash_map_[attr] = aggregation_callback();
+    return hash_map_[attr].get();
   }
 
+//   Aggregation *GetOrSetDefault(std::function<std::unique_ptr<Aggregation>()> aggregation_callback,
+//                                size_t hash)
+//   {
+//     auto it = hash_map_.find(hash);
+//     if (it != hash_map_.end())
+//     {
+//       return it->second.second.get();
+//     }
+// 
+//     if (IsOverflowAttributes())
+//     {
+//       return GetOrSetOveflowAttributes(aggregation_callback);
+//     }
+// 
+//     MetricAttributes attr{};
+//     hash_map_[hash] = {attr, aggregation_callback()};
+//     return hash_map_[hash].second.get();
+//   }
+
   Aggregation *GetOrSetDefault(const MetricAttributes &attributes,
                                std::function<std::unique_ptr<Aggregation>()> aggregation_callback,
                                size_t hash)
   {
-    auto it = hash_map_.find(hash);
+    auto it = hash_map_.find(attributes);
     if (it != hash_map_.end())
     {
-      return it->second.second.get();
+      return it->second.get();
     }
 
     if (IsOverflowAttributes())
@@ -125,10 +126,8 @@ class AttributesHashMap
       return GetOrSetOveflowAttributes(aggregation_callback);
     }
 
-    MetricAttributes attr{attributes};
-
-    hash_map_[hash] = {attr, aggregation_callback()};
-    return hash_map_[hash].second.get();
+    hash_map_[attributes] = aggregation_callback();
+    return hash_map_[attributes].get();
   }
 
   /**
@@ -139,40 +138,38 @@ class AttributesHashMap
            std::unique_ptr<Aggregation> aggr,
            size_t hash)
   {
-    auto it = hash_map_.find(hash);
+    MetricAttributes attr{attributes, attributes_processor};
+
+    auto it = hash_map_.find(attr);
     if (it != hash_map_.end())
     {
-      it->second.second = std::move(aggr);
+      it->second = std::move(aggr);
     }
     else if (IsOverflowAttributes())
     {
-      hash_map_[kOverflowAttributesHash] = {
-          MetricAttributes{{kAttributesLimitOverflowKey, kAttributesLimitOverflowValue}},
-          std::move(aggr)};
+      hash_map_[kOverflowAttributes] = std::move(aggr);
     }
     else
     {
       MetricAttributes attr{attributes, attributes_processor};
-      hash_map_[hash] = {attr, std::move(aggr)};
+      hash_map_[attr] = std::move(aggr);
     }
   }
 
   void Set(const MetricAttributes &attributes, std::unique_ptr<Aggregation> aggr, size_t hash)
   {
-    auto it = hash_map_.find(hash);
+    auto it = hash_map_.find(attributes);
     if (it != hash_map_.end())
     {
-      it->second.second = std::move(aggr);
+      it->second = std::move(aggr);
     }
     else if (IsOverflowAttributes())
     {
-      hash_map_[kOverflowAttributesHash] = {
-          MetricAttributes{{kAttributesLimitOverflowKey, kAttributesLimitOverflowValue}},
-          std::move(aggr)};
+      hash_map_[kOverflowAttributes] = std::move(aggr);
     }
     else
     {
-      hash_map_[hash] = {attributes, std::move(aggr)};
+      hash_map_[attributes] = std::move(aggr);
     }
   }
 
@@ -184,7 +181,7 @@ class AttributesHashMap
   {
     for (auto &kv : hash_map_)
     {
-      if (!callback(kv.second.first, *(kv.second.second.get())))
+      if (!callback(kv.first, *(kv.second.get())))
       {
         return false;  // callback is not prepared to consume data
       }
@@ -198,7 +195,7 @@ class AttributesHashMap
   size_t Size() { return hash_map_.size(); }
 
 private:
-  std::unordered_map<size_t, std::pair<MetricAttributes, std::unique_ptr<Aggregation>>> hash_map_;
+  std::unordered_map<MetricAttributes, std::unique_ptr<Aggregation>, FilteredOrderedAttributeMapHash> hash_map_;
   size_t attributes_limit_;
 
   Aggregation *GetOrSetOveflowAttributes(
@@ -210,15 +207,15 @@ class AttributesHashMap
 
   Aggregation *GetOrSetOveflowAttributes(std::unique_ptr<Aggregation> agg)
   {
-    auto it = hash_map_.find(kOverflowAttributesHash);
+    auto it = hash_map_.find(kOverflowAttributes);
     if (it != hash_map_.end())
     {
-      return it->second.second.get();
+      return it->second.get();
     }
 
     MetricAttributes attr{{kAttributesLimitOverflowKey, kAttributesLimitOverflowValue}};
-    hash_map_[kOverflowAttributesHash] = {attr, std::move(agg)};
-    return hash_map_[kOverflowAttributesHash].second.get();
+    hash_map_[kOverflowAttributes] = std::move(agg);
+    return hash_map_[kOverflowAttributes].get();
   }
 
   bool IsOverflowAttributes() const { return (hash_map_.size() + 1 >= attributes_limit_); }
diff --git a/sdk/include/opentelemetry/sdk/metrics/state/filtered_ordered_attribute_map.h b/sdk/include/opentelemetry/sdk/metrics/state/filtered_ordered_attribute_map.h
index 329e75bfa7..014d6464b7 100644
--- a/sdk/include/opentelemetry/sdk/metrics/state/filtered_ordered_attribute_map.h
+++ b/sdk/include/opentelemetry/sdk/metrics/state/filtered_ordered_attribute_map.h
@@ -10,6 +10,7 @@
 #include "opentelemetry/common/key_value_iterable.h"
 #include "opentelemetry/nostd/string_view.h"
 #include "opentelemetry/sdk/common/attribute_utils.h"
+#include "opentelemetry/sdk/common/attributemap_hash.h"
 #include "opentelemetry/version.h"
 
 OPENTELEMETRY_BEGIN_NAMESPACE
@@ -23,21 +24,46 @@ class FilteredOrderedAttributeMap : public opentelemetry::sdk::common::OrderedAt
 {
 public:
   FilteredOrderedAttributeMap() = default;
+
   FilteredOrderedAttributeMap(
       std::initializer_list<std::pair<nostd::string_view, opentelemetry::common::AttributeValue>>
           attributes)
       : OrderedAttributeMap(attributes)
   {}
+
   FilteredOrderedAttributeMap(const opentelemetry::common::KeyValueIterable &attributes)
       : FilteredOrderedAttributeMap(attributes, nullptr)
   {}
+
   FilteredOrderedAttributeMap(const opentelemetry::common::KeyValueIterable &attributes,
                               const opentelemetry::sdk::metrics::AttributesProcessor *processor);
+
   FilteredOrderedAttributeMap(
       std::initializer_list<std::pair<nostd::string_view, opentelemetry::common::AttributeValue>>
           attributes,
       const opentelemetry::sdk::metrics::AttributesProcessor *processor);
+
+  size_t GetHash() const { return _hash; }
+
+private:
+
+  void UpdateHash()
+  {
+    _hash = GetHashForAttributeMap(*this);
+  }
+
+  size_t _hash = 0UL;
+};
+
+class FilteredOrderedAttributeMapHash
+{
+public:
+  size_t operator()(const opentelemetry::sdk::metrics::FilteredOrderedAttributeMap &attributes) const
+  {
+    return attributes.GetHash();
+  }
 };
+
 }  // namespace metrics
 }  // namespace sdk
-OPENTELEMETRY_END_NAMESPACE
+OPENTELEMETRY_END_NAMESPACE
\ No newline at end of file
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 6e5b799e3f..9997cf5f78 100644
--- a/sdk/include/opentelemetry/sdk/metrics/state/sync_metric_storage.h
+++ b/sdk/include/opentelemetry/sdk/metrics/state/sync_metric_storage.h
@@ -95,9 +95,9 @@ class SyncMetricStorage : public MetricStorage, public SyncWritableMetricStorage
       exemplar_reservoir_->OfferMeasurement(value, {}, context, std::chrono::system_clock::now());
     }
 #endif
-    static size_t hash = opentelemetry::sdk::common::GetHash("");
+    static MetricAttributes attr = MetricAttributes{};
     std::lock_guard<opentelemetry::common::SpinLockMutex> guard(attribute_hashmap_lock_);
-    attributes_hashmap_->GetOrSetDefault(create_default_aggregation_, hash)->Aggregate(value);
+    attributes_hashmap_->GetOrSetDefault(attr, create_default_aggregation_, 0)->Aggregate(value);
   }
 
   void RecordLong(int64_t value,
@@ -148,9 +148,9 @@ class SyncMetricStorage : public MetricStorage, public SyncWritableMetricStorage
       exemplar_reservoir_->OfferMeasurement(value, {}, context, std::chrono::system_clock::now());
     }
 #endif
-    static size_t hash = opentelemetry::sdk::common::GetHash("");
+    static MetricAttributes attr = MetricAttributes{};
     std::lock_guard<opentelemetry::common::SpinLockMutex> guard(attribute_hashmap_lock_);
-    attributes_hashmap_->GetOrSetDefault(create_default_aggregation_, hash)->Aggregate(value);
+    attributes_hashmap_->GetOrSetDefault(attr, create_default_aggregation_, 0)->Aggregate(value);
   }
 
   void RecordDouble(double value,
diff --git a/sdk/src/metrics/state/filtered_ordered_attribute_map.cc b/sdk/src/metrics/state/filtered_ordered_attribute_map.cc
index baed0fce9a..5bb73fef35 100644
--- a/sdk/src/metrics/state/filtered_ordered_attribute_map.cc
+++ b/sdk/src/metrics/state/filtered_ordered_attribute_map.cc
@@ -1,6 +1,8 @@
 // Copyright The OpenTelemetry Authors
 // SPDX-License-Identifier: Apache-2.0
 
+#include "opentelemetry/sdk/common/attributemap_hash.h"
+#include "opentelemetry/sdk/common/attribute_utils.h"
 #include "opentelemetry/sdk/metrics/state/filtered_ordered_attribute_map.h"
 #include "opentelemetry/nostd/function_ref.h"
 #include "opentelemetry/sdk/metrics/view/attributes_processor.h"
@@ -10,6 +12,9 @@ namespace sdk
 {
 namespace metrics
 {
+
+using namespace opentelemetry::sdk::common;
+
 FilteredOrderedAttributeMap::FilteredOrderedAttributeMap(
     const opentelemetry::common::KeyValueIterable &attributes,
     const AttributesProcessor *processor)
@@ -23,6 +28,8 @@ FilteredOrderedAttributeMap::FilteredOrderedAttributeMap(
         }
         return true;
       });
+
+  UpdateHash();
 }
 
 FilteredOrderedAttributeMap::FilteredOrderedAttributeMap(
@@ -38,6 +45,8 @@ FilteredOrderedAttributeMap::FilteredOrderedAttributeMap(
       SetAttribute(kv.first, kv.second);
     }
   }
+
+  UpdateHash();
 }
 }  // namespace metrics
 }  // namespace sdk
diff --git a/sdk/src/metrics/state/temporal_metric_storage.cc b/sdk/src/metrics/state/temporal_metric_storage.cc
index 1302f9d642..e4739c108c 100644
--- a/sdk/src/metrics/state/temporal_metric_storage.cc
+++ b/sdk/src/metrics/state/temporal_metric_storage.cc
@@ -104,11 +104,10 @@ bool TemporalMetricStorage::buildMetrics(CollectorHandle *collector,
   {
     agg_hashmap->GetAllEnteries(
         [&merged_metrics, this](const MetricAttributes &attributes, Aggregation &aggregation) {
-          auto hash = opentelemetry::sdk::common::GetHashForAttributeMap(attributes);
-          auto agg  = merged_metrics->Get(hash);
+          auto agg  = merged_metrics->Get(attributes);
           if (agg)
           {
-            merged_metrics->Set(attributes, agg->Merge(aggregation), hash);
+            merged_metrics->Set(attributes, agg->Merge(aggregation), 0);
           }
           else
           {
@@ -116,7 +115,7 @@ bool TemporalMetricStorage::buildMetrics(CollectorHandle *collector,
                                 DefaultAggregation::CreateAggregation(
                                     aggregation_type_, instrument_descriptor_, aggregation_config_)
                                     ->Merge(aggregation),
-                                hash);
+                                0);
           }
           return true;
         });
@@ -139,17 +138,16 @@ bool TemporalMetricStorage::buildMetrics(CollectorHandle *collector,
       // merge current delta to previous cumulative
       last_aggr_hashmap->GetAllEnteries(
           [&merged_metrics, this](const MetricAttributes &attributes, Aggregation &aggregation) {
-            auto hash = opentelemetry::sdk::common::GetHashForAttributeMap(attributes);
-            auto agg  = merged_metrics->Get(hash);
+            auto agg  = merged_metrics->Get(attributes);
             if (agg)
             {
-              merged_metrics->Set(attributes, agg->Merge(aggregation), hash);
+              merged_metrics->Set(attributes, agg->Merge(aggregation), 0);
             }
             else
             {
               auto def_agg = DefaultAggregation::CreateAggregation(
                   aggregation_type_, instrument_descriptor_, aggregation_config_);
-              merged_metrics->Set(attributes, def_agg->Merge(aggregation), hash);
+              merged_metrics->Set(attributes, def_agg->Merge(aggregation), 0);
             }
             return true;
           });
diff --git a/sdk/test/metrics/attributes_hashmap_benchmark.cc b/sdk/test/metrics/attributes_hashmap_benchmark.cc
index 8c4bc69f9e..8394618ee6 100644
--- a/sdk/test/metrics/attributes_hashmap_benchmark.cc
+++ b/sdk/test/metrics/attributes_hashmap_benchmark.cc
@@ -42,7 +42,7 @@ void BM_AttributseHashMap(benchmark::State &state)
     auto hash = opentelemetry::sdk::common::GetHashForAttributeMap(attributes[i % 2]);
     hash_map.GetOrSetDefault(attributes[i % 2], create_default_aggregation, hash)
         ->Aggregate(static_cast<int64_t>(1));
-    benchmark::DoNotOptimize(hash_map.Has(hash));
+    benchmark::DoNotOptimize(hash_map.Has(attributes[i % 2]));
     m.unlock();
   };
   while (state.KeepRunning())
diff --git a/sdk/test/metrics/attributes_hashmap_test.cc b/sdk/test/metrics/attributes_hashmap_test.cc
index 153c6ada25..6281e79618 100644
--- a/sdk/test/metrics/attributes_hashmap_test.cc
+++ b/sdk/test/metrics/attributes_hashmap_test.cc
@@ -30,32 +30,32 @@ TEST(AttributesHashMap, BasicTests)
   MetricAttributes m1 = {{"k1", "v1"}};
   auto hash           = opentelemetry::sdk::common::GetHashForAttributeMap(m1);
 
-  EXPECT_EQ(hash_map.Get(hash), nullptr);
-  EXPECT_EQ(hash_map.Has(hash), false);
+  EXPECT_EQ(hash_map.Get(m1), nullptr);
+  EXPECT_EQ(hash_map.Has(m1), false);
 
   // Set
   std::unique_ptr<Aggregation> aggregation1(
       new DropAggregation());  //  = std::unique_ptr<Aggregation>(new DropAggregation);
   hash_map.Set(m1, std::move(aggregation1), hash);
-  hash_map.Get(hash)->Aggregate(static_cast<int64_t>(1));
+  hash_map.Get(m1)->Aggregate(static_cast<int64_t>(1));
   EXPECT_EQ(hash_map.Size(), 1);
-  EXPECT_EQ(hash_map.Has(hash), true);
+  EXPECT_EQ(hash_map.Has(m1), true);
 
   // Set same key again
   auto aggregation2 = std::unique_ptr<Aggregation>(new DropAggregation());
   hash_map.Set(m1, std::move(aggregation2), hash);
-  hash_map.Get(hash)->Aggregate(static_cast<int64_t>(1));
+  hash_map.Get(m1)->Aggregate(static_cast<int64_t>(1));
   EXPECT_EQ(hash_map.Size(), 1);
-  EXPECT_EQ(hash_map.Has(hash), true);
+  EXPECT_EQ(hash_map.Has(m1), true);
 
   // Set more enteria
   auto aggregation3   = std::unique_ptr<Aggregation>(new DropAggregation());
   MetricAttributes m3 = {{"k1", "v1"}, {"k2", "v2"}};
   auto hash3          = opentelemetry::sdk::common::GetHashForAttributeMap(m3);
   hash_map.Set(m3, std::move(aggregation3), hash3);
-  EXPECT_EQ(hash_map.Has(hash), true);
-  EXPECT_EQ(hash_map.Has(hash3), true);
-  hash_map.Get(hash3)->Aggregate(static_cast<int64_t>(1));
+  EXPECT_EQ(hash_map.Has(m1), true);
+  EXPECT_EQ(hash_map.Has(m3), true);
+  hash_map.Get(m3)->Aggregate(static_cast<int64_t>(1));
   EXPECT_EQ(hash_map.Size(), 2);
 
   // GetOrSetDefault
@@ -72,7 +72,7 @@ TEST(AttributesHashMap, BasicTests)
   // Set attributes with different order - shouldn't create a new entry.
   MetricAttributes m5 = {{"k2", "v2"}, {"k1", "v1"}};
   auto hash5          = opentelemetry::sdk::common::GetHashForAttributeMap(m5);
-  EXPECT_EQ(hash_map.Has(hash5), true);
+  EXPECT_EQ(hash_map.Has(m5), true);
 
   // GetAllEnteries
   size_t count = 0;
diff --git a/sdk/test/metrics/cardinality_limit_test.cc b/sdk/test/metrics/cardinality_limit_test.cc
index c785eeb5d5..c0da0dffe7 100644
--- a/sdk/test/metrics/cardinality_limit_test.cc
+++ b/sdk/test/metrics/cardinality_limit_test.cc
@@ -83,7 +83,7 @@ TEST(CardinalityLimit, AttributesHashMapBasicTests)
   // get the overflow metric point
   auto agg1 = hash_map.GetOrSetDefault(
       FilteredOrderedAttributeMap({{kAttributesLimitOverflowKey, kAttributesLimitOverflowValue}}),
-      aggregation_callback, kOverflowAttributesHash);
+      aggregation_callback, 0);
   EXPECT_NE(agg1, nullptr);
   auto sum_agg1 = static_cast<LongSumAggregation *>(agg1);
   EXPECT_EQ(nostd::get<int64_t>(nostd::get<SumPointData>(sum_agg1->ToPoint()).value_),

From adf6126b1ab891f107cf647c8001bd067ab029de Mon Sep 17 00:00:00 2001
From: Tom Tan <totan@microsoft.com>
Date: Mon, 24 Mar 2025 22:26:55 -0700
Subject: [PATCH 02/23] Fix ccardinality_limit test

---
 sdk/test/metrics/cardinality_limit_test.cc | 16 ++++++----------
 1 file changed, 6 insertions(+), 10 deletions(-)

diff --git a/sdk/test/metrics/cardinality_limit_test.cc b/sdk/test/metrics/cardinality_limit_test.cc
index c0da0dffe7..fcedafe16c 100644
--- a/sdk/test/metrics/cardinality_limit_test.cc
+++ b/sdk/test/metrics/cardinality_limit_test.cc
@@ -51,9 +51,8 @@ TEST(CardinalityLimit, AttributesHashMapBasicTests)
   for (auto i = 0; i < 10; i++)
   {
     FilteredOrderedAttributeMap attributes = {{"key", std::to_string(i)}};
-    auto hash = opentelemetry::sdk::common::GetHashForAttributeMap(attributes);
     static_cast<LongSumAggregation *>(
-        hash_map.GetOrSetDefault(attributes, aggregation_callback, hash))
+        hash_map.GetOrSetDefault(attributes, aggregation_callback, 0))
         ->Aggregate(record_value);
   }
   EXPECT_EQ(hash_map.Size(), 10);
@@ -62,9 +61,8 @@ TEST(CardinalityLimit, AttributesHashMapBasicTests)
   for (auto i = 10; i < 15; i++)
   {
     FilteredOrderedAttributeMap attributes = {{"key", std::to_string(i)}};
-    auto hash = opentelemetry::sdk::common::GetHashForAttributeMap(attributes);
     static_cast<LongSumAggregation *>(
-        hash_map.GetOrSetDefault(attributes, aggregation_callback, hash))
+        hash_map.GetOrSetDefault(attributes, aggregation_callback, 0))
         ->Aggregate(record_value);
   }
   EXPECT_EQ(hash_map.Size(), 10);  // only one more metric point should be added as overflow.
@@ -73,16 +71,15 @@ TEST(CardinalityLimit, AttributesHashMapBasicTests)
   for (auto i = 0; i < 5; i++)
   {
     FilteredOrderedAttributeMap attributes = {{"key", std::to_string(i)}};
-    auto hash = opentelemetry::sdk::common::GetHashForAttributeMap(attributes);
     static_cast<LongSumAggregation *>(
-        hash_map.GetOrSetDefault(attributes, aggregation_callback, hash))
+        hash_map.GetOrSetDefault(attributes, aggregation_callback, 0))
         ->Aggregate(record_value);
   }
   EXPECT_EQ(hash_map.Size(), 10);  // no new metric point added
 
   // get the overflow metric point
   auto agg1 = hash_map.GetOrSetDefault(
-      FilteredOrderedAttributeMap({{kAttributesLimitOverflowKey, kAttributesLimitOverflowValue}}),
+      kOverflowAttributes,
       aggregation_callback, 0);
   EXPECT_NE(agg1, nullptr);
   auto sum_agg1 = static_cast<LongSumAggregation *>(agg1);
@@ -92,10 +89,9 @@ TEST(CardinalityLimit, AttributesHashMapBasicTests)
   for (auto i = 0; i < 9; i++)
   {
     FilteredOrderedAttributeMap attributes = {{"key", std::to_string(i)}};
-    auto hash = opentelemetry::sdk::common::GetHashForAttributeMap(attributes);
     auto agg2 = hash_map.GetOrSetDefault(
-        FilteredOrderedAttributeMap({{kAttributesLimitOverflowKey, kAttributesLimitOverflowValue}}),
-        aggregation_callback, hash);
+        attributes,
+        aggregation_callback, 0);
     EXPECT_NE(agg2, nullptr);
     auto sum_agg2 = static_cast<LongSumAggregation *>(agg2);
     if (i < 5)

From 938b241af28445d6c378c4d0be8cd0e3a7674375 Mon Sep 17 00:00:00 2001
From: Tom Tan <totan@microsoft.com>
Date: Mon, 24 Mar 2025 23:05:18 -0700
Subject: [PATCH 03/23] Remove unused hash function

---
 .../sdk/common/attributemap_hash.h            | 22 --------
 .../sdk/metrics/state/sync_metric_storage.h   | 26 +---------
 sdk/test/metrics/attributes_hashmap_test.cc   | 52 ++-----------------
 3 files changed, 6 insertions(+), 94 deletions(-)

diff --git a/sdk/include/opentelemetry/sdk/common/attributemap_hash.h b/sdk/include/opentelemetry/sdk/common/attributemap_hash.h
index 8b88d3677d..7fddd02c8c 100644
--- a/sdk/include/opentelemetry/sdk/common/attributemap_hash.h
+++ b/sdk/include/opentelemetry/sdk/common/attributemap_hash.h
@@ -74,28 +74,6 @@ inline size_t GetHashForAttributeMap(const OrderedAttributeMap &attribute_map)
   return seed;
 }
 
-// Calculate hash of keys and values of KeyValueIterable, filtered using callback.
-// TODO: remove this, this is not correct!
-inline size_t GetHashForAttributeMap(
-    const opentelemetry::common::KeyValueIterable &attributes,
-    nostd::function_ref<bool(nostd::string_view)> is_key_present_callback)
-{
-  AttributeConverter converter;
-  size_t seed = 0UL;
-  attributes.ForEachKeyValue(
-      [&](nostd::string_view key, opentelemetry::common::AttributeValue value) noexcept {
-        if (!is_key_present_callback(key))
-        {
-          return true;
-        }
-        GetHash(seed, key);
-        auto attr_val = nostd::visit(converter, value);
-        nostd::visit(GetHashForAttributeValueVisitor(seed), attr_val);
-        return true;
-      });
-  return seed;
-}
-
 template <class T>
 inline size_t GetHash(T value)
 {
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 9997cf5f78..cd2cdaccaa 100644
--- a/sdk/include/opentelemetry/sdk/metrics/state/sync_metric_storage.h
+++ b/sdk/include/opentelemetry/sdk/metrics/state/sync_metric_storage.h
@@ -116,21 +116,10 @@ class SyncMetricStorage : public MetricStorage, public SyncWritableMetricStorage
                                             std::chrono::system_clock::now());
     }
 #endif
-    auto hash = opentelemetry::sdk::common::GetHashForAttributeMap(
-        attributes, [this](nostd::string_view key) {
-          if (attributes_processor_)
-          {
-            return attributes_processor_->isPresent(key);
-          }
-          else
-          {
-            return true;
-          }
-        });
 
     std::lock_guard<opentelemetry::common::SpinLockMutex> guard(attribute_hashmap_lock_);
     attributes_hashmap_
-        ->GetOrSetDefault(attributes, attributes_processor_, create_default_aggregation_, hash)
+        ->GetOrSetDefault(attributes, attributes_processor_, create_default_aggregation_, 0)
         ->Aggregate(value);
   }
 
@@ -169,20 +158,9 @@ class SyncMetricStorage : public MetricStorage, public SyncWritableMetricStorage
                                             std::chrono::system_clock::now());
     }
 #endif
-    auto hash = opentelemetry::sdk::common::GetHashForAttributeMap(
-        attributes, [this](nostd::string_view key) {
-          if (attributes_processor_)
-          {
-            return attributes_processor_->isPresent(key);
-          }
-          else
-          {
-            return true;
-          }
-        });
     std::lock_guard<opentelemetry::common::SpinLockMutex> guard(attribute_hashmap_lock_);
     attributes_hashmap_
-        ->GetOrSetDefault(attributes, attributes_processor_, create_default_aggregation_, hash)
+        ->GetOrSetDefault(attributes, attributes_processor_, create_default_aggregation_, 0)
         ->Aggregate(value);
   }
 
diff --git a/sdk/test/metrics/attributes_hashmap_test.cc b/sdk/test/metrics/attributes_hashmap_test.cc
index 6281e79618..240306942e 100644
--- a/sdk/test/metrics/attributes_hashmap_test.cc
+++ b/sdk/test/metrics/attributes_hashmap_test.cc
@@ -28,7 +28,6 @@ TEST(AttributesHashMap, BasicTests)
   AttributesHashMap hash_map;
   EXPECT_EQ(hash_map.Size(), 0);
   MetricAttributes m1 = {{"k1", "v1"}};
-  auto hash           = opentelemetry::sdk::common::GetHashForAttributeMap(m1);
 
   EXPECT_EQ(hash_map.Get(m1), nullptr);
   EXPECT_EQ(hash_map.Has(m1), false);
@@ -36,14 +35,14 @@ TEST(AttributesHashMap, BasicTests)
   // Set
   std::unique_ptr<Aggregation> aggregation1(
       new DropAggregation());  //  = std::unique_ptr<Aggregation>(new DropAggregation);
-  hash_map.Set(m1, std::move(aggregation1), hash);
+  hash_map.Set(m1, std::move(aggregation1), 0);
   hash_map.Get(m1)->Aggregate(static_cast<int64_t>(1));
   EXPECT_EQ(hash_map.Size(), 1);
   EXPECT_EQ(hash_map.Has(m1), true);
 
   // Set same key again
   auto aggregation2 = std::unique_ptr<Aggregation>(new DropAggregation());
-  hash_map.Set(m1, std::move(aggregation2), hash);
+  hash_map.Set(m1, std::move(aggregation2), 0);
   hash_map.Get(m1)->Aggregate(static_cast<int64_t>(1));
   EXPECT_EQ(hash_map.Size(), 1);
   EXPECT_EQ(hash_map.Has(m1), true);
@@ -51,8 +50,7 @@ TEST(AttributesHashMap, BasicTests)
   // Set more enteria
   auto aggregation3   = std::unique_ptr<Aggregation>(new DropAggregation());
   MetricAttributes m3 = {{"k1", "v1"}, {"k2", "v2"}};
-  auto hash3          = opentelemetry::sdk::common::GetHashForAttributeMap(m3);
-  hash_map.Set(m3, std::move(aggregation3), hash3);
+  hash_map.Set(m3, std::move(aggregation3), 0);
   EXPECT_EQ(hash_map.Has(m1), true);
   EXPECT_EQ(hash_map.Has(m3), true);
   hash_map.Get(m3)->Aggregate(static_cast<int64_t>(1));
@@ -64,14 +62,12 @@ TEST(AttributesHashMap, BasicTests)
     return std::unique_ptr<Aggregation>(new DropAggregation);
   };
   MetricAttributes m4 = {{"k1", "v1"}, {"k2", "v2"}, {"k3", "v3"}};
-  auto hash4          = opentelemetry::sdk::common::GetHashForAttributeMap(m4);
-  hash_map.GetOrSetDefault(m4, create_default_aggregation, hash4)
+  hash_map.GetOrSetDefault(m4, create_default_aggregation, 0)
       ->Aggregate(static_cast<int64_t>(1));
   EXPECT_EQ(hash_map.Size(), 3);
 
   // Set attributes with different order - shouldn't create a new entry.
   MetricAttributes m5 = {{"k2", "v2"}, {"k1", "v1"}};
-  auto hash5          = opentelemetry::sdk::common::GetHashForAttributeMap(m5);
   EXPECT_EQ(hash_map.Has(m5), true);
 
   // GetAllEnteries
@@ -89,46 +85,6 @@ std::string make_unique_string(const char *str)
   return std::string(str);
 }
 
-TEST(AttributesHashMap, HashWithKeyValueIterable)
-{
-  std::string key1   = make_unique_string("k1");
-  std::string value1 = make_unique_string("v1");
-  std::string key2   = make_unique_string("k2");
-  std::string value2 = make_unique_string("v2");
-  std::string key3   = make_unique_string("k3");
-  std::string value3 = make_unique_string("v3");
-
-  // Create mock KeyValueIterable instances with the same content but different variables
-  std::map<std::string, std::string> attributes1({{key1, value1}, {key2, value2}});
-  std::map<std::string, std::string> attributes2({{key1, value1}, {key2, value2}});
-  std::map<std::string, std::string> attributes3({{key1, value1}, {key2, value2}, {key3, value3}});
-
-  // Create a callback that filters "k3" key
-  auto is_key_filter_k3_callback = [](nostd::string_view key) {
-    if (key == "k3")
-    {
-      return false;
-    }
-    return true;
-  };
-  // Calculate hash
-  size_t hash1 = opentelemetry::sdk::common::GetHashForAttributeMap(
-      opentelemetry::common::KeyValueIterableView<std::map<std::string, std::string>>(attributes1),
-      is_key_filter_k3_callback);
-  size_t hash2 = opentelemetry::sdk::common::GetHashForAttributeMap(
-      opentelemetry::common::KeyValueIterableView<std::map<std::string, std::string>>(attributes2),
-      is_key_filter_k3_callback);
-
-  size_t hash3 = opentelemetry::sdk::common::GetHashForAttributeMap(
-      opentelemetry::common::KeyValueIterableView<std::map<std::string, std::string>>(attributes3),
-      is_key_filter_k3_callback);
-
-  // Expect the hashes to be the same because the content is the same
-  EXPECT_EQ(hash1, hash2);
-  // Expect the hashes to be the same because the content is the same
-  EXPECT_EQ(hash1, hash3);
-}
-
 TEST(AttributesHashMap, HashConsistencyAcrossStringTypes)
 {
   const char *c_str                 = "teststring";

From 274ed364f29b8573abce55ab8e57ec8199d918ae Mon Sep 17 00:00:00 2001
From: Tom Tan <totan@microsoft.com>
Date: Mon, 24 Mar 2025 23:20:26 -0700
Subject: [PATCH 04/23] Remove unnecessary hash code

---
 .../sdk/metrics/state/async_metric_storage.h  |  9 +++---
 .../sdk/metrics/state/attributes_hashmap.h    | 31 +++----------------
 .../sdk/metrics/state/sync_metric_storage.h   |  8 ++---
 .../state/filtered_ordered_attribute_map.cc   |  2 --
 .../metrics/state/temporal_metric_storage.cc  |  9 +++---
 .../metrics/attributes_hashmap_benchmark.cc   |  2 +-
 sdk/test/metrics/attributes_hashmap_test.cc   |  8 ++---
 sdk/test/metrics/cardinality_limit_test.cc    | 14 +++------
 8 files changed, 26 insertions(+), 57 deletions(-)

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 52f4d82aaf..93ffccc48c 100644
--- a/sdk/include/opentelemetry/sdk/metrics/state/async_metric_storage.h
+++ b/sdk/include/opentelemetry/sdk/metrics/state/async_metric_storage.h
@@ -77,17 +77,16 @@ class AsyncMetricStorage : public MetricStorage, public AsyncWritableMetricStora
         auto delta = prev->Diff(*aggr);
         // store received value in cumulative map, and the diff in delta map (to pass it to temporal
         // storage)
-        cumulative_hash_map_->Set(measurement.first, std::move(aggr), 0);
-        delta_hash_map_->Set(measurement.first, std::move(delta), 0);
+        cumulative_hash_map_->Set(measurement.first, std::move(aggr));
+        delta_hash_map_->Set(measurement.first, std::move(delta));
       }
       else
       {
         // store received value in cumulative and delta map.
         cumulative_hash_map_->Set(
             measurement.first,
-            DefaultAggregation::CloneAggregation(aggregation_type_, instrument_descriptor_, *aggr),
-            0);
-        delta_hash_map_->Set(measurement.first, std::move(aggr), 0);
+            DefaultAggregation::CloneAggregation(aggregation_type_, instrument_descriptor_, *aggr));
+        delta_hash_map_->Set(measurement.first, std::move(aggr));
       }
     }
   }
diff --git a/sdk/include/opentelemetry/sdk/metrics/state/attributes_hashmap.h b/sdk/include/opentelemetry/sdk/metrics/state/attributes_hashmap.h
index 1af4ae058f..da02dca883 100644
--- a/sdk/include/opentelemetry/sdk/metrics/state/attributes_hashmap.h
+++ b/sdk/include/opentelemetry/sdk/metrics/state/attributes_hashmap.h
@@ -71,9 +71,7 @@ class AttributesHashMap
    */
   Aggregation *GetOrSetDefault(const opentelemetry::common::KeyValueIterable &attributes,
                                const AttributesProcessor *attributes_processor,
-                               std::function<std::unique_ptr<Aggregation>()> aggregation_callback,
-                               // TODO: remove this, this is not correct!
-                               size_t hash)
+                               std::function<std::unique_ptr<Aggregation>()> aggregation_callback)
   {
     MetricAttributes attr{attributes, attributes_processor};
 
@@ -92,28 +90,8 @@ class AttributesHashMap
     return hash_map_[attr].get();
   }
 
-//   Aggregation *GetOrSetDefault(std::function<std::unique_ptr<Aggregation>()> aggregation_callback,
-//                                size_t hash)
-//   {
-//     auto it = hash_map_.find(hash);
-//     if (it != hash_map_.end())
-//     {
-//       return it->second.second.get();
-//     }
-// 
-//     if (IsOverflowAttributes())
-//     {
-//       return GetOrSetOveflowAttributes(aggregation_callback);
-//     }
-// 
-//     MetricAttributes attr{};
-//     hash_map_[hash] = {attr, aggregation_callback()};
-//     return hash_map_[hash].second.get();
-//   }
-
   Aggregation *GetOrSetDefault(const MetricAttributes &attributes,
-                               std::function<std::unique_ptr<Aggregation>()> aggregation_callback,
-                               size_t hash)
+                               std::function<std::unique_ptr<Aggregation>()> aggregation_callback)
   {
     auto it = hash_map_.find(attributes);
     if (it != hash_map_.end())
@@ -135,8 +113,7 @@ class AttributesHashMap
    */
   void Set(const opentelemetry::common::KeyValueIterable &attributes,
            const AttributesProcessor *attributes_processor,
-           std::unique_ptr<Aggregation> aggr,
-           size_t hash)
+           std::unique_ptr<Aggregation> aggr)
   {
     MetricAttributes attr{attributes, attributes_processor};
 
@@ -156,7 +133,7 @@ class AttributesHashMap
     }
   }
 
-  void Set(const MetricAttributes &attributes, std::unique_ptr<Aggregation> aggr, size_t hash)
+  void Set(const MetricAttributes &attributes, std::unique_ptr<Aggregation> aggr)
   {
     auto it = hash_map_.find(attributes);
     if (it != hash_map_.end())
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 cd2cdaccaa..0c9dea9c90 100644
--- a/sdk/include/opentelemetry/sdk/metrics/state/sync_metric_storage.h
+++ b/sdk/include/opentelemetry/sdk/metrics/state/sync_metric_storage.h
@@ -97,7 +97,7 @@ class SyncMetricStorage : public MetricStorage, public SyncWritableMetricStorage
 #endif
     static MetricAttributes attr = MetricAttributes{};
     std::lock_guard<opentelemetry::common::SpinLockMutex> guard(attribute_hashmap_lock_);
-    attributes_hashmap_->GetOrSetDefault(attr, create_default_aggregation_, 0)->Aggregate(value);
+    attributes_hashmap_->GetOrSetDefault(attr, create_default_aggregation_)->Aggregate(value);
   }
 
   void RecordLong(int64_t value,
@@ -119,7 +119,7 @@ class SyncMetricStorage : public MetricStorage, public SyncWritableMetricStorage
 
     std::lock_guard<opentelemetry::common::SpinLockMutex> guard(attribute_hashmap_lock_);
     attributes_hashmap_
-        ->GetOrSetDefault(attributes, attributes_processor_, create_default_aggregation_, 0)
+        ->GetOrSetDefault(attributes, attributes_processor_, create_default_aggregation_)
         ->Aggregate(value);
   }
 
@@ -139,7 +139,7 @@ class SyncMetricStorage : public MetricStorage, public SyncWritableMetricStorage
 #endif
     static MetricAttributes attr = MetricAttributes{};
     std::lock_guard<opentelemetry::common::SpinLockMutex> guard(attribute_hashmap_lock_);
-    attributes_hashmap_->GetOrSetDefault(attr, create_default_aggregation_, 0)->Aggregate(value);
+    attributes_hashmap_->GetOrSetDefault(attr, create_default_aggregation_)->Aggregate(value);
   }
 
   void RecordDouble(double value,
@@ -160,7 +160,7 @@ class SyncMetricStorage : public MetricStorage, public SyncWritableMetricStorage
 #endif
     std::lock_guard<opentelemetry::common::SpinLockMutex> guard(attribute_hashmap_lock_);
     attributes_hashmap_
-        ->GetOrSetDefault(attributes, attributes_processor_, create_default_aggregation_, 0)
+        ->GetOrSetDefault(attributes, attributes_processor_, create_default_aggregation_)
         ->Aggregate(value);
   }
 
diff --git a/sdk/src/metrics/state/filtered_ordered_attribute_map.cc b/sdk/src/metrics/state/filtered_ordered_attribute_map.cc
index 5bb73fef35..7c453bccca 100644
--- a/sdk/src/metrics/state/filtered_ordered_attribute_map.cc
+++ b/sdk/src/metrics/state/filtered_ordered_attribute_map.cc
@@ -13,8 +13,6 @@ namespace sdk
 namespace metrics
 {
 
-using namespace opentelemetry::sdk::common;
-
 FilteredOrderedAttributeMap::FilteredOrderedAttributeMap(
     const opentelemetry::common::KeyValueIterable &attributes,
     const AttributesProcessor *processor)
diff --git a/sdk/src/metrics/state/temporal_metric_storage.cc b/sdk/src/metrics/state/temporal_metric_storage.cc
index e4739c108c..8abf7193b0 100644
--- a/sdk/src/metrics/state/temporal_metric_storage.cc
+++ b/sdk/src/metrics/state/temporal_metric_storage.cc
@@ -107,15 +107,14 @@ bool TemporalMetricStorage::buildMetrics(CollectorHandle *collector,
           auto agg  = merged_metrics->Get(attributes);
           if (agg)
           {
-            merged_metrics->Set(attributes, agg->Merge(aggregation), 0);
+            merged_metrics->Set(attributes, agg->Merge(aggregation));
           }
           else
           {
             merged_metrics->Set(attributes,
                                 DefaultAggregation::CreateAggregation(
                                     aggregation_type_, instrument_descriptor_, aggregation_config_)
-                                    ->Merge(aggregation),
-                                0);
+                                    ->Merge(aggregation));
           }
           return true;
         });
@@ -141,13 +140,13 @@ bool TemporalMetricStorage::buildMetrics(CollectorHandle *collector,
             auto agg  = merged_metrics->Get(attributes);
             if (agg)
             {
-              merged_metrics->Set(attributes, agg->Merge(aggregation), 0);
+              merged_metrics->Set(attributes, agg->Merge(aggregation));
             }
             else
             {
               auto def_agg = DefaultAggregation::CreateAggregation(
                   aggregation_type_, instrument_descriptor_, aggregation_config_);
-              merged_metrics->Set(attributes, def_agg->Merge(aggregation), 0);
+              merged_metrics->Set(attributes, def_agg->Merge(aggregation));
             }
             return true;
           });
diff --git a/sdk/test/metrics/attributes_hashmap_benchmark.cc b/sdk/test/metrics/attributes_hashmap_benchmark.cc
index 8394618ee6..4e18fe8d44 100644
--- a/sdk/test/metrics/attributes_hashmap_benchmark.cc
+++ b/sdk/test/metrics/attributes_hashmap_benchmark.cc
@@ -40,7 +40,7 @@ void BM_AttributseHashMap(benchmark::State &state)
     };
     m.lock();
     auto hash = opentelemetry::sdk::common::GetHashForAttributeMap(attributes[i % 2]);
-    hash_map.GetOrSetDefault(attributes[i % 2], create_default_aggregation, hash)
+    hash_map.GetOrSetDefault(attributes[i % 2], create_default_aggregation)
         ->Aggregate(static_cast<int64_t>(1));
     benchmark::DoNotOptimize(hash_map.Has(attributes[i % 2]));
     m.unlock();
diff --git a/sdk/test/metrics/attributes_hashmap_test.cc b/sdk/test/metrics/attributes_hashmap_test.cc
index 240306942e..d6a50f7a4c 100644
--- a/sdk/test/metrics/attributes_hashmap_test.cc
+++ b/sdk/test/metrics/attributes_hashmap_test.cc
@@ -35,14 +35,14 @@ TEST(AttributesHashMap, BasicTests)
   // Set
   std::unique_ptr<Aggregation> aggregation1(
       new DropAggregation());  //  = std::unique_ptr<Aggregation>(new DropAggregation);
-  hash_map.Set(m1, std::move(aggregation1), 0);
+  hash_map.Set(m1, std::move(aggregation1));
   hash_map.Get(m1)->Aggregate(static_cast<int64_t>(1));
   EXPECT_EQ(hash_map.Size(), 1);
   EXPECT_EQ(hash_map.Has(m1), true);
 
   // Set same key again
   auto aggregation2 = std::unique_ptr<Aggregation>(new DropAggregation());
-  hash_map.Set(m1, std::move(aggregation2), 0);
+  hash_map.Set(m1, std::move(aggregation2));
   hash_map.Get(m1)->Aggregate(static_cast<int64_t>(1));
   EXPECT_EQ(hash_map.Size(), 1);
   EXPECT_EQ(hash_map.Has(m1), true);
@@ -50,7 +50,7 @@ TEST(AttributesHashMap, BasicTests)
   // Set more enteria
   auto aggregation3   = std::unique_ptr<Aggregation>(new DropAggregation());
   MetricAttributes m3 = {{"k1", "v1"}, {"k2", "v2"}};
-  hash_map.Set(m3, std::move(aggregation3), 0);
+  hash_map.Set(m3, std::move(aggregation3));
   EXPECT_EQ(hash_map.Has(m1), true);
   EXPECT_EQ(hash_map.Has(m3), true);
   hash_map.Get(m3)->Aggregate(static_cast<int64_t>(1));
@@ -62,7 +62,7 @@ TEST(AttributesHashMap, BasicTests)
     return std::unique_ptr<Aggregation>(new DropAggregation);
   };
   MetricAttributes m4 = {{"k1", "v1"}, {"k2", "v2"}, {"k3", "v3"}};
-  hash_map.GetOrSetDefault(m4, create_default_aggregation, 0)
+  hash_map.GetOrSetDefault(m4, create_default_aggregation)
       ->Aggregate(static_cast<int64_t>(1));
   EXPECT_EQ(hash_map.Size(), 3);
 
diff --git a/sdk/test/metrics/cardinality_limit_test.cc b/sdk/test/metrics/cardinality_limit_test.cc
index fcedafe16c..c497c7fa07 100644
--- a/sdk/test/metrics/cardinality_limit_test.cc
+++ b/sdk/test/metrics/cardinality_limit_test.cc
@@ -52,7 +52,7 @@ TEST(CardinalityLimit, AttributesHashMapBasicTests)
   {
     FilteredOrderedAttributeMap attributes = {{"key", std::to_string(i)}};
     static_cast<LongSumAggregation *>(
-        hash_map.GetOrSetDefault(attributes, aggregation_callback, 0))
+        hash_map.GetOrSetDefault(attributes, aggregation_callback))
         ->Aggregate(record_value);
   }
   EXPECT_EQ(hash_map.Size(), 10);
@@ -62,7 +62,7 @@ TEST(CardinalityLimit, AttributesHashMapBasicTests)
   {
     FilteredOrderedAttributeMap attributes = {{"key", std::to_string(i)}};
     static_cast<LongSumAggregation *>(
-        hash_map.GetOrSetDefault(attributes, aggregation_callback, 0))
+        hash_map.GetOrSetDefault(attributes, aggregation_callback))
         ->Aggregate(record_value);
   }
   EXPECT_EQ(hash_map.Size(), 10);  // only one more metric point should be added as overflow.
@@ -72,15 +72,13 @@ TEST(CardinalityLimit, AttributesHashMapBasicTests)
   {
     FilteredOrderedAttributeMap attributes = {{"key", std::to_string(i)}};
     static_cast<LongSumAggregation *>(
-        hash_map.GetOrSetDefault(attributes, aggregation_callback, 0))
+        hash_map.GetOrSetDefault(attributes, aggregation_callback))
         ->Aggregate(record_value);
   }
   EXPECT_EQ(hash_map.Size(), 10);  // no new metric point added
 
   // get the overflow metric point
-  auto agg1 = hash_map.GetOrSetDefault(
-      kOverflowAttributes,
-      aggregation_callback, 0);
+  auto agg1 = hash_map.GetOrSetDefault( kOverflowAttributes, aggregation_callback);
   EXPECT_NE(agg1, nullptr);
   auto sum_agg1 = static_cast<LongSumAggregation *>(agg1);
   EXPECT_EQ(nostd::get<int64_t>(nostd::get<SumPointData>(sum_agg1->ToPoint()).value_),
@@ -89,9 +87,7 @@ TEST(CardinalityLimit, AttributesHashMapBasicTests)
   for (auto i = 0; i < 9; i++)
   {
     FilteredOrderedAttributeMap attributes = {{"key", std::to_string(i)}};
-    auto agg2 = hash_map.GetOrSetDefault(
-        attributes,
-        aggregation_callback, 0);
+    auto agg2 = hash_map.GetOrSetDefault(attributes, aggregation_callback);
     EXPECT_NE(agg2, nullptr);
     auto sum_agg2 = static_cast<LongSumAggregation *>(agg2);
     if (i < 5)

From 1cad020eb760b54b9a820cb73d9bf4bd0d1fce24 Mon Sep 17 00:00:00 2001
From: Tom Tan <totan@microsoft.com>
Date: Tue, 25 Mar 2025 08:57:17 -0700
Subject: [PATCH 05/23] Add new line

---
 .../sdk/metrics/state/filtered_ordered_attribute_map.h          | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/sdk/include/opentelemetry/sdk/metrics/state/filtered_ordered_attribute_map.h b/sdk/include/opentelemetry/sdk/metrics/state/filtered_ordered_attribute_map.h
index 014d6464b7..e501b1f8f8 100644
--- a/sdk/include/opentelemetry/sdk/metrics/state/filtered_ordered_attribute_map.h
+++ b/sdk/include/opentelemetry/sdk/metrics/state/filtered_ordered_attribute_map.h
@@ -66,4 +66,4 @@ class FilteredOrderedAttributeMapHash
 
 }  // namespace metrics
 }  // namespace sdk
-OPENTELEMETRY_END_NAMESPACE
\ No newline at end of file
+OPENTELEMETRY_END_NAMESPACE

From 5ebc039a3d8769f6558cb9baec74eb129c942510 Mon Sep 17 00:00:00 2001
From: Tom Tan <totan@microsoft.com>
Date: Tue, 25 Mar 2025 09:50:00 -0700
Subject: [PATCH 06/23] Removed unused variable

---
 sdk/test/metrics/attributes_hashmap_benchmark.cc | 1 -
 1 file changed, 1 deletion(-)

diff --git a/sdk/test/metrics/attributes_hashmap_benchmark.cc b/sdk/test/metrics/attributes_hashmap_benchmark.cc
index 4e18fe8d44..1068e99127 100644
--- a/sdk/test/metrics/attributes_hashmap_benchmark.cc
+++ b/sdk/test/metrics/attributes_hashmap_benchmark.cc
@@ -39,7 +39,6 @@ void BM_AttributseHashMap(benchmark::State &state)
       return std::unique_ptr<Aggregation>(new DropAggregation);
     };
     m.lock();
-    auto hash = opentelemetry::sdk::common::GetHashForAttributeMap(attributes[i % 2]);
     hash_map.GetOrSetDefault(attributes[i % 2], create_default_aggregation)
         ->Aggregate(static_cast<int64_t>(1));
     benchmark::DoNotOptimize(hash_map.Has(attributes[i % 2]));

From e88c7aeba5fc9e5424ee2ca3c97569f8dc39cc04 Mon Sep 17 00:00:00 2001
From: Tom Tan <totan@microsoft.com>
Date: Tue, 25 Mar 2025 10:01:06 -0700
Subject: [PATCH 07/23] Avoid copying MetricAttributes

---
 .../opentelemetry/sdk/metrics/state/attributes_hashmap.h        | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/sdk/include/opentelemetry/sdk/metrics/state/attributes_hashmap.h b/sdk/include/opentelemetry/sdk/metrics/state/attributes_hashmap.h
index da02dca883..5279aa0720 100644
--- a/sdk/include/opentelemetry/sdk/metrics/state/attributes_hashmap.h
+++ b/sdk/include/opentelemetry/sdk/metrics/state/attributes_hashmap.h
@@ -48,7 +48,7 @@ class AttributesHashMap
   AttributesHashMap(size_t attributes_limit = kAggregationCardinalityLimit)
       : attributes_limit_(attributes_limit)
   {}
-  Aggregation *Get(MetricAttributes attributes) const
+  Aggregation *Get(const MetricAttributes &attributes) const
   {
     auto it = hash_map_.find(attributes);
     if (it != hash_map_.end())

From 1d0ebea0577aea0925c7bd0b6defc38fff4b039e Mon Sep 17 00:00:00 2001
From: Tom Tan <totan@microsoft.com>
Date: Tue, 25 Mar 2025 11:08:18 -0700
Subject: [PATCH 08/23] Update hash for remaining ctor

---
 .../sdk/metrics/state/attributes_hashmap.h    | 37 ++++++++++++++++++-
 .../state/filtered_ordered_attribute_map.h    |  8 +++-
 .../state/filtered_ordered_attribute_map.cc   |  1 -
 3 files changed, 41 insertions(+), 5 deletions(-)

diff --git a/sdk/include/opentelemetry/sdk/metrics/state/attributes_hashmap.h b/sdk/include/opentelemetry/sdk/metrics/state/attributes_hashmap.h
index 5279aa0720..f377038e88 100644
--- a/sdk/include/opentelemetry/sdk/metrics/state/attributes_hashmap.h
+++ b/sdk/include/opentelemetry/sdk/metrics/state/attributes_hashmap.h
@@ -108,6 +108,23 @@ class AttributesHashMap
     return hash_map_[attributes].get();
   }
 
+  Aggregation *GetOrSetDefault(MetricAttributes &&attributes,
+                               std::function<std::unique_ptr<Aggregation>()> aggregation_callback)
+  {
+    auto it = hash_map_.find(attributes);
+    if (it != hash_map_.end())
+    {
+      return it->second.get();
+    }
+
+    if (IsOverflowAttributes())
+    {
+      return GetOrSetOveflowAttributes(aggregation_callback);
+    }
+
+    auto [iter, inserted] = hash_map_.emplace(std::move(attributes), aggregation_callback());
+    return iter->second.get();
+  }
   /**
    * Set the value for given key, overwriting the value if already present
    */
@@ -128,8 +145,7 @@ class AttributesHashMap
     }
     else
     {
-      MetricAttributes attr{attributes, attributes_processor};
-      hash_map_[attr] = std::move(aggr);
+      hash_map_[std::move(attr)] = std::move(aggr);
     }
   }
 
@@ -150,6 +166,23 @@ class AttributesHashMap
     }
   }
 
+  void Set(MetricAttributes &&attributes, std::unique_ptr<Aggregation> aggr)
+  {
+    auto it = hash_map_.find(attributes);
+    if (it != hash_map_.end())
+    {
+      it->second = std::move(aggr);
+    }
+    else if (IsOverflowAttributes())
+    {
+      hash_map_[kOverflowAttributes] = std::move(aggr);
+    }
+    else
+    {
+      hash_map_[std::move(attributes)] = std::move(aggr);
+    }
+  }
+
   /**
    * Iterate the hash to yield key and value stored in hash.
    */
diff --git a/sdk/include/opentelemetry/sdk/metrics/state/filtered_ordered_attribute_map.h b/sdk/include/opentelemetry/sdk/metrics/state/filtered_ordered_attribute_map.h
index e501b1f8f8..bb85108f1d 100644
--- a/sdk/include/opentelemetry/sdk/metrics/state/filtered_ordered_attribute_map.h
+++ b/sdk/include/opentelemetry/sdk/metrics/state/filtered_ordered_attribute_map.h
@@ -29,11 +29,15 @@ class FilteredOrderedAttributeMap : public opentelemetry::sdk::common::OrderedAt
       std::initializer_list<std::pair<nostd::string_view, opentelemetry::common::AttributeValue>>
           attributes)
       : OrderedAttributeMap(attributes)
-  {}
+  {
+    UpdateHash();
+  }
 
   FilteredOrderedAttributeMap(const opentelemetry::common::KeyValueIterable &attributes)
       : FilteredOrderedAttributeMap(attributes, nullptr)
-  {}
+  {
+    // No need to update hash here as it is already updated in the constructor above
+  }
 
   FilteredOrderedAttributeMap(const opentelemetry::common::KeyValueIterable &attributes,
                               const opentelemetry::sdk::metrics::AttributesProcessor *processor);
diff --git a/sdk/src/metrics/state/filtered_ordered_attribute_map.cc b/sdk/src/metrics/state/filtered_ordered_attribute_map.cc
index 7c453bccca..138b2505c4 100644
--- a/sdk/src/metrics/state/filtered_ordered_attribute_map.cc
+++ b/sdk/src/metrics/state/filtered_ordered_attribute_map.cc
@@ -12,7 +12,6 @@ namespace sdk
 {
 namespace metrics
 {
-
 FilteredOrderedAttributeMap::FilteredOrderedAttributeMap(
     const opentelemetry::common::KeyValueIterable &attributes,
     const AttributesProcessor *processor)

From 3dcb0ad9a3ee50490d0a5e1824d47ec43b61ee34 Mon Sep 17 00:00:00 2001
From: Tom Tan <totan@microsoft.com>
Date: Tue, 25 Mar 2025 11:40:30 -0700
Subject: [PATCH 09/23] Add copy/move ctor and assignment operator for
 FilteredOrderedAttributeMap

---
 .../sdk/metrics/state/attributes_hashmap.h        |  4 ++--
 .../state/filtered_ordered_attribute_map.h        | 15 ++++++++++++---
 .../sdk/metrics/view/attributes_processor.h       |  2 ++
 3 files changed, 16 insertions(+), 5 deletions(-)

diff --git a/sdk/include/opentelemetry/sdk/metrics/state/attributes_hashmap.h b/sdk/include/opentelemetry/sdk/metrics/state/attributes_hashmap.h
index f377038e88..9110b44f77 100644
--- a/sdk/include/opentelemetry/sdk/metrics/state/attributes_hashmap.h
+++ b/sdk/include/opentelemetry/sdk/metrics/state/attributes_hashmap.h
@@ -122,8 +122,8 @@ class AttributesHashMap
       return GetOrSetOveflowAttributes(aggregation_callback);
     }
 
-    auto [iter, inserted] = hash_map_.emplace(std::move(attributes), aggregation_callback());
-    return iter->second.get();
+    auto result = hash_map_.emplace(std::move(attributes), aggregation_callback());
+    return result.first->second.get();
   }
   /**
    * Set the value for given key, overwriting the value if already present
diff --git a/sdk/include/opentelemetry/sdk/metrics/state/filtered_ordered_attribute_map.h b/sdk/include/opentelemetry/sdk/metrics/state/filtered_ordered_attribute_map.h
index bb85108f1d..718be114ed 100644
--- a/sdk/include/opentelemetry/sdk/metrics/state/filtered_ordered_attribute_map.h
+++ b/sdk/include/opentelemetry/sdk/metrics/state/filtered_ordered_attribute_map.h
@@ -4,6 +4,7 @@
 #pragma once
 
 #include <initializer_list>
+#include <limits>
 #include <utility>
 
 #include "opentelemetry/common/attribute_value.h"
@@ -47,16 +48,24 @@ class FilteredOrderedAttributeMap : public opentelemetry::sdk::common::OrderedAt
           attributes,
       const opentelemetry::sdk::metrics::AttributesProcessor *processor);
 
-  size_t GetHash() const { return _hash; }
+  //
+  // Copy and move constructors, assignment operators
+  //
+  FilteredOrderedAttributeMap(const FilteredOrderedAttributeMap &other) = default;
+  FilteredOrderedAttributeMap(FilteredOrderedAttributeMap &&other)      = default;
+  FilteredOrderedAttributeMap &operator=(const FilteredOrderedAttributeMap &other) = default;
+  FilteredOrderedAttributeMap &operator=(FilteredOrderedAttributeMap &&other) = default;
 
-private:
+  size_t GetHash() const { return _hash; }
 
   void UpdateHash()
   {
     _hash = GetHashForAttributeMap(*this);
   }
 
-  size_t _hash = 0UL;
+private:
+
+  size_t _hash = (std::numeric_limits<size_t>::max)();
 };
 
 class FilteredOrderedAttributeMapHash
diff --git a/sdk/include/opentelemetry/sdk/metrics/view/attributes_processor.h b/sdk/include/opentelemetry/sdk/metrics/view/attributes_processor.h
index 4e20e7d65f..7ab8cafb13 100644
--- a/sdk/include/opentelemetry/sdk/metrics/view/attributes_processor.h
+++ b/sdk/include/opentelemetry/sdk/metrics/view/attributes_processor.h
@@ -87,6 +87,8 @@ class FilteringAttributesProcessor : public AttributesProcessor
           }
           return true;
         });
+
+    result.UpdateHash();
     return result;
   }
 

From 6b899efa7a0e06a45162a69a9f1f006b24807aa7 Mon Sep 17 00:00:00 2001
From: Tom Tan <totan@microsoft.com>
Date: Tue, 25 Mar 2025 14:29:50 -0700
Subject: [PATCH 10/23] Add unit test on hash collision of MetricAttributes

---
 .../opentelemetry/sdk/metrics/instruments.h   |  1 +
 .../sdk/metrics/state/attributes_hashmap.h    | 15 ++++-
 sdk/test/metrics/CMakeLists.txt               |  2 +
 sdk/test/metrics/attributes_hashmap_test.cc   | 55 +++++++++++++++++++
 4 files changed, 70 insertions(+), 3 deletions(-)

diff --git a/sdk/include/opentelemetry/sdk/metrics/instruments.h b/sdk/include/opentelemetry/sdk/metrics/instruments.h
index 76ad07b2e3..6473a267f2 100644
--- a/sdk/include/opentelemetry/sdk/metrics/instruments.h
+++ b/sdk/include/opentelemetry/sdk/metrics/instruments.h
@@ -65,6 +65,7 @@ struct InstrumentDescriptor
 };
 
 using MetricAttributes               = opentelemetry::sdk::metrics::FilteredOrderedAttributeMap;
+using MetricAttributesHash           = opentelemetry::sdk::metrics::FilteredOrderedAttributeMapHash;
 using AggregationTemporalitySelector = std::function<AggregationTemporality(InstrumentType)>;
 
 /*class InstrumentSelector {
diff --git a/sdk/include/opentelemetry/sdk/metrics/state/attributes_hashmap.h b/sdk/include/opentelemetry/sdk/metrics/state/attributes_hashmap.h
index 9110b44f77..6f2d5e2cc9 100644
--- a/sdk/include/opentelemetry/sdk/metrics/state/attributes_hashmap.h
+++ b/sdk/include/opentelemetry/sdk/metrics/state/attributes_hashmap.h
@@ -42,10 +42,11 @@ class AttributeHashGenerator
   }
 };
 
-class AttributesHashMap
+template <typename CustomHash=MetricAttributesHash>
+class AttributesHashMapWithCustomHash
 {
 public:
-  AttributesHashMap(size_t attributes_limit = kAggregationCardinalityLimit)
+  AttributesHashMapWithCustomHash(size_t attributes_limit = kAggregationCardinalityLimit)
       : attributes_limit_(attributes_limit)
   {}
   Aggregation *Get(const MetricAttributes &attributes) const
@@ -204,8 +205,13 @@ class AttributesHashMap
    */
   size_t Size() { return hash_map_.size(); }
 
+#ifdef UNIT_TESTING
+  size_t BucketCount() { return hash_map_.bucket_count(); }
+  size_t BucketSize(size_t n) { return hash_map_.bucket_size(n); }
+#endif
+
 private:
-  std::unordered_map<MetricAttributes, std::unique_ptr<Aggregation>, FilteredOrderedAttributeMapHash> hash_map_;
+  std::unordered_map<MetricAttributes, std::unique_ptr<Aggregation>, CustomHash> hash_map_;
   size_t attributes_limit_;
 
   Aggregation *GetOrSetOveflowAttributes(
@@ -230,6 +236,9 @@ class AttributesHashMap
 
   bool IsOverflowAttributes() const { return (hash_map_.size() + 1 >= attributes_limit_); }
 };
+
+using AttributesHashMap = AttributesHashMapWithCustomHash<>;
+
 }  // namespace metrics
 
 }  // namespace sdk
diff --git a/sdk/test/metrics/CMakeLists.txt b/sdk/test/metrics/CMakeLists.txt
index 4e09af4d2e..cd87f400c0 100644
--- a/sdk/test/metrics/CMakeLists.txt
+++ b/sdk/test/metrics/CMakeLists.txt
@@ -38,6 +38,8 @@ foreach(
   target_link_libraries(
     ${testname} ${GTEST_BOTH_LIBRARIES} ${CMAKE_THREAD_LIBS_INIT}
     metrics_common_test_utils opentelemetry_resources)
+  target_compile_definitions(${testname} PRIVATE
+                             UNIT_TESTING)
   gtest_add_tests(
     TARGET ${testname}
     TEST_PREFIX metrics.
diff --git a/sdk/test/metrics/attributes_hashmap_test.cc b/sdk/test/metrics/attributes_hashmap_test.cc
index d6a50f7a4c..77daa65b08 100644
--- a/sdk/test/metrics/attributes_hashmap_test.cc
+++ b/sdk/test/metrics/attributes_hashmap_test.cc
@@ -70,6 +70,10 @@ TEST(AttributesHashMap, BasicTests)
   MetricAttributes m5 = {{"k2", "v2"}, {"k1", "v1"}};
   EXPECT_EQ(hash_map.Has(m5), true);
 
+  // Set attributes with different order - shouldn't create a new entry.
+  MetricAttributes m6 = {{"k1", "v2"}, {"k2", "v1"}};
+  EXPECT_EQ(hash_map.Has(m6), false);
+
   // GetAllEnteries
   size_t count = 0;
   hash_map.GetAllEnteries(
@@ -80,6 +84,57 @@ TEST(AttributesHashMap, BasicTests)
   EXPECT_EQ(count, hash_map.Size());
 }
 
+class MetricAttributeMapHashForCollision
+{
+public:
+  size_t operator()(const MetricAttributes &attributes) const
+  {
+    return 42;
+  }
+};
+
+TEST(AttributesHashMap, CollisionTest)
+{
+  // The hash on MetricsAttributes will be ignored by MetricAttributeMapHashForCollision
+  MetricAttributes m1 = {{"k1", "v1"}};
+  MetricAttributes m2 = {{"k2", "v2"}};
+  MetricAttributes m3 = {{"k1", "v1"}, {"k2", "v2"}};
+  MetricAttributes m4 = {};
+
+  AttributesHashMapWithCustomHash<MetricAttributeMapHashForCollision> hash_map;
+
+  hash_map.Set(m1, std::unique_ptr<Aggregation>(new DropAggregation()));
+  hash_map.Set(m2, std::unique_ptr<Aggregation>(new DropAggregation()));
+  hash_map.Set(m3, std::unique_ptr<Aggregation>(new DropAggregation()));
+  hash_map.Set(m4, std::unique_ptr<Aggregation>(new DropAggregation()));
+
+  EXPECT_EQ(hash_map.Size(), 4);
+  EXPECT_EQ(hash_map.Has(m1), true);
+  EXPECT_EQ(hash_map.Has(m2), true);
+  EXPECT_EQ(hash_map.Has(m3), true);
+  EXPECT_EQ(hash_map.Has(m4), true);
+
+  MetricAttributes m5 = {{"k2", "v1"}};
+  EXPECT_EQ(hash_map.Has(m5), false);
+
+  //
+  // Verify only one bucket used based on the custom hash
+  //
+  size_t total_active_buckets = 0;
+  size_t total_elements = 0;
+  for (size_t i = 0; i < hash_map.BucketCount(); i++)
+  {
+    size_t bucket_size = hash_map.BucketSize(i);
+    if (bucket_size > 0)
+    {
+      total_active_buckets++;
+      total_elements += bucket_size;
+    }
+  }
+  EXPECT_EQ(total_active_buckets, 1);
+  EXPECT_EQ(total_elements, 4);
+}
+
 std::string make_unique_string(const char *str)
 {
   return std::string(str);

From d836c10969be74bd802353f80358a45e692b1262 Mon Sep 17 00:00:00 2001
From: Tom Tan <totan@microsoft.com>
Date: Tue, 25 Mar 2025 14:39:41 -0700
Subject: [PATCH 11/23] Fix bazel build

---
 sdk/test/metrics/BUILD | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/sdk/test/metrics/BUILD b/sdk/test/metrics/BUILD
index 929511a89b..d96aa521c5 100644
--- a/sdk/test/metrics/BUILD
+++ b/sdk/test/metrics/BUILD
@@ -44,6 +44,9 @@ cc_test(
         "metrics_common_test_utils",
         "@com_google_googletest//:gtest_main",
     ],
+    copts = [
+        "-DUNIT_TESTING",
+    ],
 )
 
 otel_cc_benchmark(

From 44a467a04cab3d08fa61d3f99098d721ea44dc9b Mon Sep 17 00:00:00 2001
From: Tom Tan <totan@microsoft.com>
Date: Tue, 25 Mar 2025 14:42:36 -0700
Subject: [PATCH 12/23] Format

---
 .../sdk/metrics/state/attributes_hashmap.h        | 12 ++++++++----
 .../state/filtered_ordered_attribute_map.h        | 15 ++++++---------
 .../state/filtered_ordered_attribute_map.cc       |  4 ++--
 sdk/src/metrics/state/temporal_metric_storage.cc  |  4 ++--
 sdk/test/metrics/attributes_hashmap_test.cc       | 10 +++-------
 sdk/test/metrics/cardinality_limit_test.cc        | 11 ++++-------
 6 files changed, 25 insertions(+), 31 deletions(-)

diff --git a/sdk/include/opentelemetry/sdk/metrics/state/attributes_hashmap.h b/sdk/include/opentelemetry/sdk/metrics/state/attributes_hashmap.h
index 6f2d5e2cc9..6a329418d4 100644
--- a/sdk/include/opentelemetry/sdk/metrics/state/attributes_hashmap.h
+++ b/sdk/include/opentelemetry/sdk/metrics/state/attributes_hashmap.h
@@ -30,8 +30,9 @@ using opentelemetry::sdk::common::OrderedAttributeMap;
 constexpr size_t kAggregationCardinalityLimit = 2000;
 const std::string kAttributesLimitOverflowKey = "otel.metrics.overflow";
 const bool kAttributesLimitOverflowValue      = true;
-const MetricAttributes kOverflowAttributes = {
-    {kAttributesLimitOverflowKey, kAttributesLimitOverflowValue}};  // precalculated for optimization
+const MetricAttributes kOverflowAttributes    = {
+    {kAttributesLimitOverflowKey,
+        kAttributesLimitOverflowValue}};  // precalculated for optimization
 
 class AttributeHashGenerator
 {
@@ -42,7 +43,7 @@ class AttributeHashGenerator
   }
 };
 
-template <typename CustomHash=MetricAttributesHash>
+template <typename CustomHash = MetricAttributesHash>
 class AttributesHashMapWithCustomHash
 {
 public:
@@ -63,7 +64,10 @@ class AttributesHashMapWithCustomHash
    * @return check if key is present in hash
    *
    */
-  bool Has(const MetricAttributes& attributes) const { return hash_map_.find(attributes) != hash_map_.end(); }
+  bool Has(const MetricAttributes &attributes) const
+  {
+    return hash_map_.find(attributes) != hash_map_.end();
+  }
 
   /**
    * @return the pointer to value for given key if present.
diff --git a/sdk/include/opentelemetry/sdk/metrics/state/filtered_ordered_attribute_map.h b/sdk/include/opentelemetry/sdk/metrics/state/filtered_ordered_attribute_map.h
index 718be114ed..3db157c4d0 100644
--- a/sdk/include/opentelemetry/sdk/metrics/state/filtered_ordered_attribute_map.h
+++ b/sdk/include/opentelemetry/sdk/metrics/state/filtered_ordered_attribute_map.h
@@ -51,27 +51,24 @@ class FilteredOrderedAttributeMap : public opentelemetry::sdk::common::OrderedAt
   //
   // Copy and move constructors, assignment operators
   //
-  FilteredOrderedAttributeMap(const FilteredOrderedAttributeMap &other) = default;
-  FilteredOrderedAttributeMap(FilteredOrderedAttributeMap &&other)      = default;
+  FilteredOrderedAttributeMap(const FilteredOrderedAttributeMap &other)            = default;
+  FilteredOrderedAttributeMap(FilteredOrderedAttributeMap &&other)                 = default;
   FilteredOrderedAttributeMap &operator=(const FilteredOrderedAttributeMap &other) = default;
-  FilteredOrderedAttributeMap &operator=(FilteredOrderedAttributeMap &&other) = default;
+  FilteredOrderedAttributeMap &operator=(FilteredOrderedAttributeMap &&other)      = default;
 
   size_t GetHash() const { return _hash; }
 
-  void UpdateHash()
-  {
-    _hash = GetHashForAttributeMap(*this);
-  }
+  void UpdateHash() { _hash = GetHashForAttributeMap(*this); }
 
 private:
-
   size_t _hash = (std::numeric_limits<size_t>::max)();
 };
 
 class FilteredOrderedAttributeMapHash
 {
 public:
-  size_t operator()(const opentelemetry::sdk::metrics::FilteredOrderedAttributeMap &attributes) const
+  size_t operator()(
+      const opentelemetry::sdk::metrics::FilteredOrderedAttributeMap &attributes) const
   {
     return attributes.GetHash();
   }
diff --git a/sdk/src/metrics/state/filtered_ordered_attribute_map.cc b/sdk/src/metrics/state/filtered_ordered_attribute_map.cc
index 138b2505c4..0d3f9b606e 100644
--- a/sdk/src/metrics/state/filtered_ordered_attribute_map.cc
+++ b/sdk/src/metrics/state/filtered_ordered_attribute_map.cc
@@ -1,10 +1,10 @@
 // Copyright The OpenTelemetry Authors
 // SPDX-License-Identifier: Apache-2.0
 
-#include "opentelemetry/sdk/common/attributemap_hash.h"
-#include "opentelemetry/sdk/common/attribute_utils.h"
 #include "opentelemetry/sdk/metrics/state/filtered_ordered_attribute_map.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/view/attributes_processor.h"
 
 OPENTELEMETRY_BEGIN_NAMESPACE
diff --git a/sdk/src/metrics/state/temporal_metric_storage.cc b/sdk/src/metrics/state/temporal_metric_storage.cc
index 8abf7193b0..abf48099a8 100644
--- a/sdk/src/metrics/state/temporal_metric_storage.cc
+++ b/sdk/src/metrics/state/temporal_metric_storage.cc
@@ -104,7 +104,7 @@ bool TemporalMetricStorage::buildMetrics(CollectorHandle *collector,
   {
     agg_hashmap->GetAllEnteries(
         [&merged_metrics, this](const MetricAttributes &attributes, Aggregation &aggregation) {
-          auto agg  = merged_metrics->Get(attributes);
+          auto agg = merged_metrics->Get(attributes);
           if (agg)
           {
             merged_metrics->Set(attributes, agg->Merge(aggregation));
@@ -137,7 +137,7 @@ bool TemporalMetricStorage::buildMetrics(CollectorHandle *collector,
       // merge current delta to previous cumulative
       last_aggr_hashmap->GetAllEnteries(
           [&merged_metrics, this](const MetricAttributes &attributes, Aggregation &aggregation) {
-            auto agg  = merged_metrics->Get(attributes);
+            auto agg = merged_metrics->Get(attributes);
             if (agg)
             {
               merged_metrics->Set(attributes, agg->Merge(aggregation));
diff --git a/sdk/test/metrics/attributes_hashmap_test.cc b/sdk/test/metrics/attributes_hashmap_test.cc
index 77daa65b08..baebea3d13 100644
--- a/sdk/test/metrics/attributes_hashmap_test.cc
+++ b/sdk/test/metrics/attributes_hashmap_test.cc
@@ -62,8 +62,7 @@ TEST(AttributesHashMap, BasicTests)
     return std::unique_ptr<Aggregation>(new DropAggregation);
   };
   MetricAttributes m4 = {{"k1", "v1"}, {"k2", "v2"}, {"k3", "v3"}};
-  hash_map.GetOrSetDefault(m4, create_default_aggregation)
-      ->Aggregate(static_cast<int64_t>(1));
+  hash_map.GetOrSetDefault(m4, create_default_aggregation)->Aggregate(static_cast<int64_t>(1));
   EXPECT_EQ(hash_map.Size(), 3);
 
   // Set attributes with different order - shouldn't create a new entry.
@@ -87,10 +86,7 @@ TEST(AttributesHashMap, BasicTests)
 class MetricAttributeMapHashForCollision
 {
 public:
-  size_t operator()(const MetricAttributes &attributes) const
-  {
-    return 42;
-  }
+  size_t operator()(const MetricAttributes &attributes) const { return 42; }
 };
 
 TEST(AttributesHashMap, CollisionTest)
@@ -121,7 +117,7 @@ TEST(AttributesHashMap, CollisionTest)
   // Verify only one bucket used based on the custom hash
   //
   size_t total_active_buckets = 0;
-  size_t total_elements = 0;
+  size_t total_elements       = 0;
   for (size_t i = 0; i < hash_map.BucketCount(); i++)
   {
     size_t bucket_size = hash_map.BucketSize(i);
diff --git a/sdk/test/metrics/cardinality_limit_test.cc b/sdk/test/metrics/cardinality_limit_test.cc
index c497c7fa07..a24c402452 100644
--- a/sdk/test/metrics/cardinality_limit_test.cc
+++ b/sdk/test/metrics/cardinality_limit_test.cc
@@ -51,8 +51,7 @@ TEST(CardinalityLimit, AttributesHashMapBasicTests)
   for (auto i = 0; i < 10; i++)
   {
     FilteredOrderedAttributeMap attributes = {{"key", std::to_string(i)}};
-    static_cast<LongSumAggregation *>(
-        hash_map.GetOrSetDefault(attributes, aggregation_callback))
+    static_cast<LongSumAggregation *>(hash_map.GetOrSetDefault(attributes, aggregation_callback))
         ->Aggregate(record_value);
   }
   EXPECT_EQ(hash_map.Size(), 10);
@@ -61,8 +60,7 @@ TEST(CardinalityLimit, AttributesHashMapBasicTests)
   for (auto i = 10; i < 15; i++)
   {
     FilteredOrderedAttributeMap attributes = {{"key", std::to_string(i)}};
-    static_cast<LongSumAggregation *>(
-        hash_map.GetOrSetDefault(attributes, aggregation_callback))
+    static_cast<LongSumAggregation *>(hash_map.GetOrSetDefault(attributes, aggregation_callback))
         ->Aggregate(record_value);
   }
   EXPECT_EQ(hash_map.Size(), 10);  // only one more metric point should be added as overflow.
@@ -71,14 +69,13 @@ TEST(CardinalityLimit, AttributesHashMapBasicTests)
   for (auto i = 0; i < 5; i++)
   {
     FilteredOrderedAttributeMap attributes = {{"key", std::to_string(i)}};
-    static_cast<LongSumAggregation *>(
-        hash_map.GetOrSetDefault(attributes, aggregation_callback))
+    static_cast<LongSumAggregation *>(hash_map.GetOrSetDefault(attributes, aggregation_callback))
         ->Aggregate(record_value);
   }
   EXPECT_EQ(hash_map.Size(), 10);  // no new metric point added
 
   // get the overflow metric point
-  auto agg1 = hash_map.GetOrSetDefault( kOverflowAttributes, aggregation_callback);
+  auto agg1 = hash_map.GetOrSetDefault(kOverflowAttributes, aggregation_callback);
   EXPECT_NE(agg1, nullptr);
   auto sum_agg1 = static_cast<LongSumAggregation *>(agg1);
   EXPECT_EQ(nostd::get<int64_t>(nostd::get<SumPointData>(sum_agg1->ToPoint()).value_),

From a7698d01ced31b6e6d57d5370208cb15df77e45f Mon Sep 17 00:00:00 2001
From: Tom Tan <totan@microsoft.com>
Date: Tue, 25 Mar 2025 14:48:01 -0700
Subject: [PATCH 13/23] More format

---
 sdk/test/metrics/BUILD          | 6 +++---
 sdk/test/metrics/CMakeLists.txt | 3 +--
 2 files changed, 4 insertions(+), 5 deletions(-)

diff --git a/sdk/test/metrics/BUILD b/sdk/test/metrics/BUILD
index d96aa521c5..519cb1fc1f 100644
--- a/sdk/test/metrics/BUILD
+++ b/sdk/test/metrics/BUILD
@@ -36,6 +36,9 @@ cc_test(
 cc_test(
     name = "all_tests",
     srcs = glob(["*_test.cc"]),
+    copts = [
+        "-DUNIT_TESTING",
+    ],
     tags = [
         "metrics",
         "test",
@@ -44,9 +47,6 @@ cc_test(
         "metrics_common_test_utils",
         "@com_google_googletest//:gtest_main",
     ],
-    copts = [
-        "-DUNIT_TESTING",
-    ],
 )
 
 otel_cc_benchmark(
diff --git a/sdk/test/metrics/CMakeLists.txt b/sdk/test/metrics/CMakeLists.txt
index cd87f400c0..79b8c28a2f 100644
--- a/sdk/test/metrics/CMakeLists.txt
+++ b/sdk/test/metrics/CMakeLists.txt
@@ -38,8 +38,7 @@ foreach(
   target_link_libraries(
     ${testname} ${GTEST_BOTH_LIBRARIES} ${CMAKE_THREAD_LIBS_INIT}
     metrics_common_test_utils opentelemetry_resources)
-  target_compile_definitions(${testname} PRIVATE
-                             UNIT_TESTING)
+  target_compile_definitions(${testname} PRIVATE UNIT_TESTING)
   gtest_add_tests(
     TARGET ${testname}
     TEST_PREFIX metrics.

From 72ef04e6ffd8849bbb225c835a1c62b79db21aad Mon Sep 17 00:00:00 2001
From: Tom Tan <totan@microsoft.com>
Date: Tue, 25 Mar 2025 14:50:58 -0700
Subject: [PATCH 14/23] Update changelog

---
 CHANGELOG.md | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 62262afe08..62914b9c4e 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -15,6 +15,9 @@ Increment the:
 
 ## [Unreleased]
 
+* [METRICS SDK] Fix hash collision in MetricAttributes
+  [#3322](https://github.com/open-telemetry/opentelemetry-cpp/pull/3322)
+
 * [BUILD] Fix misssing exported definition for OTLP file exporter and forceflush
   [#3319](https://github.com/open-telemetry/opentelemetry-cpp/pull/3319)
 

From d9d0e82c414c1954304694f494dd4bff59ba1379 Mon Sep 17 00:00:00 2001
From: Tom Tan <totan@microsoft.com>
Date: Tue, 25 Mar 2025 15:14:58 -0700
Subject: [PATCH 15/23] Fix unused parameter

---
 sdk/test/metrics/attributes_hashmap_test.cc | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/sdk/test/metrics/attributes_hashmap_test.cc b/sdk/test/metrics/attributes_hashmap_test.cc
index baebea3d13..b1c9eff3e7 100644
--- a/sdk/test/metrics/attributes_hashmap_test.cc
+++ b/sdk/test/metrics/attributes_hashmap_test.cc
@@ -86,7 +86,11 @@ TEST(AttributesHashMap, BasicTests)
 class MetricAttributeMapHashForCollision
 {
 public:
-  size_t operator()(const MetricAttributes &attributes) const { return 42; }
+  size_t operator()(const MetricAttributes &attributes) const
+  {
+    (void)attributes;
+    return 42;
+  }
 };
 
 TEST(AttributesHashMap, CollisionTest)

From f17399c4467c67a047999501b9640d37ab854006 Mon Sep 17 00:00:00 2001
From: Tom Tan <totan@microsoft.com>
Date: Tue, 25 Mar 2025 17:01:33 -0700
Subject: [PATCH 16/23] Add equity operator for MetricsAttributes

---
 .../opentelemetry/sdk/metrics/state/attributes_hashmap.h | 8 ++++----
 .../sdk/metrics/state/filtered_ordered_attribute_map.h   | 9 +++++++++
 sdk/test/metrics/attributes_hashmap_test.cc              | 6 +-----
 3 files changed, 14 insertions(+), 9 deletions(-)

diff --git a/sdk/include/opentelemetry/sdk/metrics/state/attributes_hashmap.h b/sdk/include/opentelemetry/sdk/metrics/state/attributes_hashmap.h
index 6a329418d4..0d89e6ac6b 100644
--- a/sdk/include/opentelemetry/sdk/metrics/state/attributes_hashmap.h
+++ b/sdk/include/opentelemetry/sdk/metrics/state/attributes_hashmap.h
@@ -91,8 +91,8 @@ class AttributesHashMapWithCustomHash
       return GetOrSetOveflowAttributes(aggregation_callback);
     }
 
-    hash_map_[attr] = aggregation_callback();
-    return hash_map_[attr].get();
+    auto result = hash_map_.emplace(std::move(attr), aggregation_callback());
+    return result.first->second.get();
   }
 
   Aggregation *GetOrSetDefault(const MetricAttributes &attributes,
@@ -233,9 +233,9 @@ class AttributesHashMapWithCustomHash
       return it->second.get();
     }
 
-    MetricAttributes attr{{kAttributesLimitOverflowKey, kAttributesLimitOverflowValue}};
     hash_map_[kOverflowAttributes] = std::move(agg);
-    return hash_map_[kOverflowAttributes].get();
+    auto result                    = hash_map_.emplace(kOverflowAttributes, std::move(agg));
+    return result.first->second.get();
   }
 
   bool IsOverflowAttributes() const { return (hash_map_.size() + 1 >= attributes_limit_); }
diff --git a/sdk/include/opentelemetry/sdk/metrics/state/filtered_ordered_attribute_map.h b/sdk/include/opentelemetry/sdk/metrics/state/filtered_ordered_attribute_map.h
index 3db157c4d0..1e3deb0705 100644
--- a/sdk/include/opentelemetry/sdk/metrics/state/filtered_ordered_attribute_map.h
+++ b/sdk/include/opentelemetry/sdk/metrics/state/filtered_ordered_attribute_map.h
@@ -56,6 +56,15 @@ class FilteredOrderedAttributeMap : public opentelemetry::sdk::common::OrderedAt
   FilteredOrderedAttributeMap &operator=(const FilteredOrderedAttributeMap &other) = default;
   FilteredOrderedAttributeMap &operator=(FilteredOrderedAttributeMap &&other)      = default;
 
+  //
+  // equality operator
+  //
+  bool operator==(const FilteredOrderedAttributeMap &other) const
+  {
+    return _hash == other._hash && static_cast<const OrderedAttributeMap &>(*this) ==
+                                       static_cast<const OrderedAttributeMap &>(other);
+  }
+
   size_t GetHash() const { return _hash; }
 
   void UpdateHash() { _hash = GetHashForAttributeMap(*this); }
diff --git a/sdk/test/metrics/attributes_hashmap_test.cc b/sdk/test/metrics/attributes_hashmap_test.cc
index b1c9eff3e7..7984d11cef 100644
--- a/sdk/test/metrics/attributes_hashmap_test.cc
+++ b/sdk/test/metrics/attributes_hashmap_test.cc
@@ -86,11 +86,7 @@ TEST(AttributesHashMap, BasicTests)
 class MetricAttributeMapHashForCollision
 {
 public:
-  size_t operator()(const MetricAttributes &attributes) const
-  {
-    (void)attributes;
-    return 42;
-  }
+  size_t operator()(const MetricAttributes & /*attributes*/) const { return 42; }
 };
 
 TEST(AttributesHashMap, CollisionTest)

From 809a70a6804195626f758bd934482f9a7a5744cb Mon Sep 17 00:00:00 2001
From: Tom Tan <totan@microsoft.com>
Date: Tue, 25 Mar 2025 17:30:20 -0700
Subject: [PATCH 17/23] Update hash for empty MetricAttributes

---
 .../sdk/metrics/state/filtered_ordered_attribute_map.h          | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/sdk/include/opentelemetry/sdk/metrics/state/filtered_ordered_attribute_map.h b/sdk/include/opentelemetry/sdk/metrics/state/filtered_ordered_attribute_map.h
index 1e3deb0705..42d660b533 100644
--- a/sdk/include/opentelemetry/sdk/metrics/state/filtered_ordered_attribute_map.h
+++ b/sdk/include/opentelemetry/sdk/metrics/state/filtered_ordered_attribute_map.h
@@ -24,7 +24,7 @@ class AttributesProcessor;  // IWYU pragma: keep
 class FilteredOrderedAttributeMap : public opentelemetry::sdk::common::OrderedAttributeMap
 {
 public:
-  FilteredOrderedAttributeMap() = default;
+  FilteredOrderedAttributeMap() : OrderedAttributeMap() { UpdateHash(); }
 
   FilteredOrderedAttributeMap(
       std::initializer_list<std::pair<nostd::string_view, opentelemetry::common::AttributeValue>>

From 2c017e3abe132cb7eb15efcfddea87b34d3fe4b6 Mon Sep 17 00:00:00 2001
From: Tom Tan <totan@microsoft.com>
Date: Tue, 25 Mar 2025 18:47:25 -0700
Subject: [PATCH 18/23] Fix reservior test

---
 sdk/include/opentelemetry/sdk/metrics/exemplar/reservoir_cell.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/sdk/include/opentelemetry/sdk/metrics/exemplar/reservoir_cell.h b/sdk/include/opentelemetry/sdk/metrics/exemplar/reservoir_cell.h
index 03941870c0..cc315b7131 100644
--- a/sdk/include/opentelemetry/sdk/metrics/exemplar/reservoir_cell.h
+++ b/sdk/include/opentelemetry/sdk/metrics/exemplar/reservoir_cell.h
@@ -125,6 +125,7 @@ class ReservoirCell
         res.erase(it);
       }
     }
+    res.UpdateHash();
     return res;
   }
 

From a5735f3515e4736cab093b80c396287f5f56b464 Mon Sep 17 00:00:00 2001
From: Tom Tan <totan@microsoft.com>
Date: Tue, 25 Mar 2025 20:56:07 -0700
Subject: [PATCH 19/23] Remove unnecessary set

---
 .../opentelemetry/sdk/metrics/state/attributes_hashmap.h       | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/sdk/include/opentelemetry/sdk/metrics/state/attributes_hashmap.h b/sdk/include/opentelemetry/sdk/metrics/state/attributes_hashmap.h
index 0d89e6ac6b..e7b8977062 100644
--- a/sdk/include/opentelemetry/sdk/metrics/state/attributes_hashmap.h
+++ b/sdk/include/opentelemetry/sdk/metrics/state/attributes_hashmap.h
@@ -233,8 +233,7 @@ class AttributesHashMapWithCustomHash
       return it->second.get();
     }
 
-    hash_map_[kOverflowAttributes] = std::move(agg);
-    auto result                    = hash_map_.emplace(kOverflowAttributes, std::move(agg));
+    auto result = hash_map_.emplace(kOverflowAttributes, std::move(agg));
     return result.first->second.get();
   }
 

From 95a9f7f10e608f8e228386dc768df064f7f8948e Mon Sep 17 00:00:00 2001
From: Tom Tan <totan@microsoft.com>
Date: Wed, 26 Mar 2025 14:16:44 -0700
Subject: [PATCH 20/23] Fix iwyu

---
 sdk/include/opentelemetry/sdk/common/attributemap_hash.h  | 4 ----
 .../sdk/metrics/state/filtered_ordered_attribute_map.h    | 8 +++++++-
 sdk/src/metrics/state/filtered_ordered_attribute_map.cc   | 1 -
 sdk/src/metrics/state/temporal_metric_storage.cc          | 1 -
 sdk/test/metrics/attributes_hashmap_benchmark.cc          | 2 --
 sdk/test/metrics/attributes_hashmap_test.cc               | 1 -
 sdk/test/metrics/observer_result_test.cc                  | 2 --
 7 files changed, 7 insertions(+), 12 deletions(-)

diff --git a/sdk/include/opentelemetry/sdk/common/attributemap_hash.h b/sdk/include/opentelemetry/sdk/common/attributemap_hash.h
index 7fddd02c8c..e56d6aaf75 100644
--- a/sdk/include/opentelemetry/sdk/common/attributemap_hash.h
+++ b/sdk/include/opentelemetry/sdk/common/attributemap_hash.h
@@ -10,10 +10,6 @@
 #include <utility>
 #include <vector>
 
-#include "opentelemetry/common/attribute_value.h"
-#include "opentelemetry/common/key_value_iterable.h"
-#include "opentelemetry/nostd/function_ref.h"
-#include "opentelemetry/nostd/string_view.h"
 #include "opentelemetry/nostd/variant.h"
 #include "opentelemetry/sdk/common/attribute_utils.h"
 #include "opentelemetry/version.h"
diff --git a/sdk/include/opentelemetry/sdk/metrics/state/filtered_ordered_attribute_map.h b/sdk/include/opentelemetry/sdk/metrics/state/filtered_ordered_attribute_map.h
index 42d660b533..ac0dcbea4a 100644
--- a/sdk/include/opentelemetry/sdk/metrics/state/filtered_ordered_attribute_map.h
+++ b/sdk/include/opentelemetry/sdk/metrics/state/filtered_ordered_attribute_map.h
@@ -3,13 +3,19 @@
 
 #pragma once
 
+#include <stddef.h>
+#include <algorithm>
+#include <functional>
 #include <initializer_list>
 #include <limits>
+#include <map>
+#include <string>
 #include <utility>
-
+#include <vector>
 #include "opentelemetry/common/attribute_value.h"
 #include "opentelemetry/common/key_value_iterable.h"
 #include "opentelemetry/nostd/string_view.h"
+#include "opentelemetry/nostd/variant.h"
 #include "opentelemetry/sdk/common/attribute_utils.h"
 #include "opentelemetry/sdk/common/attributemap_hash.h"
 #include "opentelemetry/version.h"
diff --git a/sdk/src/metrics/state/filtered_ordered_attribute_map.cc b/sdk/src/metrics/state/filtered_ordered_attribute_map.cc
index 0d3f9b606e..85f4bc9d6e 100644
--- a/sdk/src/metrics/state/filtered_ordered_attribute_map.cc
+++ b/sdk/src/metrics/state/filtered_ordered_attribute_map.cc
@@ -4,7 +4,6 @@
 #include "opentelemetry/sdk/metrics/state/filtered_ordered_attribute_map.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/view/attributes_processor.h"
 
 OPENTELEMETRY_BEGIN_NAMESPACE
diff --git a/sdk/src/metrics/state/temporal_metric_storage.cc b/sdk/src/metrics/state/temporal_metric_storage.cc
index abf48099a8..d027c9883c 100644
--- a/sdk/src/metrics/state/temporal_metric_storage.cc
+++ b/sdk/src/metrics/state/temporal_metric_storage.cc
@@ -13,7 +13,6 @@
 #include "opentelemetry/common/timestamp.h"
 #include "opentelemetry/nostd/function_ref.h"
 #include "opentelemetry/nostd/span.h"
-#include "opentelemetry/sdk/common/attributemap_hash.h"
 #include "opentelemetry/sdk/metrics/aggregation/aggregation.h"
 #include "opentelemetry/sdk/metrics/aggregation/aggregation_config.h"
 #include "opentelemetry/sdk/metrics/aggregation/default_aggregation.h"
diff --git a/sdk/test/metrics/attributes_hashmap_benchmark.cc b/sdk/test/metrics/attributes_hashmap_benchmark.cc
index 1068e99127..8225f8ba12 100644
--- a/sdk/test/metrics/attributes_hashmap_benchmark.cc
+++ b/sdk/test/metrics/attributes_hashmap_benchmark.cc
@@ -11,11 +11,9 @@
 #include <thread>
 #include <vector>
 
-#include "opentelemetry/sdk/common/attributemap_hash.h"
 #include "opentelemetry/sdk/metrics/aggregation/aggregation.h"
 #include "opentelemetry/sdk/metrics/aggregation/drop_aggregation.h"
 #include "opentelemetry/sdk/metrics/state/attributes_hashmap.h"
-#include "opentelemetry/sdk/metrics/state/filtered_ordered_attribute_map.h"
 #include "opentelemetry/sdk/metrics/view/attributes_processor.h"
 
 using namespace opentelemetry::sdk::metrics;
diff --git a/sdk/test/metrics/attributes_hashmap_test.cc b/sdk/test/metrics/attributes_hashmap_test.cc
index 7984d11cef..7a501a5210 100644
--- a/sdk/test/metrics/attributes_hashmap_test.cc
+++ b/sdk/test/metrics/attributes_hashmap_test.cc
@@ -10,7 +10,6 @@
 #include <string>
 #include <utility>
 
-#include "opentelemetry/common/key_value_iterable_view.h"
 #include "opentelemetry/nostd/function_ref.h"
 #include "opentelemetry/nostd/string_view.h"
 #include "opentelemetry/sdk/common/attributemap_hash.h"
diff --git a/sdk/test/metrics/observer_result_test.cc b/sdk/test/metrics/observer_result_test.cc
index fc4835786c..b33141b4f7 100644
--- a/sdk/test/metrics/observer_result_test.cc
+++ b/sdk/test/metrics/observer_result_test.cc
@@ -6,10 +6,8 @@
 #include <map>
 #include <string>
 #include <unordered_map>
-#include <vector>
 
 #include "opentelemetry/common/key_value_iterable_view.h"
-#include "opentelemetry/nostd/variant.h"
 #include "opentelemetry/sdk/metrics/observer_result.h"
 #include "opentelemetry/sdk/metrics/view/attributes_processor.h"
 

From eef46df28fd4a1f31d9d3bc4d5e694c49650681c Mon Sep 17 00:00:00 2001
From: Tom Tan <totan@microsoft.com>
Date: Wed, 26 Mar 2025 15:56:17 -0700
Subject: [PATCH 21/23] More fix on iywu

---
 sdk/src/metrics/state/observable_registry.cc | 3 ---
 sdk/test/metrics/attributes_hashmap_test.cc  | 1 -
 sdk/test/metrics/cardinality_limit_test.cc   | 1 -
 3 files changed, 5 deletions(-)

diff --git a/sdk/src/metrics/state/observable_registry.cc b/sdk/src/metrics/state/observable_registry.cc
index 8db11d4605..d8d6afb01b 100644
--- a/sdk/src/metrics/state/observable_registry.cc
+++ b/sdk/src/metrics/state/observable_registry.cc
@@ -3,10 +3,8 @@
 
 #include <algorithm>
 #include <cstdint>
-#include <map>
 #include <mutex>
 #include <ostream>
-#include <string>
 #include <utility>
 #include <vector>
 
@@ -14,7 +12,6 @@
 #include "opentelemetry/metrics/async_instruments.h"
 #include "opentelemetry/metrics/observer_result.h"
 #include "opentelemetry/nostd/shared_ptr.h"
-#include "opentelemetry/nostd/variant.h"
 #include "opentelemetry/sdk/common/global_log_handler.h"
 #include "opentelemetry/sdk/metrics/async_instruments.h"
 #include "opentelemetry/sdk/metrics/instruments.h"
diff --git a/sdk/test/metrics/attributes_hashmap_test.cc b/sdk/test/metrics/attributes_hashmap_test.cc
index 7a501a5210..d4e2387916 100644
--- a/sdk/test/metrics/attributes_hashmap_test.cc
+++ b/sdk/test/metrics/attributes_hashmap_test.cc
@@ -5,7 +5,6 @@
 #include <stddef.h>
 #include <stdint.h>
 #include <functional>
-#include <map>
 #include <memory>
 #include <string>
 #include <utility>
diff --git a/sdk/test/metrics/cardinality_limit_test.cc b/sdk/test/metrics/cardinality_limit_test.cc
index a24c402452..801096b269 100644
--- a/sdk/test/metrics/cardinality_limit_test.cc
+++ b/sdk/test/metrics/cardinality_limit_test.cc
@@ -19,7 +19,6 @@
 #include "opentelemetry/nostd/function_ref.h"
 #include "opentelemetry/nostd/span.h"
 #include "opentelemetry/nostd/variant.h"
-#include "opentelemetry/sdk/common/attributemap_hash.h"
 #include "opentelemetry/sdk/metrics/aggregation/aggregation.h"
 #include "opentelemetry/sdk/metrics/aggregation/sum_aggregation.h"
 #include "opentelemetry/sdk/metrics/data/metric_data.h"

From 4a8b77fbda8eec037135c41a5c34c5cbbe248ebd Mon Sep 17 00:00:00 2001
From: Tom Tan <Tom.Tan@microsoft.com>
Date: Wed, 26 Mar 2025 22:24:20 -0700
Subject: [PATCH 22/23] Addressing feedback

---
 .../opentelemetry/sdk/metrics/state/attributes_hashmap.h  | 2 ++
 .../sdk/metrics/state/filtered_ordered_attribute_map.h    | 8 ++++----
 2 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/sdk/include/opentelemetry/sdk/metrics/state/attributes_hashmap.h b/sdk/include/opentelemetry/sdk/metrics/state/attributes_hashmap.h
index e7b8977062..b847a1cc30 100644
--- a/sdk/include/opentelemetry/sdk/metrics/state/attributes_hashmap.h
+++ b/sdk/include/opentelemetry/sdk/metrics/state/attributes_hashmap.h
@@ -78,6 +78,8 @@ class AttributesHashMapWithCustomHash
                                const AttributesProcessor *attributes_processor,
                                std::function<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);
diff --git a/sdk/include/opentelemetry/sdk/metrics/state/filtered_ordered_attribute_map.h b/sdk/include/opentelemetry/sdk/metrics/state/filtered_ordered_attribute_map.h
index ac0dcbea4a..6a0562e0e9 100644
--- a/sdk/include/opentelemetry/sdk/metrics/state/filtered_ordered_attribute_map.h
+++ b/sdk/include/opentelemetry/sdk/metrics/state/filtered_ordered_attribute_map.h
@@ -67,16 +67,16 @@ class FilteredOrderedAttributeMap : public opentelemetry::sdk::common::OrderedAt
   //
   bool operator==(const FilteredOrderedAttributeMap &other) const
   {
-    return _hash == other._hash && static_cast<const OrderedAttributeMap &>(*this) ==
+    return hash_ == other.hash_ && static_cast<const OrderedAttributeMap &>(*this) ==
                                        static_cast<const OrderedAttributeMap &>(other);
   }
 
-  size_t GetHash() const { return _hash; }
+  size_t GetHash() const { return hash_; }
 
-  void UpdateHash() { _hash = GetHashForAttributeMap(*this); }
+  void UpdateHash() { hash_ = GetHashForAttributeMap(*this); }
 
 private:
-  size_t _hash = (std::numeric_limits<size_t>::max)();
+  size_t hash_ = (std::numeric_limits<size_t>::max)();
 };
 
 class FilteredOrderedAttributeMapHash

From 8d5fe18ded591e4ae2484e8e1bf4afcad8f8e820 Mon Sep 17 00:00:00 2001
From: Tom Tan <totan@microsoft.com>
Date: Tue, 1 Apr 2025 12:00:37 -0700
Subject: [PATCH 23/23] Address feedback

---
 .../sdk/metrics/state/attributes_hashmap.h       | 16 +---------------
 sdk/test/metrics/attributes_hashmap_test.cc      |  5 -----
 2 files changed, 1 insertion(+), 20 deletions(-)

diff --git a/sdk/include/opentelemetry/sdk/metrics/state/attributes_hashmap.h b/sdk/include/opentelemetry/sdk/metrics/state/attributes_hashmap.h
index b847a1cc30..bcc7610aae 100644
--- a/sdk/include/opentelemetry/sdk/metrics/state/attributes_hashmap.h
+++ b/sdk/include/opentelemetry/sdk/metrics/state/attributes_hashmap.h
@@ -139,21 +139,7 @@ class AttributesHashMapWithCustomHash
            const AttributesProcessor *attributes_processor,
            std::unique_ptr<Aggregation> aggr)
   {
-    MetricAttributes attr{attributes, attributes_processor};
-
-    auto it = hash_map_.find(attr);
-    if (it != hash_map_.end())
-    {
-      it->second = std::move(aggr);
-    }
-    else if (IsOverflowAttributes())
-    {
-      hash_map_[kOverflowAttributes] = std::move(aggr);
-    }
-    else
-    {
-      hash_map_[std::move(attr)] = std::move(aggr);
-    }
+    Set(MetricAttributes{attributes, attributes_processor}, std::move(aggr));
   }
 
   void Set(const MetricAttributes &attributes, std::unique_ptr<Aggregation> aggr)
diff --git a/sdk/test/metrics/attributes_hashmap_test.cc b/sdk/test/metrics/attributes_hashmap_test.cc
index d4e2387916..c51f4fe800 100644
--- a/sdk/test/metrics/attributes_hashmap_test.cc
+++ b/sdk/test/metrics/attributes_hashmap_test.cc
@@ -129,11 +129,6 @@ TEST(AttributesHashMap, CollisionTest)
   EXPECT_EQ(total_elements, 4);
 }
 
-std::string make_unique_string(const char *str)
-{
-  return std::string(str);
-}
-
 TEST(AttributesHashMap, HashConsistencyAcrossStringTypes)
 {
   const char *c_str                 = "teststring";