From c5c3be378724c618ef78eae281bdbb79f1d7933c Mon Sep 17 00:00:00 2001 From: David Goffredo Date: Fri, 5 Jan 2024 12:38:58 -0500 Subject: [PATCH] tracing: dd-trace-cpp v0.1.12 (#31526) dd-trace-cpp is Datadog's core C++ tracing library, and is used by Envoy to provide tracing via Datadog. Envoy is currently consuming an older version of dd-trace-cpp. Subsequent releases of dd-trace-cpp were not compatible with the Datadog tracing extension here. These changes make the Datadog tracing extension compatible with the latest release of dd-trace-cpp, v0.1.12. The changes are mostly in unit tests. Newer versions of dd-trace-cpp send more HTTP requests to the Datadog Agent, and so tests that assumed the number of requests were broken. There are also some changes involving how timeouts are handled by dd-trace-cpp. These changes address #30957, #29354, and #31360. Signed-off-by: David Goffredo --- bazel/repository_locations.bzl | 6 +- .../tracers/datadog/agent_http_client.cc | 35 ++++-- .../tracers/datadog/agent_http_client.h | 13 ++- source/extensions/tracers/datadog/config.cc | 6 +- source/extensions/tracers/datadog/tracer.cc | 14 ++- source/extensions/tracers/datadog/tracer.h | 5 +- .../tracers/datadog/agent_http_client_test.cc | 89 ++++++++++++--- .../extensions/tracers/datadog/config_test.cc | 107 ++++++++++++++++-- .../extensions/tracers/datadog/naming_test.cc | 6 +- test/extensions/tracers/datadog/span_test.cc | 7 +- .../extensions/tracers/datadog/tracer_test.cc | 24 +++- 11 files changed, 243 insertions(+), 69 deletions(-) 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;