Skip to content

Commit 844f1a2

Browse files
nikhilbhatia08nikhilbhatia08
committed
StringViewHash Fix
1 parent faccac5 commit 844f1a2

File tree

3 files changed

+80
-40
lines changed

3 files changed

+80
-40
lines changed

‎sdk/include/opentelemetry/sdk/metrics/view/attributes_processor.h‎

Lines changed: 52 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -23,44 +23,65 @@ using MetricAttributes = opentelemetry::sdk::metrics::FilteredOrderedAttributeMa
2323

2424
/**
2525
* Hash and equality for nostd::string_view, enabling safe use in unordered_map
26-
* without requiring null termination. Supports heterogeneous lookup with
27-
* std::string and std::string_view as well.
26+
* without requiring null termination.
2827
*/
2928
struct StringViewHash
3029
{
3130
using is_transparent = void;
31+
32+
std::size_t operator()(const std::string &s) const noexcept
33+
{
34+
return std::hash<std::string>{}(s);
35+
}
36+
std::size_t operator()(std::string_view sv) const noexcept
37+
{
38+
return std::hash<std::string_view>{}(sv);
39+
}
3240
std::size_t operator()(opentelemetry::nostd::string_view sv) const noexcept
3341
{
34-
return std::hash<std::string_view>{}(
35-
std::string_view{sv.data(), sv.size()});
42+
return std::hash<std::string_view>{}(std::string_view{sv.data(), sv.size()});
3643
}
3744
};
3845

3946
struct StringViewEqual
4047
{
4148
using is_transparent = void;
42-
bool operator()(opentelemetry::nostd::string_view lhs,
43-
opentelemetry::nostd::string_view rhs) const noexcept
49+
50+
bool operator()(const std::string &lhs, const std::string &rhs) const noexcept
4451
{
45-
return lhs.size() == rhs.size() &&
46-
std::memcmp(lhs.data(), rhs.data(), lhs.size()) == 0;
52+
return lhs == rhs;
4753
}
48-
49-
bool operator()(const std::string &lhs,
50-
opentelemetry::nostd::string_view rhs) const noexcept
54+
bool operator()(const std::string &lhs, std::string_view rhs) const noexcept
5155
{
52-
return lhs.size() == rhs.size() &&
53-
std::memcmp(lhs.data(), rhs.data(), lhs.size()) == 0;
56+
return lhs.size() == rhs.size() && std::memcmp(lhs.data(), rhs.data(), lhs.size()) == 0;
5457
}
55-
56-
bool operator()(opentelemetry::nostd::string_view lhs,
57-
const std::string &rhs) const noexcept
58+
bool operator()(std::string_view lhs, const std::string &rhs) const noexcept
5859
{
59-
return rhs.size() == lhs.size() &&
60-
std::memcmp(lhs.data(), rhs.data(), rhs.size()) == 0;
60+
return lhs.size() == rhs.size() && std::memcmp(lhs.data(), rhs.data(), lhs.size()) == 0;
61+
}
62+
bool operator()(opentelemetry::nostd::string_view lhs, const std::string &rhs) const noexcept
63+
{
64+
return lhs.size() == rhs.size() && std::memcmp(lhs.data(), rhs.data(), lhs.size()) == 0;
6165
}
6266
};
6367

68+
/**
69+
* Cross-platform heterogeneous lookup wrapper.
70+
* Falls back to std::string construction on libc++ (macOS),
71+
* but uses direct lookup on libstdc++ (Linux).
72+
*/
73+
template <typename Map, typename Key>
74+
auto find_hetero(Map &map, Key &&key)
75+
{
76+
#if defined(_LIBCPP_VERSION) && !defined(_LIBCPP_HAS_UNORDERED_MAP_TRANSPARENT_LOOKUP)
77+
// libc++ (macOS) does not yet support heterogeneous lookup in unordered_map
78+
return map.find(std::string(key));
79+
#else
80+
// libstdc++ (Linux, GCC/Clang) supports it
81+
return map.find(std::forward<Key>(key));
82+
#endif
83+
}
84+
6485
/**
6586
* The AttributesProcessor is responsible for customizing which
6687
* attribute(s) are to be reported as metrics dimension(s).
@@ -106,12 +127,15 @@ class DefaultAttributesProcessor : public AttributesProcessor
106127
class FilteringAttributesProcessor : public AttributesProcessor
107128
{
108129
public:
109-
FilteringAttributesProcessor(std::unordered_map<std::string, bool, StringViewHash, StringViewEqual> &&allowed_attribute_keys = {})
130+
FilteringAttributesProcessor(
131+
std::unordered_map<std::string, bool, StringViewHash, StringViewEqual>
132+
&&allowed_attribute_keys = {})
110133
: allowed_attribute_keys_(std::move(allowed_attribute_keys))
111134
{}
112135

113136
FilteringAttributesProcessor(
114-
const std::unordered_map<std::string, bool, StringViewHash, StringViewEqual> &allowed_attribute_keys = {})
137+
const std::unordered_map<std::string, bool, StringViewHash, StringViewEqual>
138+
&allowed_attribute_keys = {})
115139
: allowed_attribute_keys_(allowed_attribute_keys)
116140
{}
117141

@@ -121,7 +145,7 @@ class FilteringAttributesProcessor : public AttributesProcessor
121145
MetricAttributes result;
122146
attributes.ForEachKeyValue(
123147
[&](nostd::string_view key, opentelemetry::common::AttributeValue value) noexcept {
124-
if (allowed_attribute_keys_.find(key) != allowed_attribute_keys_.end())
148+
if (find_hetero(allowed_attribute_keys_, key) != allowed_attribute_keys_.end())
125149
{
126150
result.SetAttribute(key, value);
127151
return true;
@@ -135,7 +159,7 @@ class FilteringAttributesProcessor : public AttributesProcessor
135159

136160
bool isPresent(nostd::string_view key) const noexcept override
137161
{
138-
return (allowed_attribute_keys_.find(key) != allowed_attribute_keys_.end());
162+
return (find_hetero(allowed_attribute_keys_, key) != allowed_attribute_keys_.end());
139163
}
140164

141165
private:
@@ -150,12 +174,14 @@ class FilteringAttributesProcessor : public AttributesProcessor
150174
class FilteringExcludeAttributesProcessor : public AttributesProcessor
151175
{
152176
public:
153-
FilteringExcludeAttributesProcessor(std::unordered_map<std::string, bool, StringViewHash, StringViewEqual> &&exclude_list = {})
177+
FilteringExcludeAttributesProcessor(
178+
std::unordered_map<std::string, bool, StringViewHash, StringViewEqual> &&exclude_list = {})
154179
: exclude_list_(std::move(exclude_list))
155180
{}
156181

157182
FilteringExcludeAttributesProcessor(
158-
const std::unordered_map<std::string, bool, StringViewHash, StringViewEqual> &exclude_list = {})
183+
const std::unordered_map<std::string, bool, StringViewHash, StringViewEqual> &exclude_list =
184+
{})
159185
: exclude_list_(exclude_list)
160186
{}
161187

@@ -165,7 +191,7 @@ class FilteringExcludeAttributesProcessor : public AttributesProcessor
165191
MetricAttributes result;
166192
attributes.ForEachKeyValue(
167193
[&](nostd::string_view key, opentelemetry::common::AttributeValue value) noexcept {
168-
if (exclude_list_.find(key) == exclude_list_.end())
194+
if (find_hetero(exclude_list_, key) == exclude_list_.end())
169195
{
170196
result.SetAttribute(key, value);
171197
return true;
@@ -179,7 +205,7 @@ class FilteringExcludeAttributesProcessor : public AttributesProcessor
179205

180206
bool isPresent(nostd::string_view key) const noexcept override
181207
{
182-
return (exclude_list_.find(key) == exclude_list_.end());
208+
return (find_hetero(exclude_list_, key) == exclude_list_.end());
183209
}
184210

185211
private:

‎sdk/test/metrics/attributes_processor_test.cc‎

Lines changed: 16 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -17,9 +17,10 @@ using namespace opentelemetry::sdk::common;
1717

1818
TEST(AttributesProcessor, FilteringAttributesProcessor)
1919
{
20-
const int kNumFilterAttributes = 3;
21-
std::unordered_map<std::string, bool> filter = {
22-
{"attr2", true}, {"attr4", true}, {"attr6", true}};
20+
const int kNumFilterAttributes = 3;
21+
std::unordered_map<std::string, bool, opentelemetry::sdk::metrics::StringViewHash,
22+
opentelemetry::sdk::metrics::StringViewEqual>
23+
filter = {{"attr2", true}, {"attr4", true}, {"attr6", true}};
2324
const int kNumAttributes = 6;
2425
std::string keys[kNumAttributes] = {"attr1", "attr2", "attr3", "attr4", "attr5", "attr6"};
2526
int values[kNumAttributes] = {10, 20, 30, 40, 50, 60};
@@ -38,9 +39,11 @@ TEST(AttributesProcessor, FilteringAttributesProcessor)
3839

3940
TEST(AttributesProcessor, FilteringAllAttributesProcessor)
4041
{
41-
const int kNumFilterAttributes = 0;
42-
std::unordered_map<std::string, bool> filter = {};
43-
const int kNumAttributes = 6;
42+
const int kNumFilterAttributes = 0;
43+
std::unordered_map<std::string, bool, opentelemetry::sdk::metrics::StringViewHash,
44+
opentelemetry::sdk::metrics::StringViewEqual>
45+
filter = {};
46+
const int kNumAttributes = 6;
4447
std::string keys[kNumAttributes] = {"attr1", "attr2", "attr3", "attr4", "attr5", "attr6"};
4548
int values[kNumAttributes] = {10, 20, 30, 40, 50, 60};
4649
std::map<std::string, int> attributes = {{keys[0], values[0]}, {keys[1], values[1]},
@@ -54,8 +57,9 @@ TEST(AttributesProcessor, FilteringAllAttributesProcessor)
5457

5558
TEST(AttributesProcessor, FilteringExcludeAttributesProcessor)
5659
{
57-
std::unordered_map<std::string, bool> filter = {
58-
{"attr2", true}, {"attr4", true}, {"attr6", true}};
60+
std::unordered_map<std::string, bool, opentelemetry::sdk::metrics::StringViewHash,
61+
opentelemetry::sdk::metrics::StringViewEqual>
62+
filter = {{"attr2", true}, {"attr4", true}, {"attr6", true}};
5963
const int kNumAttributes = 7;
6064
std::string keys[kNumAttributes] = {"attr1", "attr2", "attr3", "attr4",
6165
"attr5", "attr6", "attr7"};
@@ -76,8 +80,10 @@ TEST(AttributesProcessor, FilteringExcludeAttributesProcessor)
7680

7781
TEST(AttributesProcessor, FilteringExcludeAllAttributesProcessor)
7882
{
79-
std::unordered_map<std::string, bool> filter = {};
80-
const int kNumAttributes = 6;
83+
std::unordered_map<std::string, bool, opentelemetry::sdk::metrics::StringViewHash,
84+
opentelemetry::sdk::metrics::StringViewEqual>
85+
filter = {};
86+
const int kNumAttributes = 6;
8187
std::string keys[kNumAttributes] = {"attr1", "attr2", "attr3", "attr4", "attr5", "attr6"};
8288
int values[kNumAttributes] = {10, 20, 30, 40, 50, 60};
8389
std::map<std::string, int> attributes = {{keys[0], values[0]}, {keys[1], values[1]},

‎sdk/test/metrics/sum_aggregation_test.cc‎

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -100,7 +100,9 @@ TEST(HistogramToSumFilterAttributes, Double)
100100
std::string instrument_name = "histogram1";
101101
std::string instrument_desc = "histogram metrics";
102102

103-
std::unordered_map<std::string, bool> allowedattr;
103+
std::unordered_map<std::string, bool, opentelemetry::sdk::metrics::StringViewHash,
104+
opentelemetry::sdk::metrics::StringViewEqual>
105+
allowedattr;
104106
allowedattr["attr1"] = true;
105107
std::unique_ptr<opentelemetry::sdk::metrics::AttributesProcessor> attrproc{
106108
new opentelemetry::sdk::metrics::FilteringAttributesProcessor(allowedattr)};
@@ -154,7 +156,9 @@ TEST(HistogramToSumFilterAttributesWithCardinalityLimit, Double)
154156
std::string instrument_desc = "histogram metrics";
155157
size_t cardinality_limit = 10000;
156158

157-
std::unordered_map<std::string, bool> allowedattr;
159+
std::unordered_map<std::string, bool, opentelemetry::sdk::metrics::StringViewHash,
160+
opentelemetry::sdk::metrics::StringViewEqual>
161+
allowedattr;
158162
allowedattr["attr1"] = true;
159163
std::unique_ptr<opentelemetry::sdk::metrics::AttributesProcessor> attrproc{
160164
new opentelemetry::sdk::metrics::FilteringAttributesProcessor(allowedattr)};
@@ -281,7 +285,9 @@ TEST(CounterToSumFilterAttributes, Double)
281285
std::string instrument_name = "counter1";
282286
std::string instrument_desc = "counter metrics";
283287

284-
std::unordered_map<std::string, bool> allowedattr;
288+
std::unordered_map<std::string, bool, opentelemetry::sdk::metrics::StringViewHash,
289+
opentelemetry::sdk::metrics::StringViewEqual>
290+
allowedattr;
285291
allowedattr["attr1"] = true;
286292
std::unique_ptr<opentelemetry::sdk::metrics::AttributesProcessor> attrproc{
287293
new opentelemetry::sdk::metrics::FilteringAttributesProcessor(allowedattr)};
@@ -335,7 +341,9 @@ TEST(CounterToSumFilterAttributesWithCardinalityLimit, Double)
335341
std::string instrument_desc = "counter metrics";
336342
size_t cardinality_limit = 10000;
337343

338-
std::unordered_map<std::string, bool> allowedattr;
344+
std::unordered_map<std::string, bool, opentelemetry::sdk::metrics::StringViewHash,
345+
opentelemetry::sdk::metrics::StringViewEqual>
346+
allowedattr;
339347
allowedattr["attr1"] = true;
340348
std::unique_ptr<opentelemetry::sdk::metrics::AttributesProcessor> attrproc{
341349
new opentelemetry::sdk::metrics::FilteringAttributesProcessor(allowedattr)};

0 commit comments

Comments
 (0)