From ae152060e9a9600f8dbc99e712fecb5765b1c948 Mon Sep 17 00:00:00 2001 From: Alex E Date: Mon, 28 Oct 2024 01:26:01 -0400 Subject: [PATCH 1/9] Enable curl verbose output and add debug function callback --- .../http/client/curl/http_operation_curl.h | 6 ++ .../http/client/curl/http_operation_curl.cc | 64 ++++++++++++++++++- 2 files changed, 69 insertions(+), 1 deletion(-) diff --git a/ext/include/opentelemetry/ext/http/client/curl/http_operation_curl.h b/ext/include/opentelemetry/ext/http/client/curl/http_operation_curl.h index b6654ce3ad..b1671227e1 100644 --- a/ext/include/opentelemetry/ext/http/client/curl/http_operation_curl.h +++ b/ext/include/opentelemetry/ext/http/client/curl/http_operation_curl.h @@ -102,6 +102,12 @@ class HttpOperation static size_t ReadMemoryCallback(char *buffer, size_t size, size_t nitems, void *userp); + static int CurlLoggerCallback(const CURL * /* handle */, + curl_infotype type, + const char *data, + size_t size, + void * /* clientp */) noexcept; + #if LIBCURL_VERSION_NUM >= 0x075000 static int PreRequestCallback(void *clientp, char *conn_primary_ip, diff --git a/ext/src/http/client/curl/http_operation_curl.cc b/ext/src/http/client/curl/http_operation_curl.cc index 4de014fd82..fbf2a80fa2 100644 --- a/ext/src/http/client/curl/http_operation_curl.cc +++ b/ext/src/http/client/curl/http_operation_curl.cc @@ -22,6 +22,7 @@ #include "opentelemetry/ext/http/client/curl/http_client_curl.h" #include "opentelemetry/ext/http/client/curl/http_operation_curl.h" #include "opentelemetry/ext/http/client/http_client.h" +#include "opentelemetry/nostd/string_view.h" #include "opentelemetry/sdk/common/global_log_handler.h" #include "opentelemetry/version.h" @@ -569,6 +570,60 @@ CURLcode HttpOperation::SetCurlOffOption(CURLoption option, curl_off_t value) return rc; } +int HttpOperation::CurlLoggerCallback(const CURL * /* handle */, + curl_infotype type, + const char *data, + size_t size, + void * /* clientp */) noexcept +{ + static const auto kTlsInfo = nostd::string_view("SSL connection using"); + static const auto kFailureMsg = nostd::string_view("Recv failure:"); + static const auto kHeaderSent = nostd::string_view("Send header => "); + static const auto kHeaderRecv = nostd::string_view("Recv header => "); + + if (nostd::string_view text_to_log{data, size}; + text_to_log.empty() || std::isspace(text_to_log[0])) + { + return 0; + } + else if (type == CURLINFO_TEXT) + { + if (text_to_log.substr(0, kTlsInfo.size()) == kTlsInfo) + { + OTEL_INTERNAL_LOG_INFO(text_to_log); + } + else if (text_to_log.substr(0, kFailureMsg.size()) == kFailureMsg) + { + OTEL_INTERNAL_LOG_ERROR(text_to_log); + } +#ifdef CURL_DEBUG + else + { + OTEL_INTERNAL_LOG_DEBUG(text_to_log); + } +#endif // CURL_DEBUG + } +#ifdef CURL_DEBUG + else if (type == CURLINFO_HEADER_OUT) + { + while (!text_to_log.empty() && !std::isspace(text_to_log[0])) + { + if (const auto pos = text_to_log.find('\n'); pos != nostd::string_view::npos) + { + OTEL_INTERNAL_LOG_DEBUG(kHeaderSent << text_to_log.substr(0, pos)); + text_to_log = text_to_log.substr(pos + 1); + } + } + } + else if (type == CURLINFO_HEADER_IN) + { + OTEL_INTERNAL_LOG_DEBUG(kHeaderRecv << text_to_log); + } +#endif // CURL_DEBUG + + return 0; +} + CURLcode HttpOperation::Setup() { if (!curl_resource_.easy_handle) @@ -581,7 +636,14 @@ CURLcode HttpOperation::Setup() curl_error_message_[0] = '\0'; curl_easy_setopt(curl_resource_.easy_handle, CURLOPT_ERRORBUFFER, curl_error_message_); - rc = SetCurlLongOption(CURLOPT_VERBOSE, 0L); + rc = SetCurlLongOption(CURLOPT_VERBOSE, 1L); + if (rc != CURLE_OK) + { + return rc; + } + + rc = SetCurlPtrOption(CURLOPT_DEBUGFUNCTION, + reinterpret_cast(&HttpOperation::CurlLoggerCallback)); if (rc != CURLE_OK) { return rc; From b2e4069e7fc6760f19f874bffd078ed2aa28ae83 Mon Sep 17 00:00:00 2001 From: Alex E Date: Mon, 28 Oct 2024 18:34:24 -0400 Subject: [PATCH 2/9] Fix PR pipeline errors --- .../http/client/curl/http_operation_curl.cc | 23 +++++++++++-------- 1 file changed, 14 insertions(+), 9 deletions(-) diff --git a/ext/src/http/client/curl/http_operation_curl.cc b/ext/src/http/client/curl/http_operation_curl.cc index fbf2a80fa2..52fcc2e753 100644 --- a/ext/src/http/client/curl/http_operation_curl.cc +++ b/ext/src/http/client/curl/http_operation_curl.cc @@ -576,18 +576,18 @@ int HttpOperation::CurlLoggerCallback(const CURL * /* handle */, size_t size, void * /* clientp */) noexcept { - static const auto kTlsInfo = nostd::string_view("SSL connection using"); - static const auto kFailureMsg = nostd::string_view("Recv failure:"); - static const auto kHeaderSent = nostd::string_view("Send header => "); - static const auto kHeaderRecv = nostd::string_view("Recv header => "); + nostd::string_view text_to_log{data, size}; - if (nostd::string_view text_to_log{data, size}; - text_to_log.empty() || std::isspace(text_to_log[0])) + if (text_to_log.empty() || std::iscntrl(text_to_log[0])) { return 0; } - else if (type == CURLINFO_TEXT) + + if (type == CURLINFO_TEXT) { + static const auto kTlsInfo = nostd::string_view("SSL connection using"); + static const auto kFailureMsg = nostd::string_view("Recv failure:"); + if (text_to_log.substr(0, kTlsInfo.size()) == kTlsInfo) { OTEL_INTERNAL_LOG_INFO(text_to_log); @@ -606,9 +606,13 @@ int HttpOperation::CurlLoggerCallback(const CURL * /* handle */, #ifdef CURL_DEBUG else if (type == CURLINFO_HEADER_OUT) { - while (!text_to_log.empty() && !std::isspace(text_to_log[0])) + static const auto kHeaderSent = nostd::string_view("Send header => "); + + while (!text_to_log.empty() && !std::iscntrl(text_to_log[0])) { - if (const auto pos = text_to_log.find('\n'); pos != nostd::string_view::npos) + const auto pos = text_to_log.find('\n'); + + if (pos != nostd::string_view::npos) { OTEL_INTERNAL_LOG_DEBUG(kHeaderSent << text_to_log.substr(0, pos)); text_to_log = text_to_log.substr(pos + 1); @@ -617,6 +621,7 @@ int HttpOperation::CurlLoggerCallback(const CURL * /* handle */, } else if (type == CURLINFO_HEADER_IN) { + static const auto kHeaderRecv = nostd::string_view("Recv header => "); OTEL_INTERNAL_LOG_DEBUG(kHeaderRecv << text_to_log); } #endif // CURL_DEBUG From 6c071322b29d331152a1afb0ac6db937af334bf3 Mon Sep 17 00:00:00 2001 From: Alex E Date: Mon, 28 Oct 2024 20:20:42 -0400 Subject: [PATCH 3/9] Minor touchup to remove trailing newline --- ext/src/http/client/curl/http_operation_curl.cc | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/ext/src/http/client/curl/http_operation_curl.cc b/ext/src/http/client/curl/http_operation_curl.cc index 52fcc2e753..60ced36112 100644 --- a/ext/src/http/client/curl/http_operation_curl.cc +++ b/ext/src/http/client/curl/http_operation_curl.cc @@ -578,9 +578,9 @@ int HttpOperation::CurlLoggerCallback(const CURL * /* handle */, { nostd::string_view text_to_log{data, size}; - if (text_to_log.empty() || std::iscntrl(text_to_log[0])) + if (!text_to_log.empty() && text_to_log[size - 1] == '\n') { - return 0; + text_to_log = text_to_log.substr(0, size - 1); } if (type == CURLINFO_TEXT) @@ -614,7 +614,7 @@ int HttpOperation::CurlLoggerCallback(const CURL * /* handle */, if (pos != nostd::string_view::npos) { - OTEL_INTERNAL_LOG_DEBUG(kHeaderSent << text_to_log.substr(0, pos)); + OTEL_INTERNAL_LOG_DEBUG(kHeaderSent << text_to_log.substr(0, pos - 1)); text_to_log = text_to_log.substr(pos + 1); } } From ab2c39b03548a04d3cb41b0835e57f75f8e59134 Mon Sep 17 00:00:00 2001 From: Alex E Date: Fri, 1 Nov 2024 00:14:26 -0400 Subject: [PATCH 4/9] Add runtime flag --- exporters/otlp/src/otlp_http_client.cc | 1 + .../opentelemetry/ext/http/client/curl/http_client_curl.h | 3 +++ .../ext/http/client/curl/http_operation_curl.h | 5 ++++- ext/include/opentelemetry/ext/http/client/http_client.h | 2 ++ ext/src/http/client/curl/http_client_curl.cc | 8 ++++---- ext/src/http/client/curl/http_operation_curl.cc | 6 ++++-- .../ext/http/client/nosend/http_client_nosend.h | 3 +++ 7 files changed, 21 insertions(+), 7 deletions(-) diff --git a/exporters/otlp/src/otlp_http_client.cc b/exporters/otlp/src/otlp_http_client.cc index 876719fa1e..c330ffae2f 100644 --- a/exporters/otlp/src/otlp_http_client.cc +++ b/exporters/otlp/src/otlp_http_client.cc @@ -989,6 +989,7 @@ OtlpHttpClient::createSession( request->SetBody(body_vec); request->ReplaceHeader("Content-Type", content_type); request->ReplaceHeader("User-Agent", options_.user_agent); + request->EnableLogging(options_.console_debug); if (options_.compression == "gzip") { 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 902981f392..412c0704c2 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 @@ -102,6 +102,8 @@ class Request : public opentelemetry::ext::http::client::Request compression_ = compression; } + void EnableLogging(bool needs_to_log) noexcept override { needs_to_log_ = needs_to_log; }; + public: opentelemetry::ext::http::client::Method method_; opentelemetry::ext::http::client::HttpSslOptions ssl_options_; @@ -111,6 +113,7 @@ class Request : public opentelemetry::ext::http::client::Request std::chrono::milliseconds timeout_ms_{5000}; // ms opentelemetry::ext::http::client::Compression compression_{ opentelemetry::ext::http::client::Compression::kNone}; + bool needs_to_log_{false}; }; class Response : public opentelemetry::ext::http::client::Response diff --git a/ext/include/opentelemetry/ext/http/client/curl/http_operation_curl.h b/ext/include/opentelemetry/ext/http/client/curl/http_operation_curl.h index b1671227e1..8ba4506364 100644 --- a/ext/include/opentelemetry/ext/http/client/curl/http_operation_curl.h +++ b/ext/include/opentelemetry/ext/http/client/curl/http_operation_curl.h @@ -158,7 +158,8 @@ class HttpOperation // Default connectivity and response size options bool is_raw_response = false, std::chrono::milliseconds http_conn_timeout = default_http_conn_timeout, - bool reuse_connection = false); + bool reuse_connection = false, + bool needs_to_log_ = false); /** * Destroy CURL instance @@ -306,6 +307,8 @@ class HttpOperation const opentelemetry::ext::http::client::Compression &compression_; + const bool needs_to_log_; + // Processed response headers and body long response_code_; std::vector response_headers_; diff --git a/ext/include/opentelemetry/ext/http/client/http_client.h b/ext/include/opentelemetry/ext/http/client/http_client.h index d17215da9b..c6c7bcc321 100644 --- a/ext/include/opentelemetry/ext/http/client/http_client.h +++ b/ext/include/opentelemetry/ext/http/client/http_client.h @@ -245,6 +245,8 @@ class Request virtual void SetCompression(const Compression &compression) noexcept = 0; + virtual void EnableLogging(bool needs_to_log) noexcept = 0; + virtual ~Request() = default; }; diff --git a/ext/src/http/client/curl/http_client_curl.cc b/ext/src/http/client/curl/http_client_curl.cc index 22f20fb0e0..31e18959c6 100644 --- a/ext/src/http/client/curl/http_client_curl.cc +++ b/ext/src/http/client/curl/http_client_curl.cc @@ -116,10 +116,10 @@ void Session::SendRequest( #endif } - curl_operation_.reset(new HttpOperation(http_request_->method_, url, http_request_->ssl_options_, - callback_ptr, http_request_->headers_, - http_request_->body_, http_request_->compression_, false, - http_request_->timeout_ms_, reuse_connection)); + curl_operation_.reset(new HttpOperation( + http_request_->method_, url, http_request_->ssl_options_, callback_ptr, + http_request_->headers_, http_request_->body_, http_request_->compression_, false, + http_request_->timeout_ms_, reuse_connection, http_request_->needs_to_log_)); bool success = CURLE_OK == curl_operation_->SendAsync(this, [this, callback](HttpOperation &operation) { if (operation.WasAborted()) diff --git a/ext/src/http/client/curl/http_operation_curl.cc b/ext/src/http/client/curl/http_operation_curl.cc index 60ced36112..9a1748efe4 100644 --- a/ext/src/http/client/curl/http_operation_curl.cc +++ b/ext/src/http/client/curl/http_operation_curl.cc @@ -262,7 +262,8 @@ HttpOperation::HttpOperation(opentelemetry::ext::http::client::Method method, // Default connectivity and response size options bool is_raw_response, std::chrono::milliseconds http_conn_timeout, - bool reuse_connection) + bool reuse_connection, + bool needs_to_log) : is_aborted_(false), is_finished_(false), is_cleaned_(false), @@ -282,6 +283,7 @@ HttpOperation::HttpOperation(opentelemetry::ext::http::client::Method method, request_nwrite_(0), session_state_(opentelemetry::ext::http::client::SessionState::Created), compression_(compression), + needs_to_log_(needs_to_log), response_code_(0) { /* get a curl handle */ @@ -641,7 +643,7 @@ CURLcode HttpOperation::Setup() curl_error_message_[0] = '\0'; curl_easy_setopt(curl_resource_.easy_handle, CURLOPT_ERRORBUFFER, curl_error_message_); - rc = SetCurlLongOption(CURLOPT_VERBOSE, 1L); + rc = SetCurlLongOption(CURLOPT_VERBOSE, static_cast(needs_to_log_)); if (rc != CURLE_OK) { return rc; 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 a5f8c2f0e1..1719645569 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 @@ -69,6 +69,8 @@ class Request : public opentelemetry::ext::http::client::Request compression_ = compression; } + void EnableLogging(bool needs_to_log) noexcept override { needs_to_log_ = needs_to_log; }; + public: opentelemetry::ext::http::client::Method method_; opentelemetry::ext::http::client::HttpSslOptions ssl_options_; @@ -78,6 +80,7 @@ class Request : public opentelemetry::ext::http::client::Request std::chrono::milliseconds timeout_ms_{5000}; // ms opentelemetry::ext::http::client::Compression compression_{ opentelemetry::ext::http::client::Compression::kNone}; + bool needs_to_log_{false}; }; class Response : public opentelemetry::ext::http::client::Response From 3be305edb6934883cbcbd46a0a7af3292abfd0c3 Mon Sep 17 00:00:00 2001 From: Alex E Date: Fri, 1 Nov 2024 23:37:46 -0400 Subject: [PATCH 5/9] Add compile-time flag --- CMakeLists.txt | 2 ++ ext/src/http/client/curl/CMakeLists.txt | 5 +++++ ext/src/http/client/curl/http_operation_curl.cc | 8 +++++++- 3 files changed, 14 insertions(+), 1 deletion(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index a0710834bf..532e3c01a1 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -215,6 +215,8 @@ option( "Whether to include gzip compression for the OTLP http exporter in the SDK" OFF) +option(WITH_CURL_LOGGING "Whether to enable select CURL verbosity in OTel logs" OFF) + option(WITH_ZIPKIN "Whether to include the Zipkin exporter in the SDK" OFF) option(WITH_PROMETHEUS "Whether to include the Prometheus Client in the SDK" diff --git a/ext/src/http/client/curl/CMakeLists.txt b/ext/src/http/client/curl/CMakeLists.txt index 6a69c7de51..c812fcefbf 100644 --- a/ext/src/http/client/curl/CMakeLists.txt +++ b/ext/src/http/client/curl/CMakeLists.txt @@ -24,6 +24,11 @@ else() PRIVATE ${CURL_LIBRARIES}) endif() +if(WITH_CURL_LOGGING) + target_compile_definitions(opentelemetry_http_client_curl + PRIVATE ENABLE_CURL_LOGGING) +endif() + if(WITH_OTLP_HTTP_COMPRESSION) if(TARGET ZLIB::ZLIB) target_link_libraries( diff --git a/ext/src/http/client/curl/http_operation_curl.cc b/ext/src/http/client/curl/http_operation_curl.cc index 9a1748efe4..6a2d557e48 100644 --- a/ext/src/http/client/curl/http_operation_curl.cc +++ b/ext/src/http/client/curl/http_operation_curl.cc @@ -633,6 +633,12 @@ int HttpOperation::CurlLoggerCallback(const CURL * /* handle */, CURLcode HttpOperation::Setup() { +#ifdef ENABLE_CURL_LOGGING + static constexpr auto kEnableCurlLogging = true; +#else + static constexpr auto kEnableCurlLogging = false; +#endif // ENABLE_CURL_LOGGING + if (!curl_resource_.easy_handle) { return CURLE_FAILED_INIT; @@ -643,7 +649,7 @@ CURLcode HttpOperation::Setup() curl_error_message_[0] = '\0'; curl_easy_setopt(curl_resource_.easy_handle, CURLOPT_ERRORBUFFER, curl_error_message_); - rc = SetCurlLongOption(CURLOPT_VERBOSE, static_cast(needs_to_log_)); + rc = SetCurlLongOption(CURLOPT_VERBOSE, static_cast(needs_to_log_ || kEnableCurlLogging)); if (rc != CURLE_OK) { return rc; From 78f9a0b7b2158310d69a500c2db1dd21937a3d79 Mon Sep 17 00:00:00 2001 From: Alex E Date: Sat, 2 Nov 2024 00:54:40 -0400 Subject: [PATCH 6/9] Adhere to code and format requirements --- CMakeLists.txt | 3 ++- .../opentelemetry/ext/http/client/curl/http_client_curl.h | 2 +- .../opentelemetry/ext/http/client/curl/http_operation_curl.h | 2 +- .../test_common/ext/http/client/nosend/http_client_nosend.h | 2 +- 4 files changed, 5 insertions(+), 4 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 532e3c01a1..b2f2e2764d 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -215,7 +215,8 @@ option( "Whether to include gzip compression for the OTLP http exporter in the SDK" OFF) -option(WITH_CURL_LOGGING "Whether to enable select CURL verbosity in OTel logs" OFF) +option(WITH_CURL_LOGGING "Whether to enable select CURL verbosity in OTel logs" + OFF) option(WITH_ZIPKIN "Whether to include the Zipkin exporter in the SDK" OFF) 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 412c0704c2..e91cada13d 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 @@ -102,7 +102,7 @@ class Request : public opentelemetry::ext::http::client::Request compression_ = compression; } - void EnableLogging(bool needs_to_log) noexcept override { needs_to_log_ = needs_to_log; }; + void EnableLogging(bool needs_to_log) noexcept override { needs_to_log_ = needs_to_log; } public: opentelemetry::ext::http::client::Method method_; diff --git a/ext/include/opentelemetry/ext/http/client/curl/http_operation_curl.h b/ext/include/opentelemetry/ext/http/client/curl/http_operation_curl.h index 8ba4506364..4f3395dab9 100644 --- a/ext/include/opentelemetry/ext/http/client/curl/http_operation_curl.h +++ b/ext/include/opentelemetry/ext/http/client/curl/http_operation_curl.h @@ -159,7 +159,7 @@ class HttpOperation bool is_raw_response = false, std::chrono::milliseconds http_conn_timeout = default_http_conn_timeout, bool reuse_connection = false, - bool needs_to_log_ = false); + bool needs_to_log = false); /** * Destroy CURL instance 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 1719645569..df056d907b 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 @@ -69,7 +69,7 @@ class Request : public opentelemetry::ext::http::client::Request compression_ = compression; } - void EnableLogging(bool needs_to_log) noexcept override { needs_to_log_ = needs_to_log; }; + void EnableLogging(bool needs_to_log) noexcept override { needs_to_log_ = needs_to_log; } public: opentelemetry::ext::http::client::Method method_; From d0b8589e6b15dd680bc44169a2bbe7638237f09b Mon Sep 17 00:00:00 2001 From: Alex E Date: Sat, 2 Nov 2024 10:47:39 -0400 Subject: [PATCH 7/9] Version guard for callback --- ext/src/http/client/curl/http_operation_curl.cc | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/ext/src/http/client/curl/http_operation_curl.cc b/ext/src/http/client/curl/http_operation_curl.cc index 6a2d557e48..90744ca7c5 100644 --- a/ext/src/http/client/curl/http_operation_curl.cc +++ b/ext/src/http/client/curl/http_operation_curl.cc @@ -649,6 +649,15 @@ CURLcode HttpOperation::Setup() curl_error_message_[0] = '\0'; curl_easy_setopt(curl_resource_.easy_handle, CURLOPT_ERRORBUFFER, curl_error_message_); +// Support for custom debug function callback was added in version 7.9.6 so we guard against +// exposing the default CURL output by keeping verbosity always disabled in lower versions. +#if LIBCURL_VERSION_NUM < CURL_VERSION_BITS(7, 9, 6) + rc = SetCurlLongOption(CURLOPT_VERBOSE, 0L); + if (rc != CURLE_OK) + { + return rc; + } +#else rc = SetCurlLongOption(CURLOPT_VERBOSE, static_cast(needs_to_log_ || kEnableCurlLogging)); if (rc != CURLE_OK) { @@ -661,6 +670,7 @@ CURLcode HttpOperation::Setup() { return rc; } +#endif // Specify target URL rc = SetCurlStrOption(CURLOPT_URL, url_.c_str()); From 5dc28abf4287e70886223633e01406e2d72ebe9c Mon Sep 17 00:00:00 2001 From: Alex E Date: Sat, 2 Nov 2024 11:12:12 -0400 Subject: [PATCH 8/9] Align with naming conventions --- .../opentelemetry/ext/http/client/curl/http_client_curl.h | 4 ++-- .../ext/http/client/curl/http_operation_curl.h | 4 ++-- ext/include/opentelemetry/ext/http/client/http_client.h | 2 +- ext/src/http/client/curl/http_client_curl.cc | 2 +- ext/src/http/client/curl/http_operation_curl.cc | 6 +++--- .../test_common/ext/http/client/nosend/http_client_nosend.h | 4 ++-- 6 files changed, 11 insertions(+), 11 deletions(-) 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 e91cada13d..ef65388fe1 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 @@ -102,7 +102,7 @@ class Request : public opentelemetry::ext::http::client::Request compression_ = compression; } - void EnableLogging(bool needs_to_log) noexcept override { needs_to_log_ = needs_to_log; } + void EnableLogging(bool is_log_enabled) noexcept override { is_log_enabled_ = is_log_enabled; } public: opentelemetry::ext::http::client::Method method_; @@ -113,7 +113,7 @@ class Request : public opentelemetry::ext::http::client::Request std::chrono::milliseconds timeout_ms_{5000}; // ms opentelemetry::ext::http::client::Compression compression_{ opentelemetry::ext::http::client::Compression::kNone}; - bool needs_to_log_{false}; + bool is_log_enabled_{false}; }; class Response : public opentelemetry::ext::http::client::Response diff --git a/ext/include/opentelemetry/ext/http/client/curl/http_operation_curl.h b/ext/include/opentelemetry/ext/http/client/curl/http_operation_curl.h index 4f3395dab9..b94c53b2d0 100644 --- a/ext/include/opentelemetry/ext/http/client/curl/http_operation_curl.h +++ b/ext/include/opentelemetry/ext/http/client/curl/http_operation_curl.h @@ -159,7 +159,7 @@ class HttpOperation bool is_raw_response = false, std::chrono::milliseconds http_conn_timeout = default_http_conn_timeout, bool reuse_connection = false, - bool needs_to_log = false); + bool is_log_enabled = false); /** * Destroy CURL instance @@ -307,7 +307,7 @@ class HttpOperation const opentelemetry::ext::http::client::Compression &compression_; - const bool needs_to_log_; + const bool is_log_enabled_; // Processed response headers and body long response_code_; diff --git a/ext/include/opentelemetry/ext/http/client/http_client.h b/ext/include/opentelemetry/ext/http/client/http_client.h index c6c7bcc321..e467f9ef63 100644 --- a/ext/include/opentelemetry/ext/http/client/http_client.h +++ b/ext/include/opentelemetry/ext/http/client/http_client.h @@ -245,7 +245,7 @@ class Request virtual void SetCompression(const Compression &compression) noexcept = 0; - virtual void EnableLogging(bool needs_to_log) noexcept = 0; + virtual void EnableLogging(bool is_log_enabled) noexcept = 0; virtual ~Request() = default; }; diff --git a/ext/src/http/client/curl/http_client_curl.cc b/ext/src/http/client/curl/http_client_curl.cc index 31e18959c6..a0bb2748ba 100644 --- a/ext/src/http/client/curl/http_client_curl.cc +++ b/ext/src/http/client/curl/http_client_curl.cc @@ -119,7 +119,7 @@ void Session::SendRequest( curl_operation_.reset(new HttpOperation( http_request_->method_, url, http_request_->ssl_options_, callback_ptr, http_request_->headers_, http_request_->body_, http_request_->compression_, false, - http_request_->timeout_ms_, reuse_connection, http_request_->needs_to_log_)); + http_request_->timeout_ms_, reuse_connection, http_request_->is_log_enabled_)); bool success = CURLE_OK == curl_operation_->SendAsync(this, [this, callback](HttpOperation &operation) { if (operation.WasAborted()) diff --git a/ext/src/http/client/curl/http_operation_curl.cc b/ext/src/http/client/curl/http_operation_curl.cc index 90744ca7c5..b3e18273b0 100644 --- a/ext/src/http/client/curl/http_operation_curl.cc +++ b/ext/src/http/client/curl/http_operation_curl.cc @@ -263,7 +263,7 @@ HttpOperation::HttpOperation(opentelemetry::ext::http::client::Method method, bool is_raw_response, std::chrono::milliseconds http_conn_timeout, bool reuse_connection, - bool needs_to_log) + bool is_log_enabled) : is_aborted_(false), is_finished_(false), is_cleaned_(false), @@ -283,7 +283,7 @@ HttpOperation::HttpOperation(opentelemetry::ext::http::client::Method method, request_nwrite_(0), session_state_(opentelemetry::ext::http::client::SessionState::Created), compression_(compression), - needs_to_log_(needs_to_log), + is_log_enabled_(is_log_enabled), response_code_(0) { /* get a curl handle */ @@ -658,7 +658,7 @@ CURLcode HttpOperation::Setup() return rc; } #else - rc = SetCurlLongOption(CURLOPT_VERBOSE, static_cast(needs_to_log_ || kEnableCurlLogging)); + rc = SetCurlLongOption(CURLOPT_VERBOSE, static_cast(is_log_enabled_ || kEnableCurlLogging)); if (rc != CURLE_OK) { return rc; 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 df056d907b..211b5b3720 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 @@ -69,7 +69,7 @@ class Request : public opentelemetry::ext::http::client::Request compression_ = compression; } - void EnableLogging(bool needs_to_log) noexcept override { needs_to_log_ = needs_to_log; } + void EnableLogging(bool is_log_enabled) noexcept override { is_log_enabled_ = is_log_enabled; } public: opentelemetry::ext::http::client::Method method_; @@ -80,7 +80,7 @@ class Request : public opentelemetry::ext::http::client::Request std::chrono::milliseconds timeout_ms_{5000}; // ms opentelemetry::ext::http::client::Compression compression_{ opentelemetry::ext::http::client::Compression::kNone}; - bool needs_to_log_{false}; + bool is_log_enabled_{false}; }; class Response : public opentelemetry::ext::http::client::Response From afe9e5a69713ffe491e8dd42b6c410ab7fdadba8 Mon Sep 17 00:00:00 2001 From: Alex E Date: Tue, 5 Nov 2024 22:31:07 -0500 Subject: [PATCH 9/9] Code review feedback --- ext/src/http/client/curl/http_operation_curl.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ext/src/http/client/curl/http_operation_curl.cc b/ext/src/http/client/curl/http_operation_curl.cc index b3e18273b0..d287e0fcb2 100644 --- a/ext/src/http/client/curl/http_operation_curl.cc +++ b/ext/src/http/client/curl/http_operation_curl.cc @@ -658,7 +658,7 @@ CURLcode HttpOperation::Setup() return rc; } #else - rc = SetCurlLongOption(CURLOPT_VERBOSE, static_cast(is_log_enabled_ || kEnableCurlLogging)); + rc = SetCurlLongOption(CURLOPT_VERBOSE, (is_log_enabled_ && kEnableCurlLogging) ? 1L : 0L; if (rc != CURLE_OK) { return rc;