Skip to content

Use shared test facilities for time-tracking and dynamic-delay #541

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
mum4k merged 4 commits into from
Sep 24, 2020
Merged

Use shared test facilities for time-tracking and dynamic-delay #541

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
512910b
Use shared test facilities for time-tracking and dynamic-delay
oschaaf Sep 17, 2020
2fa83ee
Review feedback
oschaaf Sep 17, 2020
5de0290
Merge remote-tracking branch 'upstream/master' into extension-refacto…
oschaaf Sep 18, 2020
a68362e
Merge remote-tracking branch 'upstream/master' into extension-refacto…
oschaaf Sep 23, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
151 changes: 59 additions & 92 deletions test/server/http_dynamic_delay_filter_integration_test.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,16 +1,18 @@
#include <string>

#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:
Expand All @@ -20,136 +22,101 @@ 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<Envoy::Network::Address::IpVersion> {
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,
testing::ValuesIn(Envoy::TestEnvironment::getIpVersionsForTest()));

// 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
concurrency_based_linear_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 {
Expand Down
105 changes: 23 additions & 82 deletions test/server/http_time_tracking_filter_integration_test.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,18 +1,14 @@
#include <chrono>

#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"
#include "api/server/response_options.pb.validate.h"

#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"

Expand All @@ -32,67 +28,28 @@ name: time-tracking
)EOF";

class HttpTimeTrackingIntegrationTest
: public Envoy::HttpIntegrationTest,
: public HttpFilterIntegrationTestBase,
public testing::TestWithParam<Envoy::Network::Address::IpVersion> {
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,
testing::ValuesIn(Envoy::TestEnvironment::getIpVersionsForTest()));

// 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);
Expand All @@ -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);
Expand All @@ -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 {};

Expand Down