opentelemetrytracer: Allow sampler to set variant type span attribute… (

#32681) * opentelemetrytracer: Allow sampler to set variant type span attributes (#20) Allow sampler to set variant type span attributes. Signed-off-by: thomas.ebner <[email protected]> * added changelog entry Signed-off-by: thomas.ebner <[email protected]> * introduce type alias OtelAttibutes, use local variable Signed-off-by: thomas.ebner <[email protected]> * fix typo in type alias Signed-off-by: thomas.ebner <[email protected]> --------- Signed-off-by: thomas.ebner <[email protected]>
envoyproxy · Mar 7, 2024 · 7186d1b · 7186d1b
1 parent 4165adc
commit 7186d1b
Show file tree

Hide file tree

Showing 10 changed files with 158 additions and 36 deletions.
diff --git a/changelogs/current.yaml b/changelogs/current.yaml
@@ -279,6 +279,9 @@ new_features:
 - area: tracing
   change: |
     Added User-Agent header to OTLP trace exporters according to the OpenTelemetry specification.
+- area: tracing
+  change: |
+    Added support for variant span attribute type for the OpenTelemetry tracer.
 
 deprecated:
 - area: listener

diff --git a/source/extensions/tracers/opentelemetry/BUILD b/source/extensions/tracers/opentelemetry/BUILD
@@ -67,6 +67,7 @@ envoy_cc_library(
         "otlp_utils.h",
         "trace_exporter.h",
     ],
+    external_deps = ["opentelemetry_api"],
     deps = [
         "//envoy/grpc:async_client_manager_interface",
         "//envoy/upstream:cluster_manager_interface",

diff --git a/source/extensions/tracers/opentelemetry/otlp_utils.cc b/source/extensions/tracers/opentelemetry/otlp_utils.cc
@@ -1,5 +1,6 @@
 #include "source/extensions/tracers/opentelemetry/otlp_utils.h"
 
+#include <cstdint>
 #include <string>
 
 #include "source/common/common/fmt.h"
@@ -16,6 +17,40 @@ const std::string& OtlpUtils::getOtlpUserAgentHeader() {
                          fmt::format("OTel-OTLP-Exporter-Envoy/{}", Envoy::VersionInfo::version()));
 }
 
+void OtlpUtils::populateAnyValue(opentelemetry::proto::common::v1::AnyValue& value_proto,
+                                 const OTelAttribute& attribute_value) {
+  switch (attribute_value.index()) {
+  case opentelemetry::common::AttributeType::kTypeBool:
+    value_proto.set_bool_value(opentelemetry::nostd::get<bool>(attribute_value) ? true : false);
+    break;
+  case opentelemetry::common::AttributeType::kTypeInt:
+    value_proto.set_int_value(opentelemetry::nostd::get<int32_t>(attribute_value));
+    break;
+  case opentelemetry::common::AttributeType::kTypeInt64:
+    value_proto.set_int_value(opentelemetry::nostd::get<int64_t>(attribute_value));
+    break;
+  case opentelemetry::common::AttributeType::kTypeUInt:
+    value_proto.set_int_value(opentelemetry::nostd::get<uint32_t>(attribute_value));
+    break;
+  case opentelemetry::common::AttributeType::kTypeUInt64:
+    value_proto.set_int_value(opentelemetry::nostd::get<uint64_t>(attribute_value));
+    break;
+  case opentelemetry::common::AttributeType::kTypeDouble:
+    value_proto.set_double_value(opentelemetry::nostd::get<double>(attribute_value));
+    break;
+  case opentelemetry::common::AttributeType::kTypeCString:
+    value_proto.set_string_value(opentelemetry::nostd::get<const char*>(attribute_value));
+    break;
+  case opentelemetry::common::AttributeType::kTypeString: {
+    const auto sv = opentelemetry::nostd::get<opentelemetry::nostd::string_view>(attribute_value);
+    value_proto.set_string_value(sv.data(), sv.size());
+    break;
+  }
+  default:
+    return;
+  }
+}
+
 } // namespace OpenTelemetry
 } // namespace Tracers
 } // namespace Extensions

diff --git a/source/extensions/tracers/opentelemetry/otlp_utils.h b/source/extensions/tracers/opentelemetry/otlp_utils.h
@@ -2,11 +2,34 @@
 
 #include <string>
 
+#include "opentelemetry/common/attribute_value.h"
+#include "opentelemetry/proto/common/v1/common.pb.h"
+#include "opentelemetry/proto/trace/v1/trace.pb.h"
+
 namespace Envoy {
 namespace Extensions {
 namespace Tracers {
 namespace OpenTelemetry {
 
+/**
+ * @brief The type of the span.
+ * see
+ * https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/trace/api.md#spankind
+ */
+using OTelSpanKind = ::opentelemetry::proto::trace::v1::Span::SpanKind;
+
+/**
+ * @brief Open-telemetry Attribute
+ * see
+ * https://github.com/open-telemetry/opentelemetry-cpp/blob/main/api/include/opentelemetry/common/attribute_value.h
+ */
+using OTelAttribute = ::opentelemetry::common::AttributeValue;
+
+/**
+ * @brief Container holding Open-telemetry Attributes
+ */
+using OtelAttributes = std::map<std::string, OTelAttribute>;
+
 /**
  * Contains utility functions  for Otel
  */
@@ -21,6 +44,15 @@ class OtlpUtils {
    * @return std::string The User-Agent for the OTLP exporters in Envoy.
    */
   static const std::string& getOtlpUserAgentHeader();
+
+  /**
+   * @brief Set the Otel attribute on a Proto Value object
+   *
+   * @param value_proto Proto object which gets the value set.
+   * @param attribute_value Value to set on the proto object.
+   */
+  static void populateAnyValue(opentelemetry::proto::common::v1::AnyValue& value_proto,
+                               const OTelAttribute& attribute_value);
 };
 
 } // namespace OpenTelemetry

diff --git a/source/extensions/tracers/opentelemetry/samplers/dynatrace/dynatrace_sampler.cc b/source/extensions/tracers/opentelemetry/samplers/dynatrace/dynatrace_sampler.cc
@@ -85,22 +85,21 @@ class DynatraceTag {
 };
 
 // add Dynatrace specific span attributes
-void addSamplingAttributes(uint32_t sampling_exponent,
-                           std::map<std::string, std::string>& attributes) {
+void addSamplingAttributes(uint32_t sampling_exponent, OtelAttributes& attributes) {
 
   const auto multiplicity = SamplingState::toMultiplicity(sampling_exponent);
   // The denominator of the sampling ratio. If, for example, the Dynatrace OneAgent samples with a
   // probability of 1/16, the value of supportability sampling ratio would be 16.
   // Note: Ratio is also known as multiplicity.
-  attributes["supportability.atm_sampling_ratio"] = std::to_string(multiplicity);
+  attributes["supportability.atm_sampling_ratio"] = multiplicity;
 
   if (multiplicity > 1) {
     static constexpr uint64_t two_pow_56 = 1llu << 56; // 2^56
     // The sampling probability can be interpreted as the number of spans
     // that are discarded out of 2^56. The attribute is only available if the sampling.threshold is
     // not 0 and therefore sampling happened.
     const uint64_t sampling_threshold = two_pow_56 - two_pow_56 / multiplicity;
-    attributes["sampling.threshold"] = std::to_string(sampling_threshold);
+    attributes["sampling.threshold"] = sampling_threshold;
   }
 }
 
@@ -129,7 +128,7 @@ SamplingResult DynatraceSampler::shouldSample(const absl::optional<SpanContext>
                                               const std::vector<SpanContext>& /*links*/) {
 
   SamplingResult result;
-  std::map<std::string, std::string> att;
+  OtelAttributes att;
 
   // trace_context->path() returns path and query. query part is removed in getSamplingKey()
   const std::string sampling_key =
@@ -175,8 +174,9 @@ SamplingResult DynatraceSampler::shouldSample(const absl::optional<SpanContext>
   }
 
   if (!att.empty()) {
-    result.attributes = std::make_unique<const std::map<std::string, std::string>>(std::move(att));
+    result.attributes = std::make_unique<const OtelAttributes>(std::move(att));
   }
+
   return result;
 }
 

diff --git a/source/extensions/tracers/opentelemetry/samplers/sampler.h b/source/extensions/tracers/opentelemetry/samplers/sampler.h
@@ -10,8 +10,9 @@
 #include "envoy/server/tracer_config.h"
 #include "envoy/tracing/trace_context.h"
 
+#include "source/extensions/tracers/opentelemetry/otlp_utils.h"
+
 #include "absl/types/optional.h"
-#include "opentelemetry/proto/trace/v1/trace.pb.h"
 
 namespace Envoy {
 namespace Extensions {
@@ -30,18 +31,11 @@ enum class Decision {
   RecordAndSample
 };
 
-/**
- * @brief The type of the span.
- * see
- * https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/trace/api.md#spankind
- */
-using OTelSpanKind = ::opentelemetry::proto::trace::v1::Span::SpanKind;
-
 struct SamplingResult {
   /// @see Decision
   Decision decision;
   // A set of span Attributes that will also be added to the Span. Can be nullptr.
-  std::unique_ptr<const std::map<std::string, std::string>> attributes;
+  std::unique_ptr<const OtelAttributes> attributes;
   // A Tracestate that will be associated with the Span. If the sampler
   // returns an empty Tracestate here, the Tracestate will be cleared, so samplers SHOULD normally
   // return the passed-in Tracestate if they do not intend to change it

diff --git a/source/extensions/tracers/opentelemetry/tracer.cc b/source/extensions/tracers/opentelemetry/tracer.cc
@@ -8,6 +8,7 @@
 #include "source/common/common/empty_string.h"
 #include "source/common/common/hex.h"
 #include "source/common/tracing/trace_context_impl.h"
+#include "source/extensions/tracers/opentelemetry/otlp_utils.h"
 
 #include "opentelemetry/proto/collector/trace/v1/trace_service.pb.h"
 #include "opentelemetry/proto/trace/v1/trace.pb.h"
@@ -44,7 +45,7 @@ void callSampler(SamplerSharedPtr sampler, const absl::optional<SpanContext> spa
 
   if (sampling_result.attributes) {
     for (auto const& attribute : *sampling_result.attributes) {
-      new_span.setTag(attribute.first, attribute.second);
+      new_span.setAttribute(attribute.first, attribute.second);
     }
   }
   if (!sampling_result.tracestate.empty()) {
@@ -96,7 +97,7 @@ void Span::injectContext(Tracing::TraceContext& trace_context,
   traceStateHeader().setRefKey(trace_context, span_.trace_state());
 }
 
-void Span::setTag(absl::string_view name, absl::string_view value) {
+void Span::setAttribute(absl::string_view name, const OTelAttribute& attribute_value) {
   // The attribute key MUST be a non-null and non-empty string.
   if (name.empty()) {
     return;
@@ -105,7 +106,7 @@ void Span::setTag(absl::string_view name, absl::string_view value) {
   // If a value already exists for this key, overwrite it.
   for (auto& key_value : *span_.mutable_attributes()) {
     if (key_value.key() == name) {
-      key_value.mutable_value()->set_string_value(std::string{value});
+      OtlpUtils::populateAnyValue(*key_value.mutable_value(), attribute_value);
       return;
     }
   }
@@ -114,12 +115,14 @@ void Span::setTag(absl::string_view name, absl::string_view value) {
       opentelemetry::proto::common::v1::KeyValue();
   opentelemetry::proto::common::v1::AnyValue value_proto =
       opentelemetry::proto::common::v1::AnyValue();
-  value_proto.set_string_value(std::string{value});
+  OtlpUtils::populateAnyValue(value_proto, attribute_value);
   key_value.set_key(std::string{name});
   *key_value.mutable_value() = value_proto;
   *span_.add_attributes() = key_value;
 }
 
+void Span::setTag(absl::string_view name, absl::string_view value) { setAttribute(name, value); }
+
 Tracer::Tracer(OpenTelemetryTraceExporterPtr exporter, Envoy::TimeSource& time_source,
                Random::RandomGenerator& random, Runtime::Loader& runtime,
                Event::Dispatcher& dispatcher, OpenTelemetryTracerStats tracing_stats,

diff --git a/source/extensions/tracers/opentelemetry/tracer.h b/source/extensions/tracers/opentelemetry/tracer.h
@@ -146,6 +146,11 @@ class Span : Logger::Loggable<Logger::Id::tracing>, public Tracing::Span {
     span_.set_trace_state(std::string{tracestate});
   }
 
+  /**
+   * Sets a span attribute.
+   */
+  void setAttribute(absl::string_view name, const OTelAttribute& value);
+
   /**
    * Method to access the span for testing.
    */

diff --git a/test/extensions/tracers/opentelemetry/samplers/dynatrace/dynatrace_sampler_test.cc b/test/extensions/tracers/opentelemetry/samplers/dynatrace/dynatrace_sampler_test.cc
@@ -80,8 +80,9 @@ TEST_F(DynatraceSamplerTest, TestWithoutParentContext) {
                              ::opentelemetry::proto::trace::v1::Span::SPAN_KIND_SERVER, {}, {});
   EXPECT_EQ(sampling_result.decision, Decision::RecordAndSample);
   EXPECT_EQ(sampling_result.attributes->size(), 1);
-  EXPECT_STREQ(
-      sampling_result.attributes->find("supportability.atm_sampling_ratio")->second.c_str(), "1");
+  EXPECT_EQ(opentelemetry::nostd::get<uint32_t>(
+                sampling_result.attributes->find("supportability.atm_sampling_ratio")->second),
+            1);
   EXPECT_STREQ(sampling_result.tracestate.c_str(), "5b3f9fed-980df25c@dt=fw4;0;0;0;0;0;0;95");
   EXPECT_TRUE(sampling_result.isRecording());
   EXPECT_TRUE(sampling_result.isSampled());
@@ -96,8 +97,9 @@ TEST_F(DynatraceSamplerTest, TestWithUnknownParentContext) {
                              ::opentelemetry::proto::trace::v1::Span::SPAN_KIND_SERVER, {}, {});
   EXPECT_EQ(sampling_result.decision, Decision::RecordAndSample);
   EXPECT_EQ(sampling_result.attributes->size(), 1);
-  EXPECT_STREQ(
-      sampling_result.attributes->find("supportability.atm_sampling_ratio")->second.c_str(), "1");
+  EXPECT_EQ(opentelemetry::nostd::get<uint32_t>(
+                sampling_result.attributes->find("supportability.atm_sampling_ratio")->second),
+            1);
   // Dynatrace tracestate should be prepended
   EXPECT_STREQ(sampling_result.tracestate.c_str(),
                "5b3f9fed-980df25c@dt=fw4;0;0;0;0;0;0;95,some_vendor=some_value");
@@ -114,8 +116,9 @@ TEST_F(DynatraceSamplerTest, TestWithDynatraceParentContextSampled) {
                              ::opentelemetry::proto::trace::v1::Span::SPAN_KIND_SERVER, {}, {});
   EXPECT_EQ(sampling_result.decision, Decision::RecordAndSample);
   EXPECT_EQ(sampling_result.attributes->size(), 1);
-  EXPECT_STREQ(
-      sampling_result.attributes->find("supportability.atm_sampling_ratio")->second.c_str(), "1");
+  EXPECT_EQ(opentelemetry::nostd::get<uint32_t>(
+                sampling_result.attributes->find("supportability.atm_sampling_ratio")->second),
+            1);
   // tracestate should be forwarded
   EXPECT_STREQ(sampling_result.tracestate.c_str(), dt_tracestate_sampled);
   // sampling decision from parent should be respected
@@ -183,10 +186,13 @@ TEST_F(DynatraceSamplerTest, TestWithDynatraceParentContextIgnored) {
                              ::opentelemetry::proto::trace::v1::Span::SPAN_KIND_SERVER, {}, {});
   EXPECT_EQ(sampling_result.decision, Decision::Drop);
   EXPECT_EQ(sampling_result.attributes->size(), 2);
-  EXPECT_STREQ(
-      sampling_result.attributes->find("supportability.atm_sampling_ratio")->second.c_str(), "4");
-  EXPECT_STREQ(sampling_result.attributes->find("sampling.threshold")->second.c_str(),
-               "54043195528445952");
+  EXPECT_EQ(opentelemetry::nostd::get<uint32_t>(
+                sampling_result.attributes->find("supportability.atm_sampling_ratio")->second),
+            4);
+  EXPECT_EQ(opentelemetry::nostd::get<uint64_t>(
+                sampling_result.attributes->find("sampling.threshold")->second),
+            54043195528445952);
+
   // tracestate should be forwarded
   EXPECT_STREQ(sampling_result.tracestate.c_str(), dt_tracestate_ignored);
   // sampling decision from parent should be respected
@@ -205,8 +211,9 @@ TEST_F(DynatraceSamplerTest, TestWithDynatraceParentContextFromDifferentTenant)
   // sampling decision on tracestate should be ignored because it is from a different tenant.
   EXPECT_EQ(sampling_result.decision, Decision::RecordAndSample);
   EXPECT_EQ(sampling_result.attributes->size(), 1);
-  EXPECT_STREQ(
-      sampling_result.attributes->find("supportability.atm_sampling_ratio")->second.c_str(), "1");
+  EXPECT_EQ(opentelemetry::nostd::get<uint32_t>(
+                sampling_result.attributes->find("supportability.atm_sampling_ratio")->second),
+            1);
   // new Dynatrace tag should be prepended, already existing tag should be kept
   const char* exptected =
       "5b3f9fed-980df25c@dt=fw4;0;0;0;0;0;0;95,6666ad40-980df25c@dt=fw4;4;4af38366;0;0;1;2;123;"

diff --git a/test/extensions/tracers/opentelemetry/samplers/sampler_test.cc b/test/extensions/tracers/opentelemetry/samplers/sampler_test.cc
@@ -165,11 +165,18 @@ TEST_F(SamplerFactoryTest, TestWithSampler) {
                    const std::vector<SpanContext>&) {
         SamplingResult res;
         res.decision = Decision::Drop;
-        std::map<std::string, std::string> attributes;
-        attributes["key"] = "value";
-        attributes["another_key"] = "another_value";
-        res.attributes =
-            std::make_unique<const std::map<std::string, std::string>>(std::move(attributes));
+        OtelAttributes attributes;
+        attributes["char_key"] = "char_value";
+        attributes["sv_key"] = absl::string_view("sv_value");
+        attributes["bool_key"] = true;
+        attributes["int_key"] = static_cast<int32_t>(123);
+        attributes["uint_key"] = static_cast<uint32_t>(123);
+        attributes["int64_t_key"] = static_cast<int64_t>(INT64_MAX);
+        attributes["uint64_t_key"] = static_cast<uint64_t>(UINT64_MAX);
+        attributes["double_key"] = 0.123;
+        attributes["not_supported_span"] = opentelemetry::nostd::span<bool>();
+
+        res.attributes = std::make_unique<const OtelAttributes>(std::move(attributes));
         res.tracestate = "this_is=another_tracesate";
         return res;
       });
@@ -178,6 +185,41 @@ TEST_F(SamplerFactoryTest, TestWithSampler) {
   std::unique_ptr<Span> unsampled_span(dynamic_cast<Span*>(tracing_span.release()));
   EXPECT_FALSE(unsampled_span->sampled());
   EXPECT_STREQ(unsampled_span->tracestate().c_str(), "this_is=another_tracesate");
+  auto proto_span = unsampled_span->spanForTest();
+
+  auto get_attr_value =
+      [&proto_span](const char* name) -> ::opentelemetry::proto::common::v1::AnyValue* {
+    for (auto& key_value : *proto_span.mutable_attributes()) {
+      if (key_value.key() == name) {
+        return key_value.mutable_value();
+      }
+    }
+    return nullptr;
+  };
+
+  ASSERT_NE(get_attr_value("char_key"), nullptr);
+  EXPECT_STREQ(get_attr_value("char_key")->string_value().c_str(), "char_value");
+
+  ASSERT_NE(get_attr_value("sv_key"), nullptr);
+  EXPECT_STREQ(get_attr_value("sv_key")->string_value().c_str(), "sv_value");
+
+  ASSERT_NE(get_attr_value("bool_key"), nullptr);
+  EXPECT_EQ(get_attr_value("bool_key")->bool_value(), true);
+
+  ASSERT_NE(get_attr_value("int_key"), nullptr);
+  EXPECT_EQ(get_attr_value("int_key")->int_value(), 123);
+
+  ASSERT_NE(get_attr_value("uint_key"), nullptr);
+  EXPECT_EQ(get_attr_value("uint_key")->int_value(), 123);
+
+  ASSERT_NE(get_attr_value("int64_t_key"), nullptr);
+  EXPECT_EQ(get_attr_value("int64_t_key")->int_value(), INT64_MAX);
+
+  ASSERT_NE(get_attr_value("uint64_t_key"), nullptr);
+  EXPECT_EQ(get_attr_value("uint64_t_key")->int_value(), UINT64_MAX);
+
+  ASSERT_NE(get_attr_value("double_key"), nullptr);
+  EXPECT_EQ(get_attr_value("double_key")->double_value(), 0.123);
 }
 
 // Test that sampler receives trace_context