diff --git a/source/common/http/async_client_impl.cc b/source/common/http/async_client_impl.cc index 16dd4b953282..720184a96291 100644 --- a/source/common/http/async_client_impl.cc +++ b/source/common/http/async_client_impl.cc @@ -79,10 +79,14 @@ void AsyncRequestImpl::decodeHeaders(HeaderMapPtr&& headers, bool end_stream) { response_->headers().iterate([](const LowerCaseString& key, const std::string& value) -> void { log_debug(" '{}':'{}'", key.get(), value); }); #endif + bool is_canary = + response_->headers().get(Headers::get().EnvoyUpstreamCanary) == "true" || upstream_host_ + ? upstream_host_->canary() + : false; CodeUtility::ResponseStatInfo info{parent_.stats_store_, parent_.stat_prefix_, response_->headers(), true, EMPTY_STRING, EMPTY_STRING, - parent_.local_zone_name_, upstreamZone()}; + parent_.local_zone_name_, upstreamZone(), is_canary}; CodeUtility::chargeResponseStat(info); if (end_stream) { @@ -116,10 +120,13 @@ void AsyncRequestImpl::decodeTrailers(HeaderMapPtr&& trailers) { } void AsyncRequestImpl::onComplete() { + bool is_canary = + response_->headers().get(Headers::get().EnvoyUpstreamCanary) == "true" || upstream_host_ + ? upstream_host_->canary() + : false; + CodeUtility::ResponseTimingInfo info{ - parent_.stats_store_, parent_.stat_prefix_, stream_encoder_->requestCompleteTime(), - response_->headers().get(Headers::get().EnvoyUpstreamCanary) == "true" || - upstream_host_ ? upstream_host_->canary() : false, + parent_.stats_store_, parent_.stat_prefix_, stream_encoder_->requestCompleteTime(), is_canary, true, EMPTY_STRING, EMPTY_STRING, parent_.local_zone_name_, upstreamZone()}; CodeUtility::chargeResponseTiming(info); @@ -130,7 +137,7 @@ void AsyncRequestImpl::onComplete() { void AsyncRequestImpl::onResetStream(StreamResetReason) { CodeUtility::ResponseStatInfo info{parent_.stats_store_, parent_.stat_prefix_, SERVICE_UNAVAILABLE_HEADER, true, EMPTY_STRING, EMPTY_STRING, - parent_.local_zone_name_, upstreamZone()}; + parent_.local_zone_name_, upstreamZone(), false}; CodeUtility::chargeResponseStat(info); callbacks_.onFailure(AsyncClient::FailureReason::Reset); cleanup(); @@ -139,7 +146,7 @@ void AsyncRequestImpl::onResetStream(StreamResetReason) { void AsyncRequestImpl::onRequestTimeout() { CodeUtility::ResponseStatInfo info{parent_.stats_store_, parent_.stat_prefix_, REQUEST_TIMEOUT_HEADER, true, EMPTY_STRING, EMPTY_STRING, - parent_.local_zone_name_, upstreamZone()}; + parent_.local_zone_name_, upstreamZone(), false}; CodeUtility::chargeResponseStat(info); parent_.cluster_.stats().upstream_rq_timeout_.inc(); stream_encoder_->resetStream(); diff --git a/source/common/http/codes.cc b/source/common/http/codes.cc index d74776e83ea4..3ef9350921c4 100644 --- a/source/common/http/codes.cc +++ b/source/common/http/codes.cc @@ -25,7 +25,7 @@ void CodeUtility::chargeResponseStat(const ResponseStatInfo& info) { std::string group_string = groupStringForResponseCode(static_cast(response_code)); // If the response is from a canary, also create canary stats. - if (info.response_headers_.get(Headers::get().EnvoyUpstreamCanary) == "true") { + if (info.upstream_canary_) { info.store_.counter(fmt::format("{}canary.upstream_rq_{}", info.prefix_, group_string)).inc(); info.store_.counter(fmt::format("{}canary.upstream_rq_{}", info.prefix_, response_code)).inc(); } diff --git a/source/common/http/codes.h b/source/common/http/codes.h index f200ac4d330c..248e982e9b03 100644 --- a/source/common/http/codes.h +++ b/source/common/http/codes.h @@ -27,6 +27,7 @@ class CodeUtility { const std::string& request_vcluster_name_; const std::string& from_zone_; const std::string& to_zone_; + bool upstream_canary_; }; /** diff --git a/source/common/http/filter/ratelimit.cc b/source/common/http/filter/ratelimit.cc index d50e2b7968f7..9f6539148abf 100644 --- a/source/common/http/filter/ratelimit.cc +++ b/source/common/http/filter/ratelimit.cc @@ -84,7 +84,7 @@ void RateLimitFilter::complete(RateLimit::LimitStatus status) { config_->stats().counter(cluster_ratelimit_stat_prefix_ + "over_limit").inc(); Http::CodeUtility::ResponseStatInfo info{config_->stats(), cluster_stat_prefix_, TOO_MANY_REQUESTS_HEADER, true, EMPTY_STRING, - EMPTY_STRING, EMPTY_STRING, EMPTY_STRING}; + EMPTY_STRING, EMPTY_STRING, EMPTY_STRING, false}; Http::CodeUtility::chargeResponseStat(info); break; } diff --git a/source/common/router/router.cc b/source/common/router/router.cc index 661f3ebeced9..8066013d84a7 100644 --- a/source/common/router/router.cc +++ b/source/common/router/router.cc @@ -98,7 +98,8 @@ void Filter::chargeUpstreamCode(const Http::HeaderMap& response_headers) { Http::CodeUtility::ResponseStatInfo info{ config_->stats_store_, stat_prefix_, response_headers, downstream_headers_->get(Http::Headers::get().EnvoyInternalRequest) == "true", - route_->virtualHostName(), request_vcluster_name_, config_->service_zone_, upstreamZone()}; + route_->virtualHostName(), request_vcluster_name_, config_->service_zone_, upstreamZone(), + upstream_request_->upstream_canary_}; Http::CodeUtility::chargeResponseStat(info); @@ -106,7 +107,7 @@ void Filter::chargeUpstreamCode(const Http::HeaderMap& response_headers) { Http::CodeUtility::ResponseStatInfo info{ config_->stats_store_, alt_prefix, response_headers, downstream_headers_->get(Http::Headers::get().EnvoyInternalRequest) == "true", "", "", - config_->service_zone_, upstreamZone()}; + config_->service_zone_, upstreamZone(), upstream_request_->upstream_canary_}; Http::CodeUtility::chargeResponseStat(info); } @@ -382,9 +383,10 @@ void Filter::onUpstreamHeaders(Http::HeaderMapPtr&& headers, bool end_stream) { std::to_string(ms.count())); } - // TODO: Check host's canary status in addition to canary header. upstream_request_->upstream_canary_ = - headers->get(Http::Headers::get().EnvoyUpstreamCanary) == "true"; + headers->get(Http::Headers::get().EnvoyUpstreamCanary) == "true" || upstream_host_ + ? upstream_host_->canary() + : false; chargeUpstreamCode(*headers); downstream_response_started_ = true; diff --git a/test/common/http/async_client_impl_test.cc b/test/common/http/async_client_impl_test.cc index 742cbe20f4f9..2bd0cee7cafc 100644 --- a/test/common/http/async_client_impl_test.cc +++ b/test/common/http/async_client_impl_test.cc @@ -13,8 +13,8 @@ using testing::ByRef; using testing::Invoke; using testing::NiceMock; using testing::Ref; -using testing::ReturnRef; using testing::Return; +using testing::ReturnRef; namespace Http { @@ -273,8 +273,8 @@ TEST_F(AsyncClientImplTest, CanaryStatusTrue) { AsyncClientImpl client(cluster_, *this, stats_store_, dispatcher_, "from_az"); client.send(std::move(message_), callbacks_, Optional()); - HeaderMapPtr response_headers(new HeaderMapImpl{{":status", "200"}, - {"x-envoy-upstream-canary", "false"}}); + HeaderMapPtr response_headers( + new HeaderMapImpl{{":status", "200"}, {"x-envoy-upstream-canary", "false"}}); response_decoder_->decodeHeaders(std::move(response_headers), false); EXPECT_CALL(*conn_pool_.host_, canary()).WillOnce(Return(true)); EXPECT_CALL(stats_store_, deliverTimingToSinks("cluster.fake_cluster.upstream_rq_time", _)); @@ -283,10 +283,10 @@ TEST_F(AsyncClientImplTest, CanaryStatusTrue) { EXPECT_CALL(stats_store_, deliverTimingToSinks("cluster.fake_cluster.zone.from_az.to_az.upstream_rq_time", _)); - EXPECT_CALL(stats_store_, - deliverTimingToSinks("cluster.fake_cluster.canary.upstream_rq_time", _)); + EXPECT_CALL(stats_store_, + deliverTimingToSinks("cluster.fake_cluster.canary.upstream_rq_time", _)); response_decoder_->decodeData(data, true); - } +} TEST_F(AsyncClientImplTest, CanaryStatusFalse) { message_->body(Buffer::InstancePtr{new Buffer::OwnedImpl("test body")}); @@ -307,8 +307,8 @@ TEST_F(AsyncClientImplTest, CanaryStatusFalse) { AsyncClientImpl client(cluster_, *this, stats_store_, dispatcher_, "from_az"); client.send(std::move(message_), callbacks_, Optional()); - HeaderMapPtr response_headers(new HeaderMapImpl{{":status", "200"}, - {"x-envoy-upstream-canary", "false"}}); + HeaderMapPtr response_headers( + new HeaderMapImpl{{":status", "200"}, {"x-envoy-upstream-canary", "false"}}); response_decoder_->decodeHeaders(std::move(response_headers), false); EXPECT_CALL(*conn_pool_.host_, canary()).WillOnce(Return(false)); EXPECT_CALL(stats_store_, deliverTimingToSinks("cluster.fake_cluster.upstream_rq_time", _)); @@ -317,6 +317,6 @@ TEST_F(AsyncClientImplTest, CanaryStatusFalse) { EXPECT_CALL(stats_store_, deliverTimingToSinks("cluster.fake_cluster.zone.from_az.to_az.upstream_rq_time", _)); response_decoder_->decodeData(data, true); - } +} } // Http diff --git a/test/common/http/codes_test.cc b/test/common/http/codes_test.cc index 6e47afe93499..c1a225bb930a 100644 --- a/test/common/http/codes_test.cc +++ b/test/common/http/codes_test.cc @@ -22,7 +22,8 @@ class CodeUtilityTest : public testing::Test { } CodeUtility::ResponseStatInfo info{store_, "prefix.", headers, internal_request, - request_vhost_name, request_vcluster_name, from_az, to_az}; + request_vhost_name, request_vcluster_name, from_az, to_az, + true}; CodeUtility::chargeResponseStat(info); }