diff --git a/opentracing-shim/src/span_shim.cc b/opentracing-shim/src/span_shim.cc index 77f8c3a262..927b7090ab 100644 --- a/opentracing-shim/src/span_shim.cc +++ b/opentracing-shim/src/span_shim.cc @@ -24,13 +24,13 @@ void SpanShim::handleError(const opentracing::Value& value) noexcept // - false maps to Ok // - no value being set maps to Unset. auto code = StatusCode::kUnset; - const auto& str_value = utils::stringFromValue(value); + const auto& error_tag = utils::stringFromValue(value); - if (str_value == "true") + if (error_tag == "true") { code = StatusCode::kError; } - else if (str_value == "false") + else if (error_tag == "false") { code = StatusCode::kOk; } @@ -68,7 +68,7 @@ void SpanShim::SetBaggageItem(opentracing::string_view restricted_key, opentraci // Creates a new SpanContext Shim with a new OpenTelemetry Baggage containing the specified // Baggage key/value pair, and sets it as the current instance for this Span Shim. if (restricted_key.empty() || value.empty()) return; - + // This operation MUST be safe to be called concurrently. const std::lock_guard guard(context_lock_); context_ = context_.newWithKeyValue(restricted_key.data(), value.data()); } @@ -78,7 +78,7 @@ std::string SpanShim::BaggageItem(opentracing::string_view restricted_key) const // Returns the value for the specified key in the OpenTelemetry Baggage // of the current SpanContext Shim, or null if none exists. if (restricted_key.empty()) return ""; - + // This operation MUST be safe to be called concurrently. const std::lock_guard guard(context_lock_); std::string value; return context_.BaggageItem(restricted_key.data(), value) ? value : ""; diff --git a/opentracing-shim/test/propagation_test.cc b/opentracing-shim/test/propagation_test.cc index 1850d9bd73..e3b1645500 100644 --- a/opentracing-shim/test/propagation_test.cc +++ b/opentracing-shim/test/propagation_test.cc @@ -14,19 +14,7 @@ namespace common = opentelemetry::common; namespace nostd = opentelemetry::nostd; namespace shim = opentelemetry::opentracingshim; -class PropagationTest : public testing::Test -{ -protected: - virtual void SetUp() - { - } - - virtual void TearDown() - { - } -}; - -TEST_F(PropagationTest, TextMapReader_Get_LookupKey_Unsupported) +TEST(PropagationTest, TextMapReader_Get_LookupKey_Unsupported) { std::unordered_map text_map; TextMapCarrier testee{text_map}; @@ -48,7 +36,7 @@ TEST_F(PropagationTest, TextMapReader_Get_LookupKey_Unsupported) ASSERT_EQ(testee.foreach_key_call_count, 2); } -TEST_F(PropagationTest, TextMapReader_Get_LookupKey_Supported) +TEST(PropagationTest, TextMapReader_Get_LookupKey_Supported) { std::unordered_map text_map; TextMapCarrier testee{text_map}; @@ -71,7 +59,7 @@ TEST_F(PropagationTest, TextMapReader_Get_LookupKey_Supported) ASSERT_EQ(testee.foreach_key_call_count, 1); } -TEST_F(PropagationTest, TextMapReader_Keys) +TEST(PropagationTest, TextMapReader_Keys) { std::unordered_map text_map; TextMapCarrier testee{text_map}; @@ -98,7 +86,7 @@ TEST_F(PropagationTest, TextMapReader_Keys) ASSERT_EQ(testee.foreach_key_call_count, 2); } -TEST_F(PropagationTest, TextMapWriter_Set) +TEST(PropagationTest, TextMapWriter_Set) { std::unordered_map text_map; TextMapCarrier testee{text_map}; diff --git a/opentracing-shim/test/shim_mocks.h b/opentracing-shim/test/shim_mocks.h index 8467d15dab..cf60761678 100644 --- a/opentracing-shim/test/shim_mocks.h +++ b/opentracing-shim/test/shim_mocks.h @@ -11,6 +11,7 @@ #include "opentelemetry/trace/span_context.h" #include "opentelemetry/trace/span_metadata.h" #include "opentelemetry/trace/tracer.h" +#include "opentelemetry/trace/tracer_provider.h" #include "opentracing/propagation.h" #include @@ -22,6 +23,19 @@ namespace common = opentelemetry::common; namespace context = opentelemetry::context; namespace nostd = opentelemetry::nostd; +struct MockTracerProvider : public trace_api::TracerProvider +{ + nostd::shared_ptr GetTracer(nostd::string_view library_name, + nostd::string_view, + nostd::string_view) noexcept override + { + library_name_ = std::string{library_name}; + return nostd::shared_ptr(); + } + + std::string library_name_; +}; + struct MockSpan final : public trace_api::Span { void SetAttribute(nostd::string_view key, diff --git a/opentracing-shim/test/shim_utils_test.cc b/opentracing-shim/test/shim_utils_test.cc index 712d286de2..a8006195a6 100644 --- a/opentracing-shim/test/shim_utils_test.cc +++ b/opentracing-shim/test/shim_utils_test.cc @@ -17,19 +17,7 @@ namespace common = opentelemetry::common; namespace nostd = opentelemetry::nostd; namespace shim = opentelemetry::opentracingshim; -class ShimUtilsTest : public testing::Test -{ -protected: - virtual void SetUp() - { - } - - virtual void TearDown() - { - } -}; - -TEST_F(ShimUtilsTest, IsBaggageEmpty) +TEST(ShimUtilsTest, IsBaggageEmpty) { auto none = nostd::shared_ptr(nullptr); ASSERT_TRUE(shim::utils::isBaggageEmpty(none)); @@ -42,7 +30,7 @@ TEST_F(ShimUtilsTest, IsBaggageEmpty) ASSERT_FALSE(shim::utils::isBaggageEmpty(non_empty)); } -TEST_F(ShimUtilsTest, StringFromValue) +TEST(ShimUtilsTest, StringFromValue) { ASSERT_EQ(shim::utils::stringFromValue(true), "true"); ASSERT_EQ(shim::utils::stringFromValue(false), "false"); @@ -61,7 +49,7 @@ TEST_F(ShimUtilsTest, StringFromValue) ASSERT_EQ(shim::utils::stringFromValue(dict.get()), ""); } -TEST_F(ShimUtilsTest, AttributeFromValue) +TEST(ShimUtilsTest, AttributeFromValue) { auto value = shim::utils::attributeFromValue(true); ASSERT_EQ(value.index(), common::AttributeType::kTypeBool); @@ -110,7 +98,7 @@ TEST_F(ShimUtilsTest, AttributeFromValue) ASSERT_EQ(nostd::get(value), nostd::string_view{}); } -TEST_F(ShimUtilsTest, MakeOptionsShim_EmptyRefs) +TEST(ShimUtilsTest, MakeOptionsShim_EmptyRefs) { auto span_context_shim = nostd::shared_ptr(new shim::SpanContextShim( trace_api::SpanContext::GetInvalid(), baggage::Baggage::GetDefault())); @@ -126,7 +114,7 @@ TEST_F(ShimUtilsTest, MakeOptionsShim_EmptyRefs) ASSERT_EQ(nostd::get(options_shim.parent), trace_api::SpanContext::GetInvalid()); } -TEST_F(ShimUtilsTest, MakeOptionsShim_InvalidSpanContext) +TEST(ShimUtilsTest, MakeOptionsShim_InvalidSpanContext) { auto span_context_shim = nostd::shared_ptr(new shim::SpanContextShim( trace_api::SpanContext::GetInvalid(), baggage::Baggage::GetDefault())); @@ -143,7 +131,7 @@ TEST_F(ShimUtilsTest, MakeOptionsShim_InvalidSpanContext) ASSERT_EQ(nostd::get(options_shim.parent), trace_api::SpanContext::GetInvalid()); } -TEST_F(ShimUtilsTest, MakeOptionsShim_FirstChildOf) +TEST(ShimUtilsTest, MakeOptionsShim_FirstChildOf) { auto span_context_shim = nostd::shared_ptr(new shim::SpanContextShim( trace_api::SpanContext::GetInvalid(), baggage::Baggage::GetDefault())); @@ -164,7 +152,7 @@ TEST_F(ShimUtilsTest, MakeOptionsShim_FirstChildOf) ASSERT_EQ(nostd::get(options_shim.parent), span_context_shim->context()); } -TEST_F(ShimUtilsTest, MakeOptionsShim_FirstInList) +TEST(ShimUtilsTest, MakeOptionsShim_FirstInList) { auto span_context_shim = nostd::shared_ptr(new shim::SpanContextShim( trace_api::SpanContext::GetInvalid(), baggage::Baggage::GetDefault())); @@ -184,7 +172,7 @@ TEST_F(ShimUtilsTest, MakeOptionsShim_FirstInList) ASSERT_EQ(nostd::get(options_shim.parent), span_context_shim->context()); } -TEST_F(ShimUtilsTest, MakeIterableLinks) +TEST(ShimUtilsTest, MakeIterableLinks) { auto span_context_shim1 = nostd::shared_ptr(new shim::SpanContextShim( trace_api::SpanContext::GetInvalid(), baggage::Baggage::GetDefault())); @@ -228,7 +216,7 @@ TEST_F(ShimUtilsTest, MakeIterableLinks) ASSERT_EQ(nostd::get(value), "child_of"); } -TEST_F(ShimUtilsTest, MakeBaggage_EmptyRefs) +TEST(ShimUtilsTest, MakeBaggage_EmptyRefs) { auto baggage = baggage::Baggage::GetDefault()->Set("foo", "bar"); std::string value; @@ -246,7 +234,7 @@ TEST_F(ShimUtilsTest, MakeBaggage_EmptyRefs) ASSERT_EQ(value, "bar"); } -TEST_F(ShimUtilsTest, MakeBaggage_NonEmptyRefs) +TEST(ShimUtilsTest, MakeBaggage_NonEmptyRefs) { auto span_context_shim1 = nostd::shared_ptr(new shim::SpanContextShim( trace_api::SpanContext::GetInvalid(), @@ -273,7 +261,7 @@ TEST_F(ShimUtilsTest, MakeBaggage_NonEmptyRefs) ASSERT_EQ(value, "world"); } -TEST_F(ShimUtilsTest, MakeIterableTags) +TEST(ShimUtilsTest, MakeIterableTags) { opentracing::StartSpanOptions options; auto empty = shim::utils::makeIterableTags(options); diff --git a/opentracing-shim/test/span_shim_test.cc b/opentracing-shim/test/span_shim_test.cc index 48c498e261..3f6c91c80d 100644 --- a/opentracing-shim/test/span_shim_test.cc +++ b/opentracing-shim/test/span_shim_test.cc @@ -107,6 +107,37 @@ TEST_F(SpanShimTest, SetBaggageItem) ASSERT_EQ(span_shim->BaggageItem("no value"), ""); } +TEST_F(SpanShimTest, SetBaggageItem_MultiThreaded) +{ + auto span = nostd::shared_ptr(new MockSpan()); + auto tracer = shim::TracerShim::createTracerShim(); + auto tracer_shim = dynamic_cast(tracer.get()); + auto baggage = baggage::Baggage::GetDefault(); + shim::SpanShim span_shim(*tracer_shim, span, baggage); + + std::vector threads; + std::vector keys; + std::vector values; + int thread_count = 100; + + for (int index = 0; index < thread_count; ++index) + { + keys.emplace_back("key-" + std::to_string(index)); + values.emplace_back("value-" + std::to_string(index)); + threads.emplace_back(std::bind(&shim::SpanShim::SetBaggageItem, &span_shim, keys[index], values[index])); + } + + for (auto& thread : threads) + { + thread.join(); + } + + for (int index = 0; index < thread_count; ++index) + { + ASSERT_EQ(span_shim.BaggageItem(keys[index]), values[index]); + } +} + TEST_F(SpanShimTest, Log_NoEvent) { std::string name; diff --git a/opentracing-shim/test/tracer_shim_test.cc b/opentracing-shim/test/tracer_shim_test.cc index 742584d8b0..84c27a4368 100644 --- a/opentracing-shim/test/tracer_shim_test.cc +++ b/opentracing-shim/test/tracer_shim_test.cc @@ -47,6 +47,14 @@ class TracerShimTest : public testing::Test } }; +TEST_F(TracerShimTest, TracerProviderName) +{ + auto mock_provider_ptr = new MockTracerProvider(); + nostd::shared_ptr provider(mock_provider_ptr); + ASSERT_NE(shim::TracerShim::createTracerShim(provider), nullptr); + ASSERT_EQ(mock_provider_ptr->library_name_, "opentracing-shim"); +} + TEST_F(TracerShimTest, SpanReferenceToCreatingTracer) { auto span_shim = tracer_shim->StartSpan("a");