From fda43987e4f72afbef3f4824d3e64660fac7caa8 Mon Sep 17 00:00:00 2001 From: Victor Vazquez Date: Thu, 15 Oct 2020 17:45:52 -0700 Subject: [PATCH] removing log member from curl session (#789) Remove the m_logger member from curl session and use logger directly from static logger in application --- .../inc/azure/core/http/curl/curl.hpp | 25 ++------- sdk/core/azure-core/src/http/curl/curl.cpp | 55 +++++++++---------- 2 files changed, 33 insertions(+), 47 deletions(-) diff --git a/sdk/core/azure-core/inc/azure/core/http/curl/curl.hpp b/sdk/core/azure-core/inc/azure/core/http/curl/curl.hpp index ea2a98f04..2807b4e32 100644 --- a/sdk/core/azure-core/inc/azure/core/http/curl/curl.hpp +++ b/sdk/core/azure-core/inc/azure/core/http/curl/curl.hpp @@ -12,7 +12,6 @@ #include "azure/core/context.hpp" #include "azure/core/http/http.hpp" #include "azure/core/http/policy.hpp" -#include "azure/core/internal/log.hpp" #include #include @@ -104,8 +103,8 @@ namespace Azure { namespace Core { namespace Http { * @brief CURL HTTP connection pool makes it possible to re-use one curl connection to perform * more than one request. Use this component when connections are not re-used by default. * - * This pool offers static methods and it is allocated statically. There can be only one connection - * pool per application. + * This pool offers static methods and it is allocated statically. There can be only one + * connection pool per application. */ struct CurlConnectionPool { @@ -184,9 +183,9 @@ namespace Azure { namespace Core { namespace Http { * @brief Stateful component that controls sending an HTTP Request with libcurl over the wire. * * @remark This component does not use the classic libcurl easy interface to send and receive - * bytes from the network using callbacks. Instead, `CurlSession` supports working with the custom HTTP - * protocol option from libcurl to manually upload and download bytes from the network socket using - * curl_easy_send() and curl_easy_recv(). + * bytes from the network using callbacks. Instead, `CurlSession` supports working with the custom + * HTTP protocol option from libcurl to manually upload and download bytes from the network socket + * using curl_easy_send() and curl_easy_recv(). * * @remarks This component is expected to be used by an HTTP Transporter to ensure that * transporter to be reusable in multiple pipelines while every call to network is unique. @@ -529,25 +528,13 @@ namespace Azure { namespace Core { namespace Http { : this->m_contentLength == this->m_sessionTotalRead; } - /** - * @brief The function to log. - * - */ - std::function m_logger; - public: /** * @brief Construct a new Curl Session object. Init internal libcurl handler. * * @param request reference to an HTTP Request. */ - CurlSession(Request& request) - : CurlSession(request, [](std::string const& message) { (void)message; }) - { - } - - CurlSession(Request& request, std::function logger) - : m_request(request), m_logger(logger) + CurlSession(Request& request) : m_request(request) { this->m_connection = CurlConnectionPool::GetCurlConnection(this->m_request); this->m_bodyStartInBuffer = -1; diff --git a/sdk/core/azure-core/src/http/curl/curl.cpp b/sdk/core/azure-core/src/http/curl/curl.cpp index db4521bfb..9def28627 100644 --- a/sdk/core/azure-core/src/http/curl/curl.cpp +++ b/sdk/core/azure-core/src/http/curl/curl.cpp @@ -4,6 +4,7 @@ #include "azure/core/http/curl/curl.hpp" #include "azure/core/azure.hpp" #include "azure/core/http/http.hpp" +#include "azure/core/internal/log.hpp" #ifdef POSIX #include // for poll() @@ -17,6 +18,18 @@ #include namespace { +// Can be used from anywhere a little simpler +inline void LogThis(std::string const& msg) +{ + if (Azure::Core::Logging::Details::ShouldWrite( + Azure::Core::Http::LogClassification::HttpTransportAdapter)) + { + Azure::Core::Logging::Details::Write( + Azure::Core::Http::LogClassification::HttpTransportAdapter, + "[CURL Transport Adapter]: " + msg); + } +} + template inline void SetLibcurlOption( CURL* handle, @@ -108,9 +121,7 @@ int pollSocketUntilEventOrTimeout( // Windows needs this after every write to socket or performance would be reduced to 1/4 for // uploading operation. // https://github.com/Azure/azure-sdk-for-cpp/issues/644 -void WinSocketSetBuffSize( - curl_socket_t socket, - std::function logger) +void WinSocketSetBuffSize(curl_socket_t socket) { ULONG ideal; DWORD ideallen; @@ -122,24 +133,12 @@ void WinSocketSetBuffSize( // Specifies the total per-socket buffer space reserved for sends. // https://docs.microsoft.com/en-us/windows/win32/api/winsock/nf-winsock-setsockopt auto result = setsockopt(socket, SOL_SOCKET, SO_SNDBUF, (const char*)&ideal, sizeof(ideal)); - logger( + LogThis( "Windows - calling setsockopt after uploading chunk. ideal = " + std::to_string(ideal) + " result = " + std::to_string(result)); } } #endif // WINDOWS - -// Can be used from anywhere a little simpler -inline void LogThis(std::string const& msg) -{ - if (Azure::Core::Logging::Details::ShouldWrite( - Azure::Core::Http::LogClassification::HttpTransportAdapter)) - { - Azure::Core::Logging::Details::Write( - Azure::Core::Http::LogClassification::HttpTransportAdapter, - "[CURL Transport Adapter]: " + msg); - } -} } // namespace using Azure::Core::Http::CurlConnection; @@ -156,7 +155,7 @@ std::unique_ptr CurlTransport::Send(Context const& context, Request { // Create CurlSession to perform request LogThis("Creating a new session."); - auto session = std::make_unique(request, LogThis); + auto session = std::make_unique(request); CURLcode performing; // Try to send the request. If we get CURLE_UNSUPPORTED_PROTOCOL back, it means the connection is @@ -218,13 +217,13 @@ CURLcode CurlSession::Perform(Context const& context) auto hostHeader = headers.find("Host"); if (hostHeader == headers.end()) { - this->m_logger("No Host in request headers. Adding it"); + LogThis("No Host in request headers. Adding it"); this->m_request.AddHeader("Host", this->m_request.GetUrl().GetHost()); } auto isContentLengthHeaderInRequest = headers.find("content-length"); if (isContentLengthHeaderInRequest == headers.end()) { - this->m_logger("No content-length in headers. Adding it"); + LogThis("No content-length in headers. Adding it"); this->m_request.AddHeader( "content-length", std::to_string(this->m_request.GetBodyStream()->Length())); } @@ -233,21 +232,21 @@ CURLcode CurlSession::Perform(Context const& context) // use expect:100 for PUT requests. Server will decide if it can take our request if (this->m_request.GetMethod() == HttpMethod::Put) { - this->m_logger("Using 100-continue for PUT request"); + LogThis("Using 100-continue for PUT request"); this->m_request.AddHeader("expect", "100-continue"); } // Send request. If the connection assigned to this curlSession is closed or the socket is // somehow lost, libcurl will return CURLE_UNSUPPORTED_PROTOCOL // (https://curl.haxx.se/libcurl/c/curl_easy_send.html). Return the error back. - this->m_logger("Send request without payload"); + LogThis("Send request without payload"); result = SendRawHttp(context); if (result != CURLE_OK) { return result; } - this->m_logger("Parse server response"); + LogThis("Parse server response"); ReadStatusLineAndHeadersFromRawResponse(context); // Upload body for PUT @@ -256,17 +255,17 @@ CURLcode CurlSession::Perform(Context const& context) return result; } - this->m_logger("Check server response before upload starts"); + LogThis("Check server response before upload starts"); // Check server response from Expect:100-continue for PUT; // This help to prevent us from start uploading data when Server can't handle it if (this->m_lastStatusCode != HttpStatusCode::Continue) { - this->m_logger("Server rejected the upload request"); + LogThis("Server rejected the upload request"); return result; // Won't upload. } - this->m_logger("Upload payload"); + LogThis("Upload payload"); if (this->m_bodyStartInBuffer > 0) { // If internal buffer has more data after the 100-continue means Server return an error. @@ -282,7 +281,7 @@ CURLcode CurlSession::Perform(Context const& context) return result; // will throw transport exception before trying to read } - this->m_logger("Upload completed. Parse server response"); + LogThis("Upload completed. Parse server response"); ReadStatusLineAndHeadersFromRawResponse(context); return result; } @@ -389,7 +388,7 @@ CURLcode CurlSession::SendBuffer(Context const& context, uint8_t const* buffer, }; } #ifdef WINDOWS - WinSocketSetBuffSize(this->m_curlSocket, this->m_logger); + WinSocketSetBuffSize(this->m_curlSocket); #endif // WINDOWS return CURLE_OK; } @@ -759,7 +758,7 @@ int64_t CurlSession::ReadFromSocket(Context const& context, uint8_t* buffer, int } } #ifdef WINDOWS - WinSocketSetBuffSize(this->m_curlSocket, this->m_logger); + WinSocketSetBuffSize(this->m_curlSocket); #endif // WINDOWS return readBytes; }