diff --git a/CHANGELOG.md b/CHANGELOG.md index 56f43265cd..6e5f88f9c4 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/ci/do_ci.sh b/ci/do_ci.sh index 2ab3c3a4d0..cb2547d9a1 100755 --- a/ci/do_ci.sh +++ b/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 //... diff --git a/exporters/jaeger/BUILD b/exporters/jaeger/BUILD index eefacc5c5f..ea18ef4153 100644 --- a/exporters/jaeger/BUILD +++ b/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", + ], +) diff --git a/exporters/jaeger/src/recordable.cc b/exporters/jaeger/src/recordable.cc index fe18bbaa39..58ef6873e0 100644 --- a/exporters/jaeger/src/recordable.cc +++ b/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 &tags) { - if (nostd::holds_alternative(value)) + if (nostd::holds_alternative(value)) + { + AddTag(std::string{key}, int64_t{nostd::get(value)}, tags); + } + else if (nostd::holds_alternative(value)) + { + AddTag(std::string{key}, int64_t{nostd::get(value)}, tags); + } + else if (nostd::holds_alternative(value)) { AddTag(std::string{key}, nostd::get(value), tags); } @@ -39,14 +48,68 @@ void JaegerRecordable::PopulateAttribute(nostd::string_view key, { AddTag(std::string{key}, std::string{nostd::get(value)}, tags); } - // TODO: extend other AttributeType to the types supported by Jaeger. + else if (nostd::holds_alternative>(value)) + { + for (const auto &val : nostd::get>(value)) + { + AddTag(std::string{key}, val, tags); + } + } + else if (nostd::holds_alternative>(value)) + { + for (const auto &val : nostd::get>(value)) + { + AddTag(std::string{key}, int64_t{val}, tags); + } + } + else if (nostd::holds_alternative>(value)) + { + for (const auto &val : nostd::get>(value)) + { + AddTag(std::string{key}, val, tags); + } + } + else if (nostd::holds_alternative>(value)) + { + for (const auto &val : nostd::get>(value)) + { + AddTag(std::string{key}, int64_t{val}, tags); + } + } + else if (nostd::holds_alternative>(value)) + { + for (const auto &val : nostd::get>(value)) + { + AddTag(std::string{key}, val, tags); + } + } + else if (nostd::holds_alternative>(value)) + { + for (const auto &val : nostd::get>(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 &tags) { - if (nostd::holds_alternative(value)) + if (nostd::holds_alternative(value)) + { + AddTag(std::string{key}, int64_t{nostd::get(value)}, tags); + } + else if (nostd::holds_alternative(value)) + { + AddTag(std::string{key}, int64_t{nostd::get(value)}, tags); + } + else if (nostd::holds_alternative(value)) { AddTag(std::string{key}, nostd::get(value), tags); } @@ -62,7 +125,11 @@ void JaegerRecordable::PopulateAttribute(nostd::string_view key, { AddTag(std::string{key}, std::string{nostd::get(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, diff --git a/exporters/jaeger/test/jaeger_recordable_test.cc b/exporters/jaeger/test/jaeger_recordable_test.cc index 48c2003a98..586b5f7ccf 100644 --- a/exporters/jaeger/test/jaeger_recordable_test.cc +++ b/exporters/jaeger/test/jaeger_recordable_test.cc @@ -1,6 +1,7 @@ // Copyright The OpenTelemetry Authors // SPDX-License-Identifier: Apache-2.0 +#include #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 +void addTag(thrift::TagType::type tag_type, + const std::string &key, + value_type value, + vector &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 &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 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{{false, true}}); + rec.SetAttribute("key3", nostd::span{{-320, 320}}); + rec.SetAttribute("key4", nostd::span{{-640, 640}}); + rec.SetAttribute("key5", nostd::span{{320, 322}}); + rec.SetAttribute("key6", nostd::span{{4.15, 5.15}}); + rec.SetAttribute("key7", nostd::span{{"string_v1", "string_v2"}}); + + auto tags = rec.Tags(); + EXPECT_EQ(tags.size(), values.size() + 12); + + vector 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"); }