Code review for improve http client traces

aws · Mar 28, 2024 · cb111cf · cb111cf
1 parent 58097f4
commit cb111cf
Show file tree

Hide file tree

Showing 7 changed files with 18 additions and 7 deletions.
diff --git a/src/aws-cpp-sdk-core/include/aws/core/client/ClientConfiguration.h b/src/aws-cpp-sdk-core/include/aws/core/client/ClientConfiguration.h
@@ -320,7 +320,7 @@ namespace Aws
              * Enable http client (WinHTTP or CURL) traces.
              * Defaults to false, it's an optional feature.
              */
-            bool enableHttpClientTrace;
+            bool enableHttpClientTrace = false;
 
             /**
              * profileName in config file that will be used by this object to resolve more configurations.

diff --git a/src/aws-cpp-sdk-core/source/http/curl/CurlHttpClient.cpp b/src/aws-cpp-sdk-core/source/http/curl/CurlHttpClient.cpp
@@ -590,6 +590,12 @@ int CurlDebugCallback(CURL *handle, curl_infotype type, char *data, size_t size,
     return 0;
 }
 
+#if defined(ENABLE_CURL_LOGGING)
+const bool FORCE_ENABLE_CURL_LOGGING = true;
+#else
+const bool FORCE_ENABLE_CURL_LOGGING = false;
+#endif
+
 
 CurlHttpClient::CurlHttpClient(const ClientConfiguration& clientConfig) :
     Base(),
@@ -603,7 +609,7 @@ CurlHttpClient::CurlHttpClient(const ClientConfiguration& clientConfig) :
     m_proxyPort(clientConfig.proxyPort), m_verifySSL(clientConfig.verifySSL), m_caPath(clientConfig.caPath),
     m_caFile(clientConfig.caFile), m_proxyCaPath(clientConfig.proxyCaPath), m_proxyCaFile(clientConfig.proxyCaFile),
     m_disableExpectHeader(clientConfig.disableExpectHeader),
-    m_enableHttpClientTrace(clientConfig.enableHttpClientTrace),
+    m_enableHttpClientTrace(clientConfig.enableHttpClientTrace || FORCE_ENABLE_CURL_LOGGING),
     m_telemetryProvider(clientConfig.telemetryProvider)
 {
     if (clientConfig.followRedirects == FollowRedirectsPolicy::NEVER ||
@@ -750,14 +756,12 @@ std::shared_ptr<HttpResponse> CurlHttpClient::MakeRequest(const std::shared_ptr<
             curl_easy_setopt(connectionHandle, CURLOPT_FOLLOWLOCATION, 0L);
         }
 
-#if defined(ENABLE_CURL_LOGGING) || m_enableHttpClientTrace
         if (m_enableHttpClientTrace)
         {
-            AWS_LOGSTREAM_DEBUG(CURL_HTTP_CLIENT_TAG, "Activating CURL traces");
+            AWS_LOGSTREAM_TRACE(CURL_HTTP_CLIENT_TAG, "Activating CURL traces");
+            curl_easy_setopt(connectionHandle, CURLOPT_VERBOSE, 1);
+            curl_easy_setopt(connectionHandle, CURLOPT_DEBUGFUNCTION, CurlDebugCallback);
         }
-        curl_easy_setopt(connectionHandle, CURLOPT_VERBOSE, 1);
-        curl_easy_setopt(connectionHandle, CURLOPT_DEBUGFUNCTION, CurlDebugCallback);
-#endif
 
         if (m_isUsingProxy)
         {

diff --git a/src/aws-cpp-sdk-core/source/http/windows/WinHttpSyncHttpClient.cpp b/src/aws-cpp-sdk-core/source/http/windows/WinHttpSyncHttpClient.cpp
@@ -21,6 +21,7 @@
 #include <winhttp.h>
 #include <sstream>
 #include <iostream>
+#include <versionhelpers.h>
 
 using namespace Aws::Client;
 using namespace Aws::Http;
@@ -218,7 +219,9 @@ static void CALLBACK WinHttpSyncLogCallback(HINTERNET hInternet,
         }//found handler
     }
     if (!found)
+    {
         AWS_LOGSTREAM_DEBUG("WinHttp", "got unrecognized internet status " << dwInternetStatus);
+    }
 }
 
 static void WinHttpEnableHttp2(void* handle)

diff --git a/tests/aws-cpp-sdk-dynamodb-integration-tests/TableOperationTest.cpp b/tests/aws-cpp-sdk-dynamodb-integration-tests/TableOperationTest.cpp
@@ -175,6 +175,7 @@ class TableOperationTest : public ::testing::Test {
         config.httpLibOverride = transferType;
         config.executor = Aws::MakeShared<Aws::Utils::Threading::PooledThreadExecutor>(ALLOCATION_TAG, 4);
         config.disableExpectHeader = true;
+        config.enableHttpClientTrace = true;
 
         //to test proxy functionality, uncomment the next two lines.
         //config.proxyHost = "localhost";

diff --git a/tests/aws-cpp-sdk-s3-integration-tests/BucketAndObjectOperationTest.cpp b/tests/aws-cpp-sdk-s3-integration-tests/BucketAndObjectOperationTest.cpp
@@ -178,6 +178,7 @@ namespace
             config.readRateLimiter = Limiter;
             config.writeRateLimiter = Limiter;
             config.executor = Aws::MakeShared<Aws::Utils::Threading::PooledThreadExecutor>(ALLOCATION_TAG, 4);
+            config.enableHttpClientTrace = true;
 
             //to use a proxy, uncomment the next two lines.
             if (USE_PROXY_FOR_TESTS)

diff --git a/tests/aws-cpp-sdk-s3-integration-tests/S3ExpressTest.cpp b/tests/aws-cpp-sdk-s3-integration-tests/S3ExpressTest.cpp
@@ -300,6 +300,7 @@ namespace {
     void SetUp() override {
       S3ClientConfiguration configuration;
       configuration.region = "us-east-1";
+      configuration.enableHttpClientTrace = true;
       client = Aws::MakeShared<S3Client>(ALLOCATION_TAG, configuration);
     }
 

diff --git a/tests/aws-cpp-sdk-transcribestreaming-integ-tests/TranscribeTests.cpp b/tests/aws-cpp-sdk-transcribestreaming-integ-tests/TranscribeTests.cpp
@@ -51,6 +51,7 @@ class TranscribeStreamingTests : public Aws::Testing::AwsCppSdkGTestSuite
         }
 
         Aws::Client::ClientConfiguration config;
+        config.enableHttpClientTrace = true;
 #ifdef _WIN32
         // TODO: remove this once we get H2 working with WinHttp client
         config.httpLibOverride = Aws::Http::TransferLibType::WIN_INET_CLIENT;