Skip to content

Commit

Permalink
Jaeger exporter: extend supported attributes types (#1106)
Browse files Browse the repository at this point in the history
  • Loading branch information
esigo committed Dec 6, 2021
1 parent 2351125 commit c50ed1c
Show file tree
Hide file tree
Showing 5 changed files with 181 additions and 14 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
8 changes: 4 additions & 4 deletions ci/do_ci.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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 //...
Expand Down
14 changes: 14 additions & 0 deletions exporters/jaeger/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -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",
],
)
75 changes: 71 additions & 4 deletions exporters/jaeger/src/recordable.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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);
}
Expand All @@ -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);
}
Expand All @@ -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,
Expand Down
97 changes: 91 additions & 6 deletions exporters/jaeger/test/jaeger_recordable_test.cc
Original file line number Diff line number Diff line change
@@ -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"
Expand All @@ -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)
{
Expand Down Expand Up @@ -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 =
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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");
}
Expand Down

0 comments on commit c50ed1c

Please sign in to comment.