Cleanup

Author: chusitoo

ext/include/opentelemetry/ext/http/client/curl/http_client_curl.h

@@ -361,7 +361,7 @@ class HttpClient : public opentelemetry::ext::http::client::HttpClient
   std::unordered_map<uint64_t, std::shared_ptr<Session>> pending_to_abort_sessions_;
   std::unordered_map<uint64_t, HttpCurlEasyResource> pending_to_remove_session_handles_;
   std::list<std::shared_ptr<Session>> pending_to_remove_sessions_;
-  std::deque<Session *> pending_to_retry_sessions_;
+  std::deque<std::shared_ptr<Session>> pending_to_retry_sessions_;
 
   std::mutex background_thread_m_;
   std::unique_ptr<std::thread> background_thread_;

ext/src/http/client/curl/http_client_curl.cc

@@ -486,7 +486,7 @@ bool HttpClient::MaybeSpawnBackgroundThread()
 
                 if (operation->IsRetryable())
                 {
-                  self->pending_to_retry_sessions_.push_front(session);
+                  self->pending_to_retry_sessions_.push_front(hold_session);
                 }
               }
             }
@@ -790,7 +790,8 @@ bool HttpClient::doRemoveSessions()
 
 bool HttpClient::doRetrySessions()
 {
-  auto has_data = false;
+  const auto now = std::chrono::system_clock::now();
+  auto has_data  = false;
 
   // Assumptions:
   // - This is a FIFO list so older sessions, pushed at the front, always end up at the tail
@@ -807,7 +808,7 @@ bool HttpClient::doRetrySessions()
     {
       retry_it = decltype(retry_it){pending_to_retry_sessions_.erase(std::next(retry_it).base())};
     }
-    else if (operation->NextRetryTime() < std::chrono::system_clock::now())
+    else if (operation->NextRetryTime() < now)
     {
       auto easy_handle = operation->GetCurlEasyHandle();
       curl_multi_remove_handle(multi_handle_, easy_handle);

ext/src/http/client/curl/http_operation_curl.cc

@@ -289,7 +289,12 @@ HttpOperation::HttpOperation(opentelemetry::ext::http::client::Method method,
       compression_(compression),
       is_log_enabled_(is_log_enabled),
       retry_policy_(retry_policy),
-      retry_attempts_(0),
+      retry_attempts_((retry_policy.max_attempts > 0U &&
+                       retry_policy.initial_backoff > SecondsDecimal::zero() &&
+                       retry_policy.max_backoff > SecondsDecimal::zero() &&
+                       retry_policy.backoff_multiplier > 0.0f)
+                          ? 0
+                          : retry_policy.max_attempts),
       response_code_(0)
 {
   /* get a curl handle */

ext/test/http/curl_http_test.cc

@@ -350,6 +350,40 @@ TEST_F(BasicCurlHttpTests, CurlHttpOperations)
   delete handler;
 }
 
+TEST_F(BasicCurlHttpTests, RetryPolicyEnabled)
+{
+  RetryEventHandler handler;
+  http_client::HttpSslOptions no_ssl;
+  http_client::Body body;
+  http_client::Headers headers;
+  http_client::Compression compression  = http_client::Compression::kNone;
+  http_client::RetryPolicy retry_policy = {5, 1.0f, 5.0f, 1.5f};
+
+  curl::HttpOperation operation(http_client::Method::Post, "http://127.0.0.1:19000/retry/", no_ssl,
+                                &handler, headers, body, compression, false,
+                                curl::kDefaultHttpConnTimeout, false, false, retry_policy);
+
+  ASSERT_EQ(CURLE_OK, operation.Send());
+  ASSERT_TRUE(operation.IsRetryable());
+}
+
+TEST_F(BasicCurlHttpTests, RetryPolicyDisabled)
+{
+  RetryEventHandler handler;
+  http_client::HttpSslOptions no_ssl;
+  http_client::Body body;
+  http_client::Headers headers;
+  http_client::Compression compression     = http_client::Compression::kNone;
+  http_client::RetryPolicy no_retry_policy = {0, 0.0f, 0.0f, 0.0f};
+
+  curl::HttpOperation operation(http_client::Method::Post, "http://127.0.0.1:19000/retry/", no_ssl,
+                                &handler, headers, body, compression, false,
+                                curl::kDefaultHttpConnTimeout, false, false, no_retry_policy);
+
+  ASSERT_EQ(CURLE_OK, operation.Send());
+  ASSERT_FALSE(operation.IsRetryable());
+}
+
 TEST_F(BasicCurlHttpTests, ExponentialBackoffRetry)
 {
   using ::testing::AllOf;