From 2707a4c70c2e26d195921bd7c99f835a49205304 Mon Sep 17 00:00:00 2001 From: owent Date: Fri, 13 Dec 2024 17:30:22 +0800 Subject: [PATCH 1/4] Add feature checking and fix C++20 text format usage Using to check features --- api/include/opentelemetry/detail/preprocessor.h | 12 ++++++++++++ exporters/elasticsearch/src/es_log_recordable.cc | 8 +++++++- exporters/otlp/CMakeLists.txt | 6 ++++++ exporters/zipkin/CMakeLists.txt | 6 ++++++ test_common/src/http/client/nosend/CMakeLists.txt | 6 ++++++ 5 files changed, 37 insertions(+), 1 deletion(-) diff --git a/api/include/opentelemetry/detail/preprocessor.h b/api/include/opentelemetry/detail/preprocessor.h index dc8eb57824..acdf02eb08 100644 --- a/api/include/opentelemetry/detail/preprocessor.h +++ b/api/include/opentelemetry/detail/preprocessor.h @@ -11,3 +11,15 @@ #define OPENTELEMETRY_CONCAT(A, B) OPENTELEMETRY_CONCAT_(A, B) #define OPENTELEMETRY_CONCAT_(A, B) A##B + +// Import the C++20 feature-test macros +#ifdef __has_include +# if __has_include() +# include +# endif +#elif defined(_MSC_VER) && ((defined(__cplusplus) && __cplusplus >= 202002L) || \ + (defined(_MSVC_LANG) && _MSVC_LANG >= 202002L)) +# if _MSC_VER >= 1922 +# include +# endif +#endif diff --git a/exporters/elasticsearch/src/es_log_recordable.cc b/exporters/elasticsearch/src/es_log_recordable.cc index 0e7c3b0344..de2c74efa8 100644 --- a/exporters/elasticsearch/src/es_log_recordable.cc +++ b/exporters/elasticsearch/src/es_log_recordable.cc @@ -1,11 +1,17 @@ // Copyright The OpenTelemetry Authors // SPDX-License-Identifier: Apache-2.0 +#include + #include #include #include #include +#if defined(__cpp_lib_format) +# include +#endif + #include "opentelemetry/exporters/elasticsearch/es_log_recordable.h" #include "opentelemetry/logs/severity.h" #include "opentelemetry/nostd/variant.h" @@ -71,7 +77,7 @@ void ElasticSearchRecordable::SetTimestamp( // If built with with at least cpp 20 then use std::format // Otherwise use the old style to format the timestamp in UTC -#if __cplusplus >= 202002L +#if defined(__cpp_lib_format) const std::string dateStr = std::format("{:%FT%T%Ez}", timePoint); #else std::time_t time = std::chrono::system_clock::to_time_t(timePoint); diff --git a/exporters/otlp/CMakeLists.txt b/exporters/otlp/CMakeLists.txt index 34da75b563..dd032665d8 100644 --- a/exporters/otlp/CMakeLists.txt +++ b/exporters/otlp/CMakeLists.txt @@ -334,6 +334,12 @@ if(BUILD_TESTING) else() find_library(GMOCK_LIB gmock PATH_SUFFIXES lib) endif() + # Reset GMOCK_LIB if it was not found, or some target may link + # GMOCK_LIB-NOTFOUND + if(NOT GMOCK_LIB) + unset(GMOCK_LIB) + unset(GMOCK_LIB CACHE) + endif() endif() if(WITH_OTLP_GRPC) diff --git a/exporters/zipkin/CMakeLists.txt b/exporters/zipkin/CMakeLists.txt index 60f8a80a2a..99e42c4677 100644 --- a/exporters/zipkin/CMakeLists.txt +++ b/exporters/zipkin/CMakeLists.txt @@ -59,6 +59,12 @@ if(BUILD_TESTING) else() find_library(GMOCK_LIB gmock PATH_SUFFIXES lib) endif() + # Reset GMOCK_LIB if it was not found, or some target may link + # GMOCK_LIB-NOTFOUND + if(NOT GMOCK_LIB) + unset(GMOCK_LIB) + unset(GMOCK_LIB CACHE) + endif() add_executable(zipkin_exporter_test test/zipkin_exporter_test.cc) diff --git a/test_common/src/http/client/nosend/CMakeLists.txt b/test_common/src/http/client/nosend/CMakeLists.txt index 7394053a22..c8ae63240c 100644 --- a/test_common/src/http/client/nosend/CMakeLists.txt +++ b/test_common/src/http/client/nosend/CMakeLists.txt @@ -26,6 +26,12 @@ if(${BUILD_TESTING}) else() find_library(GMOCK_LIB gmock PATH_SUFFIXES lib) endif() + # Reset GMOCK_LIB if it was not found, or some target may link + # GMOCK_LIB-NOTFOUND + if(NOT GMOCK_LIB) + unset(GMOCK_LIB) + unset(GMOCK_LIB CACHE) + endif() target_link_libraries( opentelemetry_http_client_nosend opentelemetry_ext From c21d23600cde70755951060f031ef66935490794 Mon Sep 17 00:00:00 2001 From: owent Date: Fri, 13 Dec 2024 17:54:08 +0800 Subject: [PATCH 2/4] Fix __cpp_lib_format usage --- exporters/elasticsearch/src/es_log_recordable.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/exporters/elasticsearch/src/es_log_recordable.cc b/exporters/elasticsearch/src/es_log_recordable.cc index de2c74efa8..f60e3fdb6c 100644 --- a/exporters/elasticsearch/src/es_log_recordable.cc +++ b/exporters/elasticsearch/src/es_log_recordable.cc @@ -77,7 +77,7 @@ void ElasticSearchRecordable::SetTimestamp( // If built with with at least cpp 20 then use std::format // Otherwise use the old style to format the timestamp in UTC -#if defined(__cpp_lib_format) +#if defined(__cpp_lib_format) && __cpp_lib_format >= 201907 const std::string dateStr = std::format("{:%FT%T%Ez}", timePoint); #else std::time_t time = std::chrono::system_clock::to_time_t(timePoint); From d254ad074a77f9dfd0ae0a67456cb1511749978a Mon Sep 17 00:00:00 2001 From: owent Date: Fri, 13 Dec 2024 19:16:27 +0800 Subject: [PATCH 3/4] Use shared GMOCK_LIB settings. --- CMakeLists.txt | 31 +++++++++++++++++++ exporters/otlp/CMakeLists.txt | 29 ----------------- exporters/zipkin/CMakeLists.txt | 17 ---------- .../src/http/client/nosend/CMakeLists.txt | 25 --------------- 4 files changed, 31 insertions(+), 71 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index bf093f2caf..408ee43046 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -645,6 +645,37 @@ if(BUILD_TESTING) endif() message(STATUS "GTEST_INCLUDE_DIRS = ${GTEST_INCLUDE_DIRS}") message(STATUS "GTEST_BOTH_LIBRARIES = ${GTEST_BOTH_LIBRARIES}") + + # Try to find gmock + if(NOT GMOCK_LIB AND TARGET GTest::gmock) + set(GMOCK_LIB GTest::gmock) + elseif(MSVC) + # Explicitly specify that we consume GTest from shared library. The rest of + # code logic below determines whether we link Release or Debug flavor of the + # library. These flavors have different prefix on Windows, gmock and gmockd + # respectively. + add_definitions(-DGTEST_LINKED_AS_SHARED_LIBRARY=1) + if(GMOCK_LIB) + # unset GMOCK_LIB to force find_library to redo the lookup, as the cached + # entry could cause linking to incorrect flavor of gmock and leading to + # runtime error. + unset(GMOCK_LIB CACHE) + endif() + endif() + if(NOT GMOCK_LIB) + 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() + # Reset GMOCK_LIB if it was not found, or some target may link + # GMOCK_LIB-NOTFOUND + if(NOT GMOCK_LIB) + unset(GMOCK_LIB) + unset(GMOCK_LIB CACHE) + endif() + endif() + enable_testing() if(WITH_BENCHMARK) # Benchmark respects the CMAKE_PREFIX_PATH diff --git a/exporters/otlp/CMakeLists.txt b/exporters/otlp/CMakeLists.txt index dd032665d8..cd4e1b62fa 100644 --- a/exporters/otlp/CMakeLists.txt +++ b/exporters/otlp/CMakeLists.txt @@ -313,35 +313,6 @@ if(BUILD_TESTING) TEST_PREFIX exporter.otlp. TEST_LIST otlp_metrics_serialization_test) - if(NOT GMOCK_LIB AND TARGET GTest::gmock) - set(GMOCK_LIB GTest::gmock) - elseif(MSVC) - # Explicitly specify that we consume GTest from shared library. The rest of - # code logic below determines whether we link Release or Debug flavor of the - # library. These flavors have different prefix on Windows, gmock and gmockd - # respectively. - add_definitions(-DGTEST_LINKED_AS_SHARED_LIBRARY=1) - if(GMOCK_LIB) - # unset GMOCK_LIB to force find_library to redo the lookup, as the cached - # entry could cause linking to incorrect flavor of gmock and leading to - # runtime error. - unset(GMOCK_LIB CACHE) - endif() - endif() - if(NOT GMOCK_LIB) - 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() - # Reset GMOCK_LIB if it was not found, or some target may link - # GMOCK_LIB-NOTFOUND - if(NOT GMOCK_LIB) - unset(GMOCK_LIB) - unset(GMOCK_LIB CACHE) - endif() - endif() - if(WITH_OTLP_GRPC) add_executable(otlp_grpc_exporter_test test/otlp_grpc_exporter_test.cc) target_link_libraries( diff --git a/exporters/zipkin/CMakeLists.txt b/exporters/zipkin/CMakeLists.txt index 99e42c4677..5944258ca3 100644 --- a/exporters/zipkin/CMakeLists.txt +++ b/exporters/zipkin/CMakeLists.txt @@ -49,23 +49,6 @@ if(BUILD_TESTING) TEST_PREFIX exporter. TEST_LIST zipkin_recordable_test) - 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) - else() - find_library(GMOCK_LIB gmock PATH_SUFFIXES lib) - endif() - # Reset GMOCK_LIB if it was not found, or some target may link - # GMOCK_LIB-NOTFOUND - if(NOT GMOCK_LIB) - unset(GMOCK_LIB) - unset(GMOCK_LIB CACHE) - endif() - add_executable(zipkin_exporter_test test/zipkin_exporter_test.cc) target_link_libraries( diff --git a/test_common/src/http/client/nosend/CMakeLists.txt b/test_common/src/http/client/nosend/CMakeLists.txt index c8ae63240c..3f2b4c82bd 100644 --- a/test_common/src/http/client/nosend/CMakeLists.txt +++ b/test_common/src/http/client/nosend/CMakeLists.txt @@ -8,31 +8,6 @@ if(${BUILD_TESTING}) set_target_properties(opentelemetry_http_client_nosend PROPERTIES EXPORT_NAME opentelemetry_http_client_nosend) - if(MSVC) - # Explicitly specify that we consume GTest from shared library. The rest of - # code logic below determines whether we link Release or Debug flavor of the - # library. These flavors have different prefix on Windows, gmock and gmockd - # respectively. - add_definitions(-DGTEST_LINKED_AS_SHARED_LIBRARY=1) - if(GMOCK_LIB) - # unset GMOCK_LIB to force find_library to redo the lookup, as the cached - # entry could cause linking to incorrect flavor of gmock and leading to - # runtime error. - unset(GMOCK_LIB CACHE) - endif() - 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() - # Reset GMOCK_LIB if it was not found, or some target may link - # GMOCK_LIB-NOTFOUND - if(NOT GMOCK_LIB) - unset(GMOCK_LIB) - unset(GMOCK_LIB CACHE) - endif() - target_link_libraries( opentelemetry_http_client_nosend opentelemetry_ext opentelemetry_test_common ${GMOCK_LIB} ${GTEST_BOTH_LIBRARIES}) From 6ac772700c599847fffe259c4eb1b008866f773f Mon Sep 17 00:00:00 2001 From: owent Date: Fri, 13 Dec 2024 20:01:57 +0800 Subject: [PATCH 4/4] Add comment about `__cpp_lib_format` --- exporters/elasticsearch/src/es_log_recordable.cc | 1 + 1 file changed, 1 insertion(+) diff --git a/exporters/elasticsearch/src/es_log_recordable.cc b/exporters/elasticsearch/src/es_log_recordable.cc index f60e3fdb6c..5c5f101282 100644 --- a/exporters/elasticsearch/src/es_log_recordable.cc +++ b/exporters/elasticsearch/src/es_log_recordable.cc @@ -77,6 +77,7 @@ void ElasticSearchRecordable::SetTimestamp( // If built with with at least cpp 20 then use std::format // Otherwise use the old style to format the timestamp in UTC + // @see https://en.cppreference.com/w/cpp/feature_test#cpp_lib_format #if defined(__cpp_lib_format) && __cpp_lib_format >= 201907 const std::string dateStr = std::format("{:%FT%T%Ez}", timePoint); #else