From 8042b5b185018d4d8a749cff28d82b2801e5ad62 Mon Sep 17 00:00:00 2001 From: esigo Date: Sun, 19 Dec 2021 00:38:57 +0100 Subject: [PATCH 1/6] Add unit-test for Jaeger exporter --- exporters/jaeger/BUILD | 16 +++ .../exporters/jaeger/jaeger_exporter.h | 8 ++ exporters/jaeger/src/jaeger_exporter.cc | 4 + exporters/jaeger/src/thrift_sender.h | 4 + exporters/jaeger/test/jaeger_exporter_test.cc | 116 ++++++++++++++++++ 5 files changed, 148 insertions(+) create mode 100644 exporters/jaeger/test/jaeger_exporter_test.cc diff --git a/exporters/jaeger/BUILD b/exporters/jaeger/BUILD index ea18ef4153..7ccca66f50 100644 --- a/exporters/jaeger/BUILD +++ b/exporters/jaeger/BUILD @@ -116,3 +116,19 @@ cc_test( "@com_google_googletest//:gtest_main", ], ) + +cc_test( + name = "jaeger_exporter_test", + srcs = ["test/jaeger_exporter_test.cc"], + copts = ["-fexceptions"], + defines = ["BAZEL_BUILD"], + tags = [ + "jaeger", + "test", + ], + deps = [ + ":opentelemetry_exporter_jaeger_trace", + "//sdk/src/resource", + "@com_google_googletest//:gtest_main", + ], +) diff --git a/exporters/jaeger/include/opentelemetry/exporters/jaeger/jaeger_exporter.h b/exporters/jaeger/include/opentelemetry/exporters/jaeger/jaeger_exporter.h index da2ed33719..91d8090fbb 100644 --- a/exporters/jaeger/include/opentelemetry/exporters/jaeger/jaeger_exporter.h +++ b/exporters/jaeger/include/opentelemetry/exporters/jaeger/jaeger_exporter.h @@ -78,6 +78,14 @@ class JaegerExporter final : public opentelemetry::sdk::trace::SpanExporter bool is_shutdown_ = false; JaegerExporterOptions options_; std::unique_ptr sender_; + // For testing + friend class JaegerExporterTestPeer; + /** + * Create an JaegerExporter using the specified thrift sender. + * Only tests can call this constructor directly. + * @param sender the thrift sender to be used for exporting + */ + JaegerExporter(std::unique_ptr sender); }; } // namespace jaeger diff --git a/exporters/jaeger/src/jaeger_exporter.cc b/exporters/jaeger/src/jaeger_exporter.cc index 6d1291130c..08f70878c9 100644 --- a/exporters/jaeger/src/jaeger_exporter.cc +++ b/exporters/jaeger/src/jaeger_exporter.cc @@ -27,6 +27,10 @@ JaegerExporter::JaegerExporter(const JaegerExporterOptions &options) : options_( JaegerExporter::JaegerExporter() : JaegerExporter(JaegerExporterOptions()) {} +JaegerExporter::JaegerExporter(std::unique_ptr sender) + : options_(JaegerExporterOptions()), sender_(std::move(sender)) +{} + std::unique_ptr JaegerExporter::MakeRecordable() noexcept { return std::unique_ptr(new JaegerRecordable); diff --git a/exporters/jaeger/src/thrift_sender.h b/exporters/jaeger/src/thrift_sender.h index 40fcf5f09a..0ec8d47f11 100644 --- a/exporters/jaeger/src/thrift_sender.h +++ b/exporters/jaeger/src/thrift_sender.h @@ -67,6 +67,10 @@ class ThriftSender : public Sender uint32_t byte_buffer_size_ = 0; uint32_t process_bytes_size_ = 0; uint32_t max_span_bytes_ = 0; + friend class MockThriftSender; + +protected: + ThriftSender() = default; }; } // namespace jaeger diff --git a/exporters/jaeger/test/jaeger_exporter_test.cc b/exporters/jaeger/test/jaeger_exporter_test.cc new file mode 100644 index 0000000000..713f066371 --- /dev/null +++ b/exporters/jaeger/test/jaeger_exporter_test.cc @@ -0,0 +1,116 @@ +// Copyright The OpenTelemetry Authors +// SPDX-License-Identifier: Apache-2.0 + +#include +#include +#include + +#ifdef BAZEL_BUILD +# include "exporters/jaeger/src/thrift_sender.h" +#else +# include "src/thrift_sender.h" +#endif + +#include +#include "gmock/gmock.h" + +namespace trace = opentelemetry::trace; +namespace nostd = opentelemetry::nostd; +namespace sdktrace = opentelemetry::sdk::trace; +namespace common = opentelemetry::common; +namespace sdk_common = opentelemetry::sdk::common; + +using namespace jaegertracing; +using namespace opentelemetry::exporter::jaeger; +using namespace opentelemetry::sdk::instrumentationlibrary; +using std::vector; +using namespace testing; + +OPENTELEMETRY_BEGIN_NAMESPACE +namespace exporter +{ +namespace jaeger +{ + +class JaegerExporterTestPeer : public ::testing::Test +{ +public: + std::unique_ptr GetExporter(std::unique_ptr sender) + { + return std::unique_ptr(new JaegerExporter(std::move(sender))); + } + + // Get the options associated with the given exporter. + const JaegerExporterOptions &GetOptions(std::unique_ptr &exporter) + { + return exporter->options_; + } +}; + +class MockThriftSender : public ThriftSender +{ +public: + MOCK_METHOD(int, Append, (std::unique_ptr &&), (noexcept, override)); +}; + +TEST_F(JaegerExporterTestPeer, ShutdownTest) +{ + auto mock_thrift_ender = new MockThriftSender; + auto exporter = GetExporter(std::unique_ptr{mock_thrift_ender}); + + auto recordable_1 = exporter->MakeRecordable(); + recordable_1->SetName("Test span 1"); + auto recordable_2 = exporter->MakeRecordable(); + recordable_2->SetName("Test span 2"); + + // exporter shuold not be shutdown by default + nostd::span> batch_1(&recordable_1, 1); + EXPECT_CALL(*mock_thrift_ender, Append(_)).Times(Exactly(1)).WillOnce(Return(1)); + auto result = exporter->Export(batch_1); + EXPECT_EQ(sdk_common::ExportResult::kSuccess, result); + + exporter->Shutdown(); + + nostd::span> batch_2(&recordable_2, 1); + result = exporter->Export(batch_2); + EXPECT_EQ(sdk_common::ExportResult::kFailure, result); +} + +// Call Export() directly +TEST_F(JaegerExporterTestPeer, ExportTest) +{ + auto mock_thrift_ender = new MockThriftSender; + auto exporter = GetExporter(std::unique_ptr{mock_thrift_ender}); + + auto recordable_1 = exporter->MakeRecordable(); + recordable_1->SetName("Test span 1"); + auto recordable_2 = exporter->MakeRecordable(); + recordable_2->SetName("Test span 2"); + + // Test successful send + nostd::span> batch_1(&recordable_1, 1); + EXPECT_CALL(*mock_thrift_ender, Append(_)).Times(Exactly(1)).WillOnce(Return(1)); + auto result = exporter->Export(batch_1); + EXPECT_EQ(sdk_common::ExportResult::kSuccess, result); + + // Test failed send + nostd::span> batch_2(&recordable_2, 1); + EXPECT_CALL(*mock_thrift_ender, Append(_)).Times(Exactly(1)).WillOnce(Return(0)); + result = exporter->Export(batch_2); + EXPECT_EQ(sdk::common::ExportResult::kFailure, result); +} + +// Test exporter configuration options +TEST_F(JaegerExporterTestPeer, ConfigTest) +{ + JaegerExporterOptions opts; + opts.endpoint = "localhost"; + opts.server_port = 6851; + std::unique_ptr exporter(new JaegerExporter(opts)); + EXPECT_EQ(GetOptions(exporter).endpoint, "localhost"); + EXPECT_EQ(GetOptions(exporter).server_port, 6851); +} + +} // namespace jaeger +} // namespace exporter +OPENTELEMETRY_END_NAMESPACE From 15c08393cd76b1bb0beedfac0ec22f47589a3975 Mon Sep 17 00:00:00 2001 From: esigo Date: Sun, 19 Dec 2021 00:58:05 +0100 Subject: [PATCH 2/6] cmake --- exporters/jaeger/CMakeLists.txt | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/exporters/jaeger/CMakeLists.txt b/exporters/jaeger/CMakeLists.txt index 0ae9162c61..e52cc2bd4c 100644 --- a/exporters/jaeger/CMakeLists.txt +++ b/exporters/jaeger/CMakeLists.txt @@ -67,4 +67,14 @@ if(BUILD_TESTING) TARGET jaeger_recordable_test TEST_PREFIX exporter. TEST_LIST jaeger_recordable_test) + + add_executable(jaeger_exporter_test test/jaeger_exporter_test.cc) + target_link_libraries( + jaeger_exporter_test ${GTEST_BOTH_LIBRARIES} ${CMAKE_THREAD_LIBS_INIT} + opentelemetry_exporter_jaeger_trace) + + gtest_add_tests( + TARGET jaeger_exporter_test + TEST_PREFIX exporter. + TEST_LIST jaeger_exporter_test) endif() # BUILD_TESTING From a00a21fe00c332749070116603e0b7849eeeb7ba Mon Sep 17 00:00:00 2001 From: Oblivion Date: Sun, 19 Dec 2021 01:05:15 +0000 Subject: [PATCH 3/6] cmake CI --- exporters/jaeger/CMakeLists.txt | 14 +++++++++++++- exporters/jaeger/test/jaeger_exporter_test.cc | 2 +- 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/exporters/jaeger/CMakeLists.txt b/exporters/jaeger/CMakeLists.txt index e52cc2bd4c..fc87cc90d0 100644 --- a/exporters/jaeger/CMakeLists.txt +++ b/exporters/jaeger/CMakeLists.txt @@ -69,9 +69,21 @@ if(BUILD_TESTING) TEST_LIST jaeger_recordable_test) add_executable(jaeger_exporter_test test/jaeger_exporter_test.cc) + add_definitions(-DGTEST_LINKED_AS_SHARED_LIBRARY=1) + if(GMOCK_LIB) + unset(GMOCK_LIB CACHE) + endif() + if(MSVC AND CMAKE_BUILD_TYPE STREQUAL "Debug") + find_library(GMOCK_LIB gmockd PATH_SUFFIXES lib) + else() + find_library(GMOCK_LIB gmock PATH_SUFFIXES lib) + endif() target_link_libraries( jaeger_exporter_test ${GTEST_BOTH_LIBRARIES} ${CMAKE_THREAD_LIBS_INIT} - opentelemetry_exporter_jaeger_trace) + ${GMOCK_LIB} opentelemetry_exporter_jaeger_trace) + + target_include_directories(jaeger_exporter_test + PRIVATE ${CMAKE_CURRENT_LIST_DIR}/src) gtest_add_tests( TARGET jaeger_exporter_test diff --git a/exporters/jaeger/test/jaeger_exporter_test.cc b/exporters/jaeger/test/jaeger_exporter_test.cc index 713f066371..b485e13283 100644 --- a/exporters/jaeger/test/jaeger_exporter_test.cc +++ b/exporters/jaeger/test/jaeger_exporter_test.cc @@ -8,7 +8,7 @@ #ifdef BAZEL_BUILD # include "exporters/jaeger/src/thrift_sender.h" #else -# include "src/thrift_sender.h" +# include "thrift_sender.h" #endif #include From 8172d877ad28dec863f2ca61b6c08033c04695db Mon Sep 17 00:00:00 2001 From: Oblivion Date: Mon, 20 Dec 2021 19:52:10 +0000 Subject: [PATCH 4/6] comments --- exporters/jaeger/CMakeLists.txt | 6 ++++-- exporters/jaeger/test/jaeger_exporter_test.cc | 4 ---- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/exporters/jaeger/CMakeLists.txt b/exporters/jaeger/CMakeLists.txt index fc87cc90d0..5ae830aa60 100644 --- a/exporters/jaeger/CMakeLists.txt +++ b/exporters/jaeger/CMakeLists.txt @@ -70,8 +70,10 @@ if(BUILD_TESTING) add_executable(jaeger_exporter_test test/jaeger_exporter_test.cc) add_definitions(-DGTEST_LINKED_AS_SHARED_LIBRARY=1) - if(GMOCK_LIB) - unset(GMOCK_LIB CACHE) + if(MSVC) + if(GMOCK_LIB) + unset(GMOCK_LIB CACHE) + endif() endif() if(MSVC AND CMAKE_BUILD_TYPE STREQUAL "Debug") find_library(GMOCK_LIB gmockd PATH_SUFFIXES lib) diff --git a/exporters/jaeger/test/jaeger_exporter_test.cc b/exporters/jaeger/test/jaeger_exporter_test.cc index b485e13283..b91b2312cd 100644 --- a/exporters/jaeger/test/jaeger_exporter_test.cc +++ b/exporters/jaeger/test/jaeger_exporter_test.cc @@ -20,10 +20,6 @@ namespace sdktrace = opentelemetry::sdk::trace; namespace common = opentelemetry::common; namespace sdk_common = opentelemetry::sdk::common; -using namespace jaegertracing; -using namespace opentelemetry::exporter::jaeger; -using namespace opentelemetry::sdk::instrumentationlibrary; -using std::vector; using namespace testing; OPENTELEMETRY_BEGIN_NAMESPACE From ecb104ff21f07d83d6b08848e0732707313af1c0 Mon Sep 17 00:00:00 2001 From: Ehsan Saei <71217171+esigo@users.noreply.github.com> Date: Tue, 21 Dec 2021 00:35:10 +0100 Subject: [PATCH 5/6] typo --- exporters/jaeger/test/jaeger_exporter_test.cc | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/exporters/jaeger/test/jaeger_exporter_test.cc b/exporters/jaeger/test/jaeger_exporter_test.cc index b91b2312cd..4084ca789e 100644 --- a/exporters/jaeger/test/jaeger_exporter_test.cc +++ b/exporters/jaeger/test/jaeger_exporter_test.cc @@ -51,8 +51,8 @@ class MockThriftSender : public ThriftSender TEST_F(JaegerExporterTestPeer, ShutdownTest) { - auto mock_thrift_ender = new MockThriftSender; - auto exporter = GetExporter(std::unique_ptr{mock_thrift_ender}); + auto mock_thrift_sender = new MockThriftSender; + auto exporter = GetExporter(std::unique_ptr{mock_thrift_sender}); auto recordable_1 = exporter->MakeRecordable(); recordable_1->SetName("Test span 1"); @@ -61,7 +61,7 @@ TEST_F(JaegerExporterTestPeer, ShutdownTest) // exporter shuold not be shutdown by default nostd::span> batch_1(&recordable_1, 1); - EXPECT_CALL(*mock_thrift_ender, Append(_)).Times(Exactly(1)).WillOnce(Return(1)); + EXPECT_CALL(*mock_thrift_sender, Append(_)).Times(Exactly(1)).WillOnce(Return(1)); auto result = exporter->Export(batch_1); EXPECT_EQ(sdk_common::ExportResult::kSuccess, result); @@ -75,8 +75,8 @@ TEST_F(JaegerExporterTestPeer, ShutdownTest) // Call Export() directly TEST_F(JaegerExporterTestPeer, ExportTest) { - auto mock_thrift_ender = new MockThriftSender; - auto exporter = GetExporter(std::unique_ptr{mock_thrift_ender}); + auto mock_thrift_sender = new MockThriftSender; + auto exporter = GetExporter(std::unique_ptr{mock_thrift_sender}); auto recordable_1 = exporter->MakeRecordable(); recordable_1->SetName("Test span 1"); @@ -85,13 +85,13 @@ TEST_F(JaegerExporterTestPeer, ExportTest) // Test successful send nostd::span> batch_1(&recordable_1, 1); - EXPECT_CALL(*mock_thrift_ender, Append(_)).Times(Exactly(1)).WillOnce(Return(1)); + EXPECT_CALL(*mock_thrift_sender, Append(_)).Times(Exactly(1)).WillOnce(Return(1)); auto result = exporter->Export(batch_1); EXPECT_EQ(sdk_common::ExportResult::kSuccess, result); // Test failed send nostd::span> batch_2(&recordable_2, 1); - EXPECT_CALL(*mock_thrift_ender, Append(_)).Times(Exactly(1)).WillOnce(Return(0)); + EXPECT_CALL(*mock_thrift_sender, Append(_)).Times(Exactly(1)).WillOnce(Return(0)); result = exporter->Export(batch_2); EXPECT_EQ(sdk::common::ExportResult::kFailure, result); } From 49443627fbc5d7c9b3d53f7724c6593b35f4fd65 Mon Sep 17 00:00:00 2001 From: esigo Date: Tue, 21 Dec 2021 00:51:06 +0100 Subject: [PATCH 6/6] format --- exporters/jaeger/test/jaeger_exporter_test.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/exporters/jaeger/test/jaeger_exporter_test.cc b/exporters/jaeger/test/jaeger_exporter_test.cc index 4084ca789e..ab680e35fc 100644 --- a/exporters/jaeger/test/jaeger_exporter_test.cc +++ b/exporters/jaeger/test/jaeger_exporter_test.cc @@ -52,7 +52,7 @@ class MockThriftSender : public ThriftSender TEST_F(JaegerExporterTestPeer, ShutdownTest) { auto mock_thrift_sender = new MockThriftSender; - auto exporter = GetExporter(std::unique_ptr{mock_thrift_sender}); + auto exporter = GetExporter(std::unique_ptr{mock_thrift_sender}); auto recordable_1 = exporter->MakeRecordable(); recordable_1->SetName("Test span 1"); @@ -76,7 +76,7 @@ TEST_F(JaegerExporterTestPeer, ShutdownTest) TEST_F(JaegerExporterTestPeer, ExportTest) { auto mock_thrift_sender = new MockThriftSender; - auto exporter = GetExporter(std::unique_ptr{mock_thrift_sender}); + auto exporter = GetExporter(std::unique_ptr{mock_thrift_sender}); auto recordable_1 = exporter->MakeRecordable(); recordable_1->SetName("Test span 1");