diff --git a/source/extensions/tracers/datadog/BUILD b/source/extensions/tracers/datadog/BUILD index f21618260f50b..d5f86bed53094 100644 --- a/source/extensions/tracers/datadog/BUILD +++ b/source/extensions/tracers/datadog/BUILD @@ -14,11 +14,13 @@ envoy_extension_package() envoy_cc_library( name = "datadog_tracer_lib", srcs = [ + "agent_http_client.cc", "datadog_tracer_impl.cc", "dict_util.cc", "time_util.cc", ], hdrs = [ + "agent_http_client.h", "datadog_tracer_impl.h", "dict_util.h", "time_util.h", diff --git a/source/extensions/tracers/datadog/agent_http_client.cc b/source/extensions/tracers/datadog/agent_http_client.cc new file mode 100644 index 0000000000000..37a9fad328cac --- /dev/null +++ b/source/extensions/tracers/datadog/agent_http_client.cc @@ -0,0 +1,144 @@ +#include "source/extensions/tracers/datadog/agent_http_client.h" + +#include +#include +#include +#include + +#include "source/common/common/assert.h" +#include "source/common/http/message_impl.h" +#include "source/common/http/utility.h" +#include "source/extensions/tracers/datadog/dict_util.h" +#include "source/extensions/tracers/datadog/tracer_stats.h" + +#include "absl/strings/str_format.h" +#include "datadog/dict_reader.h" +#include "datadog/dict_writer.h" +#include "datadog/error.h" +#include "datadog/json.hpp" + +namespace Envoy { +namespace Extensions { +namespace Tracers { +namespace Datadog { + +AgentHTTPClient::AgentHTTPClient(Upstream::ClusterManager& cluster_manager, + const std::string& cluster, const std::string& reference_host, + TracerStats& stats) + : collector_cluster_(cluster_manager, cluster), cluster_(cluster), + reference_host_(reference_host), stats_(stats) {} + +AgentHTTPClient::~AgentHTTPClient() { + for (const auto& [request_ptr, _] : handlers_) { + RELEASE_ASSERT(request_ptr, + "null Http::AsyncClient::Request* in handler map of Datadog::AgentHTTPClient"); + request_ptr->cancel(); + } +} + +// 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"); + + auto message = std::make_unique(); + Http::RequestHeaderMap& headers = message->headers(); + headers.setReferenceMethod(Http::Headers::get().MethodValues.Post); + headers.setReferencePath(url.path); + headers.setReferenceHost(reference_host_); + + RequestHeaderWriter writer{message->headers()}; + set_headers(writer); + + message->body().add(body); + ENVOY_LOG(debug, "submitting trace(s) to {} with payload size {}", url.path, body.size()); + + if (!collector_cluster_.threadLocalCluster().has_value()) { + ENVOY_LOG(debug, "collector cluster '{}' does not exist", cluster_); + stats_.reports_skipped_no_cluster_.inc(); + return datadog::tracing::nullopt; + } + + Http::AsyncClient::Request* request = + collector_cluster_.threadLocalCluster()->get().httpAsyncClient().send( + std::move(message), *this, + Http::AsyncClient::RequestOptions().setTimeout(std::chrono::milliseconds(1000))); + if (!request) { + stats_.reports_failed_.inc(); + return datadog::tracing::Error{datadog::tracing::Error::ENVOY_HTTP_CLIENT_FAILURE, + "Failed to create request."}; + } + + handlers_.emplace(request, Handlers{std::move(on_response), std::move(on_error)}); + return datadog::tracing::nullopt; +} + +void AgentHTTPClient::drain(std::chrono::steady_clock::time_point) {} + +nlohmann::json AgentHTTPClient::config_json() const { + return nlohmann::json::object({ + {"type", "Envoy::Extensions::Tracers::Datadog::AgentHTTPClient"}, + }); +} + +// Http::AsyncClient::Callbacks + +void AgentHTTPClient::onSuccess(const Http::AsyncClient::Request& request, + Http::ResponseMessagePtr&& response) { + const std::uint64_t status = Http::Utility::getResponseStatus(response->headers()); + if (status != std::uint64_t(Http::Code::OK)) { + stats_.reports_dropped_.inc(); + } else { + ENVOY_LOG(debug, "traces successfully submitted to datadog agent"); + stats_.reports_sent_.inc(); + } + + auto found = handlers_.find(const_cast(&request)); + if (found == handlers_.end()) { + ENVOY_LOG(debug, "request at address 0x{} is not in the map of {} Handlers objects", + absl::StrFormat("%p", &request), handlers_.size()); + return; + } + + Handlers& handlers = found->second; + ResponseHeaderReader reader{response->headers()}; + handlers.on_response(status, reader, response->bodyAsString()); + + handlers_.erase(found); +} + +void AgentHTTPClient::onFailure(const Http::AsyncClient::Request& request, + Http::AsyncClient::FailureReason reason) { + auto found = handlers_.find(const_cast(&request)); + // If the request failed to start, then `post` never even registered the request. + if (found == handlers_.end()) { + return; + } + + stats_.reports_failed_.inc(); + + Handlers& handlers = found->second; + std::string message = "Failed to send request to Datadog Agent: "; + switch (reason) { + case Http::AsyncClient::FailureReason::Reset: + message += "The stream has been reset."; + break; + default: + message += "Unknown error."; + } + handlers.on_error(datadog::tracing::Error{datadog::tracing::Error::ENVOY_HTTP_CLIENT_FAILURE, + std::move(message)}); + + handlers_.erase(found); +} + +void AgentHTTPClient::onBeforeFinalizeUpstreamSpan(Tracing::Span&, const Http::ResponseHeaderMap*) { +} + +} // namespace Datadog +} // namespace Tracers +} // namespace Extensions +} // namespace Envoy diff --git a/source/extensions/tracers/datadog/agent_http_client.h b/source/extensions/tracers/datadog/agent_http_client.h new file mode 100644 index 0000000000000..900a101abb528 --- /dev/null +++ b/source/extensions/tracers/datadog/agent_http_client.h @@ -0,0 +1,106 @@ +#pragma once + +#include + +#include "envoy/http/async_client.h" + +#include "source/common/common/logger.h" +#include "source/common/upstream/cluster_update_tracker.h" + +#include "absl/container/flat_hash_map.h" +#include "datadog/http_client.h" + +namespace Envoy { +namespace Extensions { +namespace Tracers { +namespace Datadog { + +struct TracerStats; + +/** + * \c datadog::tracing::HTTPClient implementation that uses Envoy's + * \c Http::AsyncClient. The class is called \c AgentHTTPClient because it is + * not a general-purpose HTTP client. Instead, it sends requests to a specified + * cluster only. The idea is that the cluster is configured to point to a + * Datadog Agent instance. + */ +class AgentHTTPClient : public datadog::tracing::HTTPClient, + public Http::AsyncClient::Callbacks, + private Logger::Loggable { +public: + struct Handlers { + ResponseHandler on_response; + ErrorHandler on_error; + }; + +public: + /** + * Create an \c AgentHTTPClient that uses the specified \p cluster_manager + * to make requests to the specified \p cluster, where requests include the + * specified \p reference_host as the "Host" header. Use the specified + * \p stats to keep track of usage statistics. + * @param cluster_manager cluster manager from which the thread local cluster + * is retrieved in order to make HTTP requests + * @param cluster the name of the cluster to which HTTP requests are made + * @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 + */ + AgentHTTPClient(Upstream::ClusterManager& cluster_manager, const std::string& cluster, + const std::string& reference_host, TracerStats& stats); + ~AgentHTTPClient() override; + + // datadog::tracing::HTTPClient + + /** + * Send an HTTP POST request to the cluster, where the requested resource is + * \p url.path. Invoke \p set_headers immediately to obtain the HTTP request + * headers. Send \p body as the request body. Return a + * \c datadog::tracing::Error if one occurs, otherwise return + * \c datadog::tracing::nullopt. When a complete response is received, + * invoke \p on_response with the response status, response headers, and + * response body. If an error occurs before a complete response is received, + * invoke \p on_error with a \c datadog::tracing::Error. + * @param url URL from which the request path is taken + * @param set_headers callback invoked immediately to obtain request headers + * @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. + * @return \c datadog::tracing::Error if an error occurs, or + * \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; + + /** + * \c drain has no effect. It's a part of the \c datadog::tracing::HTTPClient + * that we do not need. + */ + void drain(std::chrono::steady_clock::time_point) override; + + /** + * Return a JSON representation of this object's configuration. This function + * is used in the startup banner logged by \c dd-trace-cpp. + */ + nlohmann::json config_json() const override; + + // Http::AsyncClient::Callbacks + + void onSuccess(const Http::AsyncClient::Request&, Http::ResponseMessagePtr&&) override; + void onFailure(const Http::AsyncClient::Request&, Http::AsyncClient::FailureReason) override; + void onBeforeFinalizeUpstreamSpan(Tracing::Span&, const Http::ResponseHeaderMap*) override; + +private: + absl::flat_hash_map handlers_; + Upstream::ClusterUpdateTracker collector_cluster_; + const std::string cluster_; + const std::string reference_host_; + TracerStats& stats_; +}; + +} // namespace Datadog +} // namespace Tracers +} // namespace Extensions +} // namespace Envoy diff --git a/source/extensions/tracers/datadog/datadog_tracer_impl.h b/source/extensions/tracers/datadog/datadog_tracer_impl.h index d48253b9a4e96..bcdb6ba442c64 100644 --- a/source/extensions/tracers/datadog/datadog_tracer_impl.h +++ b/source/extensions/tracers/datadog/datadog_tracer_impl.h @@ -1,7 +1,5 @@ #pragma once -#include - #include "envoy/config/trace/v3/datadog.pb.h" #include "envoy/local_info/local_info.h" #include "envoy/runtime/runtime.h" @@ -14,6 +12,7 @@ #include "source/common/upstream/cluster_update_tracker.h" #include "source/extensions/tracers/common/ot/opentracing_driver_impl.h" +#include "datadog/opentracing.h" #include "fmt/ostream.h" namespace Envoy { diff --git a/source/extensions/tracers/datadog/dict_util.h b/source/extensions/tracers/datadog/dict_util.h index a844a5cce1adb..518e4ab164928 100644 --- a/source/extensions/tracers/datadog/dict_util.h +++ b/source/extensions/tracers/datadog/dict_util.h @@ -10,11 +10,11 @@ * headers, or writing HTTP request headers. */ -#include -#include - #include +#include "datadog/dict_reader.h" +#include "datadog/dict_writer.h" + namespace Envoy { namespace Tracing { class TraceContext; diff --git a/source/extensions/tracers/datadog/time_util.h b/source/extensions/tracers/datadog/time_util.h index 723c4ded82d2a..cc645e7d9015e 100644 --- a/source/extensions/tracers/datadog/time_util.h +++ b/source/extensions/tracers/datadog/time_util.h @@ -19,10 +19,10 @@ * estimateTime function. */ -#include - #include "envoy/common/time.h" +#include "datadog/clock.h" + namespace Envoy { namespace Extensions { namespace Tracers { diff --git a/test/extensions/tracers/datadog/BUILD b/test/extensions/tracers/datadog/BUILD index 737dc0a6c1635..03ba8f950d2cf 100644 --- a/test/extensions/tracers/datadog/BUILD +++ b/test/extensions/tracers/datadog/BUILD @@ -14,6 +14,7 @@ envoy_package() envoy_extension_cc_test( name = "datadog_tracer_impl_test", srcs = [ + "agent_http_client_test.cc", "datadog_tracer_impl_test.cc", "dict_util_test.cc", "time_util_test.cc", diff --git a/test/extensions/tracers/datadog/agent_http_client_test.cc b/test/extensions/tracers/datadog/agent_http_client_test.cc new file mode 100644 index 0000000000000..3791d89b9cc9d --- /dev/null +++ b/test/extensions/tracers/datadog/agent_http_client_test.cc @@ -0,0 +1,467 @@ +#include + +#include "envoy/http/header_map.h" + +#include "source/common/http/message_impl.h" +#include "source/common/tracing/null_span_impl.h" +#include "source/extensions/tracers/datadog/agent_http_client.h" +#include "source/extensions/tracers/datadog/dict_util.h" +#include "source/extensions/tracers/datadog/tracer_stats.h" + +#include "test/mocks/http/mocks.h" +#include "test/mocks/upstream/cluster_manager.h" +#include "test/test_common/utility.h" + +#include "absl/types/optional.h" +#include "datadog/dict_writer.h" +#include "datadog/error.h" +#include "datadog/expected.h" +#include "datadog/json.hpp" +#include "datadog/optional.h" +#include "gtest/gtest.h" + +namespace Envoy { +namespace Extensions { +namespace Tracers { +namespace Datadog { +namespace { + +struct InitializedMockClusterManager { + InitializedMockClusterManager() { + instance_.initializeClusters({"fake_cluster"}, {}); + instance_.thread_local_cluster_.cluster_.info_->name_ = "fake_cluster"; + instance_.initializeThreadLocalClusters({"fake_cluster"}); + } + + NiceMock instance_; +}; + +class DatadogAgentHttpClientTest : public testing::Test { +public: + DatadogAgentHttpClientTest() + : request_(&cluster_manager_.instance_.thread_local_cluster_.async_client_), + stats_(makeTracerStats(*store_.rootScope())), + client_(cluster_manager_.instance_, "fake_cluster", "test_host", stats_) { + url_.scheme = "http"; + url_.authority = "localhost:8126"; + url_.path = "/foo/bar"; + } + +protected: + InitializedMockClusterManager cluster_manager_; + Http::MockAsyncClientRequest request_; + Stats::TestUtil::TestStore store_; + TracerStats stats_; + AgentHTTPClient client_; + datadog::tracing::HTTPClient::URL url_; + Http::AsyncClient::Callbacks* callbacks_; + testing::MockFunction + on_response_; + testing::MockFunction on_error_; +}; + +TEST_F(DatadogAgentHttpClientTest, PathFromURL) { + // The `.path` portion of the `URL` argument to `AgentHTTPClient::post` ends + // up as the "reference path" of the `Http::RequestHeaderMap`. + // That is, the URL "http://foobar.com/trace/v04" results in "/trace/v04". + + EXPECT_CALL(cluster_manager_.instance_.thread_local_cluster_.async_client_, send_(_, _, _)) + .WillOnce( + Invoke([this](Http::RequestMessagePtr& message, Http::AsyncClient::Callbacks&, + const Http::AsyncClient::RequestOptions&) -> Http::AsyncClient::Request* { + EXPECT_EQ(url_.path, message->headers().path()); + return &request_; + })); + + // `~AgentHTTPClient()` will cancel the request since we don't finish it here. + EXPECT_CALL(request_, cancel()); + + const auto ignore = [](auto&&...) {}; + datadog::tracing::Expected result = client_.post(url_, ignore, "", ignore, ignore); + EXPECT_TRUE(result) << result.error(); + EXPECT_EQ(0, stats_.reports_skipped_no_cluster_.value()); + EXPECT_EQ(0, stats_.reports_failed_.value()); +} + +TEST_F(DatadogAgentHttpClientTest, MissingThreadLocalCluster) { + // If ...`threadLocalCluster().has_value()` is false, then `post` cannot + // create a request and so will immediately return successfully but increment + // the "reports skipped no cluster" counter. + + NiceMock cluster_manager; + AgentHTTPClient client(cluster_manager, "fake_cluster", "test_host", stats_); + + const auto ignore = [](auto&&...) {}; + datadog::tracing::Expected result = client.post(url_, ignore, "", ignore, ignore); + EXPECT_TRUE(result) << result.error(); + EXPECT_EQ(1, stats_.reports_skipped_no_cluster_.value()); + EXPECT_EQ(0, stats_.reports_failed_.value()); +} + +TEST_F(DatadogAgentHttpClientTest, RequestHeaders) { + // The `set_headers` argument to `post(...)` results in the corresponding + // headers being set in `Http::RequestMessage::headers()`. + // Additionally, the "Host" header will always be the same as the + // corresponding parameter of `AgentHTTPClient`'s constructor. + + const auto set_headers = [&](datadog::tracing::DictWriter& headers) { + headers.set("foo", "bar"); + headers.set("baz-boing", "boing boing"); + headers.set("boing-boing", "boing boing"); + headers.set("boing-boing", "boing boing boing"); + }; + + EXPECT_CALL(cluster_manager_.instance_.thread_local_cluster_.async_client_, send_(_, _, _)) + .WillOnce( + Invoke([this](Http::RequestMessagePtr& message, Http::AsyncClient::Callbacks&, + const Http::AsyncClient::RequestOptions&) -> Http::AsyncClient::Request* { + EXPECT_EQ("test_host", message->headers().getHostValue()); + + EXPECT_EQ("bar", message->headers().getByKey("foo")); + EXPECT_EQ("boing boing", message->headers().getByKey("baz-boing")); + EXPECT_EQ("boing boing boing", message->headers().getByKey("boing-boing")); + + return &request_; + })); + + // `~AgentHTTPClient()` will cancel the request since we don't finish it here. + EXPECT_CALL(request_, cancel()); + + const auto ignore = [](auto&&...) {}; + datadog::tracing::Expected result = client_.post(url_, set_headers, "", ignore, ignore); + EXPECT_TRUE(result) << result.error(); + EXPECT_EQ(0, stats_.reports_skipped_no_cluster_.value()); + EXPECT_EQ(0, stats_.reports_failed_.value()); +} + +TEST_F(DatadogAgentHttpClientTest, RequestBody) { + // The `body` parameter to `AgentHTTPClient::post` corresponds to the + // resulting `Http::RequestMessage::body()`. + + const std::string body = R"body( + Butterfly in the sky + I can go twice as high + Take a look + It's in a book + A reading rainbow + + I can go anywhere + Friends to know + And ways to grow + A reading rainbow + + I can be anything + Take a look + It's in a book + A reading rainbow)body"; + + EXPECT_CALL(cluster_manager_.instance_.thread_local_cluster_.async_client_, send_(_, _, _)) + .WillOnce(Invoke( + [this, &body](Http::RequestMessagePtr& message, Http::AsyncClient::Callbacks&, + const Http::AsyncClient::RequestOptions&) -> Http::AsyncClient::Request* { + EXPECT_EQ(body, message->body().toString()); + return &request_; + })); + + // `~AgentHTTPClient()` will cancel the request since we don't finish it here. + EXPECT_CALL(request_, cancel()); + + const auto ignore = [](auto&&...) {}; + datadog::tracing::Expected result = client_.post(url_, ignore, body, ignore, ignore); + EXPECT_TRUE(result) << result.error(); + EXPECT_EQ(0, stats_.reports_skipped_no_cluster_.value()); + EXPECT_EQ(0, stats_.reports_failed_.value()); +} + +TEST_F(DatadogAgentHttpClientTest, OnResponse200) { + // When `onSuccess` is invoked on the `Http::AsyncClient::Callbacks`, the + // associated `on_response` callback is invoked with corresponding arguments. + // Additionally, if the HTTP response status is 200, `stats_.reports_sent_` is + // incremented. + + EXPECT_CALL(cluster_manager_.instance_.thread_local_cluster_.async_client_, send_(_, _, _)) + .WillOnce( + Invoke([this](Http::RequestMessagePtr&, Http::AsyncClient::Callbacks& callbacks_arg, + const Http::AsyncClient::RequestOptions&) -> Http::AsyncClient::Request* { + callbacks_ = &callbacks_arg; + return &request_; + })); + + // `callbacks_->onSuccess(...)` will cause `on_response_` to be called. + // `on_error_` will not be called. + EXPECT_CALL(on_response_, Call(200, _, "{}")); + EXPECT_CALL(on_error_, Call(_)).Times(0); + + // The request will not be canceled; neither explicitly nor in + // `~AgentHTTPClient`, because it will have been successfully fulfilled. + EXPECT_CALL(request_, cancel()).Times(0); + + const auto ignore = [](auto&&...) {}; + datadog::tracing::Expected result = + client_.post(url_, ignore, "{}", on_response_.AsStdFunction(), on_error_.AsStdFunction()); + EXPECT_TRUE(result) << result.error(); + + Http::ResponseMessagePtr msg(new Http::ResponseMessageImpl( + Http::ResponseHeaderMapPtr{new Http::TestResponseHeaderMapImpl{{":status", "200"}}})); + msg->body().add("{}"); + + callbacks_->onSuccess(request_, std::move(msg)); + EXPECT_EQ(1, stats_.reports_sent_.value()); + EXPECT_EQ(0, stats_.reports_failed_.value()); + EXPECT_EQ(0, stats_.reports_skipped_no_cluster_.value()); +} + +TEST_F(DatadogAgentHttpClientTest, OnResponseNot200) { + // When `onSuccess` is invoked on the `Http::AsyncClient::Callbacks`, the + // associated `on_response` callback is invoked with corresponding arguments. + // Additionally, if the HTTP response status is not 200, + // `stats_.reports_dropped_` is incremented. + + EXPECT_CALL(cluster_manager_.instance_.thread_local_cluster_.async_client_, send_(_, _, _)) + .WillOnce( + Invoke([this](Http::RequestMessagePtr&, Http::AsyncClient::Callbacks& callbacks_arg, + const Http::AsyncClient::RequestOptions&) -> Http::AsyncClient::Request* { + callbacks_ = &callbacks_arg; + return &request_; + })); + + // `callbacks_->onSuccess(...)` will cause `on_response_` to be called. + // The `404` value corresponds to the response sent below. + // `on_error_` will not be called. + EXPECT_CALL(on_response_, Call(404, _, "{}")); + EXPECT_CALL(on_error_, Call(_)).Times(0); + + // The request will not be canceled; neither explicitly nor in + // `~AgentHTTPClient`, because it will have been successfully fulfilled. + EXPECT_CALL(request_, cancel()).Times(0); + + const auto ignore = [](auto&&...) {}; + datadog::tracing::Expected result = + client_.post(url_, ignore, "{}", on_response_.AsStdFunction(), on_error_.AsStdFunction()); + EXPECT_TRUE(result) << result.error(); + + // The "404" below is what causes `stats.reports_failed_` to be incremented + // instead of `stats.reports_sent_`. + Http::ResponseMessagePtr msg(new Http::ResponseMessageImpl( + Http::ResponseHeaderMapPtr{new Http::TestResponseHeaderMapImpl{{":status", "404"}}})); + msg->body().add("{}"); + + callbacks_->onSuccess(request_, std::move(msg)); + EXPECT_EQ(1, stats_.reports_dropped_.value()); + EXPECT_EQ(0, stats_.reports_sent_.value()); + EXPECT_EQ(0, stats_.reports_failed_.value()); + EXPECT_EQ(0, stats_.reports_skipped_no_cluster_.value()); +} + +TEST_F(DatadogAgentHttpClientTest, OnResponseBogusRequest) { + // When `onSuccess` is invoked on the `Http::AsyncClient::Callbacks` with a + // request that is not registered with the HTTP client, then no callback is + // invoked (how would we look it up?). + + EXPECT_CALL(cluster_manager_.instance_.thread_local_cluster_.async_client_, send_(_, _, _)) + .WillOnce( + Invoke([this](Http::RequestMessagePtr&, Http::AsyncClient::Callbacks& callbacks_arg, + const Http::AsyncClient::RequestOptions&) -> Http::AsyncClient::Request* { + callbacks_ = &callbacks_arg; + return &request_; + })); + + // `callbacks_->onSuccess(...)` will not invoke any callbacks, because the + // request argument passed in is not registered with the HTTP client. + EXPECT_CALL(on_response_, Call(_, _, _)).Times(0); + EXPECT_CALL(on_error_, Call(_)).Times(0); + + // The request will will canceled by `~AgentHTTPClient` because `onSuccess` + // was passed the wrong request, and so the real request is never removed from + // the HTTP client's registry. + EXPECT_CALL(request_, cancel()); + + const auto ignore = [](auto&&...) {}; + datadog::tracing::Expected result = + client_.post(url_, ignore, "{}", on_response_.AsStdFunction(), on_error_.AsStdFunction()); + EXPECT_TRUE(result) << result.error(); + + Http::ResponseMessagePtr msg(new Http::ResponseMessageImpl( + Http::ResponseHeaderMapPtr{new Http::TestResponseHeaderMapImpl{{":status", "200"}}})); + msg->body().add("{}"); + + // The first argument to `onSuccess` should be `request_`, but instead we pass + // `bogus_request`. + Http::MockAsyncClientRequest bogus_request( + &cluster_manager_.instance_.thread_local_cluster_.async_client_); + callbacks_->onSuccess(bogus_request, std::move(msg)); +} + +TEST_F(DatadogAgentHttpClientTest, OnErrorStreamReset) { + // When `onFailure` is invoked on the `Http::AsyncClient::Callbacks` with + // `FailureReason::Reset`, the associated `on_error` callback is invoked with + // a corresponding `datadog::tracing::Error`. + + EXPECT_CALL(cluster_manager_.instance_.thread_local_cluster_.async_client_, send_(_, _, _)) + .WillOnce( + Invoke([this](Http::RequestMessagePtr&, Http::AsyncClient::Callbacks& callbacks_arg, + const Http::AsyncClient::RequestOptions&) -> Http::AsyncClient::Request* { + callbacks_ = &callbacks_arg; + return &request_; + })); + + // `callbacks_->onFailure(...)` will cause `on_error_` to be called. + // `on_response_` will not be called. + EXPECT_CALL(on_error_, Call(_)).WillOnce(Invoke([](datadog::tracing::Error error) { + EXPECT_EQ(error.code, datadog::tracing::Error::ENVOY_HTTP_CLIENT_FAILURE); + })); + EXPECT_CALL(on_response_, Call(_, _, _)).Times(0); + + // The request will not be canceled; neither explicitly nor in + // `~AgentHTTPClient`, because it will have been fulfilled. + EXPECT_CALL(request_, cancel()).Times(0); + + const auto ignore = [](auto&&...) {}; + datadog::tracing::Expected result = + client_.post(url_, ignore, "{}", on_response_.AsStdFunction(), on_error_.AsStdFunction()); + EXPECT_TRUE(result) << result.error(); + + Http::ResponseMessagePtr msg(new Http::ResponseMessageImpl( + Http::ResponseHeaderMapPtr{new Http::TestResponseHeaderMapImpl{{":status", "200"}}})); + msg->body().add("{}"); + + callbacks_->onFailure(request_, Http::AsyncClient::FailureReason::Reset); +} + +TEST_F(DatadogAgentHttpClientTest, OnErrorOther) { + // When `onFailure` is invoked on the `Http::AsyncClient::Callbacks` with any + // value other than `FailureReason::Reset`, the associated `on_error` callback + // is invoked with a corresponding `datadog::tracing::Error`. + + EXPECT_CALL(cluster_manager_.instance_.thread_local_cluster_.async_client_, send_(_, _, _)) + .WillOnce( + Invoke([this](Http::RequestMessagePtr&, Http::AsyncClient::Callbacks& callbacks_arg, + const Http::AsyncClient::RequestOptions&) -> Http::AsyncClient::Request* { + callbacks_ = &callbacks_arg; + return &request_; + })); + + // `callbacks->onFailure(...)` will cause `on_error_` to be called. + // `on_response_` will not be called. + EXPECT_CALL(on_error_, Call(_)).WillOnce(Invoke([](datadog::tracing::Error error) { + EXPECT_EQ(error.code, datadog::tracing::Error::ENVOY_HTTP_CLIENT_FAILURE); + })); + EXPECT_CALL(on_response_, Call(_, _, _)).Times(0); + + // The request will not be canceled; neither explicitly nor in + // `~AgentHTTPClient`, because it will have been fulfilled. + EXPECT_CALL(request_, cancel()).Times(0); + + const auto ignore = [](auto&&...) {}; + datadog::tracing::Expected result = + client_.post(url_, ignore, "{}", on_response_.AsStdFunction(), on_error_.AsStdFunction()); + EXPECT_TRUE(result) << result.error(); + + Http::ResponseMessagePtr msg(new Http::ResponseMessageImpl( + Http::ResponseHeaderMapPtr{new Http::TestResponseHeaderMapImpl{{":status", "200"}}})); + msg->body().add("{}"); + + const auto bogus_value = static_cast(-1); + callbacks_->onFailure(request_, bogus_value); +} + +TEST_F(DatadogAgentHttpClientTest, OnErrorBogusRequest) { + // When `onFailure` is invoked with a request that's not registered with the + // HTTP client, no callbacks are invoked. + + EXPECT_CALL(cluster_manager_.instance_.thread_local_cluster_.async_client_, send_(_, _, _)) + .WillOnce( + Invoke([this](Http::RequestMessagePtr&, Http::AsyncClient::Callbacks& callbacks_arg, + const Http::AsyncClient::RequestOptions&) -> Http::AsyncClient::Request* { + callbacks_ = &callbacks_arg; + return &request_; + })); + + EXPECT_CALL(on_error_, Call(_)).Times(0); + EXPECT_CALL(on_response_, Call(_, _, _)).Times(0); + + // The request will will canceled by `~AgentHTTPClient` because `onFailure` + // was passed the wrong request, and so the real request is never removed from + // the HTTP client's registry. + EXPECT_CALL(request_, cancel()); + + const auto ignore = [](auto&&...) {}; + datadog::tracing::Expected result = + client_.post(url_, ignore, "{}", on_response_.AsStdFunction(), on_error_.AsStdFunction()); + EXPECT_TRUE(result) << result.error(); + + Http::ResponseMessagePtr msg(new Http::ResponseMessageImpl( + Http::ResponseHeaderMapPtr{new Http::TestResponseHeaderMapImpl{{":status", "200"}}})); + msg->body().add("{}"); + + // The first argument to `onFailure` should be `request_`, but instead we pass + // `bogus_request`. + Http::MockAsyncClientRequest bogus_request( + &cluster_manager_.instance_.thread_local_cluster_.async_client_); + callbacks_->onFailure(bogus_request, Http::AsyncClient::FailureReason::Reset); +} + +TEST_F(DatadogAgentHttpClientTest, SendFailReturnsError) { + // If the underlying call to `httpAsyncClient().send(...)` returns an error, + // then the enclosing call to `AgentHTTPClient::post(...)` returns an error. + + EXPECT_CALL(cluster_manager_.instance_.thread_local_cluster_.async_client_, send_(_, _, _)) + .WillOnce( + Invoke([this](Http::RequestMessagePtr&, Http::AsyncClient::Callbacks& callbacks_arg, + const Http::AsyncClient::RequestOptions&) -> Http::AsyncClient::Request* { + callbacks_ = &callbacks_arg; + // As of this writing, any time that `send` returns `nullptr`, + // `onSuccess` will also be called with a status of 503, even though + // no request was sent and so no response was received. + // `AgentHTTPClient` does not depend on this behavior, but we + // reproduce it here for authenticity. + // The relevant branch in `AgentHTTPClient::onSuccess` is the one + // where `handlers_.find` returns `handlers.end()`. + Http::ResponseMessagePtr response( + new Http::ResponseMessageImpl(Http::ResponseHeaderMapPtr{ + new Http::TestResponseHeaderMapImpl{{":status", "503"}}})); + callbacks_arg.onSuccess(request_, std::move(response)); + return nullptr; // indicates error + })); + + // Neither callback will be invoked, because `post` fails immediately (synchronously). + EXPECT_CALL(on_error_, Call(_)).Times(0); + EXPECT_CALL(on_response_, Call(_, _, _)).Times(0); + + const auto ignore = [](auto&&...) {}; + datadog::tracing::Expected result = + client_.post(url_, ignore, "", on_response_.AsStdFunction(), on_error_.AsStdFunction()); + 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, DrainIsANoOp) { + // `AgentHTTPClient::drain` doesn't do anything. It only makes sense in + // multi-threaded contexts. + // This test is for the sake of coverage. + + // `deadline` value doesn't matter; `drain` ignores it. + const auto deadline = std::chrono::steady_clock::time_point::min(); + client_.drain(deadline); +} + +TEST_F(DatadogAgentHttpClientTest, ConfigJSONContainsTypeName) { + nlohmann::json config = client_.config_json(); + EXPECT_EQ("Envoy::Extensions::Tracers::Datadog::AgentHTTPClient", config["type"]); +} + +TEST_F(DatadogAgentHttpClientTest, OnBeforeFinalizeUpstreamSpanIsANoOp) { + // `AgentHTTPClient::onBeforeFinalizeUpstreamSpan` doesn't do anything. + // This test is for the sake of coverage. + Tracing::NullSpan null_span; + client_.onBeforeFinalizeUpstreamSpan(null_span, nullptr); +} + +} // namespace +} // namespace Datadog +} // namespace Tracers +} // namespace Extensions +} // namespace Envoy diff --git a/test/extensions/tracers/datadog/dict_util_test.cc b/test/extensions/tracers/datadog/dict_util_test.cc index d20e669c101b4..855744fde7aec 100644 --- a/test/extensions/tracers/datadog/dict_util_test.cc +++ b/test/extensions/tracers/datadog/dict_util_test.cc @@ -1,5 +1,3 @@ -#include - #include #include @@ -10,6 +8,7 @@ #include "test/mocks/http/mocks.h" #include "test/test_common/utility.h" +#include "datadog/optional.h" #include "gtest/gtest.h" namespace Envoy { diff --git a/test/extensions/tracers/datadog/time_util_test.cc b/test/extensions/tracers/datadog/time_util_test.cc index 472677f6dde95..21efa6ab8d1f6 100644 --- a/test/extensions/tracers/datadog/time_util_test.cc +++ b/test/extensions/tracers/datadog/time_util_test.cc @@ -1,9 +1,8 @@ -#include - #include "envoy/common/time.h" #include "source/extensions/tracers/datadog/time_util.h" +#include "datadog/clock.h" #include "gtest/gtest.h" namespace Envoy {