[METRICS SDK] Drop aggregation behaves as a no-op

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

@@ -157,6 +157,31 @@ struct InstrumentDescriptorUtil
   }
 };
 
+struct AggregationUtil
+{
+  static opentelemetry::nostd::string_view GetAggregationTypeString(
+      AggregationType aggregation_type) noexcept
+  {
+    switch (aggregation_type)
+    {
+      case AggregationType::kDrop:
+        return "Drop";
+      case AggregationType::kHistogram:
+        return "Histogram";
+      case AggregationType::kLastValue:
+        return "LastValue";
+      case AggregationType::kSum:
+        return "Sum";
+      case AggregationType::kDefault:
+        return "Default";
+      case AggregationType::kBase2ExponentialHistogram:
+        return "Base2ExponentialHistogram";
+      default:
+        return "Unknown";
+    }
+  }
+};
+
 struct InstrumentEqualNameCaseInsensitive
 {
   bool operator()(const InstrumentDescriptor &lhs, const InstrumentDescriptor &rhs) const noexcept

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

@@ -190,6 +190,11 @@ class Meter final : public opentelemetry::metrics::Meter
   static void WarnOnNameCaseConflict(const sdk::instrumentationscope::InstrumentationScope *scope,
                                      const InstrumentDescriptor &existing_instrument,
                                      const InstrumentDescriptor &new_instrument);
+
+  // Emits a warning that the view will create a semantic error and is skipped.
+  static void WarnOnViewSemanticError(const sdk::instrumentationscope::InstrumentationScope *scope,
+                                      const InstrumentDescriptor &existing_instrument,
+                                      const View &view);
 };
 }  // namespace metrics
 }  // namespace sdk

sdk/include/opentelemetry/sdk/metrics/state/multi_metric_storage.h

@@ -29,8 +29,21 @@ namespace metrics
 class SyncMultiMetricStorage : public SyncWritableMetricStorage
 {
 public:
+  bool HasStorage(const std::shared_ptr<SyncWritableMetricStorage> &storage) const
+  {
+    if (!storage || storages_.empty())
+    {
+      return false;
+    }
+    return std::find(storages_.begin(), storages_.end(), storage) != storages_.end();
+  }
+
   void AddStorage(const std::shared_ptr<SyncWritableMetricStorage> &storage)
   {
+    if (!storage || HasStorage(storage))
+    {
+      return;
+    }
     storages_.push_back(storage);
   }
 
@@ -82,8 +95,21 @@ class SyncMultiMetricStorage : public SyncWritableMetricStorage
 class AsyncMultiMetricStorage : public AsyncWritableMetricStorage
 {
 public:
+  bool HasStorage(const std::shared_ptr<AsyncWritableMetricStorage> &storage) const
+  {
+    if (!storage || storages_.empty())
+    {
+      return false;
+    }
+    return std::find(storages_.begin(), storages_.end(), storage) != storages_.end();
+  }
+
   void AddStorage(const std::shared_ptr<AsyncWritableMetricStorage> &storage)
   {
+    if (!storage || HasStorage(storage))
+    {
+      return;
+    }
     storages_.push_back(storage);
   }
 

sdk/src/metrics/meter.cc

@@ -21,6 +21,7 @@
 #include "opentelemetry/sdk/common/global_log_handler.h"
 #include "opentelemetry/sdk/instrumentationscope/instrumentation_scope.h"
 #include "opentelemetry/sdk/instrumentationscope/scope_configurator.h"
+#include "opentelemetry/sdk/metrics/aggregation/aggregation_config.h"
 #include "opentelemetry/sdk/metrics/async_instruments.h"
 #include "opentelemetry/sdk/metrics/data/metric_data.h"
 #include "opentelemetry/sdk/metrics/instruments.h"
@@ -60,6 +61,11 @@ struct InstrumentDescriptorLogStreamable
   std::reference_wrapper<const opentelemetry::sdk::metrics::InstrumentDescriptor> instrument;
 };
 
+struct ViewLogStreamable
+{
+  std::reference_wrapper<const opentelemetry::sdk::metrics::View> view;
+};
+
 std::ostream &operator<<(std::ostream &os,
                          const InstrumentationScopeLogStreamable &streamable) noexcept
 {
@@ -83,6 +89,27 @@ std::ostream &operator<<(std::ostream &os,
   return os;
 }
 
+std::ostream &operator<<(std::ostream &os, const ViewLogStreamable &streamable) noexcept
+{
+  using opentelemetry::sdk::metrics::AggregationUtil;
+  auto aggregation_type          = streamable.view.get().GetAggregationType();
+  const auto *aggregation_config = streamable.view.get().GetAggregationConfig();
+
+  os << "\n  name=\"" << streamable.view.get().GetName() << "\"" << "\n  description=\""
+     << streamable.view.get().GetDescription() << "\"" << "\n  aggregation_type=\""
+     << AggregationUtil::GetAggregationTypeString(aggregation_type) << "\""
+     << "\n  aggregation_config={\""
+     << (aggregation_config
+             ? AggregationUtil::GetAggregationTypeString(aggregation_config->GetType())
+             : "null")
+     << "\""
+     << (aggregation_config
+             ? ", cardinality_limit=" + std::to_string(aggregation_config->cardinality_limit_)
+             : "")
+     << "}";
+  return os;
+}
+
 }  // namespace
 
 OPENTELEMETRY_BEGIN_NAMESPACE
@@ -519,6 +546,17 @@ std::unique_ptr<SyncWritableMetricStorage> Meter::RegisterSyncMetricStorage(
        exemplar_filter_type
 #endif
   ](const View &view) {
+        OTEL_INTERNAL_LOG_DEBUG("[Meter::RegisterSyncMetricStorage] View matched."
+                                << "\nInstrumentationScope: "
+                                << InstrumentationScopeLogStreamable{*GetInstrumentationScope()}
+                                << "\nInstrument: "
+                                << InstrumentDescriptorLogStreamable{instrument_descriptor}
+                                << "\nView: " << ViewLogStreamable{view});
+
+        if (view.GetAggregationType() == AggregationType::kDrop)
+        {
+          return true;
+        }
         auto view_instr_desc = instrument_descriptor;
         if (!view.GetName().empty())
         {
@@ -552,6 +590,11 @@ std::unique_ptr<SyncWritableMetricStorage> Meter::RegisterSyncMetricStorage(
           storage_registry_.insert({view_instr_desc, sync_storage});
         }
         auto sync_multi_storage = static_cast<SyncMultiMetricStorage *>(storages.get());
+        if (sync_multi_storage->HasStorage(sync_storage))
+        {
+          WarnOnViewSemanticError(GetInstrumentationScope(), instrument_descriptor, view);
+          return true;
+        }
         sync_multi_storage->AddStorage(sync_storage);
         return true;
       });
@@ -592,6 +635,16 @@ std::unique_ptr<AsyncWritableMetricStorage> Meter::RegisterAsyncMetricStorage(
        exemplar_filter_type
 #endif
   ](const View &view) {
+        OTEL_INTERNAL_LOG_DEBUG("[Meter::RegisterAsyncMetricStorage] View matched."
+                                << "\nInstrumentationScope: "
+                                << InstrumentationScopeLogStreamable{*GetInstrumentationScope()}
+                                << "\nInstrument: "
+                                << InstrumentDescriptorLogStreamable{instrument_descriptor}
+                                << "\nView: " << ViewLogStreamable{view});
+        if (view.GetAggregationType() == AggregationType::kDrop)
+        {
+          return true;
+        }
         auto view_instr_desc = instrument_descriptor;
         if (!view.GetName().empty())
         {
@@ -625,6 +678,11 @@ std::unique_ptr<AsyncWritableMetricStorage> Meter::RegisterAsyncMetricStorage(
           storage_registry_.insert({view_instr_desc, async_storage});
         }
         auto async_multi_storage = static_cast<AsyncMultiMetricStorage *>(storages.get());
+        if (async_multi_storage->HasStorage(async_storage))
+        {
+          WarnOnViewSemanticError(GetInstrumentationScope(), instrument_descriptor, view);
+          return true;
+        }
         async_multi_storage->AddStorage(async_storage);
         return true;
       });
@@ -748,6 +806,21 @@ void Meter::WarnOnNameCaseConflict(const sdk::instrumentationscope::Instrumentat
   }
 }
 
+// Implementation of the log message recommended by the SDK specification for semantic errors caused
+// by a View configuration. See
+// https://github.com/open-telemetry/opentelemetry-specification/blob/v1.60.0/specification/metrics/sdk.md?plain=1#L438
+void Meter::WarnOnViewSemanticError(const sdk::instrumentationscope::InstrumentationScope *scope,
+                                    const InstrumentDescriptor &existing_instrument,
+                                    const View &view)
+{
+  OTEL_INTERNAL_LOG_WARN(
+      "[Meter::WarnOnViewSemanticError] The matched View may cause a semantic error in "
+      "the data exported from this meter by configuring a conflicting metric and is not applied."
+      << "\nScope: " << InstrumentationScopeLogStreamable{*scope}
+      << "\nInstrument: " << InstrumentDescriptorLogStreamable{existing_instrument}
+      << "\nView: " << ViewLogStreamable{view});
+}
+
 }  // namespace metrics
 }  // namespace sdk
 OPENTELEMETRY_END_NAMESPACE