Merge branch 'main' into fix_exporter_option_1845

open-telemetry · Nov 11, 2023 · 97a309c · 97a309c
2 parents 35f92d5 + 6322607
commit 97a309c
Show file tree

Hide file tree

Showing 17 changed files with 75 additions and 51 deletions.
diff --git 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)
 * [EXPORTER] Rework OTLP/HTTP and OTLP/GRPC exporter options
   [#2388](https://github.com/open-telemetry/opentelemetry-cpp/pull/2388)
 

diff --git 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
@@ -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
@@ -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
@@ -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<OtlpHttpClient *, std::shared_ptr<http_client::HttpClient>>
   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
@@ -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 <google/protobuf/message_lite.h>
@@ -103,7 +104,7 @@ class OtlpHttpLogRecordExporterTestPeer : public ::testing::Test
   static std::pair<OtlpHttpClient *, std::shared_ptr<http_client::HttpClient>>
   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
@@ -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 <google/protobuf/message_lite.h>
@@ -109,7 +110,7 @@ class OtlpHttpMetricExporterTestPeer : public ::testing::Test
   static std::pair<OtlpHttpClient *, std::shared_ptr<http_client::HttpClient>>
   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
@@ -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<Request> 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<std::thread> 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
@@ -17,10 +17,6 @@ class HttpClientFactory
   static std::shared_ptr<HttpClientSync> CreateSync();
 
   static std::shared_ptr<HttpClient> Create();
-
-#ifdef ENABLE_TEST
-  static std::shared_ptr<HttpClient> CreateNoSend();
-#endif
 };
 }  // namespace client
 }  // namespace http

diff --git a/sdk/src/logs/logger_provider.cc b/sdk/src/logs/logger_provider.cc
@@ -57,6 +57,12 @@ nostd::shared_ptr<opentelemetry::logs::Logger> 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<std::mutex> lock_guard{lock_};
 
@@ -84,18 +90,9 @@ nostd::shared_ptr<opentelemetry::logs::Logger> LoggerProvider::GetLogger(
   }
   */
 
-  // https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/logs/data-model.md#field-instrumentationscope
-  std::unique_ptr<instrumentationscope::InstrumentationScope> 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<instrumentationscope::InstrumentationScope> lib =
+      instrumentationscope::InstrumentationScope::Create(library_name, library_version, schema_url,
+                                                         attributes);
 
   loggers_.push_back(std::shared_ptr<opentelemetry::sdk::logs::Logger>(
       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
@@ -111,6 +111,20 @@ TEST(LoggerProviderSDK, EventLoggerProviderFactory)
   auto event_logger = elp->CreateEventLogger(logger1, "otel-cpp.test");
 }
 
+TEST(LoggerPviderSDK, LoggerEquityCheck)
+{
+  auto lp = std::shared_ptr<logs_api::LoggerProvider>(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:

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
@@ -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<HttpClient> 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
@@ -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 <map>
-#  include <string>
-#  include <vector>
+#include <map>
+#include <string>
+#include <vector>
 
-#  include <gtest/gtest.h>
-#  include "gmock/gmock.h"
+#include <gtest/gtest.h>
+#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
@@ -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
@@ -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
@@ -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/...ient/nosend/http_client_factory_nosend.cc → ...client/nosend/http_client_test_factory.cc b/...ient/nosend/http_client_factory_nosend.cc → ...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::HttpClient> http_client::HttpClientFactory::CreateNoSend()
+std::shared_ptr<http_client::HttpClient> http_client::HttpClientTestFactory::Create()
 {
   return std::make_shared<http_client::nosend::HttpClient>();
 }
-#endif