From 50b0a5d017e3151e50f2cc2cd314b60d8878f047 Mon Sep 17 00:00:00 2001 From: Pavel Pautov Date: Tue, 12 Mar 2024 23:43:09 -0700 Subject: [PATCH 1/5] Unify CMake and Nginx build system defaults. Provide generic environment variable to adjust CMake settings from Nginx build system. --- config | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/config b/config index 11d3317..75378e2 100644 --- a/config +++ b/config @@ -1,10 +1,9 @@ ngx_addon_name=ngx_otel_module cmake -D NGX_OTEL_NGINX_BUILD_DIR=$NGX_OBJS \ - -D NGX_OTEL_FETCH_DEPS=OFF \ - -D NGX_OTEL_PROTO_DIR=$NGX_OTEL_PROTO_DIR \ -D CMAKE_LIBRARY_OUTPUT_DIRECTORY=$PWD/$NGX_OBJS \ -D "CMAKE_C_FLAGS=$NGX_CC_OPT" \ -D "CMAKE_CXX_FLAGS=$NGX_CC_OPT" \ -D "CMAKE_MODULE_LINKER_FLAGS=$NGX_LD_OPT" \ + $NGX_OTEL_CMAKE_OPTS \ -S $ngx_addon_dir -B $NGX_OBJS/otel || exit 1 From 93480833d591fcecd7257d5accc3c5fb3ec76175 Mon Sep 17 00:00:00 2001 From: Pavel Pautov Date: Mon, 15 Jul 2024 16:12:19 -0700 Subject: [PATCH 2/5] Support custom versions of auto-fetched build dependencies. --- CMakeLists.txt | 46 ++++++++++++++++++++++++++++++---------------- 1 file changed, 30 insertions(+), 16 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 8588f52..645c5d5 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -6,7 +6,10 @@ set(NGX_OTEL_NGINX_BUILD_DIR "" set(NGX_OTEL_NGINX_DIR "${NGX_OTEL_NGINX_BUILD_DIR}/.." CACHE PATH "Nginx source dir") -set(NGX_OTEL_FETCH_DEPS ON CACHE BOOL "Download dependencies") +set(NGX_OTEL_GRPC e241f37befe7ba4688effd84bfbf99b0f681a2f7 # v1.49.4 + CACHE STRING "gRPC tag to download or 'package' to use preinstalled") +set(NGX_OTEL_SDK 11d5d9e0d8fd8ba876c8994714cc2647479b6574 # v1.11.0 + CACHE STRING "OTel SDK tag to download or 'package' to use preinstalled") set(NGX_OTEL_PROTO_DIR "" CACHE PATH "OTel proto files root") set(NGX_OTEL_DEV OFF CACHE BOOL "Enforce compiler warnings") @@ -16,13 +19,16 @@ endif() set(CMAKE_CXX_VISIBILITY_PRESET hidden) -if(NGX_OTEL_FETCH_DEPS) +if(NGX_OTEL_GRPC STREQUAL "package") + find_package(protobuf REQUIRED) + find_package(gRPC REQUIRED) +else() include(FetchContent) FetchContent_Declare( grpc GIT_REPOSITORY https://github.com/grpc/grpc - GIT_TAG e241f37befe7ba4688effd84bfbf99b0f681a2f7 # v1.49.4 + GIT_TAG ${NGX_OTEL_GRPC} GIT_SUBMODULES third_party/protobuf third_party/abseil-cpp third_party/re2 GIT_SHALLOW ON) @@ -32,10 +38,29 @@ if(NGX_OTEL_FETCH_DEPS) set(gRPC_SSL_PROVIDER package CACHE INTERNAL "") set(gRPC_ZLIB_PROVIDER package CACHE INTERNAL "") + set(CMAKE_POSITION_INDEPENDENT_CODE ON) + + FetchContent_MakeAvailable(grpc) + + # reconsider once https://github.com/grpc/grpc/issues/36023 is done + target_compile_definitions(grpc PRIVATE GRPC_NO_XDS GRPC_NO_RLS) + + set_property(DIRECTORY ${grpc_SOURCE_DIR} + PROPERTY EXCLUDE_FROM_ALL YES) + + add_library(gRPC::grpc++ ALIAS grpc++) + add_executable(gRPC::grpc_cpp_plugin ALIAS grpc_cpp_plugin) +endif() + +if(NGX_OTEL_SDK STREQUAL "package") + find_package(opentelemetry-cpp REQUIRED) +else() + include(FetchContent) + FetchContent_Declare( otelcpp GIT_REPOSITORY https://github.com/open-telemetry/opentelemetry-cpp - GIT_TAG 11d5d9e0d8fd8ba876c8994714cc2647479b6574 # v1.11.0 + GIT_TAG ${NGX_OTEL_SDK} GIT_SUBMODULES third_party/opentelemetry-proto GIT_SHALLOW ON) @@ -45,13 +70,8 @@ if(NGX_OTEL_FETCH_DEPS) set(CMAKE_POSITION_INDEPENDENT_CODE ON) set(CMAKE_POLICY_DEFAULT_CMP0063 NEW) - FetchContent_MakeAvailable(grpc otelcpp) + FetchContent_MakeAvailable(otelcpp) - # reconsider once https://github.com/grpc/grpc/issues/36023 is done - target_compile_definitions(grpc PRIVATE GRPC_NO_XDS GRPC_NO_RLS) - - set_property(DIRECTORY ${grpc_SOURCE_DIR} - PROPERTY EXCLUDE_FROM_ALL YES) set_property(DIRECTORY ${otelcpp_SOURCE_DIR} PROPERTY EXCLUDE_FROM_ALL YES) @@ -61,12 +81,6 @@ if(NGX_OTEL_FETCH_DEPS) endif() add_library(opentelemetry-cpp::trace ALIAS opentelemetry_trace) - add_library(gRPC::grpc++ ALIAS grpc++) - add_executable(gRPC::grpc_cpp_plugin ALIAS grpc_cpp_plugin) -else() - find_package(opentelemetry-cpp REQUIRED) - find_package(protobuf REQUIRED) - find_package(gRPC REQUIRED) endif() set(PROTO_DIR ${NGX_OTEL_PROTO_DIR}) From 5c736bd507fef46c96b831c190596c21cced85b8 Mon Sep 17 00:00:00 2001 From: Pavel Pautov Date: Tue, 16 Jul 2024 14:13:31 -0700 Subject: [PATCH 3/5] Support building with latest gRPC versions (up to v1.65.0). --- CMakeLists.txt | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 645c5d5..b745342 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -34,10 +34,14 @@ else() set(gRPC_USE_PROTO_LITE ON CACHE INTERNAL "") set(gRPC_INSTALL OFF CACHE INTERNAL "") + set(gRPC_USE_SYSTEMD OFF CACHE INTERNAL "") + set(gRPC_DOWNLOAD_ARCHIVES OFF CACHE INTERNAL "") set(gRPC_CARES_PROVIDER package CACHE INTERNAL "") set(gRPC_SSL_PROVIDER package CACHE INTERNAL "") set(gRPC_ZLIB_PROVIDER package CACHE INTERNAL "") + set(protobuf_INSTALL OFF CACHE INTERNAL "") + set(CMAKE_POSITION_INDEPENDENT_CODE ON) FetchContent_MakeAvailable(grpc) @@ -108,8 +112,8 @@ add_custom_command( --plugin protoc-gen-grpc=$ ${PROTOS} # remove inconsequential UTF8 check during serialization to aid performance - COMMAND sed -i.bak - -e [[/ ::PROTOBUF_NAMESPACE_ID::internal::WireFormatLite::VerifyUtf8String(/,/);/d]] + COMMAND sed -i.bak -E + -e [[/ ::(PROTOBUF_NAMESPACE_ID|google::protobuf)::internal::WireFormatLite::VerifyUtf8String\(/,/\);/d]] ${PROTO_SOURCES} DEPENDS ${PROTOS} protobuf::protoc gRPC::grpc_cpp_plugin VERBATIM) From 1ac41dcd9b7d1bf57d86e3704f1886378a05527a Mon Sep 17 00:00:00 2001 From: Pavel Pautov Date: Tue, 16 Jul 2024 22:45:46 -0700 Subject: [PATCH 4/5] Don't force C++ standard for user builds. This fixes build against C++17 enabled prebuilt dependencies. --- CMakeLists.txt | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index b745342..7a74f0a 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -118,10 +118,10 @@ add_custom_command( DEPENDS ${PROTOS} protobuf::protoc gRPC::grpc_cpp_plugin VERBATIM) -set(CMAKE_CXX_STANDARD 11) -set(CMAKE_CXX_EXTENSIONS OFF) - if (NGX_OTEL_DEV) + set(CMAKE_CXX_STANDARD 11) + set(CMAKE_CXX_EXTENSIONS OFF) + add_compile_options(-Wall -Wtype-limits -Werror) endif() From aedcd7d3ac14bd610f8bea357b6d1dadaf44f2f5 Mon Sep 17 00:00:00 2001 From: Pavel Pautov Date: Wed, 17 Jul 2024 16:34:06 -0700 Subject: [PATCH 5/5] Use Abseil logging for gRPC v1.65.0 and above. Original logging method is now deprecated and results in error message on Nginx startup. --- src/grpc_log.cpp | 24 +++++++++++++++++++----- 1 file changed, 19 insertions(+), 5 deletions(-) diff --git a/src/grpc_log.cpp b/src/grpc_log.cpp index 84d796c..bb83364 100644 --- a/src/grpc_log.cpp +++ b/src/grpc_log.cpp @@ -2,8 +2,8 @@ #include "grpc_log.hpp" -#include #include +#include #if GOOGLE_PROTOBUF_VERSION < 4022000 @@ -36,9 +36,9 @@ class ProtobufLog { #include #include -class ProtobufLog : absl::LogSink { +class NgxLogSink : absl::LogSink { public: - ProtobufLog() + NgxLogSink() { absl::InitializeLog(); absl::AddLogSink(this); @@ -46,7 +46,7 @@ class ProtobufLog : absl::LogSink { absl::SetStderrThreshold(static_cast(100)); } - ~ProtobufLog() override { absl::RemoveLogSink(this); } + ~NgxLogSink() override { absl::RemoveLogSink(this); } void Send(const absl::LogEntry& entry) override { @@ -61,12 +61,19 @@ class ProtobufLog : absl::LogSink { ngx_str_t message { entry.text_message().size(), (u_char*)entry.text_message().data() }; - ngx_log_error(level, ngx_cycle->log, 0, "OTel/protobuf: %V", &message); + ngx_log_error(level, ngx_cycle->log, 0, "OTel/grpc: %V", &message); } }; +typedef NgxLogSink ProtobufLog; + #endif +#if (GRPC_CPP_VERSION_MAJOR < 1) || \ + (GRPC_CPP_VERSION_MAJOR == 1 && GRPC_CPP_VERSION_MINOR < 65) + +#include + class GrpcLog { public: GrpcLog() { gpr_set_log_function(grpcLogHandler); } @@ -87,6 +94,13 @@ class GrpcLog { ProtobufLog protoLog; }; +#else + +// newer gRPC implies newer protobuf, and both use Abseil for logging +typedef NgxLogSink GrpcLog; + +#endif + void initGrpcLog() { static GrpcLog init;