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

CHANGELOG.md

@@ -52,6 +52,10 @@ Increment the:
   a running request still belongs to
   [#4396](https://github.com/open-telemetry/opentelemetry-cpp/issues/4396)
 
+* [METRICS SDK] Drop aggregation behaves as a no-op, prevent duplicate metric recording
+  from conflicting streams, and warn if stream conflicts were configured
+  [#4515](https://github.com/open-telemetry/opentelemetry-cpp/pull/4515)
+
 * [BUG] Compare the curl seek offset in `curl_off_t` before narrowing it to
   `size_t`, so an offset past the `size_t` range cannot wrap back inside the
   request body on a 32-bit build
@@ -65,6 +69,27 @@ Increment the:
 * [RELEASE] Bump main branch to 1.29.0-dev (#4259)
   [#4259](https://github.com/open-telemetry/opentelemetry-cpp/pull/4259)
 
+* [EXPORTER] Fix the Elasticsearch log exporter aborting the process when a log
+  record's body or attributes contain bytes that are not valid UTF-8. The
+  exporter now substitutes the replacement character for the invalid bytes
+  and continues, instead of `nlohmann::json::dump()` throwing out of a
+  `noexcept` function.
+  [#4439](https://github.com/open-telemetry/opentelemetry-cpp/issues/4439)
+
+* [CONFIGURATION] Apply general `attribute_limits` per individual limit field.
+  If a model-specific limit is set it is used, otherwise the matching general
+  limit, otherwise the model-specific default. Limit fields on
+  `AttributeLimitsConfiguration`, `SpanLimitsConfiguration`, and
+  `LogRecordLimitsConfiguration` are now optional so omitted keys and YAML
+  `null` are distinct from explicit values. This is a breaking change to the
+  experimental configuration model.
+  [#4467](https://github.com/open-telemetry/opentelemetry-cpp/issues/4467)
+
+* [CONFIGURATION] Add a configuration builder for the host resource detector
+  [#4451](https://github.com/open-telemetry/opentelemetry-cpp/issues/4451)
+
+* [CONFIGURATION] Cleanup build targets and docs
+  [#4486](https://github.com/open-telemetry/opentelemetry-cpp/pull/4486)
 * docs: update supported development platforms (#4260)
   [#4260](https://github.com/open-telemetry/opentelemetry-cpp/pull/4260)
 

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,12 @@ 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 will be ignored.
+  static void WarnOnViewSemanticError(const sdk::instrumentationscope::InstrumentationScope *scope,
+                                      const InstrumentDescriptor &existing_instrument,
+                                      const InstrumentDescriptor &stream,
+                                      const View &view);
 };
 }  // namespace metrics
 }  // namespace sdk

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

@@ -3,6 +3,7 @@
 
 #pragma once
 
+#include <algorithm>
 #include <cstdint>
 #include <memory>
 #include <unordered_map>
@@ -29,9 +30,14 @@ namespace metrics
 class SyncMultiMetricStorage : public SyncWritableMetricStorage
 {
 public:
-  void AddStorage(const std::shared_ptr<SyncWritableMetricStorage> &storage)
+  bool AddStorage(const std::shared_ptr<SyncWritableMetricStorage> &storage)
   {
+    if (!storage || HasStorage(storage))
+    {
+      return false;
+    }
     storages_.push_back(storage);
+    return true;
   }
 
   void RecordLong(int64_t value, const opentelemetry::context::Context &context) noexcept override
@@ -76,15 +82,25 @@ class SyncMultiMetricStorage : public SyncWritableMetricStorage
 #endif
 
 private:
+  bool HasStorage(const std::shared_ptr<SyncWritableMetricStorage> &storage) const
+  {
+    return std::find(storages_.begin(), storages_.end(), storage) != storages_.end();
+  }
+
   std::vector<std::shared_ptr<SyncWritableMetricStorage>> storages_;
 };
 
 class AsyncMultiMetricStorage : public AsyncWritableMetricStorage
 {
 public:
-  void AddStorage(const std::shared_ptr<AsyncWritableMetricStorage> &storage)
+  bool AddStorage(const std::shared_ptr<AsyncWritableMetricStorage> &storage)
   {
+    if (!storage || HasStorage(storage))
+    {
+      return false;
+    }
     storages_.push_back(storage);
+    return true;
   }
 
   void RecordLong(
@@ -108,6 +124,11 @@ class AsyncMultiMetricStorage : public AsyncWritableMetricStorage
   }
 
 private:
+  bool HasStorage(const std::shared_ptr<AsyncWritableMetricStorage> &storage) const
+  {
+    return std::find(storages_.begin(), storages_.end(), storage) != storages_.end();
+  }
+
   std::vector<std::shared_ptr<AsyncWritableMetricStorage>> storages_;
 };
 

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,26 @@ 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 +545,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,7 +589,12 @@ std::unique_ptr<SyncWritableMetricStorage> Meter::RegisterSyncMetricStorage(
           storage_registry_.insert({view_instr_desc, sync_storage});
         }
         auto sync_multi_storage = static_cast<SyncMultiMetricStorage *>(storages.get());
-        sync_multi_storage->AddStorage(sync_storage);
+        if (!sync_multi_storage->AddStorage(sync_storage))
+        {
+          WarnOnViewSemanticError(GetInstrumentationScope(), instrument_descriptor, view_instr_desc,
+                                  view);
+          return true;
+        }
         return true;
       });
 
@@ -592,6 +634,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,7 +677,13 @@ std::unique_ptr<AsyncWritableMetricStorage> Meter::RegisterAsyncMetricStorage(
           storage_registry_.insert({view_instr_desc, async_storage});
         }
         auto async_multi_storage = static_cast<AsyncMultiMetricStorage *>(storages.get());
-        async_multi_storage->AddStorage(async_storage);
+        if (!async_multi_storage->AddStorage(async_storage))
+        {
+          WarnOnViewSemanticError(GetInstrumentationScope(), instrument_descriptor, view_instr_desc,
+                                  view);
+          return true;
+        }
+
         return true;
       });
   if (!success)
@@ -748,6 +806,26 @@ 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. Views that create conflicting metric identities will be ignored and this
+// warning will be emitted. See
+// https://github.com/open-telemetry/opentelemetry-specification/blob/v1.60.0/specification/metrics/sdk.md?plain=1#L438
+// https://github.com/open-telemetry/opentelemetry-specification/blob/v1.60.0/specification/metrics/data-model.md#opentelemetry-protocol-data-model-producer-recommendations
+void Meter::WarnOnViewSemanticError(const sdk::instrumentationscope::InstrumentationScope *scope,
+                                    const InstrumentDescriptor &existing_instrument,
+                                    const InstrumentDescriptor &stream,
+                                    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. "
+      "To resolve this warning consider adjusting the View configuration to rename the stream."
+      << "\nScope: " << InstrumentationScopeLogStreamable{*scope}
+      << "\nInstrument: " << InstrumentDescriptorLogStreamable{existing_instrument}
+      << "\nMetric stream: " << InstrumentDescriptorLogStreamable{stream}
+      << "\nView: " << ViewLogStreamable{view});
+}
+
 }  // namespace metrics
 }  // namespace sdk
 OPENTELEMETRY_END_NAMESPACE