From 7afc1b39c3e0e905b6f8096b545a96fd7f076de6 Mon Sep 17 00:00:00 2001 From: Alyssa Wilk Date: Mon, 29 Apr 2019 16:44:13 -0400 Subject: [PATCH 1/2] http: tracking 100s from upstream Signed-off-by: Alyssa Wilk --- source/common/router/router.cc | 7 +++++-- source/common/router/router.h | 3 ++- test/common/router/router_test.cc | 3 +++ 3 files changed, 10 insertions(+), 3 deletions(-) diff --git a/source/common/router/router.cc b/source/common/router/router.cc index e06c421decdb..36edea44898e 100644 --- a/source/common/router/router.cc +++ b/source/common/router/router.cc @@ -720,7 +720,10 @@ void Filter::handleNon5xxResponseHeaders(const Http::HeaderMap& headers, } } -void Filter::onUpstream100ContinueHeaders(Http::HeaderMapPtr&& headers) { +void Filter::onUpstream100ContinueHeaders(Http::HeaderMapPtr&& headers, + UpstreamRequest& upstream_request) { + const uint64_t response_code = Http::Utility::getResponseStatus(*headers); + chargeUpstreamCode(response_code, *headers, upstream_request.upstream_host_, false); ENVOY_STREAM_LOG(debug, "upstream 100 continue", *callbacks_); downstream_response_started_ = true; @@ -1030,7 +1033,7 @@ Filter::UpstreamRequest::~UpstreamRequest() { void Filter::UpstreamRequest::decode100ContinueHeaders(Http::HeaderMapPtr&& headers) { ASSERT(100 == Http::Utility::getResponseStatus(*headers)); - parent_.onUpstream100ContinueHeaders(std::move(headers)); + parent_.onUpstream100ContinueHeaders(std::move(headers), *this); } void Filter::UpstreamRequest::decodeHeaders(Http::HeaderMapPtr&& headers, bool end_stream) { diff --git a/source/common/router/router.h b/source/common/router/router.h index 0c560ba8fe80..e00811cd3982 100644 --- a/source/common/router/router.h +++ b/source/common/router/router.h @@ -382,7 +382,8 @@ class Filter : Logger::Loggable, void onPerTryTimeout(UpstreamRequest& upstream_request); void onRequestComplete(); void onResponseTimeout(); - void onUpstream100ContinueHeaders(Http::HeaderMapPtr&& headers); + void onUpstream100ContinueHeaders(Http::HeaderMapPtr&& headers, + UpstreamRequest& upstream_request); // Handle an upstream request aborted due to a local timeout. void onUpstreamTimeoutAbort(StreamInfo::ResponseFlag response_flag); // Handle an "aborted" upstream request, meaning we didn't see response diff --git a/test/common/router/router_test.cc b/test/common/router/router_test.cc index e083c5e587ad..2d73433e59c7 100644 --- a/test/common/router/router_test.cc +++ b/test/common/router/router_test.cc @@ -1585,6 +1585,9 @@ TEST_F(RouterTest, RetryUpstreamReset100ContinueResponseStarted) { EXPECT_CALL(callbacks_, encode100ContinueHeaders_(_)); Http::HeaderMapPtr continue_headers(new Http::TestHeaderMapImpl{{":status", "100"}}); response_decoder->decode100ContinueHeaders(std::move(continue_headers)); + EXPECT_EQ( + 1U, + cm_.thread_local_cluster_.cluster_.info_->stats_store_.counter("upstream_rq_100").value()); EXPECT_CALL(cm_.conn_pool_.host_->outlier_detector_, putHttpResponseCode(503)); encoder1.stream_.resetStream(Http::StreamResetReason::RemoteReset); } From 09a43fb73ef11788532d9315cc3c26d931ee1c7c Mon Sep 17 00:00:00 2001 From: Alyssa Wilk Date: Mon, 29 Apr 2019 17:01:22 -0400 Subject: [PATCH 2/2] reviewer comments Signed-off-by: Alyssa Wilk --- source/common/router/router.cc | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/source/common/router/router.cc b/source/common/router/router.cc index 36edea44898e..3e0d222ce889 100644 --- a/source/common/router/router.cc +++ b/source/common/router/router.cc @@ -722,8 +722,7 @@ void Filter::handleNon5xxResponseHeaders(const Http::HeaderMap& headers, void Filter::onUpstream100ContinueHeaders(Http::HeaderMapPtr&& headers, UpstreamRequest& upstream_request) { - const uint64_t response_code = Http::Utility::getResponseStatus(*headers); - chargeUpstreamCode(response_code, *headers, upstream_request.upstream_host_, false); + chargeUpstreamCode(100, *headers, upstream_request.upstream_host_, false); ENVOY_STREAM_LOG(debug, "upstream 100 continue", *callbacks_); downstream_response_started_ = true;