Add bundle version of utf8_range to validate attributes

CHANGELOG.md

@@ -15,6 +15,9 @@ Increment the:
 
 ## [Unreleased]
 
+* [SDK] Add bundle version of utf8_range to validate attributes
+  [#3512](https://github.com/open-telemetry/opentelemetry-cpp/pull/3512)
+
 * [TEST] Remove workaround for metrics cardinality limit test
   [#3663](https://github.com/open-telemetry/opentelemetry-cpp/pull/3663)
 * [METRICS] Allow registering one callback for multiple instruments

exporters/otlp/src/otlp_populate_attribute_utils.cc

@@ -13,6 +13,7 @@
 #include "opentelemetry/nostd/string_view.h"
 #include "opentelemetry/nostd/variant.h"
 #include "opentelemetry/sdk/common/attribute_utils.h"
+#include "opentelemetry/sdk/common/attribute_validity.h"
 #include "opentelemetry/sdk/instrumentationscope/instrumentation_scope.h"
 #include "opentelemetry/sdk/resource/resource.h"
 #include "opentelemetry/version.h"
@@ -85,8 +86,18 @@ void OtlpPopulateAttributeUtils::PopulateAnyValue(
   }
   else if (nostd::holds_alternative<nostd::string_view>(value))
   {
-    proto_value->set_string_value(nostd::get<nostd::string_view>(value).data(),
-                                  nostd::get<nostd::string_view>(value).size());
+    if (allow_bytes &&
+        !opentelemetry::sdk::common::AttributeIsValidString(nostd::get<nostd::string_view>(value)))
+    {
+      proto_value->set_bytes_value(
+          reinterpret_cast<const void *>(nostd::get<nostd::string_view>(value).data()),
+          nostd::get<nostd::string_view>(value).size());
+    }
+    else
+    {
+      proto_value->set_string_value(nostd::get<nostd::string_view>(value).data(),
+                                    nostd::get<nostd::string_view>(value).size());
+    }
   }
   else if (nostd::holds_alternative<nostd::span<const uint8_t>>(value))
   {
@@ -159,7 +170,15 @@ void OtlpPopulateAttributeUtils::PopulateAnyValue(
     auto array_value = proto_value->mutable_array_value();
     for (const auto &val : nostd::get<nostd::span<const nostd::string_view>>(value))
     {
-      array_value->add_values()->set_string_value(val.data(), val.size());
+      if (allow_bytes && !opentelemetry::sdk::common::AttributeIsValidString(val))
+      {
+        array_value->add_values()->set_bytes_value(reinterpret_cast<const void *>(val.data()),
+                                                   val.size());
+      }
+      else
+      {
+        array_value->add_values()->set_string_value(val.data(), val.size());
+      }
     }
   }
 }
@@ -224,7 +243,17 @@ void OtlpPopulateAttributeUtils::PopulateAnyValue(
   }
   else if (nostd::holds_alternative<std::string>(value))
   {
-    proto_value->set_string_value(nostd::get<std::string>(value));
+    if (allow_bytes &&
+        !opentelemetry::sdk::common::AttributeIsValidString(nostd::get<std::string>(value)))
+    {
+      proto_value->set_bytes_value(
+          reinterpret_cast<const void *>(nostd::get<std::string>(value).data()),
+          nostd::get<std::string>(value).size());
+    }
+    else
+    {
+      proto_value->set_string_value(nostd::get<std::string>(value));
+    }
   }
   else if (nostd::holds_alternative<std::vector<bool>>(value))
   {
@@ -281,7 +310,15 @@ void OtlpPopulateAttributeUtils::PopulateAnyValue(
     auto array_value = proto_value->mutable_array_value();
     for (const auto &val : nostd::get<std::vector<std::string>>(value))
     {
-      array_value->add_values()->set_string_value(val);
+      if (allow_bytes && !opentelemetry::sdk::common::AttributeIsValidString(val))
+      {
+        array_value->add_values()->set_bytes_value(reinterpret_cast<const void *>(val.data()),
+                                                   val.size());
+      }
+      else
+      {
+        array_value->add_values()->set_string_value(val);
+      }
     }
   }
 }

functional/configuration/run_test.sh

@@ -15,5 +15,5 @@ export EXAMPLE_BIN_DIR="${BUILD_DIR}/examples/configuration/"
 # Make sure `example_yaml` is in the path
 export PATH=${PATH}:${EXAMPLE_BIN_DIR}
 
-shelltest ./shelltests
+shelltest --all --color ./shelltests
 

functional/configuration/shelltests/kitchen-sink.test

@@ -20,20 +20,20 @@ SDK CREATED
   severity_text      : DEBUG
   body               : body
   resource           : 
-    telemetry.sdk.version: 1.24.0-dev
-    service.version: 1.0.0
-    double_array_key: [1.1,2.2]
+    telemetry.sdk.language: cpp
+    string_key: value
+    telemetry.sdk.name: opentelemetry
     double_key: 1.1
-    bool_key: 1
+    service.namespace: my-namespace
+    string_array_key: [value1,value2]
     int_array_key: [1,2]
+    service.version: 1.0.0
     int_key: 1
-    string_array_key: [value1,value2]
-    string_key: value
-    service.namespace: my-namespace
-    telemetry.sdk.language: cpp
-    bool_array_key: [1,0]
     service.name: unknown_service
-    telemetry.sdk.name: opentelemetry
+    telemetry.sdk.version: 1.24.0-dev
+    bool_array_key: [1,0]
+    bool_key: 1
+    double_array_key: [1.1,2.2]
   attributes         : 
   event_id           : 0
   event_name         : 

sdk/include/opentelemetry/sdk/common/attribute_validity.h

@@ -0,0 +1,129 @@
+// Copyright The OpenTelemetry Authors
+// SPDX-License-Identifier: Apache-2.0
+
+#pragma once
+
+#include <cstddef>
+#include <cstdint>
+#include <string>
+#include <vector>
+
+#include "opentelemetry/common/attribute_value.h"
+#include "opentelemetry/common/key_value_iterable.h"
+#include "opentelemetry/nostd/function_ref.h"
+#include "opentelemetry/nostd/span.h"
+#include "opentelemetry/nostd/string_view.h"
+#include "opentelemetry/sdk/common/attribute_utils.h"
+#include "opentelemetry/version.h"
+
+OPENTELEMETRY_BEGIN_NAMESPACE
+namespace sdk
+{
+namespace common
+{
+
+OPENTELEMETRY_EXPORT bool AttributeIsValidString(nostd::string_view value) noexcept;
+
+/**
+ * Validate if an attribute value is valid.
+ */
+struct AttributeValidator
+{
+  bool operator()(bool /*v*/) noexcept { return true; }
+  bool operator()(int32_t /*v*/) noexcept { return true; }
+  bool operator()(uint32_t /*v*/) noexcept { return true; }
+  bool operator()(int64_t /*v*/) noexcept { return true; }
+  bool operator()(uint64_t /*v*/) noexcept { return true; }
+  bool operator()(double /*v*/) noexcept { return true; }
+  bool operator()(nostd::string_view v) noexcept { return AttributeIsValidString(v); }
+  bool operator()(const std::string &v) noexcept { return AttributeIsValidString(v); }
+  bool operator()(const char *v) noexcept { return AttributeIsValidString(v); }
+  bool operator()(nostd::span<const uint8_t> /*v*/) noexcept { return true; }
+  bool operator()(nostd::span<const bool> /*v*/) noexcept { return true; }
+  bool operator()(nostd::span<const int32_t> /*v*/) noexcept { return true; }
+  bool operator()(nostd::span<const uint32_t> /*v*/) noexcept { return true; }
+  bool operator()(nostd::span<const int64_t> /*v*/) noexcept { return true; }
+  bool operator()(nostd::span<const uint64_t> /*v*/) noexcept { return true; }
+  bool operator()(nostd::span<const double> /*v*/) noexcept { return true; }
+  bool operator()(nostd::span<const nostd::string_view> v) noexcept
+  {
+    for (const auto &s : v)
+    {
+      if (!AttributeIsValidString(s))
+      {
+        return false;
+      }
+    }
+    return true;
+  }
+  bool operator()(const std::vector<bool> & /*v*/) noexcept { return true; }
+  bool operator()(const std::vector<int32_t> & /*v*/) noexcept { return true; }
+  bool operator()(const std::vector<uint32_t> & /*v*/) noexcept { return true; }
+  bool operator()(const std::vector<int64_t> & /*v*/) noexcept { return true; }
+  bool operator()(const std::vector<double> & /*v*/) noexcept { return true; }
+  bool operator()(const std::vector<std::string> &v)
+  {
+    for (const auto &s : v)
+    {
+      if (!AttributeIsValidString(s))
+      {
+        return false;
+      }
+    }
+    return true;
+  }
+  bool operator()(const std::vector<uint64_t> & /*v*/) noexcept { return true; }
+  bool operator()(const std::vector<uint8_t> & /*v*/) noexcept { return true; }
+
+  OPENTELEMETRY_EXPORT static bool IsValid(const std::string &value) noexcept;
+
+  OPENTELEMETRY_EXPORT static bool IsValid(nostd::string_view value) noexcept;
+
+  OPENTELEMETRY_EXPORT static bool IsValid(const OwnedAttributeValue &value) noexcept;
+
+  OPENTELEMETRY_EXPORT static bool IsValid(
+      const opentelemetry::common::AttributeValue &value) noexcept;
+
+  OPENTELEMETRY_EXPORT static bool IsAllValid(const AttributeMap &attributes) noexcept;
+
+  OPENTELEMETRY_EXPORT static bool IsAllValid(const OrderedAttributeMap &attributes) noexcept;
+
+  OPENTELEMETRY_EXPORT static void Filter(AttributeMap &attributes, nostd::string_view log_hint);
+
+  OPENTELEMETRY_EXPORT static void Filter(OrderedAttributeMap &attributes,
+                                          nostd::string_view log_hint);
+};
+
+/**
+ * Supports internal iteration over a collection of key-value pairs and filtering of invalid
+ * attributes.
+ */
+class OPENTELEMETRY_EXPORT KeyValueFilterIterable : public opentelemetry::common::KeyValueIterable
+{
+public:
+  KeyValueFilterIterable(const opentelemetry::common::KeyValueIterable &origin,
+                         opentelemetry::nostd::string_view log_hint) noexcept;
+
+  ~KeyValueFilterIterable() override;
+
+  bool ForEachKeyValue(
+      opentelemetry::nostd::function_ref<bool(opentelemetry::nostd::string_view,
+                                              opentelemetry::common::AttributeValue)> callback)
+      const noexcept override;
+
+  size_t size() const noexcept override;
+
+private:
+  // Pointer to the original KeyValueIterable
+  const opentelemetry::common::KeyValueIterable *origin_;
+
+  // Size of valid attributes
+  mutable size_t size_;
+
+  // Log hint for invalid attributes
+  opentelemetry::nostd::string_view log_hint_;
+};
+
+}  // namespace common
+}  // namespace sdk
+OPENTELEMETRY_END_NAMESPACE

sdk/include/opentelemetry/sdk/instrumentationscope/instrumentation_scope.h

@@ -12,6 +12,8 @@
 #include "opentelemetry/nostd/unique_ptr.h"
 #include "opentelemetry/nostd/variant.h"
 #include "opentelemetry/sdk/common/attribute_utils.h"
+#include "opentelemetry/sdk/common/attribute_validity.h"
+#include "opentelemetry/sdk/common/global_log_handler.h"
 #include "opentelemetry/version.h"
 
 OPENTELEMETRY_BEGIN_NAMESPACE
@@ -42,6 +44,7 @@ class InstrumentationScope
       nostd::string_view schema_url               = "",
       InstrumentationScopeAttributes &&attributes = {})
   {
+    common::AttributeValidator::Filter(attributes, "[InstrumentationScope]");
     return nostd::unique_ptr<InstrumentationScope>(
         new InstrumentationScope{name, version, schema_url, std::move(attributes)});
   }
@@ -60,8 +63,19 @@ class InstrumentationScope
       nostd::string_view schema_url,
       const InstrumentationScopeAttributes &attributes)
   {
-    return nostd::unique_ptr<InstrumentationScope>(new InstrumentationScope{
-        name, version, schema_url, InstrumentationScopeAttributes(attributes)});
+    // Copy attributes only when we find some invalid attributes and try to remove them.
+    if (common::AttributeValidator::IsAllValid(attributes))
+    {
+      return nostd::unique_ptr<InstrumentationScope>(new InstrumentationScope{
+          name, version, schema_url, InstrumentationScopeAttributes(attributes)});
+    }
+    else
+    {
+      InstrumentationScopeAttributes copy_attributes = attributes;
+      common::AttributeValidator::Filter(copy_attributes, "[InstrumentationScope]");
+      return nostd::unique_ptr<InstrumentationScope>(new InstrumentationScope{
+          name, version, schema_url, InstrumentationScopeAttributes(copy_attributes)});
+    }
   }
 
   /**
@@ -88,6 +102,19 @@ class InstrumentationScope
     result->attributes_.reserve(opentelemetry::nostd::size(arg));
     for (auto &argv : arg)
     {
+      if (!common::AttributeValidator::IsValid(argv.first))
+      {
+        OTEL_INTERNAL_LOG_WARN("[InstrumentationScope] Invalid attribute key "
+                               << std::string{argv.first} << ". This attribute will be ignored.");
+        continue;
+      }
+
+      if (!common::AttributeValidator::IsValid(argv.second))
+      {
+        OTEL_INTERNAL_LOG_WARN("[InstrumentationScope] Invalid attribute value for "
+                               << std::string{argv.first} << ". This attribute will be ignored.");
+        continue;
+      }
       result->SetAttribute(argv.first, argv.second);
     }
 
@@ -148,6 +175,19 @@ class InstrumentationScope
   void SetAttribute(nostd::string_view key,
                     const opentelemetry::common::AttributeValue &value) noexcept
   {
+    if (!common::AttributeValidator::IsValid(key))
+    {
+      OTEL_INTERNAL_LOG_WARN("[InstrumentationScope] Invalid attribute key "
+                             << std::string{key} << ". This attribute will be ignored.");
+      return;
+    }
+
+    if (!common::AttributeValidator::IsValid(value))
+    {
+      OTEL_INTERNAL_LOG_WARN("[InstrumentationScope] Invalid attribute value for "
+                             << std::string{key} << ". This attribute will be ignored.");
+      return;
+    }
     attributes_[std::string(key)] =
         nostd::visit(opentelemetry::sdk::common::AttributeConverter(), value);
   }

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

@@ -11,6 +11,7 @@
 #include "opentelemetry/common/attribute_value.h"
 #include "opentelemetry/common/key_value_iterable.h"
 #include "opentelemetry/nostd/string_view.h"
+#include "opentelemetry/sdk/common/attribute_validity.h"
 #include "opentelemetry/sdk/common/custom_hash_equality.h"
 #include "opentelemetry/sdk/metrics/state/filtered_ordered_attribute_map.h"
 #include "opentelemetry/version.h"
@@ -59,7 +60,8 @@ class DefaultAttributesProcessor : public AttributesProcessor
   MetricAttributes process(
       const opentelemetry::common::KeyValueIterable &attributes) const noexcept override
   {
-    MetricAttributes result(attributes);
+    MetricAttributes result(
+        opentelemetry::sdk::common::KeyValueFilterIterable(attributes, "[Metrics] "));
     return result;
   }
 
@@ -86,7 +88,9 @@ class FilteringAttributesProcessor : public AttributesProcessor
       const opentelemetry::common::KeyValueIterable &attributes) const noexcept override
   {
     MetricAttributes result;
-    attributes.ForEachKeyValue(
+    opentelemetry::sdk::common::KeyValueFilterIterable validate_attributes{attributes,
+                                                                           "[Metrics] "};
+    validate_attributes.ForEachKeyValue(
         [&](nostd::string_view key, opentelemetry::common::AttributeValue value) noexcept {
           if (opentelemetry::sdk::common::find_heterogeneous(allowed_attribute_keys_, key) !=
               allowed_attribute_keys_.end())