diff --git a/test/server/http_dynamic_delay_filter_integration_test.cc b/test/server/http_dynamic_delay_filter_integration_test.cc index c4a38e429..3840a11ec 100644 --- a/test/server/http_dynamic_delay_filter_integration_test.cc +++ b/test/server/http_dynamic_delay_filter_integration_test.cc @@ -1,16 +1,18 @@ #include -#include "external/envoy/test/integration/http_integration.h" - #include "api/server/response_options.pb.h" #include "server/configuration.h" #include "server/http_dynamic_delay_filter.h" +#include "test/server/http_filter_integration_test_base.h" + #include "gtest/gtest.h" namespace Nighthawk { +const Envoy::Http::LowerCaseString kDelayHeaderString("x-envoy-fault-delay-request"); + /** * Support class for testing the dynamic delay filter. We rely on the fault filter for * inducing the actual delay, so this aims to prove that: @@ -20,57 +22,17 @@ namespace Nighthawk { * - Failure modes work. * - TODO(#393): An end to end test which proves that the interaction between this filter * and the fault filter work as expected. + * + * The Dynamic Delay filter communicates with the fault filter by adding kDelayHeaderString + * to the request headers. We use that in tests below to verify expectations. The fault filter + * accepts input values via request headers specified in milliseconds, so our expectations are + * also using milliseconds. */ class HttpDynamicDelayIntegrationTest - : public Envoy::HttpIntegrationTest, + : public HttpFilterIntegrationTestBase, public testing::TestWithParam { -protected: - HttpDynamicDelayIntegrationTest() - : HttpIntegrationTest(Envoy::Http::CodecClient::Type::HTTP1, GetParam()), - request_headers_({{":method", "GET"}, {":path", "/"}, {":authority", "host"}}), - delay_header_string_(Envoy::Http::LowerCaseString("x-envoy-fault-delay-request")) {} - - // We don't override SetUp(): tests in this file will call setup() instead to avoid having to - // create a fixture per filter configuration. - void setup(const std::string& config) { - config_helper_.addFilter(config); - HttpIntegrationTest::initialize(); - } - - // Fetches a response with request-level configuration set in the request header. - Envoy::IntegrationStreamDecoderPtr getResponse(absl::string_view request_level_config, - bool setup_for_upstream_request = true) { - const Envoy::Http::LowerCaseString key("x-nighthawk-test-server-config"); - Envoy::Http::TestRequestHeaderMapImpl request_headers = request_headers_; - request_headers.setCopy(key, request_level_config); - return getResponse(request_headers, setup_for_upstream_request); - } - - // Fetches a response with the default request headers, expecting the fake upstream to supply - // the response. - Envoy::IntegrationStreamDecoderPtr getResponse() { return getResponse(request_headers_); } - - // Fetches a response using the provided request headers. When setup_for_upstream_request - // is true, the expectation will be that an upstream request will be needed to provide a - // response. If it is set to false, the extension is expected to supply the response, and - // no upstream request ought to occur. - Envoy::IntegrationStreamDecoderPtr - getResponse(const Envoy::Http::TestRequestHeaderMapImpl& request_headers, - bool setup_for_upstream_request = true) { - cleanupUpstreamAndDownstream(); - codec_client_ = makeHttpConnection(lookupPort("http")); - Envoy::IntegrationStreamDecoderPtr response; - if (setup_for_upstream_request) { - response = sendRequestAndWaitForResponse(request_headers, 0, default_response_headers_, 0); - } else { - response = codec_client_->makeHeaderOnlyRequest(request_headers); - response->waitForEndStream(); - } - return response; - } - - const Envoy::Http::TestRequestHeaderMapImpl request_headers_; - const Envoy::Http::LowerCaseString delay_header_string_; +public: + HttpDynamicDelayIntegrationTest() : HttpFilterIntegrationTestBase(GetParam()){}; }; INSTANTIATE_TEST_SUITE_P(IpVersions, HttpDynamicDelayIntegrationTest, @@ -78,69 +40,72 @@ INSTANTIATE_TEST_SUITE_P(IpVersions, HttpDynamicDelayIntegrationTest, // Verify expectations with an empty dynamic-delay configuration. TEST_P(HttpDynamicDelayIntegrationTest, NoStaticConfiguration) { - setup(R"( + initializeFilterConfiguration(R"( name: dynamic-delay typed_config: "@type": type.googleapis.com/nighthawk.server.ResponseOptions )"); - // Don't send any config request header - getResponse(); - EXPECT_EQ(upstream_request_->headers().get(delay_header_string_), nullptr); - // Send a config request header with an empty / default config. Should be a no-op. - getResponse("{}"); - EXPECT_EQ(upstream_request_->headers().get(delay_header_string_), nullptr); - // Send a config request header, this should become effective. - getResponse("{static_delay: \"1.6s\"}"); - EXPECT_EQ(upstream_request_->headers().get(delay_header_string_)->value().getStringView(), - "1600"); - - // Send a malformed config request header. This ought to shortcut and directly reply, - // hence we don't expect an upstream request. - auto response = getResponse("bad_json", false); - EXPECT_EQ(Envoy::Http::Utility::getResponseStatus(response->headers()), 500); - EXPECT_EQ( - response->body(), - "dynamic-delay didn't understand the request: Error merging json config: Unable to parse " - "JSON as proto (INVALID_ARGUMENT:Unexpected token.\nbad_json\n^): bad_json"); - // Send an empty config header, which ought to trigger failure mode as well. - response = getResponse("", false); - EXPECT_EQ(Envoy::Http::Utility::getResponseStatus(response->headers()), 500); - EXPECT_EQ( - response->body(), - "dynamic-delay didn't understand the request: Error merging json config: Unable to " - "parse JSON as proto (INVALID_ARGUMENT:Unexpected end of string. Expected a value.\n\n^): "); + // Don't send any config request header ... + getResponse(ResponseOrigin::UPSTREAM); + // ... we shouldn't observe any delay being requested via the upstream request headers. + EXPECT_EQ(upstream_request_->headers().get(kDelayHeaderString), nullptr); + + // Send a config request header with an empty / default configuration .... + setRequestLevelConfiguration("{}"); + getResponse(ResponseOrigin::UPSTREAM); + // ... we shouldn't observe any delay being requested via the upstream request headers. + EXPECT_EQ(upstream_request_->headers().get(kDelayHeaderString), nullptr); + + // Send a config request header requesting a 1.6s delay... + setRequestLevelConfiguration("{static_delay: \"1.6s\"}"); + getResponse(ResponseOrigin::UPSTREAM); + // ...we should observe a delay of 1.6s in the upstream request. + EXPECT_EQ(upstream_request_->headers().get(kDelayHeaderString)->value().getStringView(), "1600"); } // Verify expectations with static/file-based static_delay configuration. TEST_P(HttpDynamicDelayIntegrationTest, StaticConfigurationStaticDelay) { - setup(R"EOF( + initializeFilterConfiguration(R"EOF( name: dynamic-delay typed_config: "@type": type.googleapis.com/nighthawk.server.ResponseOptions static_delay: 1.33s )EOF"); - getResponse(); - EXPECT_EQ(upstream_request_->headers().get(delay_header_string_)->value().getStringView(), - "1330"); - getResponse("{}"); - EXPECT_EQ(upstream_request_->headers().get(delay_header_string_)->value().getStringView(), - "1330"); - getResponse("{static_delay: \"0.2s\"}"); + + // Without any request-level configuration, we expect the statically configured static delay to + // apply. + getResponse(ResponseOrigin::UPSTREAM); + EXPECT_EQ(upstream_request_->headers().get(kDelayHeaderString)->value().getStringView(), "1330"); + + // With an empty request-level configuration, we expect the statically configured static delay to + // apply. + setRequestLevelConfiguration("{}"); + getResponse(ResponseOrigin::UPSTREAM); + EXPECT_EQ(upstream_request_->headers().get(kDelayHeaderString)->value().getStringView(), "1330"); + + // Overriding the statically configured static delay via request-level configuration should be + // reflected in the output. + setRequestLevelConfiguration("{static_delay: \"0.2s\"}"); + getResponse(ResponseOrigin::UPSTREAM); // TODO(#392): This fails, because the duration is a two-field message: it would make here to see // both the number of seconds and nanoseconds to be overridden. // However, the seconds part is set to '0', which equates to the default of the underlying int // type, and the fact that we are using proto3, which doesn't merge default values. // Hence the following expectation will fail, as it yields 1200 instead of the expected 200. - // EXPECT_EQ(upstream_request_->headers().get(delay_header_string_)->value().getStringView(), + // EXPECT_EQ(upstream_request_->headers().get(kDelayHeaderString)->value().getStringView(), // "200"); - getResponse("{static_delay: \"2.2s\"}"); - EXPECT_EQ(upstream_request_->headers().get(delay_header_string_)->value().getStringView(), - "2200"); + + // Overriding the statically configured static delay via request-level configuration should be + // reflected in the output. + setRequestLevelConfiguration("{static_delay: \"2.2s\"}"); + getResponse(ResponseOrigin::UPSTREAM); + // 2.2 seconds -> 2200 ms. + EXPECT_EQ(upstream_request_->headers().get(kDelayHeaderString)->value().getStringView(), "2200"); } // Verify expectations with static/file-based concurrency_based_linear_delay configuration. TEST_P(HttpDynamicDelayIntegrationTest, StaticConfigurationConcurrentDelay) { - setup(R"EOF( + initializeFilterConfiguration(R"EOF( name: dynamic-delay typed_config: "@type": type.googleapis.com/nighthawk.server.ResponseOptions @@ -148,8 +113,10 @@ name: dynamic-delay minimal_delay: 0.05s concurrency_delay_factor: 0.01s )EOF"); - getResponse(); - EXPECT_EQ(upstream_request_->headers().get(delay_header_string_)->value().getStringView(), "60"); + getResponse(ResponseOrigin::UPSTREAM); + // Based on the algorithm of concurrency_based_linear_delay, for the first request we expect to + // observe the configured minimal_delay + concurrency_delay_factor = 0.06s -> 60ms. + EXPECT_EQ(upstream_request_->headers().get(kDelayHeaderString)->value().getStringView(), "60"); } class ComputeTest : public testing::Test { diff --git a/test/server/http_time_tracking_filter_integration_test.cc b/test/server/http_time_tracking_filter_integration_test.cc index 55d08e9c9..5f1348c56 100644 --- a/test/server/http_time_tracking_filter_integration_test.cc +++ b/test/server/http_time_tracking_filter_integration_test.cc @@ -1,10 +1,5 @@ #include -#include "envoy/upstream/cluster_manager.h" -#include "envoy/upstream/upstream.h" - -#include "external/envoy/test/common/upstream/utility.h" -#include "external/envoy/test/integration/http_integration.h" #include "external/envoy/test/test_common/simulated_time_system.h" #include "api/server/response_options.pb.h" @@ -12,7 +7,8 @@ #include "server/configuration.h" #include "server/http_time_tracking_filter.h" -#include "server/well_known_headers.h" + +#include "test/server/http_filter_integration_test_base.h" #include "gtest/gtest.h" @@ -32,53 +28,10 @@ name: time-tracking )EOF"; class HttpTimeTrackingIntegrationTest - : public Envoy::HttpIntegrationTest, + : public HttpFilterIntegrationTestBase, public testing::TestWithParam { -protected: - HttpTimeTrackingIntegrationTest() - : HttpIntegrationTest(Envoy::Http::CodecClient::Type::HTTP1, GetParam()), - request_headers_({{":method", "GET"}, {":path", "/"}, {":authority", "host"}}) {} - - // We don't override SetUp(): tests in this file will call setup() instead to avoid having to - // create a fixture per filter configuration. - void setup(const std::string& config) { - config_helper_.addFilter(config); - HttpIntegrationTest::initialize(); - } - - // Fetches a response with request-level configuration set in the request header. - Envoy::IntegrationStreamDecoderPtr getResponse(absl::string_view request_level_config, - bool setup_for_upstream_request = true) { - Envoy::Http::TestRequestHeaderMapImpl request_headers = request_headers_; - request_headers.setCopy(Nighthawk::Server::TestServer::HeaderNames::get().TestServerConfig, - request_level_config); - return getResponse(request_headers, setup_for_upstream_request); - } - - // Fetches a response with the default request headers, expecting the fake upstream to supply - // the response. - Envoy::IntegrationStreamDecoderPtr getResponse() { return getResponse(request_headers_); } - - // Fetches a response using the provided request headers. When setup_for_upstream_request - // is true, the expectation will be that an upstream request will be needed to provide a - // response. If it is set to false, the extension is expected to supply the response, and - // no upstream request ought to occur. - Envoy::IntegrationStreamDecoderPtr - getResponse(const Envoy::Http::TestRequestHeaderMapImpl& request_headers, - bool setup_for_upstream_request = true) { - cleanupUpstreamAndDownstream(); - codec_client_ = makeHttpConnection(lookupPort("http")); - Envoy::IntegrationStreamDecoderPtr response; - if (setup_for_upstream_request) { - response = sendRequestAndWaitForResponse(request_headers, 0, default_response_headers_, 0); - } else { - response = codec_client_->makeHeaderOnlyRequest(request_headers); - response->waitForEndStream(); - } - return response; - } - - const Envoy::Http::TestRequestHeaderMapImpl request_headers_; +public: + HttpTimeTrackingIntegrationTest() : HttpFilterIntegrationTestBase(GetParam()){}; }; INSTANTIATE_TEST_SUITE_P(IpVersions, HttpTimeTrackingIntegrationTest, @@ -86,13 +39,17 @@ INSTANTIATE_TEST_SUITE_P(IpVersions, HttpTimeTrackingIntegrationTest, // Verify expectations with static/file-based time-tracking configuration. TEST_P(HttpTimeTrackingIntegrationTest, ReturnsPositiveLatencyForStaticConfiguration) { - setup(fmt::format(kProtoConfigTemplate, kDefaultProtoFragment)); - Envoy::IntegrationStreamDecoderPtr response = getResponse(); + initializeFilterConfiguration(fmt::format(kProtoConfigTemplate, kDefaultProtoFragment)); + + // As the first request doesn't have a prior one, we should not observe a delta. + Envoy::IntegrationStreamDecoderPtr response = getResponse(ResponseOrigin::UPSTREAM); int64_t latency; const Envoy::Http::HeaderEntry* latency_header_1 = response->headers().get(Envoy::Http::LowerCaseString(kLatencyResponseHeaderName)); EXPECT_EQ(latency_header_1, nullptr); - response = getResponse(); + + // On the second request we should observe a delta. + response = getResponse(ResponseOrigin::UPSTREAM); const Envoy::Http::HeaderEntry* latency_header_2 = response->headers().get(Envoy::Http::LowerCaseString(kLatencyResponseHeaderName)); ASSERT_NE(latency_header_2, nullptr); @@ -102,14 +59,17 @@ TEST_P(HttpTimeTrackingIntegrationTest, ReturnsPositiveLatencyForStaticConfigura // Verify expectations with an empty time-tracking configuration. TEST_P(HttpTimeTrackingIntegrationTest, ReturnsPositiveLatencyForPerRequestConfiguration) { - setup(fmt::format(kProtoConfigTemplate, "")); - // Don't send any config request header - getResponse(); - // Send a config request header with an empty / default config. Should be a no-op. - getResponse("{}"); - // Send a config request header, this should become effective. - Envoy::IntegrationStreamDecoderPtr response = - getResponse(fmt::format("{{{}}}", kDefaultProtoFragment)); + initializeFilterConfiguration(fmt::format(kProtoConfigTemplate, "")); + // As the first request doesn't have a prior one, we should not observe a delta. + setRequestLevelConfiguration("{}"); + Envoy::IntegrationStreamDecoderPtr response = getResponse(ResponseOrigin::UPSTREAM); + EXPECT_EQ(response->headers().get(Envoy::Http::LowerCaseString(kLatencyResponseHeaderName)), + nullptr); + + // With request level configuration indicating that the timing header should be emitted, + // we should be able to observe it. + setRequestLevelConfiguration(fmt::format("{{{}}}", kDefaultProtoFragment)); + response = getResponse(ResponseOrigin::UPSTREAM); const Envoy::Http::HeaderEntry* latency_header = response->headers().get(Envoy::Http::LowerCaseString(kLatencyResponseHeaderName)); ASSERT_NE(latency_header, nullptr); @@ -120,25 +80,6 @@ TEST_P(HttpTimeTrackingIntegrationTest, ReturnsPositiveLatencyForPerRequestConfi EXPECT_GT(latency, 0); } -TEST_P(HttpTimeTrackingIntegrationTest, BehavesWellWithBadPerRequestConfiguration) { - setup(fmt::format(kProtoConfigTemplate, "")); - // Send a malformed config request header. This ought to shortcut and directly reply, - // hence we don't expect an upstream request. - Envoy::IntegrationStreamDecoderPtr response = getResponse("bad_json", false); - EXPECT_EQ(Envoy::Http::Utility::getResponseStatus(response->headers()), 500); - EXPECT_EQ( - response->body(), - "time-tracking didn't understand the request: Error merging json config: Unable to parse " - "JSON as proto (INVALID_ARGUMENT:Unexpected token.\nbad_json\n^): bad_json"); - // Send an empty config header, which ought to trigger failure mode as well. - response = getResponse("", false); - EXPECT_EQ(Envoy::Http::Utility::getResponseStatus(response->headers()), 500); - EXPECT_EQ( - response->body(), - "time-tracking didn't understand the request: Error merging json config: Unable to " - "parse JSON as proto (INVALID_ARGUMENT:Unexpected end of string. Expected a value.\n\n^): "); -} - class HttpTimeTrackingFilterConfigTest : public testing::Test, public Envoy::Event::TestUsingSimulatedTime {};