[Metrics SDK] Fix hash calculation for nostd::string

sdk/include/opentelemetry/sdk/common/attributemap_hash.h

@@ -35,6 +35,15 @@ inline void GetHash(size_t &seed, const std::vector<T> &arg)
   }
 }
 
+// Specialization for const char*
+// this creates an intermediate string.
+template <>
+inline void GetHash<const char *>(size_t &seed, const char *const &arg)
+{
+  std::hash<std::string> hasher;
+  seed ^= hasher(std::string(arg)) + 0x9e3779b9 + (seed << 6) + (seed >> 2);
+}
+
 struct GetHashForAttributeValueVisitor
 {
   GetHashForAttributeValueVisitor(size_t &seed) : seed_(seed) {}
@@ -71,7 +80,7 @@ inline size_t GetHashForAttributeMap(
         {
           return true;
         }
-        GetHash(seed, key.data());
+        GetHash(seed, key);
         auto attr_val = nostd::visit(converter, value);
         nostd::visit(GetHashForAttributeValueVisitor(seed), attr_val);
         return true;

sdk/test/metrics/attributes_hashmap_test.cc

@@ -11,9 +11,40 @@
 using namespace opentelemetry::sdk::metrics;
 namespace nostd = opentelemetry::nostd;
 
-TEST(AttributesHashMap, BasicTests)
+// Mock KeyValueIterable implementation
+class MockKeyValueIterable : public opentelemetry::common::KeyValueIterable
 {
+public:
+  MockKeyValueIterable(std::initializer_list<std::pair<std::string, std::string>> init)
+  {
+    for (const auto &kv : init)
+    {
+      attributes_.emplace_back(kv.first, kv.second);
+    }
+  }
+
+  bool ForEachKeyValue(
+      nostd::function_ref<bool(nostd::string_view, opentelemetry::common::AttributeValue)> callback)
+      const noexcept override
+  {
+    for (const auto &kv : attributes_)
+    {
+      if (!callback(kv.first, kv.second))
+      {
+        return false;
+      }
+    }
+    return true;
+  }
+
+  size_t size() const noexcept override { return attributes_.size(); }
 
+private:
+  std::vector<std::pair<std::string, opentelemetry::common::AttributeValue>> attributes_;
+};
+
+TEST(AttributesHashMap, BasicTests)
+{
   // Empty map
   AttributesHashMap hash_map;
   EXPECT_EQ(hash_map.Size(), 0);
@@ -73,3 +104,72 @@ TEST(AttributesHashMap, BasicTests)
       });
   EXPECT_EQ(count, hash_map.Size());
 }
+
+std::string make_unique_string(const char *str)
+{
+  return std::string(str);
+}
+
+TEST(AttributesHashMap, HashWithKeyValueIterable)
+{
+  // Create mock KeyValueIterable instances with the same content but different variables
+  MockKeyValueIterable attributes1({{make_unique_string("k1"), make_unique_string("v1")},
+                                    {make_unique_string("k2"), make_unique_string("v2")}});
+  MockKeyValueIterable attributes2({{make_unique_string("k1"), make_unique_string("v1")},
+                                    {make_unique_string("k2"), make_unique_string("v2")}});
+  MockKeyValueIterable attributes3({{make_unique_string("k1"), make_unique_string("v1")},
+                                    {make_unique_string("k2"), make_unique_string("v2")},
+                                    {make_unique_string("k3"), make_unique_string("v3")}});
+
+  // 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(attributes1, is_key_filter_k3_callback);
+  size_t hash2 =
+      opentelemetry::sdk::common::GetHashForAttributeMap(attributes2, is_key_filter_k3_callback);
+
+  size_t hash3 =
+      opentelemetry::sdk::common::GetHashForAttributeMap(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";
+  std::string std_str               = "teststring";
+  nostd::string_view nostd_str_view = "teststring";
+#if __cplusplus >= 201703L
+  std::string_view std_str_view = "teststring";
+#endif
+
+  size_t hash_c_str          = 0;
+  size_t hash_std_str        = 0;
+  size_t hash_nostd_str_view = 0;
+#if __cplusplus >= 201703L
+  size_t hash_std_str_view = 0;
+#endif
+
+  opentelemetry::sdk::common::GetHash(hash_c_str, c_str);
+  opentelemetry::sdk::common::GetHash(hash_std_str, std_str);
+  opentelemetry::sdk::common::GetHash(hash_nostd_str_view, nostd_str_view);
+#if __cplusplus >= 201703L
+  opentelemetry::sdk::common::GetHash(hash_std_str_view, std_str_view);
+#endif
+
+  EXPECT_EQ(hash_c_str, hash_std_str);
+  EXPECT_EQ(hash_c_str, hash_nostd_str_view);
+#if __cplusplus >= 201703L
+  EXPECT_EQ(hash_c_str, hash_std_str_view);
+#endif
+}

sdk/test/metrics/cardinality_limit_test.cc

@@ -47,14 +47,47 @@ TEST(CardinalityLimit, AttributesHashMapBasicTests)
         ->Aggregate(record_value);
   }
   EXPECT_EQ(hash_map.Size(), 10);  // only one more metric point should be added as overflow.
+  // record 5 more measurements to already existing (and not-overflow) metric points. They
+  // should get aggregated to these existing metric points.
+  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))
+        ->Aggregate(record_value);
+  }
+  EXPECT_EQ(hash_map.Size(), 10);  // no new metric point added
+
   // get the overflow metric point
-  auto agg = hash_map.GetOrSetDefault(
+  auto agg1 = hash_map.GetOrSetDefault(
       FilteredOrderedAttributeMap({{kAttributesLimitOverflowKey, kAttributesLimitOverflowValue}}),
       aggregation_callback, kOverflowAttributesHash);
-  EXPECT_NE(agg, nullptr);
-  auto sum_agg = static_cast<LongSumAggregation *>(agg);
-  EXPECT_EQ(nostd::get<int64_t>(nostd::get<SumPointData>(sum_agg->ToPoint()).value_),
+  EXPECT_NE(agg1, nullptr);
+  auto sum_agg1 = static_cast<LongSumAggregation *>(agg1);
+  EXPECT_EQ(nostd::get<int64_t>(nostd::get<SumPointData>(sum_agg1->ToPoint()).value_),
             record_value * 6);  // 1 from previous 10, 5 from current 5.
+  // get remaining metric points
+  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);
+    EXPECT_NE(agg2, nullptr);
+    auto sum_agg2 = static_cast<LongSumAggregation *>(agg2);
+    if (i < 5)
+    {
+      EXPECT_EQ(nostd::get<int64_t>(nostd::get<SumPointData>(sum_agg2->ToPoint()).value_),
+                record_value * 2);  // 1 from first recording, 1 from third recording
+    }
+    else
+    {
+      EXPECT_EQ(nostd::get<int64_t>(nostd::get<SumPointData>(sum_agg2->ToPoint()).value_),
+                record_value);  // 1 from first recording
+    }
+  }
 }
 
 class WritableMetricStorageCardinalityLimitTestFixture