Jaeger exporter: extend supported attributes types

CHANGELOG.md

@@ -15,6 +15,7 @@ Increment the:
 
 ## [Unreleased]
 
+* [EXPORTER] Bugfix: Jaeger exporter: extend supported attributes types ([#1106](https://github.com/open-telemetry/opentelemetry-cpp/pull/1106))
 * [EXPORTER] Fix otlp generates null span ids ([#1106](https://github.com/open-telemetry/opentelemetry-cpp/pull/1106))
 
 ## [1.1.0] 2021-11-19

ci/do_ci.sh

@@ -173,10 +173,10 @@ elif [[ "$1" == "bazel.legacy.test" ]]; then
   bazel $BAZEL_STARTUP_OPTIONS test $BAZEL_TEST_OPTIONS -- //... -//exporters/otlp/... -//exporters/prometheus/...
   exit 0
 elif [[ "$1" == "bazel.noexcept" ]]; then
-  # there are some exceptions and error handling code from the Prometheus Client
-  # that make this test always fail. ignore Prometheus exporter in the noexcept here.
-  bazel $BAZEL_STARTUP_OPTIONS build --copt=-fno-exceptions $BAZEL_OPTIONS -- //... -//exporters/prometheus/...
-  bazel $BAZEL_STARTUP_OPTIONS test --copt=-fno-exceptions $BAZEL_TEST_OPTIONS -- //... -//exporters/prometheus/...
+  # there are some exceptions and error handling code from the Prometheus and Jaeger Clients
+  # that make this test always fail. ignore Prometheus and Jaeger exporters in the noexcept here.
+  bazel $BAZEL_STARTUP_OPTIONS build --copt=-fno-exceptions $BAZEL_OPTIONS -- //... -//exporters/prometheus/... -//exporters/jaeger/...
+  bazel $BAZEL_STARTUP_OPTIONS test --copt=-fno-exceptions $BAZEL_TEST_OPTIONS -- //... -//exporters/prometheus/... -//exporters/jaeger/...
   exit 0
 elif [[ "$1" == "bazel.asan" ]]; then
   bazel $BAZEL_STARTUP_OPTIONS test --config=asan $BAZEL_TEST_OPTIONS //...

exporters/jaeger/BUILD

@@ -102,3 +102,17 @@ cc_library(
         ":jaeger_exporter",
     ],
 )
+
+cc_test(
+    name = "jaeger_recordable_test",
+    srcs = ["test/jaeger_recordable_test.cc"],
+    tags = [
+        "jaeger",
+        "test",
+    ],
+    deps = [
+        ":opentelemetry_exporter_jaeger_trace",
+        "//sdk/src/resource",
+        "@com_google_googletest//:gtest_main",
+    ],
+)

exporters/jaeger/src/recordable.cc

@@ -2,6 +2,7 @@
 // SPDX-License-Identifier: Apache-2.0
 
 #include "opentelemetry/exporters/jaeger/recordable.h"
+#include "opentelemetry/sdk/common/global_log_handler.h"
 #include "opentelemetry/sdk/resource/experimental_semantic_conventions.h"
 
 OPENTELEMETRY_BEGIN_NAMESPACE
@@ -19,7 +20,15 @@ void JaegerRecordable::PopulateAttribute(nostd::string_view key,
                                          const common::AttributeValue &value,
                                          std::vector<thrift::Tag> &tags)
 {
-  if (nostd::holds_alternative<int64_t>(value))
+  if (nostd::holds_alternative<int32_t>(value))
+  {
+    AddTag(std::string{key}, int64_t{nostd::get<int32_t>(value)}, tags);
+  }
+  else if (nostd::holds_alternative<uint32_t>(value))
+  {
+    AddTag(std::string{key}, int64_t{nostd::get<uint32_t>(value)}, tags);
+  }
+  else if (nostd::holds_alternative<int64_t>(value))
   {
     AddTag(std::string{key}, nostd::get<int64_t>(value), tags);
   }
@@ -39,14 +48,68 @@ void JaegerRecordable::PopulateAttribute(nostd::string_view key,
   {
     AddTag(std::string{key}, std::string{nostd::get<nostd::string_view>(value)}, tags);
   }
-  // TODO: extend other AttributeType to the types supported by Jaeger.
+  else if (nostd::holds_alternative<nostd::span<const bool>>(value))
+  {
+    for (const auto &val : nostd::get<nostd::span<const bool>>(value))
+    {
+      AddTag(std::string{key}, val, tags);
+    }
+  }
+  else if (nostd::holds_alternative<nostd::span<const int32_t>>(value))
+  {
+    for (const auto &val : nostd::get<nostd::span<const int32_t>>(value))
+    {
+      AddTag(std::string{key}, int64_t{val}, tags);
+    }
+  }
+  else if (nostd::holds_alternative<nostd::span<const int64_t>>(value))
+  {
+    for (const auto &val : nostd::get<nostd::span<const int64_t>>(value))
+    {
+      AddTag(std::string{key}, val, tags);
+    }
+  }
+  else if (nostd::holds_alternative<nostd::span<const uint32_t>>(value))
+  {
+    for (const auto &val : nostd::get<nostd::span<const uint32_t>>(value))
+    {
+      AddTag(std::string{key}, int64_t{val}, tags);
+    }
+  }
+  else if (nostd::holds_alternative<nostd::span<const double>>(value))
+  {
+    for (const auto &val : nostd::get<nostd::span<const double>>(value))
+    {
+      AddTag(std::string{key}, val, tags);
+    }
+  }
+  else if (nostd::holds_alternative<nostd::span<const nostd::string_view>>(value))
+  {
+    for (const auto &val : nostd::get<nostd::span<const nostd::string_view>>(value))
+    {
+      AddTag(std::string{key}, std::string{val}, tags);
+    }
+  }
+  else
+  {
+    OTEL_INTERNAL_LOG_ERROR(
+        "[TRACE JAEGER Exporter] SetAttribute() failed, attribute type not supported ");
+  }
 }
 
 void JaegerRecordable::PopulateAttribute(nostd::string_view key,
                                          const sdk::common::OwnedAttributeValue &value,
                                          std::vector<thrift::Tag> &tags)
 {
-  if (nostd::holds_alternative<int64_t>(value))
+  if (nostd::holds_alternative<int32_t>(value))
+  {
+    AddTag(std::string{key}, int64_t{nostd::get<int32_t>(value)}, tags);
+  }
+  else if (nostd::holds_alternative<uint32_t>(value))
+  {
+    AddTag(std::string{key}, int64_t{nostd::get<uint32_t>(value)}, tags);
+  }
+  else if (nostd::holds_alternative<int64_t>(value))
   {
     AddTag(std::string{key}, nostd::get<int64_t>(value), tags);
   }
@@ -62,7 +125,11 @@ void JaegerRecordable::PopulateAttribute(nostd::string_view key,
   {
     AddTag(std::string{key}, std::string{nostd::get<std::string>(value)}, tags);
   }
-  // TODO: extend other OwnedAttributeType to the types supported by Jaeger.
+  else
+  {
+    OTEL_INTERNAL_LOG_ERROR(
+        "[TRACE JAEGER Exporter] SetAttribute() failed, attribute type not supported ");
+  }
 }
 
 void JaegerRecordable::SetIdentity(const trace::SpanContext &span_context,

exporters/jaeger/test/jaeger_recordable_test.cc

@@ -1,6 +1,7 @@
 // Copyright The OpenTelemetry Authors
 // SPDX-License-Identifier: Apache-2.0
 
+#include <vector>
 #include "opentelemetry/exporters/jaeger/recordable.h"
 #include "opentelemetry/sdk/instrumentationlibrary/instrumentation_library.h"
 #include "opentelemetry/sdk/trace/simple_processor.h"
@@ -18,6 +19,7 @@ namespace common   = opentelemetry::common;
 using namespace jaegertracing;
 using namespace opentelemetry::exporter::jaeger;
 using namespace opentelemetry::sdk::instrumentationlibrary;
+using std::vector;
 
 TEST(JaegerSpanRecordable, SetIdentity)
 {
@@ -127,8 +129,6 @@ TEST(JaegerSpanRecordable, AddEvent)
 {
   JaegerRecordable rec;
 
-  nostd::string_view name = "Test Event";
-
   std::chrono::system_clock::time_point event_time = std::chrono::system_clock::now();
   common::SystemTimestamp event_timestamp(event_time);
   uint64_t epoch_us =
@@ -156,6 +156,95 @@ TEST(JaegerSpanRecordable, AddEvent)
   }
 }
 
+template <typename value_type>
+void addTag(thrift::TagType::type tag_type,
+            const std::string &key,
+            value_type value,
+            vector<thrift::Tag> &tags)
+{
+  thrift::Tag tag;
+
+  tag.__set_key(key);
+  tag.__set_vType(tag_type);
+  if (tag_type == thrift::TagType::LONG)
+  {
+    tag.__set_vLong(value);
+  }
+  else if (tag_type == thrift::TagType::DOUBLE)
+  {
+    tag.__set_vDouble(value);
+  }
+  else if (tag_type == thrift::TagType::BOOL)
+  {
+    tag.__set_vBool(value);
+  }
+
+  tags.push_back(tag);
+}
+
+void addTag(const std::string &key, std::string value, vector<thrift::Tag> &tags)
+{
+  thrift::Tag tag;
+
+  tag.__set_key(key);
+  tag.__set_vType(thrift::TagType::STRING);
+  tag.__set_vStr(value);
+
+  tags.push_back(tag);
+}
+
+TEST(JaegerSpanRecordable, SetAttributes)
+{
+  JaegerRecordable rec;
+  std::string string_val{"string_val"};
+  vector<common::AttributeValue> values{
+      bool{false},
+      int32_t{-32},
+      int64_t{-64},
+      uint32_t{32},
+      double{3.14},
+      string_val.c_str(),
+      nostd::string_view{"string_view"},
+  };
+  for (const auto &val : values)
+  {
+    rec.SetAttribute("key1", val);
+  }
+  rec.SetAttribute("key2", nostd::span<const bool>{{false, true}});
+  rec.SetAttribute("key3", nostd::span<const int32_t>{{-320, 320}});
+  rec.SetAttribute("key4", nostd::span<const int64_t>{{-640, 640}});
+  rec.SetAttribute("key5", nostd::span<const uint32_t>{{320, 322}});
+  rec.SetAttribute("key6", nostd::span<const double>{{4.15, 5.15}});
+  rec.SetAttribute("key7", nostd::span<const nostd::string_view>{{"string_v1", "string_v2"}});
+
+  auto tags = rec.Tags();
+  EXPECT_EQ(tags.size(), values.size() + 12);
+
+  vector<thrift::Tag> expected_tags;
+  addTag(thrift::TagType::BOOL, "key1", bool{false}, expected_tags);
+  addTag(thrift::TagType::LONG, "key1", int32_t{-32}, expected_tags);
+  addTag(thrift::TagType::LONG, "key1", int64_t{-64}, expected_tags);
+  addTag(thrift::TagType::LONG, "key1", int32_t{32}, expected_tags);
+  addTag(thrift::TagType::DOUBLE, "key1", double{3.14}, expected_tags);
+  addTag("key1", string_val, expected_tags);
+  addTag("key1", std::string{"string_view"}, expected_tags);
+
+  addTag(thrift::TagType::BOOL, "key2", bool{false}, expected_tags);
+  addTag(thrift::TagType::BOOL, "key2", bool{true}, expected_tags);
+  addTag(thrift::TagType::LONG, "key3", int32_t{-320}, expected_tags);
+  addTag(thrift::TagType::LONG, "key3", int32_t{320}, expected_tags);
+  addTag(thrift::TagType::LONG, "key4", int64_t{-640}, expected_tags);
+  addTag(thrift::TagType::LONG, "key4", int64_t{640}, expected_tags);
+  addTag(thrift::TagType::LONG, "key5", uint32_t{320}, expected_tags);
+  addTag(thrift::TagType::LONG, "key5", uint32_t{322}, expected_tags);
+  addTag(thrift::TagType::DOUBLE, "key6", double{4.15}, expected_tags);
+  addTag(thrift::TagType::DOUBLE, "key6", double{5.15}, expected_tags);
+  addTag("key7", std::string{"string_v1"}, expected_tags);
+  addTag("key7", std::string{"string_v2"}, expected_tags);
+
+  EXPECT_EQ(tags, expected_tags);
+}
+
 TEST(JaegerSpanRecordable, SetInstrumentationLibrary)
 {
   JaegerRecordable rec;
@@ -194,19 +283,15 @@ TEST(JaegerSpanRecordable, SetResource)
   EXPECT_GE(resource_tags.size(), 2);
   EXPECT_EQ(service_name, service_name_value);
 
-  bool found_key1 = false;
-  bool found_key2 = false;
   for (const auto &tag : resource_tags)
   {
     if (tag.key == "key1")
     {
-      found_key1 = true;
       EXPECT_EQ(tag.vType, thrift::TagType::STRING);
       EXPECT_EQ(tag.vStr, "value1");
     }
     else if (tag.key == "key2")
     {
-      found_key2 = true;
       EXPECT_EQ(tag.vType, thrift::TagType::STRING);
       EXPECT_EQ(tag.vStr, "value2");
     }