diff --git a/bazel/repository_locations.bzl b/bazel/repository_locations.bzl index 3dbac5e7b850c..73a5e421ea277 100644 --- a/bazel/repository_locations.bzl +++ b/bazel/repository_locations.bzl @@ -535,13 +535,13 @@ REPOSITORY_LOCATIONS_SPEC = dict( project_name = "Datadog C++ Tracing Library", project_desc = "Datadog distributed tracing for C++", project_url = "https://github.com/DataDog/dd-trace-cpp", - version = "0.1.8", - sha256 = "77162c26ba976b8b18e6daf50beaec39389286840733b08e1627d4e5572d4b51", + version = "0.1.12", + sha256 = "9609a9192fecf730473743662e9f5ed57b4c348c12a0e16dd11ed2e592462ebe", strip_prefix = "dd-trace-cpp-{version}", urls = ["https://github.com/DataDog/dd-trace-cpp/archive/v{version}.tar.gz"], use_category = ["observability_ext"], extensions = ["envoy.tracers.datadog"], - release_date = "2023-03-31", + release_date = "2023-11-17", cpe = "N/A", license = "Apache-2.0", license_url = "https://github.com/DataDog/dd-trace-cpp/blob/v{version}/LICENSE.md", diff --git a/source/extensions/tracers/datadog/agent_http_client.cc b/source/extensions/tracers/datadog/agent_http_client.cc index 37a9fad328cac..670dc0b79243c 100644 --- a/source/extensions/tracers/datadog/agent_http_client.cc +++ b/source/extensions/tracers/datadog/agent_http_client.cc @@ -24,9 +24,9 @@ namespace Datadog { AgentHTTPClient::AgentHTTPClient(Upstream::ClusterManager& cluster_manager, const std::string& cluster, const std::string& reference_host, - TracerStats& stats) + TracerStats& stats, TimeSource& time_source) : collector_cluster_(cluster_manager, cluster), cluster_(cluster), - reference_host_(reference_host), stats_(stats) {} + reference_host_(reference_host), stats_(stats), time_source_(time_source) {} AgentHTTPClient::~AgentHTTPClient() { for (const auto& [request_ptr, _] : handlers_) { @@ -38,12 +38,10 @@ AgentHTTPClient::~AgentHTTPClient() { // datadog::tracing::HTTPClient -datadog::tracing::Expected AgentHTTPClient::post(const URL& url, HeadersSetter set_headers, - std::string body, - ResponseHandler on_response, - ErrorHandler on_error) { - ENVOY_LOG(debug, "flushing traces"); - +datadog::tracing::Expected +AgentHTTPClient::post(const URL& url, HeadersSetter set_headers, std::string body, + ResponseHandler on_response, ErrorHandler on_error, + std::chrono::steady_clock::time_point deadline) { auto message = std::make_unique(); Http::RequestHeaderMap& headers = message->headers(); headers.setReferenceMethod(Http::Headers::get().MethodValues.Post); @@ -54,7 +52,23 @@ datadog::tracing::Expected AgentHTTPClient::post(const URL& url, HeadersSe set_headers(writer); message->body().add(body); - ENVOY_LOG(debug, "submitting trace(s) to {} with payload size {}", url.path, body.size()); + + const auto timeout = std::chrono::duration_cast( + deadline - time_source_.monotonicTime()); + if (timeout.count() <= 0) { + std::string message = "request deadline expired already"; + if (timeout.count() < 0) { + message += ' '; + message += std::to_string(-timeout.count()); + message += " milliseconds ago"; + } + stats_.reports_dropped_.inc(); + return datadog::tracing::Error{datadog::tracing::Error::ENVOY_HTTP_CLIENT_FAILURE, + std::move(message)}; + } + + ENVOY_LOG(debug, "submitting data to {} with payload size {} and {} ms timeout", url.path, + body.size(), timeout.count()); if (!collector_cluster_.threadLocalCluster().has_value()) { ENVOY_LOG(debug, "collector cluster '{}' does not exist", cluster_); @@ -64,8 +78,7 @@ datadog::tracing::Expected AgentHTTPClient::post(const URL& url, HeadersSe Http::AsyncClient::Request* request = collector_cluster_.threadLocalCluster()->get().httpAsyncClient().send( - std::move(message), *this, - Http::AsyncClient::RequestOptions().setTimeout(std::chrono::milliseconds(1000))); + std::move(message), *this, Http::AsyncClient::RequestOptions().setTimeout(timeout)); if (!request) { stats_.reports_failed_.inc(); return datadog::tracing::Error{datadog::tracing::Error::ENVOY_HTTP_CLIENT_FAILURE, diff --git a/source/extensions/tracers/datadog/agent_http_client.h b/source/extensions/tracers/datadog/agent_http_client.h index 900a101abb528..2a38d4ce23802 100644 --- a/source/extensions/tracers/datadog/agent_http_client.h +++ b/source/extensions/tracers/datadog/agent_http_client.h @@ -45,9 +45,10 @@ class AgentHTTPClient : public datadog::tracing::HTTPClient, * @param reference_host the value to use for the "Host" HTTP request header * @param stats a collection of counters used to keep track of events, such as * when a request fails + * @param time_source clocks used for calculating request timeouts */ AgentHTTPClient(Upstream::ClusterManager& cluster_manager, const std::string& cluster, - const std::string& reference_host, TracerStats& stats); + const std::string& reference_host, TracerStats& stats, TimeSource& time_source); ~AgentHTTPClient() override; // datadog::tracing::HTTPClient @@ -66,13 +67,14 @@ class AgentHTTPClient : public datadog::tracing::HTTPClient, * @param body data to send as POST request body * @param on_response callback to invoke when a complete response is received * @param on_error callback to invoke when an error occurs before a complete - * response is received. + * response is received + * @param deadline time after which a response is not expected * @return \c datadog::tracing::Error if an error occurs, or - * \c datadog::tracing::nullopt otherwise. + * \c datadog::tracing::nullopt otherwise */ datadog::tracing::Expected post(const URL& url, HeadersSetter set_headers, std::string body, - ResponseHandler on_response, - ErrorHandler on_error) override; + ResponseHandler on_response, ErrorHandler on_error, + std::chrono::steady_clock::time_point deadline) override; /** * \c drain has no effect. It's a part of the \c datadog::tracing::HTTPClient @@ -98,6 +100,7 @@ class AgentHTTPClient : public datadog::tracing::HTTPClient, const std::string cluster_; const std::string reference_host_; TracerStats& stats_; + TimeSource& time_source_; }; } // namespace Datadog diff --git a/source/extensions/tracers/datadog/config.cc b/source/extensions/tracers/datadog/config.cc index 56d49e7e3094d..aa1e4832debc3 100644 --- a/source/extensions/tracers/datadog/config.cc +++ b/source/extensions/tracers/datadog/config.cc @@ -1,5 +1,6 @@ #include "source/extensions/tracers/datadog/config.h" +#include #include #include @@ -43,10 +44,11 @@ std::string DatadogTracerFactory::makeCollectorReferenceHost( Tracing::DriverSharedPtr DatadogTracerFactory::createTracerDriverTyped( const envoy::config::trace::v3::DatadogConfig& proto_config, Server::Configuration::TracerFactoryContext& context) { + auto& factory_context = context.serverFactoryContext(); return std::make_shared( proto_config.collector_cluster(), makeCollectorReferenceHost(proto_config), - makeConfig(proto_config), context.serverFactoryContext().clusterManager(), - context.serverFactoryContext().scope(), context.serverFactoryContext().threadLocal()); + makeConfig(proto_config), factory_context.clusterManager(), factory_context.scope(), + factory_context.threadLocal(), factory_context.timeSource()); } /** diff --git a/source/extensions/tracers/datadog/tracer.cc b/source/extensions/tracers/datadog/tracer.cc index c3acc38a15eff..b24265a777f42 100644 --- a/source/extensions/tracers/datadog/tracer.cc +++ b/source/extensions/tracers/datadog/tracer.cc @@ -31,11 +31,12 @@ namespace { std::shared_ptr makeThreadLocalTracer( datadog::tracing::TracerConfig config, Upstream::ClusterManager& cluster_manager, const std::string& collector_cluster, const std::string& collector_reference_host, - TracerStats& tracer_stats, Event::Dispatcher& dispatcher, spdlog::logger& logger) { + TracerStats& tracer_stats, Event::Dispatcher& dispatcher, spdlog::logger& logger, + TimeSource& time_source) { config.logger = std::make_shared(logger); config.agent.event_scheduler = std::make_shared(dispatcher); config.agent.http_client = std::make_shared( - cluster_manager, collector_cluster, collector_reference_host, tracer_stats); + cluster_manager, collector_cluster, collector_reference_host, tracer_stats, time_source); datadog::tracing::Expected maybe_config = datadog::tracing::finalize_config(config); @@ -57,7 +58,7 @@ Tracer::ThreadLocalTracer::ThreadLocalTracer(const datadog::tracing::FinalizedTr Tracer::Tracer(const std::string& collector_cluster, const std::string& collector_reference_host, const datadog::tracing::TracerConfig& config, Upstream::ClusterManager& cluster_manager, Stats::Scope& scope, - ThreadLocal::SlotAllocator& thread_local_slot_allocator) + ThreadLocal::SlotAllocator& thread_local_slot_allocator, TimeSource& time_source) : tracer_stats_(makeTracerStats(scope)), thread_local_slot_( ThreadLocal::TypedSlot::makeUnique(thread_local_slot_allocator)) { @@ -66,10 +67,11 @@ Tracer::Tracer(const std::string& collector_cluster, const std::string& collecto allow_added_via_api); thread_local_slot_->set([&logger = ENVOY_LOGGER(), collector_cluster, collector_reference_host, - config, &tracer_stats = tracer_stats_, - &cluster_manager](Event::Dispatcher& dispatcher) { + config, &tracer_stats = tracer_stats_, &cluster_manager, + &time_source](Event::Dispatcher& dispatcher) { return makeThreadLocalTracer(config, cluster_manager, collector_cluster, - collector_reference_host, tracer_stats, dispatcher, logger); + collector_reference_host, tracer_stats, dispatcher, logger, + time_source); }); } diff --git a/source/extensions/tracers/datadog/tracer.h b/source/extensions/tracers/datadog/tracer.h index 670f382fc3053..e6334161bc013 100644 --- a/source/extensions/tracers/datadog/tracer.h +++ b/source/extensions/tracers/datadog/tracer.h @@ -51,12 +51,13 @@ class Tracer : public Tracing::Driver, private Logger::Loggable on_response_; testing::MockFunction on_error_; + Event::SimulatedTimeSystem time_; }; TEST_F(DatadogAgentHttpClientTest, PathFromURL) { @@ -86,7 +87,8 @@ TEST_F(DatadogAgentHttpClientTest, PathFromURL) { EXPECT_CALL(request_, cancel()); const auto ignore = [](auto&&...) {}; - datadog::tracing::Expected result = client_.post(url_, ignore, "", ignore, ignore); + datadog::tracing::Expected result = client_.post( + url_, ignore, "", ignore, ignore, time_.monotonicTime() + std::chrono::seconds(1)); EXPECT_TRUE(result) << result.error(); EXPECT_EQ(0, stats_.reports_skipped_no_cluster_.value()); EXPECT_EQ(0, stats_.reports_failed_.value()); @@ -98,10 +100,11 @@ TEST_F(DatadogAgentHttpClientTest, MissingThreadLocalCluster) { // the "reports skipped no cluster" counter. NiceMock cluster_manager; - AgentHTTPClient client(cluster_manager, "fake_cluster", "test_host", stats_); + AgentHTTPClient client(cluster_manager, "fake_cluster", "test_host", stats_, time_); const auto ignore = [](auto&&...) {}; - datadog::tracing::Expected result = client.post(url_, ignore, "", ignore, ignore); + datadog::tracing::Expected result = client.post( + url_, ignore, "", ignore, ignore, time_.monotonicTime() + std::chrono::seconds(1)); EXPECT_TRUE(result) << result.error(); EXPECT_EQ(1, stats_.reports_skipped_no_cluster_.value()); EXPECT_EQ(0, stats_.reports_failed_.value()); @@ -137,7 +140,8 @@ TEST_F(DatadogAgentHttpClientTest, RequestHeaders) { EXPECT_CALL(request_, cancel()); const auto ignore = [](auto&&...) {}; - datadog::tracing::Expected result = client_.post(url_, set_headers, "", ignore, ignore); + datadog::tracing::Expected result = client_.post( + url_, set_headers, "", ignore, ignore, time_.monotonicTime() + std::chrono::seconds(1)); EXPECT_TRUE(result) << result.error(); EXPECT_EQ(0, stats_.reports_skipped_no_cluster_.value()); EXPECT_EQ(0, stats_.reports_failed_.value()); @@ -176,7 +180,8 @@ TEST_F(DatadogAgentHttpClientTest, RequestBody) { EXPECT_CALL(request_, cancel()); const auto ignore = [](auto&&...) {}; - datadog::tracing::Expected result = client_.post(url_, ignore, body, ignore, ignore); + datadog::tracing::Expected result = client_.post( + url_, ignore, body, ignore, ignore, time_.monotonicTime() + std::chrono::seconds(1)); EXPECT_TRUE(result) << result.error(); EXPECT_EQ(0, stats_.reports_skipped_no_cluster_.value()); EXPECT_EQ(0, stats_.reports_failed_.value()); @@ -207,7 +212,8 @@ TEST_F(DatadogAgentHttpClientTest, OnResponse200) { const auto ignore = [](auto&&...) {}; datadog::tracing::Expected result = - client_.post(url_, ignore, "{}", on_response_.AsStdFunction(), on_error_.AsStdFunction()); + client_.post(url_, ignore, "{}", on_response_.AsStdFunction(), on_error_.AsStdFunction(), + time_.monotonicTime() + std::chrono::seconds(1)); EXPECT_TRUE(result) << result.error(); Http::ResponseMessagePtr msg(new Http::ResponseMessageImpl( @@ -246,7 +252,8 @@ TEST_F(DatadogAgentHttpClientTest, OnResponseNot200) { const auto ignore = [](auto&&...) {}; datadog::tracing::Expected result = - client_.post(url_, ignore, "{}", on_response_.AsStdFunction(), on_error_.AsStdFunction()); + client_.post(url_, ignore, "{}", on_response_.AsStdFunction(), on_error_.AsStdFunction(), + time_.monotonicTime() + std::chrono::seconds(1)); EXPECT_TRUE(result) << result.error(); // The "404" below is what causes `stats.reports_failed_` to be incremented @@ -287,7 +294,8 @@ TEST_F(DatadogAgentHttpClientTest, OnResponseBogusRequest) { const auto ignore = [](auto&&...) {}; datadog::tracing::Expected result = - client_.post(url_, ignore, "{}", on_response_.AsStdFunction(), on_error_.AsStdFunction()); + client_.post(url_, ignore, "{}", on_response_.AsStdFunction(), on_error_.AsStdFunction(), + time_.monotonicTime() + std::chrono::seconds(1)); EXPECT_TRUE(result) << result.error(); Http::ResponseMessagePtr msg(new Http::ResponseMessageImpl( @@ -327,7 +335,8 @@ TEST_F(DatadogAgentHttpClientTest, OnErrorStreamReset) { const auto ignore = [](auto&&...) {}; datadog::tracing::Expected result = - client_.post(url_, ignore, "{}", on_response_.AsStdFunction(), on_error_.AsStdFunction()); + client_.post(url_, ignore, "{}", on_response_.AsStdFunction(), on_error_.AsStdFunction(), + time_.monotonicTime() + std::chrono::seconds(1)); EXPECT_TRUE(result) << result.error(); Http::ResponseMessagePtr msg(new Http::ResponseMessageImpl( @@ -363,7 +372,8 @@ TEST_F(DatadogAgentHttpClientTest, OnErrorOther) { const auto ignore = [](auto&&...) {}; datadog::tracing::Expected result = - client_.post(url_, ignore, "{}", on_response_.AsStdFunction(), on_error_.AsStdFunction()); + client_.post(url_, ignore, "{}", on_response_.AsStdFunction(), on_error_.AsStdFunction(), + time_.monotonicTime() + std::chrono::seconds(1)); EXPECT_TRUE(result) << result.error(); Http::ResponseMessagePtr msg(new Http::ResponseMessageImpl( @@ -396,7 +406,8 @@ TEST_F(DatadogAgentHttpClientTest, OnErrorBogusRequest) { const auto ignore = [](auto&&...) {}; datadog::tracing::Expected result = - client_.post(url_, ignore, "{}", on_response_.AsStdFunction(), on_error_.AsStdFunction()); + client_.post(url_, ignore, "{}", on_response_.AsStdFunction(), on_error_.AsStdFunction(), + time_.monotonicTime() + std::chrono::seconds(1)); EXPECT_TRUE(result) << result.error(); Http::ResponseMessagePtr msg(new Http::ResponseMessageImpl( @@ -439,13 +450,53 @@ TEST_F(DatadogAgentHttpClientTest, SendFailReturnsError) { const auto ignore = [](auto&&...) {}; datadog::tracing::Expected result = - client_.post(url_, ignore, "", on_response_.AsStdFunction(), on_error_.AsStdFunction()); + client_.post(url_, ignore, "", on_response_.AsStdFunction(), on_error_.AsStdFunction(), + time_.monotonicTime() + std::chrono::seconds(1)); ASSERT_FALSE(result); EXPECT_EQ(datadog::tracing::Error::ENVOY_HTTP_CLIENT_FAILURE, result.error().code); EXPECT_EQ(1, stats_.reports_failed_.value()); EXPECT_EQ(0, stats_.reports_skipped_no_cluster_.value()); } +TEST_F(DatadogAgentHttpClientTest, SendCalculatedTimeoutIsZero) { + // If the `deadline` argument to `AgentHTTPClient::post` is such that the + // timeout calculated by `post` truncates to exactly zero milliseconds, then + // `post` will return an error and increment `stats_.reports_dropped_`. + + NiceMock cluster_manager; + AgentHTTPClient client(cluster_manager, "fake_cluster", "test_host", stats_, time_); + + const auto ignore = [](auto&&...) {}; + const auto deadline = time_.monotonicTime() + std::chrono::seconds(1); + time_.setMonotonicTime(deadline); + datadog::tracing::Expected result = client.post(url_, ignore, "", ignore, ignore, deadline); + ASSERT_FALSE(result); + EXPECT_EQ(datadog::tracing::Error::ENVOY_HTTP_CLIENT_FAILURE, result.error().code); + EXPECT_EQ(1, stats_.reports_dropped_.value()); + EXPECT_EQ(0, stats_.reports_skipped_no_cluster_.value()); + EXPECT_EQ(0, stats_.reports_failed_.value()); +} + +TEST_F(DatadogAgentHttpClientTest, SendCalculatedTimeoutIsNegative) { + // If the `deadline` argument to `AgentHTTPClient::post` is such that the + // timeout calculated by `post` truncates to a negative number of + // milliseconds, then `post` will return an error and increment + // `stats_.reports_dropped_`. + + NiceMock cluster_manager; + AgentHTTPClient client(cluster_manager, "fake_cluster", "test_host", stats_, time_); + + const auto ignore = [](auto&&...) {}; + const auto deadline = time_.monotonicTime() + std::chrono::seconds(1); + time_.setMonotonicTime(deadline + std::chrono::seconds(1)); + datadog::tracing::Expected result = client.post(url_, ignore, "", ignore, ignore, deadline); + ASSERT_FALSE(result); + EXPECT_EQ(datadog::tracing::Error::ENVOY_HTTP_CLIENT_FAILURE, result.error().code); + EXPECT_EQ(1, stats_.reports_dropped_.value()); + EXPECT_EQ(0, stats_.reports_skipped_no_cluster_.value()); + EXPECT_EQ(0, stats_.reports_failed_.value()); +} + TEST_F(DatadogAgentHttpClientTest, DrainIsANoOp) { // `AgentHTTPClient::drain` doesn't do anything. It only makes sense in // multi-threaded contexts. @@ -485,7 +536,8 @@ TEST_F(DatadogAgentHttpClientTest, SkipReportIfCollectorClusterHasBeenRemoved) { // Attempt to send a request. const auto ignore = [](auto&&...) {}; - datadog::tracing::Expected result = client_.post(url_, ignore, "", ignore, ignore); + datadog::tracing::Expected result = client_.post( + url_, ignore, "", ignore, ignore, time_.monotonicTime() + std::chrono::seconds(1)); EXPECT_TRUE(result); // Verify observability. @@ -507,7 +559,8 @@ TEST_F(DatadogAgentHttpClientTest, SkipReportIfCollectorClusterHasBeenRemoved) { // Attempt to send a request. const auto ignore = [](auto&&...) {}; - datadog::tracing::Expected result = client_.post(url_, ignore, "", ignore, ignore); + datadog::tracing::Expected result = client_.post( + url_, ignore, "", ignore, ignore, time_.monotonicTime() + std::chrono::seconds(1)); EXPECT_TRUE(result); // Verify observability. @@ -531,7 +584,8 @@ TEST_F(DatadogAgentHttpClientTest, SkipReportIfCollectorClusterHasBeenRemoved) { // Attempt to send a request. const auto ignore = [](auto&&...) {}; - datadog::tracing::Expected result = client_.post(url_, ignore, "", ignore, ignore); + datadog::tracing::Expected result = client_.post( + url_, ignore, "", ignore, ignore, time_.monotonicTime() + std::chrono::seconds(1)); EXPECT_TRUE(result); // Complete in-flight request. @@ -558,7 +612,8 @@ TEST_F(DatadogAgentHttpClientTest, SkipReportIfCollectorClusterHasBeenRemoved) { // Attempt to send a request. const auto ignore = [](auto&&...) {}; - datadog::tracing::Expected result = client_.post(url_, ignore, "", ignore, ignore); + datadog::tracing::Expected result = client_.post( + url_, ignore, "", ignore, ignore, time_.monotonicTime() + std::chrono::seconds(1)); EXPECT_TRUE(result); // Complete in-flight request. diff --git a/test/extensions/tracers/datadog/config_test.cc b/test/extensions/tracers/datadog/config_test.cc index 1a412108253e9..2ceaa128e4627 100644 --- a/test/extensions/tracers/datadog/config_test.cc +++ b/test/extensions/tracers/datadog/config_test.cc @@ -68,7 +68,7 @@ class DatadogConfigTest : public testing::Test { tracer_ = std::make_unique( datadog_config.collector_cluster(), DatadogTracerFactory::makeCollectorReferenceHost(datadog_config), - DatadogTracerFactory::makeConfig(datadog_config), cm_, *stats_.rootScope(), tls_); + DatadogTracerFactory::makeConfig(datadog_config), cm_, *stats_.rootScope(), tls_, time_); } void setupValidDriver() { @@ -91,6 +91,7 @@ class DatadogConfigTest : public testing::Test { NiceMock config_; std::unique_ptr tracer_; + Event::SimulatedTimeSystem time_; }; TEST_F(DatadogConfigTest, ConfigureTracer) { @@ -111,9 +112,35 @@ TEST_F(DatadogConfigTest, ConfigureTracer) { { auto datadog_config = makeConfig("collector_cluster: fake_cluster"); - cm_.initializeClusters({"fake_cluster"}, {}); + + EXPECT_CALL(tls_.dispatcher_, createTimer_(testing::_)); + Http::MockAsyncClientRequest request(&cm_.thread_local_cluster_.async_client_); + Http::AsyncClient::Callbacks* callbacks; + EXPECT_CALL(cm_.thread_local_cluster_.async_client_, send_(_, _, _)) + .WillOnce( + Invoke([&](Http::RequestMessagePtr&, Http::AsyncClient::Callbacks& callbacks_arg, + const Http::AsyncClient::RequestOptions&) -> Http::AsyncClient::Request* { + callbacks = &callbacks_arg; + return &request; + })); + setup(datadog_config, true); + + Http::ResponseMessagePtr msg(new Http::ResponseMessageImpl( + Http::ResponseHeaderMapPtr{new Http::TestResponseHeaderMapImpl{{":status", "202"}}})); + msg->body().add("{}"); + callbacks->onSuccess(request, std::move(msg)); + + EXPECT_CALL(cm_.thread_local_cluster_.async_client_, send_(_, _, _)) + .WillOnce( + Invoke([&](Http::RequestMessagePtr&, Http::AsyncClient::Callbacks& callbacks_arg, + const Http::AsyncClient::RequestOptions&) -> Http::AsyncClient::Request* { + callbacks = &callbacks_arg; + return &request; + })); + EXPECT_CALL(request, cancel()); + tracer_.reset(); } } @@ -144,7 +171,33 @@ TEST_F(DatadogConfigTest, AllowCollectorClusterToBeAddedViaApi) { auto datadog_config = makeConfig("collector_cluster: fake_cluster"); + EXPECT_CALL(tls_.dispatcher_, createTimer_(testing::_)); + Http::MockAsyncClientRequest request(&cm_.thread_local_cluster_.async_client_); + Http::AsyncClient::Callbacks* callbacks; + EXPECT_CALL(cm_.thread_local_cluster_.async_client_, send_(_, _, _)) + .WillOnce( + Invoke([&](Http::RequestMessagePtr&, Http::AsyncClient::Callbacks& callbacks_arg, + const Http::AsyncClient::RequestOptions&) -> Http::AsyncClient::Request* { + callbacks = &callbacks_arg; + return &request; + })); + setup(datadog_config, true); + + Http::ResponseMessagePtr msg(new Http::ResponseMessageImpl( + Http::ResponseHeaderMapPtr{new Http::TestResponseHeaderMapImpl{{":status", "202"}}})); + msg->body().add("{}"); + callbacks->onSuccess(request, std::move(msg)); + + EXPECT_CALL(cm_.thread_local_cluster_.async_client_, send_(_, _, _)) + .WillOnce( + Invoke([&](Http::RequestMessagePtr&, Http::AsyncClient::Callbacks& callbacks_arg, + const Http::AsyncClient::RequestOptions&) -> Http::AsyncClient::Request* { + callbacks = &callbacks_arg; + return &request; + })); + EXPECT_CALL(request, cancel()); + tracer_.reset(); } TEST_F(DatadogConfigTest, CollectorHostname) { @@ -155,17 +208,33 @@ TEST_F(DatadogConfigTest, CollectorHostname) { collector_hostname: fake_host )EOF"); cm_.initializeClusters({"fake_cluster"}, {}); - setup(datadog_config, true); + EXPECT_CALL(tls_.dispatcher_, createTimer_(testing::_)); Http::MockAsyncClientRequest request(&cm_.thread_local_cluster_.async_client_); - Http::AsyncClient::Callbacks* callback; - const absl::optional timeout(std::chrono::seconds(1)); - EXPECT_CALL(cm_.thread_local_cluster_.async_client_, - send_(_, _, Http::AsyncClient::RequestOptions().setTimeout(timeout))) + Http::AsyncClient::Callbacks* callbacks; + EXPECT_CALL(cm_.thread_local_cluster_.async_client_, send_(_, _, _)) + .WillOnce( + Invoke([&](Http::RequestMessagePtr& message, Http::AsyncClient::Callbacks& callbacks_arg, + const Http::AsyncClient::RequestOptions&) -> Http::AsyncClient::Request* { + callbacks = &callbacks_arg; + + EXPECT_EQ("fake_host", message->headers().getHostValue()); + + return &request; + })); + + setup(datadog_config, true); + + Http::ResponseMessagePtr msg(new Http::ResponseMessageImpl( + Http::ResponseHeaderMapPtr{new Http::TestResponseHeaderMapImpl{{":status", "202"}}})); + msg->body().add("{}"); + callbacks->onSuccess(request, std::move(msg)); + + EXPECT_CALL(cm_.thread_local_cluster_.async_client_, send_(_, _, _)) .WillOnce( - Invoke([&](Http::RequestMessagePtr& message, Http::AsyncClient::Callbacks& callbacks, + Invoke([&](Http::RequestMessagePtr& message, Http::AsyncClient::Callbacks& callbacks_arg, const Http::AsyncClient::RequestOptions&) -> Http::AsyncClient::Request* { - callback = &callbacks; + callbacks = &callbacks_arg; // This is the crux of this test. EXPECT_EQ("fake_host", message->headers().getHostValue()); @@ -182,9 +251,23 @@ TEST_F(DatadogConfigTest, CollectorHostname) { timer_->invokeCallback(); - Http::ResponseMessagePtr msg(new Http::ResponseMessageImpl(Http::ResponseHeaderMapPtr{ - new Http::TestResponseHeaderMapImpl{{":status", "200"}, {"content-length", "0"}}})); - callback->onSuccess(request, std::move(msg)); + msg.reset(new Http::ResponseMessageImpl( + Http::ResponseHeaderMapPtr{new Http::TestResponseHeaderMapImpl{{":status", "200"}}})); + msg->body().add("{}"); + callbacks->onSuccess(request, std::move(msg)); + + EXPECT_CALL(cm_.thread_local_cluster_.async_client_, send_(_, _, _)) + .WillOnce( + Invoke([&](Http::RequestMessagePtr& message, Http::AsyncClient::Callbacks& callbacks_arg, + const Http::AsyncClient::RequestOptions&) -> Http::AsyncClient::Request* { + callbacks = &callbacks_arg; + + EXPECT_EQ("fake_host", message->headers().getHostValue()); + + return &request; + })); + EXPECT_CALL(request, cancel()); + tracer_.reset(); } } // namespace diff --git a/test/extensions/tracers/datadog/naming_test.cc b/test/extensions/tracers/datadog/naming_test.cc index 0d45cde32b1ac..b98182179bb20 100644 --- a/test/extensions/tracers/datadog/naming_test.cc +++ b/test/extensions/tracers/datadog/naming_test.cc @@ -108,7 +108,8 @@ void DatadogTracerNamingTest::serviceNameTest(const std::string& config_yaml, DatadogTracerFactory::makeConfig(config_proto), cluster_manager_, *store_.rootScope(), - thread_local_slot_allocator_}; + thread_local_slot_allocator_, + time_}; // Any values will do for the sake of this test. What we care about is the // `expected_service_name`. @@ -175,7 +176,8 @@ TEST_F(DatadogTracerNamingTest, OperationNameAndResourceName) { DatadogTracerFactory::makeConfig(config_proto), cluster_manager_, *store_.rootScope(), - thread_local_slot_allocator_}; + thread_local_slot_allocator_, + time_}; // Any values will do for the sake of this test. What we care about are the // operation names and the resource names. diff --git a/test/extensions/tracers/datadog/span_test.cc b/test/extensions/tracers/datadog/span_test.cc index 2b2f624817814..d0be623997a24 100644 --- a/test/extensions/tracers/datadog/span_test.cc +++ b/test/extensions/tracers/datadog/span_test.cc @@ -79,7 +79,9 @@ class MockIDGenerator : public datadog::tracing::IDGenerator { std::uint64_t span_id() const override { return id_; } - datadog::tracing::TraceID trace_id() const override { return datadog::tracing::TraceID{id_}; } + datadog::tracing::TraceID trace_id(const datadog::tracing::TimePoint&) const override { + return datadog::tracing::TraceID{id_}; + } }; class DatadogTracerSpanTest : public testing::Test { @@ -90,8 +92,7 @@ class DatadogTracerSpanTest : public testing::Test { tracer_( // Override the tracer's ID generator so that all trace IDs and span // IDs are 0xcafebabe. - *datadog::tracing::finalize_config(config_), std::make_shared(id_), - datadog::tracing::default_clock), + *datadog::tracing::finalize_config(config_), std::make_shared(id_)), span_(tracer_.create_span()) {} private: diff --git a/test/extensions/tracers/datadog/tracer_test.cc b/test/extensions/tracers/datadog/tracer_test.cc index 7cd37a8a33291..ad7ef1011e6f9 100644 --- a/test/extensions/tracers/datadog/tracer_test.cc +++ b/test/extensions/tracers/datadog/tracer_test.cc @@ -73,9 +73,11 @@ TEST_F(DatadogTracerTest, Breathing) { // does not throw exceptions. datadog::tracing::TracerConfig config; config.defaults.service = "envoy"; + config.report_traces = false; + config.report_telemetry = false; Tracer tracer("fake_cluster", "test_host", config, cluster_manager_, *store_.rootScope(), - thread_local_slot_allocator_); + thread_local_slot_allocator_, time_); } TEST_F(DatadogTracerTest, NoOpMode) { @@ -83,6 +85,8 @@ TEST_F(DatadogTracerTest, NoOpMode) { // `startSpan` subsequently returns `NullSpan` instances. datadog::tracing::TracerConfig config; config.defaults.service = "envoy"; + config.report_traces = false; + config.report_telemetry = false; datadog::tracing::TraceSamplerConfig::Rule invalid_rule; // The `sample_rate`, below, is invalid (should be between 0.0 and 1.0). // As a result, the constructor of `Tracer` will fail to initialize the @@ -92,7 +96,7 @@ TEST_F(DatadogTracerTest, NoOpMode) { config.trace_sampler.rules.push_back(invalid_rule); Tracer tracer("fake_cluster", "test_host", config, cluster_manager_, *store_.rootScope(), - thread_local_slot_allocator_); + thread_local_slot_allocator_, time_); Tracing::TestTraceContextImpl context{}; // Any values will do for the sake of this test. @@ -116,12 +120,14 @@ TEST_F(DatadogTracerTest, SpanProperties) { // resulting span. datadog::tracing::TracerConfig config; config.defaults.service = "envoy"; + config.report_traces = false; + config.report_telemetry = false; // Configure the tracer to keep all spans. We then override that // configuration in the `Tracing::Decision`, below. config.trace_sampler.sample_rate = 1.0; // 100% Tracer tracer("fake_cluster", "test_host", config, cluster_manager_, *store_.rootScope(), - thread_local_slot_allocator_); + thread_local_slot_allocator_, time_); Tracing::TestTraceContextImpl context{}; // A sampling decision of "false" forces the created trace to be dropped, @@ -165,9 +171,11 @@ TEST_F(DatadogTracerTest, ExtractionSuccess) { // the extracted trace. datadog::tracing::TracerConfig config; config.defaults.service = "envoy"; + config.report_traces = false; + config.report_telemetry = false; Tracer tracer("fake_cluster", "test_host", config, cluster_manager_, *store_.rootScope(), - thread_local_slot_allocator_); + thread_local_slot_allocator_, time_); // Any values will do for the sake of this test. Tracing::Decision decision; @@ -204,9 +212,11 @@ TEST_F(DatadogTracerTest, ExtractionFailure) { // will be the start of a new trace). datadog::tracing::TracerConfig config; config.defaults.service = "envoy"; + config.report_traces = false; + config.report_telemetry = false; Tracer tracer("fake_cluster", "test_host", config, cluster_manager_, *store_.rootScope(), - thread_local_slot_allocator_); + thread_local_slot_allocator_, time_); // Any values will do for the sake of this test. Tracing::Decision decision; @@ -311,8 +321,10 @@ TEST_F(DatadogTracerTest, EnvoySamplingVersusExtractedSampling) { EnvVarGuard guard{"DD_TRACE_PROPAGATION_STYLE", style_name}; datadog::tracing::TracerConfig config; config.defaults.service = "envoy"; + config.report_traces = false; + config.report_telemetry = false; Tracer tracer("fake_cluster", "test_host", config, cluster_manager_, *store_.rootScope(), - thread_local_slot_allocator_); + thread_local_slot_allocator_, time_); Tracing::Decision envoy_decision; envoy_decision.reason = Tracing::Reason::Sampling;