From 86ee88615b813ea08901db029343847e77d0f38b Mon Sep 17 00:00:00 2001 From: Tom Tan Date: Fri, 10 Nov 2023 01:53:12 -0800 Subject: [PATCH 1/2] [SDK] Fix GetLogger with empty library name (#2398) --- CHANGELOG.md | 2 ++ sdk/src/logs/logger_provider.cc | 21 +++++++++------------ sdk/test/logs/logger_provider_sdk_test.cc | 14 ++++++++++++++ 3 files changed, 25 insertions(+), 12 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9acee42235..7317b2b5f1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -25,6 +25,8 @@ Increment the: [#2385](https://github.com/open-telemetry/opentelemetry-cpp/pull/2385) * [API] Add a new AddLink() operation to Span [#2380](https://github.com/open-telemetry/opentelemetry-cpp/pull/2380) +* [SDK] Fix GetLogger with empty library + name[#2398](https://github.com/open-telemetry/opentelemetry-cpp/pull/2398) Important changes: diff --git a/sdk/src/logs/logger_provider.cc b/sdk/src/logs/logger_provider.cc index d64863ed73..46eaa01054 100644 --- a/sdk/src/logs/logger_provider.cc +++ b/sdk/src/logs/logger_provider.cc @@ -57,6 +57,12 @@ nostd::shared_ptr LoggerProvider::GetLogger( nostd::string_view schema_url, const opentelemetry::common::KeyValueIterable &attributes) noexcept { + // https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/logs/data-model.md#field-instrumentationscope + if (library_name.empty()) + { + library_name = logger_name; + } + // Ensure only one thread can read/write from the map of loggers std::lock_guard lock_guard{lock_}; @@ -84,18 +90,9 @@ nostd::shared_ptr LoggerProvider::GetLogger( } */ - // https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/logs/data-model.md#field-instrumentationscope - std::unique_ptr lib; - if (library_name.empty()) - { - lib = instrumentationscope::InstrumentationScope::Create(logger_name, library_version, - schema_url, attributes); - } - else - { - lib = instrumentationscope::InstrumentationScope::Create(library_name, library_version, - schema_url, attributes); - } + std::unique_ptr lib = + instrumentationscope::InstrumentationScope::Create(library_name, library_version, schema_url, + attributes); loggers_.push_back(std::shared_ptr( new Logger(logger_name, context_, std::move(lib)))); diff --git a/sdk/test/logs/logger_provider_sdk_test.cc b/sdk/test/logs/logger_provider_sdk_test.cc index 51f40178ba..ecb9ea6f9c 100644 --- a/sdk/test/logs/logger_provider_sdk_test.cc +++ b/sdk/test/logs/logger_provider_sdk_test.cc @@ -111,6 +111,20 @@ TEST(LoggerProviderSDK, EventLoggerProviderFactory) auto event_logger = elp->CreateEventLogger(logger1, "otel-cpp.test"); } +TEST(LoggerPviderSDK, LoggerEquityCheck) +{ + auto lp = std::shared_ptr(new LoggerProvider()); + nostd::string_view schema_url{"https://opentelemetry.io/schemas/1.11.0"}; + + auto logger1 = lp->GetLogger("logger1", "opentelelemtry_library", "", schema_url); + auto logger2 = lp->GetLogger("logger1", "opentelelemtry_library", "", schema_url); + EXPECT_EQ(logger1, logger2); + + auto logger3 = lp->GetLogger("logger3"); + auto another_logger3 = lp->GetLogger("logger3"); + EXPECT_EQ(logger3, another_logger3); +} + class DummyLogRecordable final : public opentelemetry::sdk::logs::Recordable { public: From 63226075207c2807f61ae670355c1e705a4ce1d1 Mon Sep 17 00:00:00 2001 From: WenTao Ou Date: Sat, 11 Nov 2023 18:20:51 +0800 Subject: [PATCH 2/2] [TEST] Fix compiling problem and removed -DENABLE_TEST (#2401) --- CMakeLists.txt | 1 - ci/do_ci.ps1 | 2 +- ci/do_ci.sh | 2 +- .../otlp/test/otlp_http_exporter_test.cc | 3 ++- .../otlp_http_log_record_exporter_test.cc | 3 ++- .../test/otlp_http_metric_exporter_test.cc | 3 ++- .../ext/http/client/curl/http_client_curl.h | 5 +--- .../ext/http/client/http_client_factory.h | 4 --- .../http/client/http_client_test_factory.h | 22 ++++++++++++++++ .../http/client/nosend/http_client_nosend.h | 26 +++++++++---------- test_common/src/http/client/nosend/BUILD | 2 +- .../src/http/client/nosend/CMakeLists.txt | 2 +- .../http/client/nosend/http_client_nosend.cc | 4 +-- ..._nosend.cc => http_client_test_factory.cc} | 10 +++---- 14 files changed, 50 insertions(+), 39 deletions(-) create mode 100644 test_common/include/opentelemetry/test_common/ext/http/client/http_client_test_factory.h rename test_common/src/http/client/nosend/{http_client_factory_nosend.cc => http_client_test_factory.cc} (51%) diff --git a/CMakeLists.txt b/CMakeLists.txt index b72e58a963..de5e714916 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -579,7 +579,6 @@ list(APPEND CMAKE_PREFIX_PATH "${CMAKE_BINARY_DIR}") include(CTest) if(BUILD_TESTING) - add_definitions(-DENABLE_TEST) if(EXISTS ${CMAKE_BINARY_DIR}/lib/libgtest.a) # Prefer GTest from build tree. GTest is not always working with # CMAKE_PREFIX_PATH diff --git a/ci/do_ci.ps1 b/ci/do_ci.ps1 index b97617e96f..0b6cd9513e 100644 --- a/ci/do_ci.ps1 +++ b/ci/do_ci.ps1 @@ -27,7 +27,7 @@ $VCPKG_DIR = Join-Path "$SRC_DIR" "tools" "vcpkg" switch ($action) { "bazel.build" { - bazel build --copt=-DENABLE_TEST $BAZEL_OPTIONS --action_env=VCPKG_DIR=$VCPKG_DIR --deleted_packages=opentracing-shim -- //... + bazel build $BAZEL_OPTIONS --action_env=VCPKG_DIR=$VCPKG_DIR --deleted_packages=opentracing-shim -- //... $exit = $LASTEXITCODE if ($exit -ne 0) { exit $exit diff --git a/ci/do_ci.sh b/ci/do_ci.sh index d9dc0c178f..f679c6af04 100755 --- a/ci/do_ci.sh +++ b/ci/do_ci.sh @@ -69,7 +69,7 @@ fi echo "make command: ${MAKE_COMMAND}" echo "IWYU option: ${IWYU}" -BAZEL_OPTIONS_DEFAULT="--copt=-DENABLE_TEST --copt=-DENABLE_METRICS_EXEMPLAR_PREVIEW" +BAZEL_OPTIONS_DEFAULT="--copt=-DENABLE_METRICS_EXEMPLAR_PREVIEW" BAZEL_OPTIONS="--cxxopt=-std=c++14 $BAZEL_OPTIONS_DEFAULT" BAZEL_TEST_OPTIONS="$BAZEL_OPTIONS --test_output=errors" diff --git a/exporters/otlp/test/otlp_http_exporter_test.cc b/exporters/otlp/test/otlp_http_exporter_test.cc index 42258d2b45..1c76ad0d02 100644 --- a/exporters/otlp/test/otlp_http_exporter_test.cc +++ b/exporters/otlp/test/otlp_http_exporter_test.cc @@ -19,6 +19,7 @@ # include "opentelemetry/sdk/trace/batch_span_processor.h" # include "opentelemetry/sdk/trace/batch_span_processor_options.h" # include "opentelemetry/sdk/trace/tracer_provider.h" +# include "opentelemetry/test_common/ext/http/client/http_client_test_factory.h" # include "opentelemetry/test_common/ext/http/client/nosend/http_client_nosend.h" # include "opentelemetry/trace/provider.h" @@ -102,7 +103,7 @@ class OtlpHttpExporterTestPeer : public ::testing::Test static std::pair> GetMockOtlpHttpClient(HttpRequestContentType content_type, bool async_mode = false) { - auto http_client = http_client::HttpClientFactory::CreateNoSend(); + auto http_client = http_client::HttpClientTestFactory::Create(); return {new OtlpHttpClient(MakeOtlpHttpClientOptions(content_type, async_mode), http_client), http_client}; } diff --git a/exporters/otlp/test/otlp_http_log_record_exporter_test.cc b/exporters/otlp/test/otlp_http_log_record_exporter_test.cc index df89ca17fa..44fa812a35 100644 --- a/exporters/otlp/test/otlp_http_log_record_exporter_test.cc +++ b/exporters/otlp/test/otlp_http_log_record_exporter_test.cc @@ -22,6 +22,7 @@ # include "opentelemetry/sdk/logs/exporter.h" # include "opentelemetry/sdk/logs/logger_provider.h" # include "opentelemetry/sdk/resource/resource.h" +# include "opentelemetry/test_common/ext/http/client/http_client_test_factory.h" # include "opentelemetry/test_common/ext/http/client/nosend/http_client_nosend.h" # include @@ -103,7 +104,7 @@ class OtlpHttpLogRecordExporterTestPeer : public ::testing::Test static std::pair> GetMockOtlpHttpClient(HttpRequestContentType content_type, bool async_mode = false) { - auto http_client = http_client::HttpClientFactory::CreateNoSend(); + auto http_client = http_client::HttpClientTestFactory::Create(); return {new OtlpHttpClient(MakeOtlpHttpClientOptions(content_type, async_mode), http_client), http_client}; } diff --git a/exporters/otlp/test/otlp_http_metric_exporter_test.cc b/exporters/otlp/test/otlp_http_metric_exporter_test.cc index ed7c4dba30..8b7688adc0 100644 --- a/exporters/otlp/test/otlp_http_metric_exporter_test.cc +++ b/exporters/otlp/test/otlp_http_metric_exporter_test.cc @@ -24,6 +24,7 @@ #include "opentelemetry/sdk/metrics/export/metric_producer.h" #include "opentelemetry/sdk/metrics/instruments.h" #include "opentelemetry/sdk/resource/resource.h" +#include "opentelemetry/test_common/ext/http/client/http_client_test_factory.h" #include "opentelemetry/test_common/ext/http/client/nosend/http_client_nosend.h" #include @@ -109,7 +110,7 @@ class OtlpHttpMetricExporterTestPeer : public ::testing::Test static std::pair> GetMockOtlpHttpClient(HttpRequestContentType content_type, bool async_mode = false) { - auto http_client = http_client::HttpClientFactory::CreateNoSend(); + auto http_client = http_client::HttpClientTestFactory::Create(); return {new OtlpHttpClient(MakeOtlpHttpClientOptions(content_type, async_mode), http_client), http_client}; } diff --git a/ext/include/opentelemetry/ext/http/client/curl/http_client_curl.h b/ext/include/opentelemetry/ext/http/client/curl/http_client_curl.h index 876d9fab4f..45ee13d55c 100644 --- a/ext/include/opentelemetry/ext/http/client/curl/http_client_curl.h +++ b/ext/include/opentelemetry/ext/http/client/curl/http_client_curl.h @@ -37,6 +37,7 @@ class HttpCurlGlobalInitializer HttpCurlGlobalInitializer(HttpCurlGlobalInitializer &&) = delete; HttpCurlGlobalInitializer &operator=(const HttpCurlGlobalInitializer &) = delete; + HttpCurlGlobalInitializer &operator=(HttpCurlGlobalInitializer &&) = delete; HttpCurlGlobalInitializer(); @@ -190,9 +191,7 @@ class Session : public opentelemetry::ext::http::client::Session, */ const std::string &GetBaseUri() const { return host_; } -#ifdef ENABLE_TEST std::shared_ptr GetRequest() { return http_request_; } -#endif inline HttpClient &GetHttpClient() noexcept { return http_client_; } inline const HttpClient &GetHttpClient() const noexcept { return http_client_; } @@ -327,7 +326,6 @@ class HttpClient : public opentelemetry::ext::http::client::HttpClient void ScheduleAbortSession(uint64_t session_id); void ScheduleRemoveSession(uint64_t session_id, HttpCurlEasyResource &&resource); -#ifdef ENABLE_TEST void WaitBackgroundThreadExit() { std::unique_ptr background_thread; @@ -341,7 +339,6 @@ class HttpClient : public opentelemetry::ext::http::client::HttpClient background_thread->join(); } } -#endif private: void wakeupBackgroundThread(); diff --git a/ext/include/opentelemetry/ext/http/client/http_client_factory.h b/ext/include/opentelemetry/ext/http/client/http_client_factory.h index 8df1e578d2..43e15cf255 100644 --- a/ext/include/opentelemetry/ext/http/client/http_client_factory.h +++ b/ext/include/opentelemetry/ext/http/client/http_client_factory.h @@ -17,10 +17,6 @@ class HttpClientFactory static std::shared_ptr CreateSync(); static std::shared_ptr Create(); - -#ifdef ENABLE_TEST - static std::shared_ptr CreateNoSend(); -#endif }; } // namespace client } // namespace http diff --git a/test_common/include/opentelemetry/test_common/ext/http/client/http_client_test_factory.h b/test_common/include/opentelemetry/test_common/ext/http/client/http_client_test_factory.h new file mode 100644 index 0000000000..51f9502bb8 --- /dev/null +++ b/test_common/include/opentelemetry/test_common/ext/http/client/http_client_test_factory.h @@ -0,0 +1,22 @@ +// Copyright The OpenTelemetry Authors +// SPDX-License-Identifier: Apache-2.0 + +#pragma once +#include "opentelemetry/ext/http/client/http_client.h" + +OPENTELEMETRY_BEGIN_NAMESPACE +namespace ext +{ +namespace http +{ +namespace client +{ +class HttpClientTestFactory +{ +public: + static std::shared_ptr Create(); +}; +} // namespace client +} // namespace http +} // namespace ext +OPENTELEMETRY_END_NAMESPACE diff --git a/test_common/include/opentelemetry/test_common/ext/http/client/nosend/http_client_nosend.h b/test_common/include/opentelemetry/test_common/ext/http/client/nosend/http_client_nosend.h index 7a6fadbd11..7dddde13d4 100644 --- a/test_common/include/opentelemetry/test_common/ext/http/client/nosend/http_client_nosend.h +++ b/test_common/include/opentelemetry/test_common/ext/http/client/nosend/http_client_nosend.h @@ -3,17 +3,16 @@ #pragma once -#ifdef ENABLE_TEST -# include "opentelemetry/ext/http/client/http_client.h" -# include "opentelemetry/ext/http/common/url_parser.h" -# include "opentelemetry/version.h" +#include "opentelemetry/ext/http/client/http_client.h" +#include "opentelemetry/ext/http/common/url_parser.h" +#include "opentelemetry/version.h" -# include -# include -# include +#include +#include +#include -# include -# include "gmock/gmock.h" +#include +#include "gmock/gmock.h" OPENTELEMETRY_BEGIN_NAMESPACE namespace ext @@ -37,12 +36,12 @@ class Request : public opentelemetry::ext::http::client::Request method_ = method; } -# ifdef ENABLE_HTTP_SSL_PREVIEW +#ifdef ENABLE_HTTP_SSL_PREVIEW void SetSslOptions(const HttpSslOptions &ssl_options) noexcept override { ssl_options_ = ssl_options; } -# endif /* ENABLE_HTTP_SSL_PREVIEW */ +#endif /* ENABLE_HTTP_SSL_PREVIEW */ void SetBody(opentelemetry::ext::http::client::Body &body) noexcept override { @@ -66,9 +65,9 @@ class Request : public opentelemetry::ext::http::client::Request public: opentelemetry::ext::http::client::Method method_; -# ifdef ENABLE_HTTP_SSL_PREVIEW +#ifdef ENABLE_HTTP_SSL_PREVIEW opentelemetry::ext::http::client::HttpSslOptions ssl_options_; -# endif /* ENABLE_HTTP_SSL_PREVIEW */ +#endif /* ENABLE_HTTP_SSL_PREVIEW */ opentelemetry::ext::http::client::Body body_; opentelemetry::ext::http::client::Headers headers_; std::string uri_; @@ -186,4 +185,3 @@ class HttpClient : public opentelemetry::ext::http::client::HttpClient } // namespace http } // namespace ext OPENTELEMETRY_END_NAMESPACE -#endif diff --git a/test_common/src/http/client/nosend/BUILD b/test_common/src/http/client/nosend/BUILD index 7aaf2a61b5..fa7ba623ab 100644 --- a/test_common/src/http/client/nosend/BUILD +++ b/test_common/src/http/client/nosend/BUILD @@ -6,8 +6,8 @@ package(default_visibility = ["//visibility:public"]) cc_library( name = "http_client_nosend", srcs = [ - "http_client_factory_nosend.cc", "http_client_nosend.cc", + "http_client_test_factory.cc", ], include_prefix = "src/http/client/nosend", tags = [ diff --git a/test_common/src/http/client/nosend/CMakeLists.txt b/test_common/src/http/client/nosend/CMakeLists.txt index 1f32861b41..92a2c1f98c 100644 --- a/test_common/src/http/client/nosend/CMakeLists.txt +++ b/test_common/src/http/client/nosend/CMakeLists.txt @@ -2,7 +2,7 @@ # SPDX-License-Identifier: Apache-2.0 if(${BUILD_TESTING}) - add_library(opentelemetry_http_client_nosend http_client_factory_nosend.cc + add_library(opentelemetry_http_client_nosend http_client_test_factory.cc http_client_nosend.cc) set_target_properties(opentelemetry_http_client_nosend diff --git a/test_common/src/http/client/nosend/http_client_nosend.cc b/test_common/src/http/client/nosend/http_client_nosend.cc index dd14e4404a..98cae0476a 100644 --- a/test_common/src/http/client/nosend/http_client_nosend.cc +++ b/test_common/src/http/client/nosend/http_client_nosend.cc @@ -1,8 +1,7 @@ // Copyright The OpenTelemetry Authors // SPDX-License-Identifier: Apache-2.0 -#ifdef ENABLE_TEST -# include "opentelemetry/test_common/ext/http/client/nosend/http_client_nosend.h" +#include "opentelemetry/test_common/ext/http/client/nosend/http_client_nosend.h" OPENTELEMETRY_BEGIN_NAMESPACE namespace ext @@ -96,4 +95,3 @@ void HttpClient::CleanupSession(uint64_t /* session_id */) {} } // namespace http } // namespace ext OPENTELEMETRY_END_NAMESPACE -#endif diff --git a/test_common/src/http/client/nosend/http_client_factory_nosend.cc b/test_common/src/http/client/nosend/http_client_test_factory.cc similarity index 51% rename from test_common/src/http/client/nosend/http_client_factory_nosend.cc rename to test_common/src/http/client/nosend/http_client_test_factory.cc index c70d1b9578..215c173549 100644 --- a/test_common/src/http/client/nosend/http_client_factory_nosend.cc +++ b/test_common/src/http/client/nosend/http_client_test_factory.cc @@ -1,15 +1,13 @@ // Copyright The OpenTelemetry Authors // SPDX-License-Identifier: Apache-2.0 -#ifdef ENABLE_TEST -# include "opentelemetry/ext/http/client/http_client.h" -# include "opentelemetry/ext/http/client/http_client_factory.h" -# include "opentelemetry/test_common/ext/http/client/nosend/http_client_nosend.h" +#include "opentelemetry/test_common/ext/http/client/http_client_test_factory.h" +#include "opentelemetry/ext/http/client/http_client.h" +#include "opentelemetry/test_common/ext/http/client/nosend/http_client_nosend.h" namespace http_client = opentelemetry::ext::http::client; -std::shared_ptr http_client::HttpClientFactory::CreateNoSend() +std::shared_ptr http_client::HttpClientTestFactory::Create() { return std::make_shared(); } -#endif