From 8c6e74eb3db9f6b235d36fe3d3deb10c0683acb5 Mon Sep 17 00:00:00 2001 From: David Goffredo Date: Thu, 9 Nov 2023 14:46:34 -0500 Subject: [PATCH] code review suggestions: - `easy_setopt_timeout` -> `easy_setopt_timeout_ms` - remove `HTTPClient::Deadline` alias - pass `Clock` into `CurlImpl` - `finalize_config(const TracerConfig&, const Clock&)` - `FinalizedTracerConfig::clock` - `finalize_config(const DatadogAgentConfig&, const Clock&)` - `default_http_client(const shared_ptr&, const Clock&)` - `FinalizedDatadogAgentConfig::clock` - `DatadogAgent::DatadogAgent` - more specific `Error::CURL_DEADLINE_EXCEEDED_BEFORE_REQUEST_START`, and a little context in the error message - `dummy_timeout` -> `dummy_deadline` - `TEST_CASE` for `Error::CURL_DEADLINE_EXCEEDED_BEFORE_REQUEST_START` --- src/datadog/curl.cpp | 61 ++++++++++++--------- src/datadog/curl.h | 13 +++-- src/datadog/datadog_agent.cpp | 3 +- src/datadog/datadog_agent.h | 2 +- src/datadog/datadog_agent_config.cpp | 7 ++- src/datadog/datadog_agent_config.h | 7 ++- src/datadog/default_http_client.h | 4 +- src/datadog/default_http_client_curl.cpp | 4 +- src/datadog/default_http_client_null.cpp | 4 +- src/datadog/error.h | 1 + src/datadog/http_client.h | 10 ++-- src/datadog/tracer.cpp | 21 +++----- src/datadog/tracer.h | 4 -- src/datadog/tracer_config.cpp | 8 ++- src/datadog/tracer_config.h | 10 +++- test/mocks/http_clients.h | 7 +-- test/test_curl.cpp | 67 +++++++++++++++--------- test/test_trace_sampler.cpp | 9 ++-- test/test_tracer.cpp | 4 +- 19 files changed, 146 insertions(+), 100 deletions(-) diff --git a/src/datadog/curl.cpp b/src/datadog/curl.cpp index f0a2010d..341fb77f 100644 --- a/src/datadog/curl.cpp +++ b/src/datadog/curl.cpp @@ -96,7 +96,7 @@ CURLcode CurlLibrary::easy_setopt_writefunction(CURL *handle, return curl_easy_setopt(handle, CURLOPT_WRITEFUNCTION, on_write); } -CURLcode CurlLibrary::easy_setopt_timeout(CURL *handle, long timeout_ms) { +CURLcode CurlLibrary::easy_setopt_timeout_ms(CURL *handle, long timeout_ms) { return curl_easy_setopt(handle, CURLOPT_TIMEOUT_MS, timeout_ms); } @@ -167,6 +167,7 @@ class CurlImpl { std::mutex mutex_; CurlLibrary &curl_; const std::shared_ptr logger_; + Clock clock_; CURLM *multi_handle_; std::unordered_set request_handles_; std::list new_handles_; @@ -174,7 +175,6 @@ class CurlImpl { int num_active_handles_; std::condition_variable no_requests_; std::thread event_loop_; - Clock clock_; struct Request { CurlLibrary *curl = nullptr; @@ -185,7 +185,7 @@ class CurlImpl { char error_buffer[CURL_ERROR_SIZE] = ""; std::unordered_map response_headers_lower; std::string response_body; - HTTPClient::Deadline deadline; + std::chrono::steady_clock::time_point deadline; ~Request(); }; @@ -228,13 +228,14 @@ class CurlImpl { static StringView trim(StringView); public: - explicit CurlImpl(const std::shared_ptr &, CurlLibrary &, - const Curl::ThreadGenerator &); + explicit CurlImpl(const std::shared_ptr &, const Clock &, + CurlLibrary &, const Curl::ThreadGenerator &); ~CurlImpl(); Expected post(const URL &url, HeadersSetter set_headers, std::string body, ResponseHandler on_response, - ErrorHandler on_error, HTTPClient::Deadline deadline); + ErrorHandler on_error, + std::chrono::steady_clock::time_point deadline); void drain(std::chrono::steady_clock::time_point deadline); }; @@ -249,21 +250,24 @@ void throw_on_error(CURLcode result) { } // namespace -Curl::Curl(const std::shared_ptr &logger) : Curl(logger, libcurl) {} +Curl::Curl(const std::shared_ptr &logger, const Clock &clock) + : Curl(logger, clock, libcurl) {} -Curl::Curl(const std::shared_ptr &logger, CurlLibrary &curl) - : Curl(logger, curl, +Curl::Curl(const std::shared_ptr &logger, const Clock &clock, + CurlLibrary &curl) + : Curl(logger, clock, curl, [](auto &&func) { return std::thread(std::move(func)); }) {} -Curl::Curl(const std::shared_ptr &logger, CurlLibrary &curl, - const Curl::ThreadGenerator &make_thread) - : impl_(new CurlImpl{logger, curl, make_thread}) {} +Curl::Curl(const std::shared_ptr &logger, const Clock &clock, + CurlLibrary &curl, const Curl::ThreadGenerator &make_thread) + : impl_(new CurlImpl{logger, clock, curl, make_thread}) {} Curl::~Curl() { delete impl_; } Expected Curl::post(const URL &url, HeadersSetter set_headers, std::string body, ResponseHandler on_response, - ErrorHandler on_error, Deadline deadline) { + ErrorHandler on_error, + std::chrono::steady_clock::time_point deadline) { return impl_->post(url, set_headers, body, on_response, on_error, deadline); } @@ -275,10 +279,11 @@ nlohmann::json Curl::config_json() const { return nlohmann::json::object({{"type", "datadog::tracing::Curl"}}); } -CurlImpl::CurlImpl(const std::shared_ptr &logger, CurlLibrary &curl, - const Curl::ThreadGenerator &make_thread) +CurlImpl::CurlImpl(const std::shared_ptr &logger, const Clock &clock, + CurlLibrary &curl, const Curl::ThreadGenerator &make_thread) : curl_(curl), logger_(logger), + clock_(clock), shutting_down_(false), num_active_handles_(0) { curl_.global_init(CURL_GLOBAL_ALL); @@ -323,11 +328,10 @@ CurlImpl::~CurlImpl() { curl_.global_cleanup(); } -Expected CurlImpl::post(const HTTPClient::URL &url, - HeadersSetter set_headers, std::string body, - ResponseHandler on_response, - ErrorHandler on_error, - HTTPClient::Deadline deadline) try { +Expected CurlImpl::post( + const HTTPClient::URL &url, HeadersSetter set_headers, std::string body, + ResponseHandler on_response, ErrorHandler on_error, + std::chrono::steady_clock::time_point deadline) try { if (multi_handle_ == nullptr) { return Error{Error::CURL_HTTP_CLIENT_NOT_RUNNING, "Unable to send request via libcurl because the HTTP client " @@ -509,10 +513,19 @@ void CurlImpl::run() { auto *request = reinterpret_cast(user_data); const auto timeout = request->deadline - clock_().tick; - if (timeout <= HTTPClient::Deadline::duration::zero()) { + if (timeout <= std::chrono::steady_clock::time_point::duration::zero()) { + std::string message; + message += + "Request deadline exceeded before request was even added to " + "libcurl " + "event loop. Deadline was "; + message += std::to_string( + -std::chrono::duration_cast(timeout) + .count()); + message += " nanoseconds ago."; request->on_error( - Error{Error::CURL_REQUEST_FAILURE, - "Request not processed as the deadline has been reached."}); + Error{Error::CURL_DEADLINE_EXCEEDED_BEFORE_REQUEST_START, + std::move(message)}); curl_.easy_cleanup(handle); delete request; @@ -520,7 +533,7 @@ void CurlImpl::run() { continue; } - throw_on_error(curl_.easy_setopt_timeout( + log_on_error(curl_.easy_setopt_timeout_ms( handle, std::chrono::duration_cast(timeout) .count())); log_on_error(curl_.multi_add_handle(multi_handle_, handle)); diff --git a/src/datadog/curl.h b/src/datadog/curl.h index 7c9cf6f7..6c1ef377 100644 --- a/src/datadog/curl.h +++ b/src/datadog/curl.h @@ -17,6 +17,7 @@ #include #include +#include "clock.h" #include "http_client.h" #include "json_fwd.hpp" @@ -59,7 +60,7 @@ class CurlLibrary { virtual CURLcode easy_setopt_url(CURL *handle, const char *url); virtual CURLcode easy_setopt_writedata(CURL *handle, void *data); virtual CURLcode easy_setopt_writefunction(CURL *handle, WriteCallback); - virtual CURLcode easy_setopt_timeout(CURL *handle, long timeout_ms); + virtual CURLcode easy_setopt_timeout_ms(CURL *handle, long timeout_ms); virtual const char *easy_strerror(CURLcode error); virtual void global_cleanup(); virtual CURLcode global_init(long flags); @@ -87,16 +88,18 @@ class Curl : public HTTPClient { public: using ThreadGenerator = std::function &&)>; - explicit Curl(const std::shared_ptr &); - Curl(const std::shared_ptr &, CurlLibrary &); - Curl(const std::shared_ptr &, CurlLibrary &, const ThreadGenerator &); + explicit Curl(const std::shared_ptr &, const Clock &); + Curl(const std::shared_ptr &, const Clock &, CurlLibrary &); + Curl(const std::shared_ptr &, const Clock &, CurlLibrary &, + const ThreadGenerator &); ~Curl(); Curl(const Curl &) = delete; Expected post(const URL &url, HeadersSetter set_headers, std::string body, ResponseHandler on_response, - ErrorHandler on_error, Deadline deadline) override; + ErrorHandler on_error, + std::chrono::steady_clock::time_point deadline) override; void drain(std::chrono::steady_clock::time_point deadline) override; diff --git a/src/datadog/datadog_agent.cpp b/src/datadog/datadog_agent.cpp index 9d47e686..362176d6 100644 --- a/src/datadog/datadog_agent.cpp +++ b/src/datadog/datadog_agent.cpp @@ -125,9 +125,8 @@ std::variant parse_agent_traces_response( } // namespace DatadogAgent::DatadogAgent(const FinalizedDatadogAgentConfig& config, - const Clock& clock, const std::shared_ptr& logger) - : clock_(clock), + : clock_(config.clock), logger_(logger), traces_endpoint_(traces_endpoint(config.url)), http_client_(config.http_client), diff --git a/src/datadog/datadog_agent.h b/src/datadog/datadog_agent.h index 700eacb2..1db2b00e 100644 --- a/src/datadog/datadog_agent.h +++ b/src/datadog/datadog_agent.h @@ -46,7 +46,7 @@ class DatadogAgent : public Collector { void flush(); public: - DatadogAgent(const FinalizedDatadogAgentConfig&, const Clock& clock, + DatadogAgent(const FinalizedDatadogAgentConfig&, const std::shared_ptr&); ~DatadogAgent(); diff --git a/src/datadog/datadog_agent_config.cpp b/src/datadog/datadog_agent_config.cpp index a10b0e56..5ba97ccc 100644 --- a/src/datadog/datadog_agent_config.cpp +++ b/src/datadog/datadog_agent_config.cpp @@ -84,11 +84,14 @@ Expected DatadogAgentConfig::parse(StringView input) { } Expected finalize_config( - const DatadogAgentConfig& config, const std::shared_ptr& logger) { + const DatadogAgentConfig& config, const std::shared_ptr& logger, + const Clock& clock) { FinalizedDatadogAgentConfig result; + result.clock = clock; + if (!config.http_client) { - result.http_client = default_http_client(logger); + result.http_client = default_http_client(logger, clock); // `default_http_client` might return a `Curl` instance depending on how // this library was built. If it returns `nullptr`, then there's no // built-in default, and so the user must provide a value. diff --git a/src/datadog/datadog_agent_config.h b/src/datadog/datadog_agent_config.h index 6e91f209..9eb96f35 100644 --- a/src/datadog/datadog_agent_config.h +++ b/src/datadog/datadog_agent_config.h @@ -16,6 +16,7 @@ #include #include +#include "clock.h" #include "expected.h" #include "http_client.h" #include "string_view.h" @@ -59,7 +60,7 @@ struct DatadogAgentConfig { class FinalizedDatadogAgentConfig { friend Expected finalize_config( - const DatadogAgentConfig& config, const std::shared_ptr& logger); + const DatadogAgentConfig&, const std::shared_ptr&, const Clock&); FinalizedDatadogAgentConfig() = default; @@ -70,10 +71,12 @@ class FinalizedDatadogAgentConfig { std::chrono::steady_clock::duration flush_interval; std::chrono::steady_clock::duration request_timeout; std::chrono::steady_clock::duration shutdown_timeout; + Clock clock; }; Expected finalize_config( - const DatadogAgentConfig& config, const std::shared_ptr& logger); + const DatadogAgentConfig& config, const std::shared_ptr& logger, + const Clock& clock); } // namespace tracing } // namespace datadog diff --git a/src/datadog/default_http_client.h b/src/datadog/default_http_client.h index 83ab410b..cfeda7e5 100644 --- a/src/datadog/default_http_client.h +++ b/src/datadog/default_http_client.h @@ -9,6 +9,8 @@ #include +#include "clock.h" + namespace datadog { namespace tracing { @@ -16,7 +18,7 @@ class HTTPClient; class Logger; std::shared_ptr default_http_client( - const std::shared_ptr& logger); + const std::shared_ptr& logger, const Clock& clock); } // namespace tracing } // namespace datadog diff --git a/src/datadog/default_http_client_curl.cpp b/src/datadog/default_http_client_curl.cpp index 1970430e..375acede 100644 --- a/src/datadog/default_http_client_curl.cpp +++ b/src/datadog/default_http_client_curl.cpp @@ -12,8 +12,8 @@ namespace datadog { namespace tracing { std::shared_ptr default_http_client( - const std::shared_ptr& logger) { - return std::make_shared(logger); + const std::shared_ptr& logger, const Clock& clock) { + return std::make_shared(logger, clock); } } // namespace tracing diff --git a/src/datadog/default_http_client_null.cpp b/src/datadog/default_http_client_null.cpp index c15f4fb6..b5a73ece 100644 --- a/src/datadog/default_http_client_null.cpp +++ b/src/datadog/default_http_client_null.cpp @@ -9,8 +9,8 @@ namespace datadog { namespace tracing { -std::shared_ptr default_http_client( - const std::shared_ptr&) { +std::shared_ptr default_http_client(const std::shared_ptr &, + const Clock &) { return nullptr; } diff --git a/src/datadog/error.h b/src/datadog/error.h index 3bbe78b3..4326f08d 100644 --- a/src/datadog/error.h +++ b/src/datadog/error.h @@ -70,6 +70,7 @@ struct Error { MULTIPLE_PROPAGATION_STYLE_ENVIRONMENT_VARIABLES = 45, DUPLICATE_PROPAGATION_STYLE = 46, ZERO_TRACE_ID = 47, + CURL_DEADLINE_EXCEEDED_BEFORE_REQUEST_START = 48, }; Code code; diff --git a/src/datadog/http_client.h b/src/datadog/http_client.h index 0c8ee09b..5cc994f2 100644 --- a/src/datadog/http_client.h +++ b/src/datadog/http_client.h @@ -36,7 +36,6 @@ class HTTPClient { // `ErrorHandler` is for errors encountered by `HTTPClient`, not for // error-indicating HTTP responses. using ErrorHandler = std::function; - using Deadline = std::chrono::steady_clock::time_point; // Send a POST request to the specified `url`. Set request headers by calling // the specified `set_headers` callback. Include the specified `body` at the @@ -45,13 +44,14 @@ class HTTPClient { // response status). Invoke the specified `on_error` if an error occurs // outside of HTTP, such as a connection failure. If an error occurs while // preparing the request, return an `Error`. - virtual Expected post(const URL& url, HeadersSetter set_headers, - std::string body, ResponseHandler on_response, - ErrorHandler on_error, Deadline deadline) = 0; + virtual Expected post( + const URL& url, HeadersSetter set_headers, std::string body, + ResponseHandler on_response, ErrorHandler on_error, + std::chrono::steady_clock::time_point deadline) = 0; // Wait until there are no more outstanding requests, or until the specified // `deadline`. - virtual void drain(Deadline deadline) = 0; + virtual void drain(std::chrono::steady_clock::time_point deadline) = 0; // Return a JSON representation of this object's configuration. The JSON // representation is an object with the following properties: diff --git a/src/datadog/tracer.cpp b/src/datadog/tracer.cpp index 16b21fff..feefc310 100644 --- a/src/datadog/tracer.cpp +++ b/src/datadog/tracer.cpp @@ -200,26 +200,18 @@ Expected extract_b3( } // namespace Tracer::Tracer(const FinalizedTracerConfig& config) - : Tracer(config, default_id_generator(config.trace_id_128_bit), - default_clock) {} + : Tracer(config, default_id_generator(config.trace_id_128_bit)) {} Tracer::Tracer(const FinalizedTracerConfig& config, const std::shared_ptr& generator) - : Tracer(config, generator, default_clock) {} - -Tracer::Tracer(const FinalizedTracerConfig& config, const Clock& clock) - : Tracer(config, default_id_generator(config.trace_id_128_bit), clock) {} - -Tracer::Tracer(const FinalizedTracerConfig& config, - const std::shared_ptr& generator, - const Clock& clock) : logger_(config.logger), collector_(/* see constructor body */), trace_sampler_( - std::make_shared(config.trace_sampler, clock)), - span_sampler_(std::make_shared(config.span_sampler, clock)), + std::make_shared(config.trace_sampler, config.clock)), + span_sampler_( + std::make_shared(config.span_sampler, config.clock)), generator_(generator), - clock_(clock), + clock_(config.clock), defaults_(std::make_shared(config.defaults)), injection_styles_(config.injection_styles), extraction_styles_(config.extraction_styles), @@ -231,8 +223,7 @@ Tracer::Tracer(const FinalizedTracerConfig& config, } else { auto& agent_config = std::get(config.collector); - collector_ = - std::make_shared(agent_config, clock, config.logger); + collector_ = std::make_shared(agent_config, config.logger); } if (config.log_on_startup) { diff --git a/src/datadog/tracer.h b/src/datadog/tracer.h index e2c54f85..a164a3a4 100644 --- a/src/datadog/tracer.h +++ b/src/datadog/tracer.h @@ -47,10 +47,6 @@ class Tracer { explicit Tracer(const FinalizedTracerConfig& config); Tracer(const FinalizedTracerConfig& config, const std::shared_ptr& generator); - Tracer(const FinalizedTracerConfig& config, const Clock& clock); - Tracer(const FinalizedTracerConfig& config, - const std::shared_ptr& generator, - const Clock& clock); // Create a new trace and return the root span of the trace. Optionally // specify a `config` indicating the attributes of the root span. diff --git a/src/datadog/tracer_config.cpp b/src/datadog/tracer_config.cpp index c4445a03..2e70b9c0 100644 --- a/src/datadog/tracer_config.cpp +++ b/src/datadog/tracer_config.cpp @@ -265,8 +265,14 @@ Expected finalize_propagation_styles(FinalizedTracerConfig &result, } // namespace Expected finalize_config(const TracerConfig &config) { + return finalize_config(config, default_clock); +} + +Expected finalize_config(const TracerConfig &config, + const Clock &clock) { FinalizedTracerConfig result; + result.clock = clock; result.defaults = config.defaults; if (auto service_env = lookup(environment::DD_SERVICE)) { @@ -314,7 +320,7 @@ Expected finalize_config(const TracerConfig &config) { if (!report_traces) { result.collector = std::make_shared(); } else if (!config.collector) { - auto finalized = finalize_config(config.agent, result.logger); + auto finalized = finalize_config(config.agent, result.logger, clock); if (auto *error = finalized.if_error()) { return std::move(*error); } diff --git a/src/datadog/tracer_config.h b/src/datadog/tracer_config.h index 275a0202..f1f87701 100644 --- a/src/datadog/tracer_config.h +++ b/src/datadog/tracer_config.h @@ -9,6 +9,7 @@ #include #include +#include "clock.h" #include "datadog_agent_config.h" #include "error.h" #include "expected.h" @@ -108,7 +109,7 @@ struct TracerConfig { // `FinalizedTracerConfig` must be obtained by calling `finalize_config`. class FinalizedTracerConfig { friend Expected finalize_config( - const TracerConfig& config); + const TracerConfig& config, const Clock& clock); FinalizedTracerConfig() = default; public: @@ -129,12 +130,19 @@ class FinalizedTracerConfig { std::shared_ptr logger; bool log_on_startup; bool trace_id_128_bit; + + Clock clock; }; // Return a `FinalizedTracerConfig` from the specified `config` and from any // relevant environment variables. If any configuration is invalid, return an // `Error`. +// Optionally specify a `clock` used to calculate span start times, span +// durations, and timeouts. If `clock` is not specified, then `default_clock` +// is used. Expected finalize_config(const TracerConfig& config); +Expected finalize_config(const TracerConfig& config, + const Clock& clock); } // namespace tracing } // namespace datadog diff --git a/test/mocks/http_clients.h b/test/mocks/http_clients.h index eea1cdf1..ad59006c 100644 --- a/test/mocks/http_clients.h +++ b/test/mocks/http_clients.h @@ -40,9 +40,10 @@ struct MockHTTPClient : public HTTPClient { ResponseHandler on_response_; ErrorHandler on_error_; - Expected post(const URL&, HeadersSetter set_headers, - std::string /*body*/, ResponseHandler on_response, - ErrorHandler on_error, Deadline /*deadline*/) override { + Expected post( + const URL&, HeadersSetter set_headers, std::string /*body*/, + ResponseHandler on_response, ErrorHandler on_error, + std::chrono::steady_clock::time_point /*deadline*/) override { std::lock_guard lock{mutex_}; if (!post_error) { on_response_ = on_response; diff --git a/test/test_curl.cpp b/test/test_curl.cpp index 357108e7..1201dd08 100644 --- a/test/test_curl.cpp +++ b/test/test_curl.cpp @@ -71,7 +71,7 @@ class SingleRequestMockCurlLibrary : public CurlLibrary { return CURLE_OK; } - CURLcode easy_setopt_timeout(CURL *, long) override { + CURLcode easy_setopt_timeout_ms(CURL *, long) override { // TBD? return CURLE_OK; } @@ -141,7 +141,7 @@ class SingleRequestMockCurlLibrary : public CurlLibrary { TEST_CASE("parse response headers and body", "[curl]") { const auto logger = std::make_shared(); SingleRequestMockCurlLibrary library; - const auto client = std::make_shared(logger, library); + const auto client = std::make_shared(logger, default_clock, library); SECTION("in the tracer") { // The tracer doesn't read response headers, at least as of this writing. @@ -210,15 +210,15 @@ TEST_CASE("bad multi-handle means error mode", "[curl]") { const auto logger = std::make_shared(); MockCurlLibrary library; - const auto client = std::make_shared(logger, library); + const auto client = std::make_shared(logger, default_clock, library); REQUIRE(logger->first_error().code == Error::CURL_HTTP_CLIENT_SETUP_FAILED); const auto ignore = [](auto &&...) {}; const HTTPClient::URL url = {"http", "whatever", ""}; - const auto dummy_timeout = + const auto dummy_deadline = std::chrono::steady_clock::now() + std::chrono::seconds(10); const auto result = - client->post(url, ignore, "dummy body", ignore, ignore, dummy_timeout); + client->post(url, ignore, "dummy body", ignore, ignore, dummy_deadline); REQUIRE_FALSE(result); REQUIRE(result.error().code == Error::CURL_HTTP_CLIENT_NOT_RUNNING); } @@ -228,19 +228,19 @@ TEST_CASE("bad std::thread means error mode", "[curl]") { // where calls to `post` always return an error. const auto logger = std::make_shared(); CurlLibrary libcurl; // the default implementation - const auto client = - std::make_shared(logger, libcurl, [](auto &&) -> std::thread { + const auto client = std::make_shared( + logger, default_clock, libcurl, [](auto &&) -> std::thread { throw std::system_error( std::make_error_code(std::errc::resource_unavailable_try_again)); }); REQUIRE(logger->first_error().code == Error::CURL_HTTP_CLIENT_SETUP_FAILED); const auto ignore = [](auto &&...) {}; - const auto dummy_timeout = + const auto dummy_deadline = std::chrono::steady_clock::now() + std::chrono::seconds(10); const HTTPClient::URL url = {"http", "whatever", ""}; const auto result = - client->post(url, ignore, "dummy body", ignore, ignore, dummy_timeout); + client->post(url, ignore, "dummy body", ignore, ignore, dummy_deadline); REQUIRE_FALSE(result); REQUIRE(result.error().code == Error::CURL_HTTP_CLIENT_NOT_RUNNING); } @@ -255,14 +255,14 @@ TEST_CASE("fail to allocate request handle", "[curl]") { const auto logger = std::make_shared(); MockCurlLibrary library; - const auto client = std::make_shared(logger, library); + const auto client = std::make_shared(logger, default_clock, library); const auto ignore = [](auto &&...) {}; const HTTPClient::URL url = {"http", "whatever", ""}; - const auto dummy_timeout = + const auto dummy_deadline = std::chrono::steady_clock::now() + std::chrono::seconds(10); const auto result = - client->post(url, ignore, "dummy body", ignore, ignore, dummy_timeout); + client->post(url, ignore, "dummy body", ignore, ignore, dummy_deadline); REQUIRE_FALSE(result); REQUIRE(result.error().code == Error::CURL_REQUEST_SETUP_FAILED); } @@ -373,7 +373,7 @@ TEST_CASE("setopt failures", "[curl]") { library.fail = which_fails; const auto logger = std::make_shared(); - const auto client = std::make_shared(logger, library); + const auto client = std::make_shared(logger, default_clock, library); const auto ignore = [](auto &&...) {}; HTTPClient::URL url; @@ -386,10 +386,10 @@ TEST_CASE("setopt failures", "[curl]") { url.path = "/trace/thing"; } - const auto dummy_timeout = + const auto dummy_deadline = std::chrono::steady_clock::now() + std::chrono::seconds(10); const auto result = - client->post(url, ignore, "dummy body", ignore, ignore, dummy_timeout); + client->post(url, ignore, "dummy body", ignore, ignore, dummy_deadline); REQUIRE_FALSE(result); REQUIRE(result.error().code == Error::CURL_REQUEST_SETUP_FAILED); } @@ -397,13 +397,13 @@ TEST_CASE("setopt failures", "[curl]") { TEST_CASE("handles are always cleaned up", "[curl]") { const auto logger = std::make_shared(); SingleRequestMockCurlLibrary library; - auto client = std::make_shared(logger, library); + auto client = std::make_shared(logger, default_clock, library); SECTION("when the response is delivered") { Optional post_error; std::exception_ptr exception; const HTTPClient::URL url = {"http", "whatever", ""}; - const auto dummy_timeout = + const auto dummy_deadline = std::chrono::steady_clock::now() + std::chrono::seconds(10); const auto result = client->post( url, [](const auto &) {}, "whatever", @@ -416,7 +416,7 @@ TEST_CASE("handles are always cleaned up", "[curl]") { exception = std::current_exception(); } }, - [&](const Error &error) { post_error = error; }, dummy_timeout); + [&](const Error &error) { post_error = error; }, dummy_deadline); REQUIRE(result); client->drain(std::chrono::steady_clock::now() + std::chrono::seconds(1)); @@ -430,12 +430,12 @@ TEST_CASE("handles are always cleaned up", "[curl]") { Optional post_error; const HTTPClient::URL url = {"http", "whatever", ""}; const auto ignore = [](auto &&...) {}; - const auto dummy_timeout = + const auto dummy_deadline = std::chrono::steady_clock::now() + std::chrono::seconds(10); library.message_result_ = CURLE_COULDNT_CONNECT; // any error would do const auto result = client->post( url, ignore, "whatever", ignore, - [&](const Error &error) { post_error = error; }, dummy_timeout); + [&](const Error &error) { post_error = error; }, dummy_deadline); REQUIRE(result); client->drain(std::chrono::steady_clock::now() + std::chrono::seconds(1)); @@ -445,11 +445,11 @@ TEST_CASE("handles are always cleaned up", "[curl]") { SECTION("when we shut down while a request is in flight") { const HTTPClient::URL url = {"http", "whatever", ""}; const auto ignore = [](auto &&...) {}; - const auto dummy_timeout = + const auto dummy_deadline = std::chrono::steady_clock::now() + std::chrono::seconds(10); library.delay_message_ = true; const auto result = - client->post(url, ignore, "whatever", ignore, ignore, dummy_timeout); + client->post(url, ignore, "whatever", ignore, ignore, dummy_deadline); REQUIRE(result); // Destroy the `Curl` object. @@ -461,5 +461,24 @@ TEST_CASE("handles are always cleaned up", "[curl]") { REQUIRE(library.created_handles_ == library.destroyed_handles_); } -// TODO: "multi_*" failures -// TODO: "getinfo" failures +TEST_CASE("post() deadline exceeded before request start", "[curl]") { + Curl client{std::make_shared(), default_clock}; + + const auto ignore = [](auto &&...) {}; + const HTTPClient::URL url = {"http", "whatever", ""}; + const std::string body; + const auto deadline = + std::chrono::steady_clock::now() - std::chrono::milliseconds(1); + Optional error_delivered; + + const auto result = client.post( + url, ignore, body, ignore, [&](Error error) { error_delivered = error; }, + deadline); + REQUIRE(result); + + client.drain(std::chrono::steady_clock::now() + std::chrono::seconds(1)); + + REQUIRE(error_delivered); + REQUIRE(error_delivered->code == + Error::CURL_DEADLINE_EXCEEDED_BEFORE_REQUEST_START); +} diff --git a/test/test_trace_sampler.cpp b/test/test_trace_sampler.cpp index 76fb469a..68a117cf 100644 --- a/test/test_trace_sampler.cpp +++ b/test/test_trace_sampler.cpp @@ -127,13 +127,14 @@ TEST_CASE("trace sampling rate limiter") { config.collector = collector; config.logger = std::make_shared(); - auto finalized = finalize_config(config); - REQUIRE(finalized); - TimePoint current_time = default_clock(); // Modify `current_time` to advance the clock. auto clock = [¤t_time]() { return current_time; }; - Tracer tracer{*finalized, clock}; + + auto finalized = finalize_config(config, clock); + REQUIRE(finalized); + + Tracer tracer{*finalized}; for (std::size_t i = 0; i < test_case.burst_size; ++i) { auto span = tracer.create_span(); diff --git a/test/test_tracer.cpp b/test/test_tracer.cpp index d21f44d8..c7c03472 100644 --- a/test/test_tracer.cpp +++ b/test/test_tracer.cpp @@ -1034,9 +1034,9 @@ TEST_CASE("128-bit trace IDs") { config.extraction_styles.push_back(PropagationStyle::W3C); config.extraction_styles.push_back(PropagationStyle::DATADOG); config.extraction_styles.push_back(PropagationStyle::B3); - const auto finalized = finalize_config(config); + const auto finalized = finalize_config(config, clock); REQUIRE(finalized); - Tracer tracer{*finalized, clock}; + Tracer tracer{*finalized}; TraceID trace_id; // used below the following SECTIONs SECTION("are generated") {