-
Notifications
You must be signed in to change notification settings - Fork 501
Description
In TemporalMetricStorage::buildMetrics, the attribute hash map is copied into a dedicated collection hash map through GetAllEntries, but since std::unordered_map provides no deterministic iteration order, the traversal sequence is undefined. As a result, if kOverflowAttributes happens to be visited early, the final attribute in the iteration—which should not be kOverflowAttributes because it has already been processed—may be skipped, since the last bucket is always reserved for kOverflowAttributes. This creates a subtle correctness issue when copying attributes into the collection map.
opentelemetry-cpp/sdk/src/metrics/state/temporal_metric_storage.cc
Lines 103 to 118 in 8a5907f
agg_hashmap->GetAllEnteries( | |
[&merged_metrics, this](const MetricAttributes &attributes, Aggregation &aggregation) { | |
auto agg = merged_metrics->Get(attributes); | |
if (agg) | |
{ | |
merged_metrics->Set(attributes, agg->Merge(aggregation)); | |
} | |
else | |
{ | |
merged_metrics->Set(attributes, | |
DefaultAggregation::CreateAggregation( | |
aggregation_type_, instrument_descriptor_, aggregation_config_) | |
->Merge(aggregation)); | |
} | |
return true; | |
}); |
Here is the function which could have issue when copying attribute hash map, IsOverflowAttributes
doesn't check the actual attribute is kOverflowAttributes
and treat it as a regular attribute. Adding a proper check for kOverflowAttributes in this function would prevent incorrect handling and ensure that all valid attributes are reliably copied during collection.
opentelemetry-cpp/sdk/include/opentelemetry/sdk/metrics/state/attributes_hashmap.h
Lines 101 to 118 in 8a5907f
Aggregation *GetOrSetDefault( | |
const MetricAttributes &attributes, | |
nostd::function_ref<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); | |
} | |
hash_map_[attributes] = aggregation_callback(); | |
return hash_map_[attributes].get(); | |
} |