Skip to content

dfp: adding connection timing test #19766

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
alyssawilk merged 3 commits into from
Feb 2, 2022
Merged

dfp: adding connection timing test #19766

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
ed938f4
dfp: adding connection timing test
alyssawilk Feb 1, 2022
a4c9eac
spelling
alyssawilk Feb 1, 2022
1b24500
comments
alyssawilk Feb 1, 2022
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
73 changes: 62 additions & 11 deletions test/extensions/filters/http/dynamic_forward_proxy/proxy_filter_integration_test.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,12 +15,15 @@ namespace {
class ProxyFilterIntegrationTest : public testing::TestWithParam<Network::Address::IpVersion>,
public HttpIntegrationTest {
public:
ProxyFilterIntegrationTest() : HttpIntegrationTest(Http::CodecType::HTTP1, GetParam()) {}
ProxyFilterIntegrationTest() : HttpIntegrationTest(Http::CodecType::HTTP1, GetParam()) {
upstream_tls_ = true;
}

void initializeWithArgs(uint64_t max_hosts = 1024, uint32_t max_pending_requests = 1024,
const std::string& override_auto_sni_header = "") {
setUpstreamProtocol(Http::CodecType::HTTP1);
const std::string filename = TestEnvironment::temporaryPath("dns_cache.txt");
::unlink(filename.c_str());

@RyanTheOptimist RyanTheOptimist Feb 1, 2022

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Out of curiosity, with your new O_TRUNC writes, is this still required?

@alyssawilk alyssawilk Feb 1, 2022

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think so - not all the tests write the file and the file shouldnt exist in the tests which don't use one


const std::string filter = fmt::format(R"EOF(
name: dynamic_forward_proxy
Expand Down Expand Up @@ -143,6 +146,9 @@ name: envoy.clusters.dynamic_forward_proxy
}
}

void testConnectionTiming(IntegrationStreamDecoderPtr& response, bool cached_dns,

@RyanTheOptimist RyanTheOptimist Feb 1, 2022

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: newline before?

@alyssawilk alyssawilk Feb 1, 2022

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

int64_t original_usec);

bool upstream_tls_{};
std::string upstream_cert_name_{"upstreamlocalhost"};
CdsHelper cds_helper_;
Expand All @@ -151,6 +157,42 @@ name: envoy.clusters.dynamic_forward_proxy
bool use_cache_file_{};
};

int64_t getHeaderValue(const Http::ResponseHeaderMap& headers, absl::string_view name) {
EXPECT_FALSE(headers.get(Http::LowerCaseString(name)).empty());
int64_t val;
if (!headers.get(Http::LowerCaseString(name)).empty() &&
absl::SimpleAtoi(headers.get(Http::LowerCaseString(name))[0]->value().getStringView(),
&val)) {
return val;
}
return 0;
}

void ProxyFilterIntegrationTest::testConnectionTiming(IntegrationStreamDecoderPtr& response,
bool cached_dns, int64_t original_usec) {
int64_t dns_start = getHeaderValue(response->headers(), "dns_start");
int64_t dns_end = getHeaderValue(response->headers(), "dns_end");
int64_t connect_start = getHeaderValue(response->headers(), "upstream_connect_start");
int64_t connect_end = getHeaderValue(response->headers(), "upstream_connect_complete");
int64_t handshake_end = getHeaderValue(response->headers(), "upstream_handshake_complete");
int64_t request_send_end = getHeaderValue(response->headers(), "request_send_end");
int64_t response_begin = getHeaderValue(response->headers(), "response_begin");
Event::DispatcherImpl dispatcher("foo", *api_, timeSystem());

ASSERT_LT(original_usec, dns_start);
ASSERT_LE(dns_start, dns_end);
if (cached_dns) {
ASSERT_GE(dns_end, connect_start);
} else {
ASSERT_LE(dns_end, connect_start);
}
ASSERT_LE(connect_start, connect_end);
ASSERT_LE(connect_end, handshake_end);
ASSERT_LE(handshake_end, request_send_end);
ASSERT_LE(request_send_end, response_begin);
ASSERT_LT(handshake_end, timeSystem().monotonicTime().time_since_epoch().count());
}

class ProxyFilterWithSimtimeIntegrationTest : public Event::TestUsingSimulatedTime,
public ProxyFilterIntegrationTest {};

Expand All @@ -164,6 +206,13 @@ INSTANTIATE_TEST_SUITE_P(IpVersions, ProxyFilterIntegrationTest,
// A basic test where we pause a request to lookup localhost, and then do another request which
// should hit the TLS cache.
TEST_P(ProxyFilterIntegrationTest, RequestWithBody) {
int64_t original_usec = dispatcher_->timeSource().monotonicTime().time_since_epoch().count();

config_helper_.prependFilter(fmt::format(R"EOF(
name: stream-info-to-headers-filter
typed_config:
"@type": type.googleapis.com/google.protobuf.Empty)EOF"));

initializeWithArgs();
codec_client_ = makeHttpConnection(lookupPort("http"));
const Http::TestRequestHeaderMapImpl request_headers{
Expand All @@ -176,20 +225,24 @@ TEST_P(ProxyFilterIntegrationTest, RequestWithBody) {
auto response =
sendRequestAndWaitForResponse(request_headers, 1024, default_response_headers_, 1024);
checkSimpleRequestSuccess(1024, 1024, response.get());
testConnectionTiming(response, false, original_usec);
EXPECT_EQ(1, test_server_->counter("dns_cache.foo.dns_query_attempt")->value());
EXPECT_EQ(1, test_server_->counter("dns_cache.foo.host_added")->value());
// Make sure dns timings are tracked for cache-misses.
ASSERT_FALSE(response->headers().get(Http::LowerCaseString("dns_start")).empty());
ASSERT_FALSE(response->headers().get(Http::LowerCaseString("dns_end")).empty());

// Now send another request. This should hit the DNS cache.
response = sendRequestAndWaitForResponse(request_headers, 512, default_response_headers_, 512);
checkSimpleRequestSuccess(512, 512, response.get());
testConnectionTiming(response, true, original_usec);
EXPECT_EQ(1, test_server_->counter("dns_cache.foo.dns_query_attempt")->value());
EXPECT_EQ(1, test_server_->counter("dns_cache.foo.host_added")->value());
// Make sure dns timings are tracked for cache-hits.
ASSERT_FALSE(response->headers().get(Http::LowerCaseString("dns_start")).empty());
ASSERT_FALSE(response->headers().get(Http::LowerCaseString("dns_end")).empty());

const Extensions::TransportSockets::Tls::SslHandshakerImpl* ssl_socket =
dynamic_cast<const Extensions::TransportSockets::Tls::SslHandshakerImpl*>(
fake_upstream_connection_->connection().ssl().get());
EXPECT_STREQ("localhost", SSL_get_servername(ssl_socket->ssl(), TLSEXT_NAMETYPE_host_name));
}

// Currently if the first DNS resolution fails, the filter will continue with
Expand Down Expand Up @@ -297,8 +350,8 @@ TEST_P(ProxyFilterIntegrationTest, DNSCacheHostOverflow) {
EXPECT_EQ(1, test_server_->counter("dns_cache.foo.host_overflow")->value());
}

// Verify that upstream TLS works with auto verification for SAN as well as auto setting SNI.
TEST_P(ProxyFilterIntegrationTest, UpstreamTls) {
// Verify that the filter works without TLS.
TEST_P(ProxyFilterIntegrationTest, UpstreamCleartext) {
upstream_tls_ = true;
initializeWithArgs();
codec_client_ = makeHttpConnection(lookupPort("http"));
Expand All @@ -312,11 +365,6 @@ TEST_P(ProxyFilterIntegrationTest, UpstreamTls) {
auto response = codec_client_->makeHeaderOnlyRequest(request_headers);
waitForNextUpstreamRequest();

const Extensions::TransportSockets::Tls::SslHandshakerImpl* ssl_socket =
dynamic_cast<const Extensions::TransportSockets::Tls::SslHandshakerImpl*>(
fake_upstream_connection_->connection().ssl().get());
EXPECT_STREQ("localhost", SSL_get_servername(ssl_socket->ssl(), TLSEXT_NAMETYPE_host_name));

upstream_request_->encodeHeaders(default_response_headers_, true);
ASSERT_TRUE(response->waitForEndStream());
checkSimpleRequestSuccess(0, 0, response.get());
Expand Down Expand Up @@ -433,6 +481,7 @@ TEST_P(ProxyFilterIntegrationTest, UseCacheFile) {
}

TEST_P(ProxyFilterIntegrationTest, UseCacheFileAndTestHappyEyeballs) {
upstream_tls_ = false; // upstream creation doesn't handle autonomous_upstream_
autonomous_upstream_ = true;

config_helper_.addRuntimeOverride("envoy.reloadable_features.allow_multiple_dns_addresses",
Expand Down Expand Up @@ -461,6 +510,8 @@ TEST_P(ProxyFilterIntegrationTest, UseCacheFileAndTestHappyEyeballs) {
}

TEST_P(ProxyFilterIntegrationTest, MultipleRequestsLowStreamLimit) {
upstream_tls_ = false; // config below uses bootstrap, tls config is in cluster_

setDownstreamProtocol(Http::CodecType::HTTP2);
setUpstreamProtocol(Http::CodecType::HTTP2);

Expand Down
59 changes: 27 additions & 32 deletions test/integration/filters/stream_info_to_headers_filter.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,7 @@
namespace Envoy {
namespace {

uint64_t toMs(MonotonicTime time) {
return std::chrono::duration_cast<std::chrono::seconds>(time.time_since_epoch()).count();
}
std::string toUsec(MonotonicTime time) { return absl::StrCat(time.time_since_epoch().count()); }

} // namespace

Expand All @@ -32,13 +30,12 @@ class StreamInfoToHeadersFilter : public Http::PassThroughFilter {
StreamInfo::StreamInfo& stream_info = decoder_callbacks_->streamInfo();

if (stream_info.downstreamTiming().getValue(dns_start).has_value()) {
headers.addCopy(
Http::LowerCaseString("dns_start"),
absl::StrCat(toMs(stream_info.downstreamTiming().getValue(dns_start).value())));
headers.addCopy(Http::LowerCaseString("dns_start"),
toUsec(stream_info.downstreamTiming().getValue(dns_start).value()));
}
if (stream_info.downstreamTiming().getValue(dns_end).has_value()) {
headers.addCopy(Http::LowerCaseString("dns_end"),
absl::StrCat(toMs(stream_info.downstreamTiming().getValue(dns_end).value())));
toUsec(stream_info.downstreamTiming().getValue(dns_end).value()));
}
if (decoder_callbacks_->streamInfo().upstreamInfo()) {
if (decoder_callbacks_->streamInfo().upstreamInfo()->upstreamSslConnection()) {
Expand All @@ -48,33 +45,31 @@ class StreamInfoToHeadersFilter : public Http::PassThroughFilter {
}
headers.addCopy(Http::LowerCaseString("num_streams"),
decoder_callbacks_->streamInfo().upstreamInfo()->upstreamNumStreams());
}

return Http::FilterHeadersStatus::Continue;
}
Http::FilterTrailersStatus encodeTrailers(Http::ResponseTrailerMap& trailers) override {
ASSERT(decoder_callbacks_->streamInfo().upstreamInfo());
StreamInfo::UpstreamTiming& upstream_timing =
decoder_callbacks_->streamInfo().upstreamInfo()->upstreamTiming();
// Upstream metrics aren't available until the response is complete.
if (upstream_timing.upstream_connect_start_.has_value()) {
trailers.addCopy(
Http::LowerCaseString("upstream_connect_start"),
absl::StrCat(upstream_timing.upstream_connect_start_.value().time_since_epoch().count()));
}
if (upstream_timing.upstream_connect_complete_.has_value()) {
trailers.addCopy(
Http::LowerCaseString("upstream_connect_complete"),
absl::StrCat(
upstream_timing.upstream_connect_complete_.value().time_since_epoch().count()));
}
if (upstream_timing.upstream_handshake_complete_.has_value()) {
trailers.addCopy(
Http::LowerCaseString("upstream_handshake_complete"),
absl::StrCat(
upstream_timing.upstream_handshake_complete_.value().time_since_epoch().count()));
StreamInfo::UpstreamTiming& upstream_timing =
decoder_callbacks_->streamInfo().upstreamInfo()->upstreamTiming();
if (upstream_timing.upstream_connect_start_.has_value()) {
headers.addCopy(Http::LowerCaseString("upstream_connect_start"),
toUsec(upstream_timing.upstream_connect_start_.value()));
}
if (upstream_timing.upstream_connect_complete_.has_value()) {
headers.addCopy(Http::LowerCaseString("upstream_connect_complete"),
toUsec(upstream_timing.upstream_connect_complete_.value()));
}
if (upstream_timing.upstream_handshake_complete_.has_value()) {
headers.addCopy(Http::LowerCaseString("upstream_handshake_complete"),
toUsec(upstream_timing.upstream_handshake_complete_.value()));
}
if (upstream_timing.last_upstream_tx_byte_sent_.has_value()) {
headers.addCopy(Http::LowerCaseString("request_send_end"),
toUsec(upstream_timing.last_upstream_tx_byte_sent_.value()));
}
if (upstream_timing.first_upstream_rx_byte_received_.has_value()) {
headers.addCopy(Http::LowerCaseString("response_begin"),
toUsec(upstream_timing.first_upstream_rx_byte_received_.value()));
}
}
return Http::FilterTrailersStatus::Continue;
return Http::FilterHeadersStatus::Continue;
}
};

Expand Down
5 changes: 2 additions & 3 deletions test/integration/multiplexed_upstream_integration_test.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -153,9 +153,8 @@ void MultiplexedUpstreamIntegrationTest::bidirectionalStreaming(uint32_t bytes)
ASSERT_EQ(response->headers().get(Http::LowerCaseString("alpn"))[0]->value().getStringView(),
expected_alpn);

ASSERT_FALSE(response->trailers()->get(Http::LowerCaseString("upstream_connect_start")).empty());
ASSERT_FALSE(
response->trailers()->get(Http::LowerCaseString("upstream_connect_complete")).empty());
ASSERT_FALSE(response->headers().get(Http::LowerCaseString("upstream_connect_start")).empty());
ASSERT_FALSE(response->headers().get(Http::LowerCaseString("upstream_connect_complete")).empty());

ASSERT_FALSE(response->headers().get(Http::LowerCaseString("num_streams")).empty());
EXPECT_EQ(
Expand Down
2 changes: 1 addition & 1 deletion tools/base/envoy_python.bzl
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
load("@rules_python//python:defs.bzl", "py_binary", "py_library")
load("@rules_python//python:defs.bzl", "py_binary")
load("@base_pip3//:requirements.bzl", base_entry_point = "entry_point")

def envoy_py_test(name, package, visibility, envoy_prefix = "@envoy"):
Expand Down