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 9a09fac9aa..689436f4d0 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 @@ -51,10 +51,24 @@ class HttpCurlGlobalInitializer HttpCurlGlobalInitializer(); + friend class HttpClientTestPeer; + + // Test constructor to simulate init failure without calling curl_global_init + HttpCurlGlobalInitializer(CURLcode code, bool is_initialized) + : init_code_{code}, is_initialized_{is_initialized} + {} + + CURLcode init_code_{CURLE_OK}; + bool is_initialized_{false}; + public: ~HttpCurlGlobalInitializer(); static nostd::shared_ptr GetInstance(); + + // added accessors here + bool IsValid() const noexcept { return is_initialized_; } + CURLcode GetStatus() const noexcept { return init_code_; } }; class Request : public opentelemetry::ext::http::client::Request @@ -253,6 +267,11 @@ class HttpClientSync : public opentelemetry::ext::http::client::HttpClientSync const opentelemetry::ext::http::client::Headers &headers, const opentelemetry::ext::http::client::Compression &compression) noexcept override { + if (!curl_global_initializer_ || !curl_global_initializer_->IsValid()) + { + return opentelemetry::ext::http::client::Result( + nullptr, opentelemetry::ext::http::client::SessionState::CreateFailed); + } opentelemetry::ext::http::client::Body body; HttpOperation curl_operation(opentelemetry::ext::http::client::Method::Get, url.data(), @@ -283,6 +302,11 @@ class HttpClientSync : public opentelemetry::ext::http::client::HttpClientSync const opentelemetry::ext::http::client::Headers &headers, const opentelemetry::ext::http::client::Compression &compression) noexcept override { + if (!curl_global_initializer_ || !curl_global_initializer_->IsValid()) + { + return opentelemetry::ext::http::client::Result( + nullptr, opentelemetry::ext::http::client::SessionState::CreateFailed); + } HttpOperation curl_operation(opentelemetry::ext::http::client::Method::Post, url.data(), ssl_options, nullptr, headers, body, compression); curl_operation.SendSync(); @@ -308,6 +332,7 @@ class HttpClientSync : public opentelemetry::ext::http::client::HttpClientSync ~HttpClientSync() override {} private: + friend class HttpClientTestPeer; nostd::shared_ptr curl_global_initializer_; }; @@ -325,6 +350,12 @@ class HttpClient : public opentelemetry::ext::http::client::HttpClient ~HttpClient() override; + inline bool IsValid() const noexcept + { + return curl_global_initializer_ && curl_global_initializer_->IsValid() && + multi_handle_ != nullptr; + } + std::shared_ptr CreateSession( nostd::string_view url) noexcept override; @@ -366,6 +397,7 @@ class HttpClient : public opentelemetry::ext::http::client::HttpClient bool doRetrySessions(bool report_all); void resetMultiHandle(); + nostd::shared_ptr curl_global_initializer_; std::mutex multi_handle_m_; CURLM *multi_handle_; std::atomic next_session_id_{0}; @@ -387,8 +419,6 @@ class HttpClient : public opentelemetry::ext::http::client::HttpClient std::chrono::milliseconds background_thread_wait_for_; std::atomic is_shutdown_{false}; - - nostd::shared_ptr curl_global_initializer_; }; } // namespace curl diff --git a/ext/src/http/client/curl/http_client_curl.cc b/ext/src/http/client/curl/http_client_curl.cc index 87f2c123ad..71ce795c3c 100644 --- a/ext/src/http/client/curl/http_client_curl.cc +++ b/ext/src/http/client/curl/http_client_curl.cc @@ -48,13 +48,21 @@ namespace curl { HttpCurlGlobalInitializer::HttpCurlGlobalInitializer() + : init_code_(curl_global_init(CURL_GLOBAL_ALL)), is_initialized_(init_code_ == CURLE_OK) { - curl_global_init(CURL_GLOBAL_ALL); + if (!is_initialized_) + { + OTEL_INTERNAL_LOG_ERROR("[HTTP Client Curl] curl_global_init failed with error code: " + << static_cast(init_code_)); + } } HttpCurlGlobalInitializer::~HttpCurlGlobalInitializer() { - curl_global_cleanup(); + if (is_initialized_) + { + curl_global_cleanup(); + } } nostd::shared_ptr HttpCurlGlobalInitializer::GetInstance() @@ -141,6 +149,18 @@ static int deflateInPlace(z_stream *strm, unsigned char *buf, uint32_t len, uint void Session::SendRequest( std::shared_ptr callback) noexcept { + // Returning early before MaybeSpawnBackgroundThread() ensures the background IO + // loop never starts with an uninitialized or null multi_handle_. + if (!http_client_.IsValid()) + { + if (callback) + { + callback->OnEvent(opentelemetry::ext::http::client::SessionState::CreateFailed, + "curl initialization failed"); + } + is_session_active_.store(false, std::memory_order_release); + return; + } is_session_active_.store(true, std::memory_order_release); const auto &url = host_ + http_request_->uri_; auto callback_ptr = callback.get(); @@ -271,24 +291,28 @@ void Session::FinishOperation() } HttpClient::HttpClient() - : multi_handle_(curl_multi_init()), + : curl_global_initializer_(HttpCurlGlobalInitializer::GetInstance()), + multi_handle_(curl_global_initializer_ && curl_global_initializer_->IsValid() + ? curl_multi_init() + : nullptr), next_session_id_{0}, max_sessions_per_connection_{8}, background_thread_instrumentation_(nullptr), scheduled_delay_milliseconds_{std::chrono::milliseconds(256)}, - background_thread_wait_for_{std::chrono::minutes{1}}, - curl_global_initializer_(HttpCurlGlobalInitializer::GetInstance()) + background_thread_wait_for_{std::chrono::minutes{1}} {} HttpClient::HttpClient( const std::shared_ptr &thread_instrumentation) - : multi_handle_(curl_multi_init()), + : curl_global_initializer_(HttpCurlGlobalInitializer::GetInstance()), + multi_handle_(curl_global_initializer_ && curl_global_initializer_->IsValid() + ? curl_multi_init() + : nullptr), next_session_id_{0}, max_sessions_per_connection_{8}, background_thread_instrumentation_(thread_instrumentation), scheduled_delay_milliseconds_{std::chrono::milliseconds(256)}, - background_thread_wait_for_{std::chrono::minutes{1}}, - curl_global_initializer_(HttpCurlGlobalInitializer::GetInstance()) + background_thread_wait_for_{std::chrono::minutes{1}} {} HttpClient::~HttpClient() @@ -317,7 +341,11 @@ HttpClient::~HttpClient() } { std::lock_guard lock_guard{multi_handle_m_}; - curl_multi_cleanup(multi_handle_); + if (multi_handle_) + { + curl_multi_cleanup(multi_handle_); + multi_handle_ = nullptr; + } } } diff --git a/ext/test/http/curl_http_test.cc b/ext/test/http/curl_http_test.cc index 36c0302ffe..4a9c02bb16 100644 --- a/ext/test/http/curl_http_test.cc +++ b/ext/test/http/curl_http_test.cc @@ -1,11 +1,11 @@ // Copyright The OpenTelemetry Authors // SPDX-License-Identifier: Apache-2.0 +#include #include #include "gtest/gtest.h" #ifdef ENABLE_OTLP_RETRY_PREVIEW -# include # include "gmock/gmock.h" #endif // ENABLE_OTLP_RETRY_PREVIEW @@ -56,6 +56,35 @@ class HttpClientTestPeer { public: static void ResetMultiHandle(HttpClient &client) { client.resetMultiHandle(); } + // mock initializer + static nostd::shared_ptr CreateMockInitializer(CURLcode code, + bool is_valid) + { + return nostd::shared_ptr( + new HttpCurlGlobalInitializer(code, is_valid)); + } + // injects the mock initializer into an HttpClient + static void SetInitializer(HttpClient &client, + const nostd::shared_ptr &initializer) + { + client.curl_global_initializer_ = initializer; + if (!initializer || !initializer->IsValid()) + { + std::lock_guard lk(client.multi_handle_m_); + if (client.multi_handle_) + { + curl_multi_cleanup(client.multi_handle_); + client.multi_handle_ = nullptr; + } + } + } + + // injects the mock initializer into an HttpClientSync + static void SetSyncInitializer(HttpClientSync &client, + const nostd::shared_ptr &initializer) + { + client.curl_global_initializer_ = initializer; + } }; } // namespace curl } // namespace client @@ -77,6 +106,10 @@ class CustomEventHandler : public http_client::EventHandler { switch (state) { + // CreateFailed occurs when initialization or request preparation fails before + // network dispatch (e.g., if curl_global_init failed or session setup failed). + // Marking is_called_ ensures tests expecting early failure notifications register it. + case http_client::SessionState::CreateFailed: case http_client::SessionState::ConnectFailed: case http_client::SessionState::SendFailed: { is_called_.store(true, std::memory_order_release); @@ -1095,6 +1128,52 @@ TEST_F(BasicCurlHttpTests, GzipIncompressibleData) session_manager->CancelAllSessions(); session_manager->FinishAllSessions(); } + #endif // ENABLE_OTLP_COMPRESSION_PREVIEW +// 2 new tests +TEST_F(BasicCurlHttpTests, GlobalInitFailureHttpClient) +{ + curl::HttpClient client; + auto mock_init = + http_client::curl::HttpClientTestPeer::CreateMockInitializer(CURLE_FAILED_INIT, false); + http_client::curl::HttpClientTestPeer::SetInitializer(client, mock_init); + + EXPECT_FALSE(client.IsValid()); + + auto session = client.CreateSession("http://127.0.0.1:19000/get/"); + auto request = session->CreateRequest(); + request->SetMethod(http_client::Method::Get); + request->SetUri("get/"); + + auto handler = std::make_shared(); + session->SendRequest(handler); + + EXPECT_FALSE(session->IsSessionActive()); + EXPECT_TRUE(handler->is_called_.load(std::memory_order_acquire)); + EXPECT_FALSE(handler->got_response_.load(std::memory_order_acquire)); + + EXPECT_TRUE(client.CancelAllSessions()); + EXPECT_TRUE(client.FinishAllSessions()); +} + +TEST_F(BasicCurlHttpTests, GlobalInitFailureHttpClientSync) +{ + curl::HttpClientSync sync_client; + auto mock_init = + http_client::curl::HttpClientTestPeer::CreateMockInitializer(CURLE_FAILED_INIT, false); + http_client::curl::HttpClientTestPeer::SetSyncInitializer(sync_client, mock_init); + + http_client::HttpSslOptions ssl_opts; + auto get_result = + sync_client.Get("http://127.0.0.1:19000/get/", ssl_opts, {}, http_client::Compression::kNone); + EXPECT_FALSE(get_result); + EXPECT_EQ(get_result.GetSessionState(), http_client::SessionState::CreateFailed); + + http_client::Body body; + auto post_result = sync_client.Post("http://127.0.0.1:19000/post/", ssl_opts, body, {}, + http_client::Compression::kNone); + EXPECT_FALSE(post_result); + EXPECT_EQ(post_result.GetSessionState(), http_client::SessionState::CreateFailed); +} } // namespace