[SDK] support aggregation of identical instruments

CHANGELOG.md

@@ -33,6 +33,9 @@ Increment the:
 * [API] Add Enabled method to Tracer
   [#3357](https://github.com/open-telemetry/opentelemetry-cpp/pull/3357)
 
+* [SDK] Aggregate identical metrics instruments and detect duplicates
+  [#3358](https://github.com/open-telemetry/opentelemetry-cpp/pull/3358)
+
 ## [1.20 2025-04-01]
 
 * [BUILD] Update opentelemetry-proto version

sdk/include/opentelemetry/sdk/metrics/instruments.h

@@ -3,6 +3,8 @@
 
 #pragma once
 
+#include <algorithm>
+#include <cctype>
 #include <functional>
 #include <string>
 
@@ -65,6 +67,112 @@ struct InstrumentDescriptor
   InstrumentValueType value_type_;
 };
 
+struct InstrumentDescriptorUtil
+{
+  // Case-insensitive comparison of two ASCII strings used to evaluate equality of instrument names
+  static bool CaseInsensitiveAsciiEquals(const std::string &lhs, const std::string &rhs) noexcept
+  {
+    return lhs.size() == rhs.size() &&
+           std::equal(lhs.begin(), lhs.end(), rhs.begin(), [](char a, char b) {
+             return std::tolower(static_cast<unsigned char>(a)) ==
+                    std::tolower(static_cast<unsigned char>(b));
+           });
+  }
+
+  // Implementation of the specification requirements on duplicate instruments
+  // An instrument is a duplicate if it has the same name (case-insensitive) as another instrument,
+  // but different instrument kind, unit, or description.
+  // https://github.com/open-telemetry/opentelemetry-specification/blob/9c8c30631b0e288de93df7452f91ed47f6fba330/specification/metrics/sdk.md?plain=1#L869
+  static bool IsDuplicate(const InstrumentDescriptor &lhs, const InstrumentDescriptor &rhs) noexcept
+  {
+    const bool names_match = CaseInsensitiveAsciiEquals(lhs.name_, rhs.name_);
+    const bool kinds_match = (lhs.type_ == rhs.type_) && (lhs.value_type_ == rhs.value_type_);
+    const bool units_match = (lhs.unit_ == rhs.unit_);
+    const bool descriptions_match = (lhs.description_ == rhs.description_);
+
+    return names_match && (!kinds_match || !units_match || !descriptions_match);
+  }
+
+  static opentelemetry::nostd::string_view GetInstrumentTypeString(InstrumentType type) noexcept
+  {
+    switch (type)
+    {
+      case InstrumentType::kCounter:
+        return "Counter";
+      case InstrumentType::kUpDownCounter:
+        return "UpDownCounter";
+      case InstrumentType::kHistogram:
+        return "Histogram";
+      case InstrumentType::kObservableCounter:
+        return "ObservableCounter";
+      case InstrumentType::kObservableUpDownCounter:
+        return "ObservableUpDownCounter";
+      case InstrumentType::kObservableGauge:
+        return "ObservableGauge";
+      case InstrumentType::kGauge:
+        return "Gauge";
+      default:
+        return "Unknown";
+    }
+  }
+
+  static opentelemetry::nostd::string_view GetInstrumentValueTypeString(
+      InstrumentValueType value_type) noexcept
+  {
+    switch (value_type)
+    {
+      case InstrumentValueType::kInt:
+        return "Int";
+      case InstrumentValueType::kLong:
+        return "Long";
+      case InstrumentValueType::kFloat:
+        return "Float";
+      case InstrumentValueType::kDouble:
+        return "Double";
+      default:
+        return "Unknown";
+    }
+  }
+};
+
+struct InstrumentEqualNameCaseInsensitive
+{
+  bool operator()(const InstrumentDescriptor &lhs, const InstrumentDescriptor &rhs) const noexcept
+  {
+    const bool names_match =
+        InstrumentDescriptorUtil::CaseInsensitiveAsciiEquals(lhs.name_, rhs.name_);
+    const bool kinds_match = (lhs.type_ == rhs.type_) && (lhs.value_type_ == rhs.value_type_);
+    const bool units_match = (lhs.unit_ == rhs.unit_);
+    const bool descriptions_match = (lhs.description_ == rhs.description_);
+
+    return names_match && kinds_match && units_match && descriptions_match;
+  }
+};
+
+// Hash for InstrumentDescriptor
+// Identical instruments must have the same hash value
+// Two instruments are identical when all identifying fields (case-insensitive name , kind,
+// description, unit) are equal.
+struct InstrumentDescriptorHash
+{
+  std::size_t operator()(const InstrumentDescriptor &instrument_descriptor) const noexcept
+  {
+    std::size_t hashcode{};
+
+    for (char c : instrument_descriptor.name_)
+    {
+      sdk::common::GetHash(hashcode,
+                           static_cast<char>(std::tolower(static_cast<unsigned char>(c))));
+    }
+
+    sdk::common::GetHash(hashcode, instrument_descriptor.description_);
+    sdk::common::GetHash(hashcode, instrument_descriptor.unit_);
+    sdk::common::GetHash(hashcode, static_cast<uint32_t>(instrument_descriptor.type_));
+    sdk::common::GetHash(hashcode, static_cast<uint32_t>(instrument_descriptor.value_type_));
+    return hashcode;
+  }
+};
+
 using MetricAttributes               = opentelemetry::sdk::metrics::FilteredOrderedAttributeMap;
 using MetricAttributesHash           = opentelemetry::sdk::metrics::FilteredOrderedAttributeMapHash;
 using AggregationTemporalitySelector = std::function<AggregationTemporality(InstrumentType)>;

sdk/include/opentelemetry/sdk/metrics/meter.h

@@ -136,8 +136,12 @@ class Meter final : public opentelemetry::metrics::Meter
   // meter-context.
   std::unique_ptr<sdk::instrumentationscope::InstrumentationScope> scope_;
   std::weak_ptr<sdk::metrics::MeterContext> meter_context_;
-  // Mapping between instrument-name and Aggregation Storage.
-  std::unordered_map<std::string, std::shared_ptr<MetricStorage>> storage_registry_;
+  // Mapping between instrument descriptor and Aggregation Storage.
+  using MetricStorageMap = std::unordered_map<InstrumentDescriptor,
+                                              std::shared_ptr<MetricStorage>,
+                                              InstrumentDescriptorHash,
+                                              InstrumentEqualNameCaseInsensitive>;
+  MetricStorageMap storage_registry_;
   std::shared_ptr<ObservableRegistry> observable_registry_;
   MeterConfig meter_config_;
   std::unique_ptr<SyncWritableMetricStorage> RegisterSyncMetricStorage(
@@ -164,6 +168,19 @@ class Meter final : public opentelemetry::metrics::Meter
     return instrument_validator.ValidateName(name) && instrument_validator.ValidateUnit(unit) &&
            instrument_validator.ValidateDescription(description);
   }
+
+  // This function checks if the instrument is a duplicate of an existing one
+  // and emits a warning through the internal logger.
+  static void WarnOnDuplicateInstrument(
+      const sdk::instrumentationscope::InstrumentationScope *scope,
+      const MetricStorageMap &storage_registry,
+      const InstrumentDescriptor &new_instrument);
+
+  // This function checks if the instrument has a name case conflict with an existing one
+  // and emits a warning through the internal logger.
+  static void WarnOnNameCaseConflict(const sdk::instrumentationscope::InstrumentationScope *scope,
+                                     const InstrumentDescriptor &existing_instrument,
+                                     const InstrumentDescriptor &new_instrument);
 };
 }  // namespace metrics
 }  // namespace sdk

sdk/src/metrics/meter.cc

@@ -43,6 +43,44 @@
 #  include "opentelemetry/sdk/metrics/exemplar/reservoir_utils.h"
 #endif
 
+namespace
+{
+
+struct InstrumentationScopeLogStreamable
+{
+  const opentelemetry::sdk::instrumentationscope::InstrumentationScope &scope;
+};
+
+struct InstrumentDescriptorLogStreamable
+{
+  const opentelemetry::sdk::metrics::InstrumentDescriptor &instrument;
+};
+
+std::ostream &operator<<(std::ostream &os,
+                         const InstrumentationScopeLogStreamable &streamable) noexcept
+{
+  os << "\n  name=\"" << streamable.scope.GetName() << "\"" << "\n  schema_url=\""
+     << streamable.scope.GetSchemaURL() << "\"" << "\n  version=\"" << streamable.scope.GetVersion()
+     << "\"";
+  return os;
+}
+
+std::ostream &operator<<(std::ostream &os,
+                         const InstrumentDescriptorLogStreamable &streamable) noexcept
+{
+  os << "\n  name=\"" << streamable.instrument.name_ << "\"" << "\n  description=\""
+     << streamable.instrument.description_ << "\"" << "\n  unit=\"" << streamable.instrument.unit_
+     << "\"" << "\n  kind=\""
+     << opentelemetry::sdk::metrics::InstrumentDescriptorUtil::GetInstrumentValueTypeString(
+            streamable.instrument.value_type_)
+     << opentelemetry::sdk::metrics::InstrumentDescriptorUtil::GetInstrumentTypeString(
+            streamable.instrument.type_)
+     << "\"";
+  return os;
+}
+
+}  // namespace
+
 OPENTELEMETRY_BEGIN_NAMESPACE
 namespace sdk
 {
@@ -486,18 +524,31 @@ std::unique_ptr<SyncWritableMetricStorage> Meter::RegisterSyncMetricStorage(
         {
           view_instr_desc.description_ = view.GetDescription();
         }
-        auto multi_storage = static_cast<SyncMultiMetricStorage *>(storages.get());
-
-        auto storage = std::shared_ptr<SyncMetricStorage>(new SyncMetricStorage(
-            view_instr_desc, view.GetAggregationType(), &view.GetAttributesProcessor(),
+        std::shared_ptr<SyncMetricStorage> sync_storage{};
+        auto storage_iter = storage_registry_.find(view_instr_desc);
+        if (storage_iter != storage_registry_.end())
+        {
+          WarnOnNameCaseConflict(GetInstrumentationScope(), storage_iter->first, view_instr_desc);
+          // static_pointer_cast is okay here. If storage_registry_.find is successful
+          // InstrumentEqualNameCaseInsensitive ensures that the
+          // instrument type and value type are the same for the existing and new instrument.
+          sync_storage = std::static_pointer_cast<SyncMetricStorage>(storage_iter->second);
+        }
+        else
+        {
+          WarnOnDuplicateInstrument(GetInstrumentationScope(), storage_registry_, view_instr_desc);
+          sync_storage = std::shared_ptr<SyncMetricStorage>(new SyncMetricStorage(
+              view_instr_desc, view.GetAggregationType(), &view.GetAttributesProcessor(),
 #ifdef ENABLE_METRICS_EXEMPLAR_PREVIEW
-            exemplar_filter_type,
-            GetExemplarReservoir(view.GetAggregationType(), view.GetAggregationConfig(),
-                                 instrument_descriptor),
+              exemplar_filter_type,
+              GetExemplarReservoir(view.GetAggregationType(), view.GetAggregationConfig(),
+                                   view_instr_desc),
 #endif
-            view.GetAggregationConfig()));
-        storage_registry_[instrument_descriptor.name_] = storage;
-        multi_storage->AddStorage(storage);
+              view.GetAggregationConfig()));
+          storage_registry_.insert({view_instr_desc, sync_storage});
+        }
+        auto sync_multi_storage = static_cast<SyncMultiMetricStorage *>(storages.get());
+        sync_multi_storage->AddStorage(sync_storage);
         return true;
       });
 
@@ -546,16 +597,31 @@ std::unique_ptr<AsyncWritableMetricStorage> Meter::RegisterAsyncMetricStorage(
         {
           view_instr_desc.description_ = view.GetDescription();
         }
-        auto storage = std::shared_ptr<AsyncMetricStorage>(new AsyncMetricStorage(
-            view_instr_desc, view.GetAggregationType(),
+        std::shared_ptr<AsyncMetricStorage> async_storage{};
+        auto storage_iter = storage_registry_.find(view_instr_desc);
+        if (storage_iter != storage_registry_.end())
+        {
+          WarnOnNameCaseConflict(GetInstrumentationScope(), storage_iter->first, view_instr_desc);
+          // static_pointer_cast is okay here. If storage_registry_.find is successful
+          // InstrumentEqualNameCaseInsensitive ensures that the
+          // instrument type and value type are the same for the existing and new instrument.
+          async_storage = std::static_pointer_cast<AsyncMetricStorage>(storage_iter->second);
+        }
+        else
+        {
+          WarnOnDuplicateInstrument(GetInstrumentationScope(), storage_registry_, view_instr_desc);
+          async_storage = std::shared_ptr<AsyncMetricStorage>(new AsyncMetricStorage(
+              view_instr_desc, view.GetAggregationType(),
 #ifdef ENABLE_METRICS_EXEMPLAR_PREVIEW
-            exemplar_filter_type,
-            GetExemplarReservoir(view.GetAggregationType(), view.GetAggregationConfig(),
-                                 instrument_descriptor),
+              exemplar_filter_type,
+              GetExemplarReservoir(view.GetAggregationType(), view.GetAggregationConfig(),
+                                   view_instr_desc),
 #endif
-            view.GetAggregationConfig()));
-        storage_registry_[instrument_descriptor.name_] = storage;
-        static_cast<AsyncMultiMetricStorage *>(storages.get())->AddStorage(storage);
+              view.GetAggregationConfig()));
+          storage_registry_.insert({view_instr_desc, async_storage});
+        }
+        auto async_multi_storage = static_cast<AsyncMultiMetricStorage *>(storages.get());
+        async_multi_storage->AddStorage(async_storage);
         return true;
       });
   if (!success)
@@ -596,6 +662,73 @@ std::vector<MetricData> Meter::Collect(CollectorHandle *collector,
   return metric_data_list;
 }
 
+// Implementation of the log message recommended by the SDK specification for duplicate instruments.
+// See
+// https://github.com/open-telemetry/opentelemetry-specification/blob/9c8c30631b0e288de93df7452f91ed47f6fba330/specification/metrics/sdk.md?plain=1#L882
+void Meter::WarnOnDuplicateInstrument(const sdk::instrumentationscope::InstrumentationScope *scope,
+                                      const MetricStorageMap &storage_registry,
+                                      const InstrumentDescriptor &new_instrument)
+{
+  for (const auto &element : storage_registry)
+  {
+    const auto &existing_instrument = element.first;
+    if (InstrumentDescriptorUtil::IsDuplicate(existing_instrument, new_instrument))
+    {
+      std::string resolution_info{""};
+
+      if (existing_instrument.type_ != new_instrument.type_ ||
+          existing_instrument.value_type_ != new_instrument.value_type_)
+      {
+        resolution_info +=
+            "\nDifferent instrument kinds found. Consider configuring a View to change the name of "
+            "the duplicate instrument.";
+      }
+
+      if (existing_instrument.unit_ != new_instrument.unit_)
+      {
+        resolution_info += "\nDifferent instrument units found.";
+      }
+
+      if (existing_instrument.description_ != new_instrument.description_)
+      {
+        resolution_info +=
+            "\nDifferent instrument descriptions found. Consider configuring a View to change the "
+            "description of the duplicate instrument.";
+      }
+
+      OTEL_INTERNAL_LOG_WARN(
+          "[Meter::WarnOnDuplicateInstrument] Creating a duplicate instrument of the same "
+          "case-insensitive name. This may cause "
+          "semantic errors in the data exported from this meter."
+          << resolution_info << "\nScope: " << InstrumentationScopeLogStreamable{*scope}
+          << "\nExisting instrument: " << InstrumentDescriptorLogStreamable{existing_instrument}
+          << "\nDuplicate instrument: " << InstrumentDescriptorLogStreamable{new_instrument});
+      return;
+    }
+  }
+}
+
+// Implementation of the log message recommended by the SDK specification for name case conflicts.
+// See
+// https://github.com/open-telemetry/opentelemetry-specification/blob/9c8c30631b0e288de93df7452f91ed47f6fba330/specification/metrics/sdk.md?plain=1#L910
+void Meter::WarnOnNameCaseConflict(const sdk::instrumentationscope::InstrumentationScope *scope,
+                                   const InstrumentDescriptor &existing_instrument,
+                                   const InstrumentDescriptor &new_instrument)
+{
+  if (InstrumentDescriptorUtil::CaseInsensitiveAsciiEquals(existing_instrument.name_,
+                                                           new_instrument.name_) &&
+      existing_instrument.name_ != new_instrument.name_)
+  {
+    OTEL_INTERNAL_LOG_WARN(
+        "[Meter::WarnOnNameCaseConflict] Instrument name case conflict detected on creation. "
+        "Returning the existing instrument with the first-seen instrument name. To resolve this "
+        "warning consider configuring a View to rename the duplicate instrument."
+        << "\nScope: " << InstrumentationScopeLogStreamable{*scope}
+        << "\nExisting instrument: " << InstrumentDescriptorLogStreamable{existing_instrument}
+        << "\nDuplicate instrument: " << InstrumentDescriptorLogStreamable{new_instrument});
+  }
+}
+
 }  // namespace metrics
 }  // namespace sdk
 OPENTELEMETRY_END_NAMESPACE

sdk/test/metrics/CMakeLists.txt

@@ -33,7 +33,8 @@ foreach(
   metric_reader_test
   observable_registry_test
   periodic_exporting_metric_reader_test
-  instrument_metadata_validator_test)
+  instrument_metadata_validator_test
+  instrument_descriptor_test)
   add_executable(${testname} "${testname}.cc")
   target_link_libraries(
     ${testname} ${GTEST_BOTH_LIBRARIES} ${CMAKE_THREAD_LIBS_INIT}