From ff4e6cb1c7e478c8022f759b4def70bfb8ee31c7 Mon Sep 17 00:00:00 2001 From: Roman Dzhabarov Date: Tue, 23 Aug 2016 11:25:24 -0700 Subject: [PATCH 01/10] initial commit for per az stats. --- source/common/http/async_client_impl.cc | 11 ++++++--- source/common/http/codes.cc | 15 +++++++++++ source/common/http/codes.h | 4 +++ source/common/http/filter/ratelimit.cc | 2 +- source/common/router/router.cc | 17 ++++++++++--- source/common/router/router.h | 13 ++++++---- source/server/config/http/router.cc | 3 ++- test/common/http/codes_test.cc | 33 ++++++++++++++++--------- test/common/router/router_test.cc | 14 ++++++++++- 9 files changed, 85 insertions(+), 27 deletions(-) diff --git a/source/common/http/async_client_impl.cc b/source/common/http/async_client_impl.cc index b178063a4bd1..52ac695e0371 100644 --- a/source/common/http/async_client_impl.cc +++ b/source/common/http/async_client_impl.cc @@ -77,7 +77,8 @@ void AsyncRequestImpl::decodeHeaders(HeaderMapPtr&& headers, bool end_stream) { #endif CodeUtility::ResponseStatInfo info{parent_.stats_store_, parent_.stat_prefix_, - response_->headers(), true, EMPTY_STRING, EMPTY_STRING}; + response_->headers(), true, EMPTY_STRING, EMPTY_STRING, + EMPTY_STRING, EMPTY_STRING}; CodeUtility::chargeResponseStat(info); if (end_stream) { @@ -115,7 +116,7 @@ void AsyncRequestImpl::onComplete() { CodeUtility::ResponseTimingInfo info{ parent_.stats_store_, parent_.stat_prefix_, stream_encoder_->requestCompleteTime(), response_->headers().get(Headers::get().EnvoyUpstreamCanary) == "true", true, EMPTY_STRING, - EMPTY_STRING}; + EMPTY_STRING, EMPTY_STRING, EMPTY_STRING}; CodeUtility::chargeResponseTiming(info); callbacks_.onSuccess(std::move(response_)); @@ -124,7 +125,8 @@ void AsyncRequestImpl::onComplete() { void AsyncRequestImpl::onResetStream(StreamResetReason) { CodeUtility::ResponseStatInfo info{parent_.stats_store_, parent_.stat_prefix_, - SERVICE_UNAVAILABLE_HEADER, true, EMPTY_STRING, EMPTY_STRING}; + SERVICE_UNAVAILABLE_HEADER, true, EMPTY_STRING, EMPTY_STRING, + EMPTY_STRING, EMPTY_STRING}; CodeUtility::chargeResponseStat(info); callbacks_.onFailure(AsyncClient::FailureReason::Reset); cleanup(); @@ -132,7 +134,8 @@ void AsyncRequestImpl::onResetStream(StreamResetReason) { void AsyncRequestImpl::onRequestTimeout() { CodeUtility::ResponseStatInfo info{parent_.stats_store_, parent_.stat_prefix_, - REQUEST_TIMEOUT_HEADER, true, EMPTY_STRING, EMPTY_STRING}; + REQUEST_TIMEOUT_HEADER, true, EMPTY_STRING, EMPTY_STRING, + EMPTY_STRING, EMPTY_STRING}; 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 80e3ecc14190..a60a5dd9c8bd 100644 --- a/source/common/http/codes.cc +++ b/source/common/http/codes.cc @@ -48,6 +48,14 @@ void CodeUtility::chargeResponseStat(const ResponseStatInfo& info) { info.store_.counter(fmt::format("vhost.{}.vcluster.{}.upstream_rq_{}", info.request_vhost_name_, info.request_vcluster_name_, response_code)).inc(); } + + // Handle per zone stats. + if (!info.from_az_.empty() && !info.to_az_.empty()) { + info.store_.counter(fmt::format("{}zone.{}.{}.upstream_rq_{}", info.prefix_, info.from_az_, + info.to_az_, group_string)).inc(); + info.store_.counter(fmt::format("{}zone.{}.{}.upstream_rq_{}", info.prefix_, info.from_az_, + info.to_az_, response_code)).inc(); + } } void CodeUtility::chargeResponseTiming(const ResponseTimingInfo& info) { @@ -71,6 +79,13 @@ void CodeUtility::chargeResponseTiming(const ResponseTimingInfo& info) { info.request_vcluster_name_ + ".upstream_rq_time", ms); } + + // Handle per zone stats. + if (!info.from_az_.empty() && !info.to_az_.empty()) { + info.store_.deliverTimingToSinks( + fmt::format("{}zone.{}.{}.upstream_rq_time", info.prefix_, info.from_az_, info.to_az_), + ms); + } } } diff --git a/source/common/http/codes.h b/source/common/http/codes.h index 0dd37fc1d3cb..2dcd406a9922 100644 --- a/source/common/http/codes.h +++ b/source/common/http/codes.h @@ -25,6 +25,8 @@ class CodeUtility { bool internal_request_; const std::string& request_vhost_name_; const std::string& request_vcluster_name_; + const std::string& from_az_; + const std::string& to_az_; }; /** @@ -42,6 +44,8 @@ class CodeUtility { bool internal_request_; const std::string& request_vhost_name_; const std::string& request_vcluster_name_; + const std::string& from_az_; + const std::string& to_az_; }; /** diff --git a/source/common/http/filter/ratelimit.cc b/source/common/http/filter/ratelimit.cc index 737fb7d89916..d50e2b7968f7 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}; Http::CodeUtility::chargeResponseStat(info); break; } diff --git a/source/common/router/router.cc b/source/common/router/router.cc index 2b28ac8d938e..df4122b0d85b 100644 --- a/source/common/router/router.cc +++ b/source/common/router/router.cc @@ -11,6 +11,7 @@ #include "envoy/upstream/upstream.h" #include "common/common/assert.h" +#include "common/common/empty_string.h" #include "common/common/enum_to_int.h" #include "common/common/utility.h" #include "common/http/codes.h" @@ -90,17 +91,21 @@ Filter::~Filter() { void Filter::chargeUpstreamCode(const Http::HeaderMap& response_headers) { if (!callbacks_->requestInfo().healthCheck()) { + const std::string& from_az = config_->service_zone_; + const std::string& to_az = upstream_host_ ? upstream_host_->zone() : EMPTY_STRING; + Http::CodeUtility::ResponseStatInfo info{ config_->stats_store_, stat_prefix_, response_headers, downstream_headers_->get(Http::Headers::get().EnvoyInternalRequest) == "true", - route_->virtualHostName(), request_vcluster_name_}; + route_->virtualHostName(), request_vcluster_name_, from_az, to_az}; Http::CodeUtility::chargeResponseStat(info); for (const std::string& alt_prefix : alt_stat_prefixes_) { Http::CodeUtility::ResponseStatInfo info{ config_->stats_store_, alt_prefix, response_headers, - downstream_headers_->get(Http::Headers::get().EnvoyInternalRequest) == "true", "", ""}; + downstream_headers_->get(Http::Headers::get().EnvoyInternalRequest) == "true", "", "", + from_az, to_az}; Http::CodeUtility::chargeResponseStat(info); } @@ -408,13 +413,16 @@ void Filter::onUpstreamComplete() { upstream_request_->upstream_encoder_->resetStream(); } + const std::string& from_az = config_->service_zone_; + const std::string& to_az = upstream_host_ ? upstream_host_->zone() : EMPTY_STRING; + if (!callbacks_->requestInfo().healthCheck()) { Http::CodeUtility::ResponseTimingInfo info{ config_->stats_store_, stat_prefix_, upstream_request_->upstream_encoder_->requestCompleteTime(), upstream_request_->upstream_canary_, downstream_headers_->get(Http::Headers::get().EnvoyInternalRequest) == "true", - route_->virtualHostName(), request_vcluster_name_}; + route_->virtualHostName(), request_vcluster_name_, from_az, to_az}; Http::CodeUtility::chargeResponseTiming(info); @@ -423,7 +431,8 @@ void Filter::onUpstreamComplete() { config_->stats_store_, alt_prefix, upstream_request_->upstream_encoder_->requestCompleteTime(), upstream_request_->upstream_canary_, - downstream_headers_->get(Http::Headers::get().EnvoyInternalRequest) == "true", "", ""}; + downstream_headers_->get(Http::Headers::get().EnvoyInternalRequest) == "true", "", "", + from_az, to_az}; Http::CodeUtility::chargeResponseTiming(info); } diff --git a/source/common/router/router.h b/source/common/router/router.h index 7a11a51049de..8776d08fd4a0 100644 --- a/source/common/router/router.h +++ b/source/common/router/router.h @@ -65,16 +65,17 @@ class FilterUtility { */ class FilterConfig { public: - FilterConfig(const std::string& stat_prefix, Stats::Store& stats, Upstream::ClusterManager& cm, - Runtime::Loader& runtime, Runtime::RandomGenerator& random, - ShadowWriterPtr&& shadow_writer) - : stats_store_(stats), cm_(cm), runtime_(runtime), random_(random), - stats_{ALL_ROUTER_STATS(POOL_COUNTER_PREFIX(stats, stat_prefix))}, + FilterConfig(const std::string& stat_prefix, const std::string& service_zone, Stats::Store& stats, + Upstream::ClusterManager& cm, Runtime::Loader& runtime, + Runtime::RandomGenerator& random, ShadowWriterPtr&& shadow_writer) + : stats_store_(stats), service_zone_(service_zone), cm_(cm), runtime_(runtime), + random_(random), stats_{ALL_ROUTER_STATS(POOL_COUNTER_PREFIX(stats, stat_prefix))}, shadow_writer_(std::move(shadow_writer)) {} ShadowWriter& shadowWriter() { return *shadow_writer_; } Stats::Store& stats_store_; + const std::string service_zone_; Upstream::ClusterManager& cm_; Runtime::Loader& runtime_; Runtime::RandomGenerator& random_; @@ -123,6 +124,7 @@ class Filter : Logger::Loggable, public Http::StreamDecoderF // Http::PooledStreamEncoderCallbacks void onUpstreamHostSelected(Upstream::HostDescriptionPtr host) override { + parent_.upstream_host_ = host; parent_.callbacks_->requestInfo().onUpstreamHostSelected(host); } @@ -176,6 +178,7 @@ class Filter : Logger::Loggable, public Http::StreamDecoderF Http::HeaderMap* downstream_trailers_{}; bool downstream_end_stream_{}; bool do_shadowing_{}; + Upstream::HostDescriptionPtr upstream_host_; }; class ProdFilter : public Filter { diff --git a/source/server/config/http/router.cc b/source/server/config/http/router.cc index 40fb1a42d007..d4dcc0f13102 100644 --- a/source/server/config/http/router.cc +++ b/source/server/config/http/router.cc @@ -18,7 +18,8 @@ class FilterConfig : public HttpFilterConfigFactory { } Router::FilterConfigPtr config(new Router::FilterConfig( - stat_prefix, server.stats(), server.clusterManager(), server.runtime(), server.random(), + stat_prefix, server.options().serviceZone(), server.stats(), server.clusterManager(), + server.runtime(), server.random(), Router::ShadowWriterPtr{new Router::ShadowWriterImpl(server.clusterManager())})); return [config](Http::FilterChainFactoryCallbacks& callbacks) -> void { diff --git a/test/common/http/codes_test.cc b/test/common/http/codes_test.cc index 7eb2d644ce08..6e47afe93499 100644 --- a/test/common/http/codes_test.cc +++ b/test/common/http/codes_test.cc @@ -1,3 +1,4 @@ +#include "common/common/empty_string.h" #include "common/http/codes.h" #include "common/http/header_map_impl.h" #include "common/stats/stats_impl.h" @@ -11,15 +12,17 @@ namespace Http { class CodeUtilityTest : public testing::Test { public: void addResponse(uint64_t code, bool canary, bool internal_request, - const std::string& request_vhost_name, - const std::string& request_vcluster_name) { + const std::string& request_vhost_name = EMPTY_STRING, + const std::string& request_vcluster_name = EMPTY_STRING, + const std::string& from_az = EMPTY_STRING, + const std::string& to_az = EMPTY_STRING) { HeaderMapImpl headers{{":status", std::to_string(code)}}; if (canary) { headers.addViaMove("x-envoy-upstream-canary", "true"); } CodeUtility::ResponseStatInfo info{store_, "prefix.", headers, internal_request, - request_vhost_name, request_vcluster_name}; + request_vhost_name, request_vcluster_name, from_az, to_az}; CodeUtility::chargeResponseStat(info); } @@ -28,10 +31,10 @@ class CodeUtilityTest : public testing::Test { }; TEST_F(CodeUtilityTest, NoCanary) { - addResponse(201, false, false, "", ""); - addResponse(301, false, true, "", ""); - addResponse(401, false, false, "", ""); - addResponse(501, false, true, "", ""); + addResponse(201, false, false); + addResponse(301, false, true); + addResponse(401, false, false); + addResponse(501, false, true); EXPECT_EQ(1U, store_.counter("prefix.upstream_rq_2xx").value()); EXPECT_EQ(1U, store_.counter("prefix.upstream_rq_201").value()); @@ -54,9 +57,9 @@ TEST_F(CodeUtilityTest, NoCanary) { } TEST_F(CodeUtilityTest, Canary) { - addResponse(200, true, true, "", ""); - addResponse(300, false, false, "", ""); - addResponse(500, true, false, "", ""); + addResponse(200, true, true); + addResponse(300, false, false); + addResponse(500, true, false); EXPECT_EQ(1U, store_.counter("prefix.upstream_rq_2xx").value()); EXPECT_EQ(1U, store_.counter("prefix.upstream_rq_200").value()); @@ -146,17 +149,25 @@ TEST_F(CodeUtilityTest, RequestVirtualCluster) { EXPECT_EQ(1U, store_.counter("vhost.test-vhost.vcluster.test-cluster.upstream_rq_200").value()); } +TEST_F(CodeUtilityTest, PerZoneStats) { + addResponse(200, false, false, "", "", "from_az", "to_az"); + + EXPECT_EQ(1U, store_.counter("prefix.zone.from_az.to_az.upstream_rq_200").value()); + EXPECT_EQ(1U, store_.counter("prefix.zone.from_az.to_az.upstream_rq_2xx").value()); +} + TEST(CodeUtilityResponseTimingTest, All) { Stats::MockStore store; CodeUtility::ResponseTimingInfo info{store, "prefix.", std::chrono::system_clock::now(), true, - true, "vhost_name", "req_vcluster_name"}; + true, "vhost_name", "req_vcluster_name", "from_az", "to_az"}; EXPECT_CALL(store, deliverTimingToSinks("prefix.upstream_rq_time", _)); EXPECT_CALL(store, deliverTimingToSinks("prefix.canary.upstream_rq_time", _)); EXPECT_CALL(store, deliverTimingToSinks("prefix.internal.upstream_rq_time", _)); EXPECT_CALL(store, deliverTimingToSinks( "vhost.vhost_name.vcluster.req_vcluster_name.upstream_rq_time", _)); + EXPECT_CALL(store, deliverTimingToSinks("prefix.zone.from_az.to_az.upstream_rq_time", _)); CodeUtility::chargeResponseTiming(info); } diff --git a/test/common/router/router_test.cc b/test/common/router/router_test.cc index f2d6fdbb62d6..0aa5f78a8ec1 100644 --- a/test/common/router/router_test.cc +++ b/test/common/router/router_test.cc @@ -37,11 +37,12 @@ class RouterTest : public testing::Test { public: RouterTest() : shadow_writer_(new MockShadowWriter()), - config_(new FilterConfig("test.", stats_store_, cm_, runtime_, random_, + config_(new FilterConfig("test.", "from_az", stats_store_, cm_, runtime_, random_, ShadowWriterPtr{shadow_writer_})), router_(config_) { router_.setDecoderFilterCallbacks(callbacks_); ON_CALL(*cm_.conn_pool_.host_, url()).WillByDefault(ReturnRef(host_url_)); + ON_CALL(*cm_.conn_pool_.host_, zone()).WillByDefault(ReturnRef(upstream_zone_)); } void expectResponseTimerCreate() { @@ -56,6 +57,7 @@ class RouterTest : public testing::Test { EXPECT_CALL(*per_try_timeout_, disableTimer()); } + std::string upstream_zone_{"to_az"}; Stats::IsolatedStoreImpl stats_store_; NiceMock cm_; NiceMock runtime_; @@ -523,6 +525,10 @@ TEST_F(RouterTest, RetryUpstream5xxNotComplete) { EXPECT_EQ(1U, stats_store_.counter("cluster.fake_cluster.retry.upstream_rq_503").value()); EXPECT_EQ(1U, stats_store_.counter("cluster.fake_cluster.upstream_rq_200").value()); + EXPECT_EQ( + 1U, stats_store_.counter("cluster.fake_cluster.zone.from_az.to_az.upstream_rq_200").value()); + EXPECT_EQ( + 1U, stats_store_.counter("cluster.fake_cluster.zone.from_az.to_az.upstream_rq_2xx").value()); } TEST_F(RouterTest, Shadow) { @@ -597,6 +603,12 @@ TEST_F(RouterTest, AltStatName) { .value()); EXPECT_EQ(1U, stats_store_.counter("cluster.fake_cluster.canary.upstream_rq_200").value()); EXPECT_EQ(1U, stats_store_.counter("cluster.fake_cluster.alt_stat.upstream_rq_200").value()); + EXPECT_EQ(1U, + stats_store_.counter("cluster.fake_cluster.alt_stat.zone.from_az.to_az.upstream_rq_200") + .value()); + EXPECT_EQ(1U, + stats_store_.counter("cluster.fake_cluster.alt_stat.zone.from_az.to_az.upstream_rq_200") + .value()); } TEST_F(RouterTest, Redirect) { From fa5985ade40cf8e2b8ba5aed3ecedaafcc0e07a0 Mon Sep 17 00:00:00 2001 From: Roman Dzhabarov Date: Wed, 24 Aug 2016 13:07:02 -0700 Subject: [PATCH 02/10] Inject az stats into AsyncClient and cover with tests. --- source/common/http/async_client_impl.cc | 17 +++++---- source/common/http/async_client_impl.h | 9 +++-- .../common/upstream/cluster_manager_impl.cc | 17 ++++----- source/common/upstream/cluster_manager_impl.h | 6 ++-- test/common/http/async_client_impl_test.cc | 35 +++++++++++++------ 5 files changed, 56 insertions(+), 28 deletions(-) diff --git a/source/common/http/async_client_impl.cc b/source/common/http/async_client_impl.cc index 52ac695e0371..c43661aba779 100644 --- a/source/common/http/async_client_impl.cc +++ b/source/common/http/async_client_impl.cc @@ -15,9 +15,9 @@ const HeaderMapImpl AsyncRequestImpl::REQUEST_TIMEOUT_HEADER{ AsyncClientImpl::AsyncClientImpl(const Upstream::Cluster& cluster, AsyncClientConnPoolFactory& factory, Stats::Store& stats_store, - Event::Dispatcher& dispatcher) + Event::Dispatcher& dispatcher, const std::string& local_zone_name) : cluster_(cluster), factory_(factory), stats_store_(stats_store), dispatcher_(dispatcher), - stat_prefix_(fmt::format("cluster.{}.", cluster.name())) {} + local_zone_name_(local_zone_name), stat_prefix_(fmt::format("cluster.{}.", cluster.name())) {} AsyncClientImpl::~AsyncClientImpl() { ASSERT(active_requests_.empty()); } @@ -76,9 +76,10 @@ void AsyncRequestImpl::decodeHeaders(HeaderMapPtr&& headers, bool end_stream) { -> void { log_debug(" '{}':'{}'", key.get(), value); }); #endif + const std::string& to_az = upstream_host_ ? upstream_host_->zone() : EMPTY_STRING; CodeUtility::ResponseStatInfo info{parent_.stats_store_, parent_.stat_prefix_, response_->headers(), true, EMPTY_STRING, EMPTY_STRING, - EMPTY_STRING, EMPTY_STRING}; + parent_.local_zone_name_, to_az}; CodeUtility::chargeResponseStat(info); if (end_stream) { @@ -112,11 +113,12 @@ void AsyncRequestImpl::decodeTrailers(HeaderMapPtr&& trailers) { } void AsyncRequestImpl::onComplete() { + const std::string& to_az = upstream_host_ ? upstream_host_->zone() : EMPTY_STRING; // TODO: Check host's canary status in addition to canary header. CodeUtility::ResponseTimingInfo info{ parent_.stats_store_, parent_.stat_prefix_, stream_encoder_->requestCompleteTime(), response_->headers().get(Headers::get().EnvoyUpstreamCanary) == "true", true, EMPTY_STRING, - EMPTY_STRING, EMPTY_STRING, EMPTY_STRING}; + EMPTY_STRING, parent_.local_zone_name_, to_az}; CodeUtility::chargeResponseTiming(info); callbacks_.onSuccess(std::move(response_)); @@ -124,18 +126,21 @@ void AsyncRequestImpl::onComplete() { } void AsyncRequestImpl::onResetStream(StreamResetReason) { + const std::string& to_az = upstream_host_ ? upstream_host_->zone() : EMPTY_STRING; + CodeUtility::ResponseStatInfo info{parent_.stats_store_, parent_.stat_prefix_, SERVICE_UNAVAILABLE_HEADER, true, EMPTY_STRING, EMPTY_STRING, - EMPTY_STRING, EMPTY_STRING}; + parent_.local_zone_name_, to_az}; CodeUtility::chargeResponseStat(info); callbacks_.onFailure(AsyncClient::FailureReason::Reset); cleanup(); } void AsyncRequestImpl::onRequestTimeout() { + const std::string& to_az = upstream_host_ ? upstream_host_->zone() : EMPTY_STRING; CodeUtility::ResponseStatInfo info{parent_.stats_store_, parent_.stat_prefix_, REQUEST_TIMEOUT_HEADER, true, EMPTY_STRING, EMPTY_STRING, - EMPTY_STRING, EMPTY_STRING}; + parent_.local_zone_name_, to_az}; CodeUtility::chargeResponseStat(info); parent_.cluster_.stats().upstream_rq_timeout_.inc(); stream_encoder_->resetStream(); diff --git a/source/common/http/async_client_impl.h b/source/common/http/async_client_impl.h index afc1b693afda..715eb564d1cb 100644 --- a/source/common/http/async_client_impl.h +++ b/source/common/http/async_client_impl.h @@ -33,7 +33,8 @@ class AsyncRequestImpl; class AsyncClientImpl final : public AsyncClient { public: AsyncClientImpl(const Upstream::Cluster& cluster, AsyncClientConnPoolFactory& factory, - Stats::Store& stats_store, Event::Dispatcher& dispatcher); + Stats::Store& stats_store, Event::Dispatcher& dispatcher, + const std::string& local_zone_name); ~AsyncClientImpl(); // Http::AsyncClient @@ -45,6 +46,7 @@ class AsyncClientImpl final : public AsyncClient { AsyncClientConnPoolFactory& factory_; Stats::Store& stats_store_; Event::Dispatcher& dispatcher_; + const std::string local_zone_name_; const std::string stat_prefix_; std::list> active_requests_; @@ -80,7 +82,9 @@ class AsyncRequestImpl final : public AsyncClient::Request, void onResetStream(StreamResetReason reason) override; // Http::PooledStreamEncoderCallbacks - void onUpstreamHostSelected(Upstream::HostDescriptionPtr) override {} + void onUpstreamHostSelected(Upstream::HostDescriptionPtr upstream_host) override { + upstream_host_ = upstream_host; + } void onComplete(); @@ -93,6 +97,7 @@ class AsyncRequestImpl final : public AsyncClient::Request, Event::TimerPtr request_timeout_; std::unique_ptr response_; PooledStreamEncoderPtr stream_encoder_; + Upstream::HostDescriptionPtr upstream_host_; static const HeaderMapImpl SERVICE_UNAVAILABLE_HEADER; static const HeaderMapImpl REQUEST_TIMEOUT_HEADER; diff --git a/source/common/upstream/cluster_manager_impl.cc b/source/common/upstream/cluster_manager_impl.cc index 8b78bd22ed59..b0359d654c7f 100644 --- a/source/common/upstream/cluster_manager_impl.cc +++ b/source/common/upstream/cluster_manager_impl.cc @@ -41,10 +41,10 @@ ClusterManagerImpl::ClusterManagerImpl(const Json::Object& config, Stats::Store& } tls.set(thread_local_slot_, - [this, &stats, &runtime, &random](Event::Dispatcher& dispatcher) + [this, &stats, &runtime, &random, &local_zone_name](Event::Dispatcher& dispatcher) -> ThreadLocal::ThreadLocalObjectPtr { - return ThreadLocal::ThreadLocalObjectPtr{ - new ThreadLocalClusterManagerImpl(*this, dispatcher, runtime, random)}; + return ThreadLocal::ThreadLocalObjectPtr{new ThreadLocalClusterManagerImpl( + *this, dispatcher, runtime, random, local_zone_name)}; }); // To avoid threading issues, for those clusters that start with hosts already in them (like @@ -202,11 +202,11 @@ Http::AsyncClient& ClusterManagerImpl::httpAsyncClientForCluster(const std::stri ClusterManagerImpl::ThreadLocalClusterManagerImpl::ThreadLocalClusterManagerImpl( ClusterManagerImpl& parent, Event::Dispatcher& dispatcher, Runtime::Loader& runtime, - Runtime::RandomGenerator& random) + Runtime::RandomGenerator& random, const std::string& local_zone_name) : parent_(parent), dispatcher_(dispatcher) { for (auto& cluster : parent.primary_clusters_) { - thread_local_clusters_[cluster.first].reset( - new ClusterEntry(*this, *cluster.second, runtime, random, parent.stats_, dispatcher)); + thread_local_clusters_[cluster.first].reset(new ClusterEntry( + *this, *cluster.second, runtime, random, parent.stats_, dispatcher, local_zone_name)); } for (auto& cluster : thread_local_clusters_) { @@ -251,9 +251,10 @@ void ClusterManagerImpl::ThreadLocalClusterManagerImpl::shutdown() { ClusterManagerImpl::ThreadLocalClusterManagerImpl::ClusterEntry::ClusterEntry( ThreadLocalClusterManagerImpl& parent, const Cluster& cluster, Runtime::Loader& runtime, - Runtime::RandomGenerator& random, Stats::Store& stats_store, Event::Dispatcher& dispatcher) + Runtime::RandomGenerator& random, Stats::Store& stats_store, Event::Dispatcher& dispatcher, + const std::string& local_zone_name) : parent_(parent), primary_cluster_(cluster), - http_async_client_(cluster, *this, stats_store, dispatcher) { + http_async_client_(cluster, *this, stats_store, dispatcher, local_zone_name) { switch (cluster.lbType()) { case LoadBalancerType::LeastRequest: { diff --git a/source/common/upstream/cluster_manager_impl.h b/source/common/upstream/cluster_manager_impl.h index 4b00de9c0012..1dd4f682d779 100644 --- a/source/common/upstream/cluster_manager_impl.h +++ b/source/common/upstream/cluster_manager_impl.h @@ -65,7 +65,8 @@ class ClusterManagerImpl : public ClusterManager { struct ClusterEntry : public Http::AsyncClientConnPoolFactory { ClusterEntry(ThreadLocalClusterManagerImpl& parent, const Cluster& cluster, Runtime::Loader& runtime, Runtime::RandomGenerator& random, - Stats::Store& stats_store, Event::Dispatcher& dispatcher); + Stats::Store& stats_store, Event::Dispatcher& dispatcher, + const std::string& local_zone_name); // Http::AsyncClientConnPoolFactory Http::ConnectionPool::Instance* connPool() override; @@ -80,7 +81,8 @@ class ClusterManagerImpl : public ClusterManager { typedef std::unique_ptr ClusterEntryPtr; ThreadLocalClusterManagerImpl(ClusterManagerImpl& parent, Event::Dispatcher& dispatcher, - Runtime::Loader& runtime, Runtime::RandomGenerator& random); + Runtime::Loader& runtime, Runtime::RandomGenerator& random, + const std::string& local_zone_name); static void updateClusterMembership(const std::string& name, ConstHostVectorPtr hosts, ConstHostVectorPtr healthy_hosts, diff --git a/test/common/http/async_client_impl_test.cc b/test/common/http/async_client_impl_test.cc index cb0bfe15b695..cbc7c9ea0526 100644 --- a/test/common/http/async_client_impl_test.cc +++ b/test/common/http/async_client_impl_test.cc @@ -13,16 +13,21 @@ using testing::ByRef; using testing::Invoke; using testing::NiceMock; using testing::Ref; +using testing::ReturnRef; namespace Http { class AsyncClientImplTest : public testing::Test, public AsyncClientConnPoolFactory { public: - AsyncClientImplTest() { HttpTestUtility::addDefaultHeaders(message_->headers()); } + AsyncClientImplTest() { + HttpTestUtility::addDefaultHeaders(message_->headers()); + ON_CALL(*conn_pool_.host_, zone()).WillByDefault(ReturnRef(upstream_zone_)); + } // Http::AsyncClientConnPoolFactory Http::ConnectionPool::Instance* connPool() override { return &conn_pool_; } + std::string upstream_zone_{"to_az"}; MessagePtr message_{new RequestMessageImpl()}; MockAsyncClientCallbacks callbacks_; ConnectionPool::MockInstance conn_pool_; @@ -50,16 +55,20 @@ TEST_F(AsyncClientImplTest, Basic) { EXPECT_CALL(stream_encoder_, encodeData(BufferEqual(&data), true)); EXPECT_CALL(callbacks_, onSuccess_(_)); - AsyncClientImpl client(cluster_, *this, stats_store_, dispatcher_); + AsyncClientImpl client(cluster_, *this, stats_store_, dispatcher_, "from_az"); client.send(std::move(message_), callbacks_, Optional()); - EXPECT_CALL(stats_store_, counter("cluster.fake_cluster.upstream_rq_2xx")); + EXPECT_CALL(stats_store_, counter("cluster.fake_cluster.zone.from_az.to_az.upstream_rq_200")); + EXPECT_CALL(stats_store_, counter("cluster.fake_cluster.zone.from_az.to_az.upstream_rq_2xx")); EXPECT_CALL(stats_store_, counter("cluster.fake_cluster.upstream_rq_200")); + EXPECT_CALL(stats_store_, counter("cluster.fake_cluster.upstream_rq_2xx")); EXPECT_CALL(stats_store_, counter("cluster.fake_cluster.internal.upstream_rq_2xx")); EXPECT_CALL(stats_store_, counter("cluster.fake_cluster.internal.upstream_rq_200")); EXPECT_CALL(stats_store_, deliverTimingToSinks("cluster.fake_cluster.upstream_rq_time", _)); EXPECT_CALL(stats_store_, deliverTimingToSinks("cluster.fake_cluster.internal.upstream_rq_time", _)); + EXPECT_CALL(stats_store_, + deliverTimingToSinks("cluster.fake_cluster.zone.from_az.to_az.upstream_rq_time", _)); HeaderMapPtr response_headers(new HeaderMapImpl{{":status", "200"}}); response_decoder_->decodeHeaders(std::move(response_headers), false); @@ -82,7 +91,7 @@ TEST_F(AsyncClientImplTest, MultipleRequests) { EXPECT_CALL(stream_encoder_, encodeHeaders(HeaderMapEqualRef(ByRef(message_->headers())), false)); EXPECT_CALL(stream_encoder_, encodeData(BufferEqual(&data), true)); - AsyncClientImpl client(cluster_, *this, stats_store_, dispatcher_); + AsyncClientImpl client(cluster_, *this, stats_store_, dispatcher_, "from_az"); client.send(std::move(message_), callbacks_, Optional()); // Send request 2. @@ -129,7 +138,7 @@ TEST_F(AsyncClientImplTest, Trailers) { EXPECT_CALL(stream_encoder_, encodeData(BufferEqual(&data), true)); EXPECT_CALL(callbacks_, onSuccess_(_)); - AsyncClientImpl client(cluster_, *this, stats_store_, dispatcher_); + AsyncClientImpl client(cluster_, *this, stats_store_, dispatcher_, "from_az"); client.send(std::move(message_), callbacks_, Optional()); HeaderMapPtr response_headers(new HeaderMapImpl{{":status", "200"}}); response_decoder_->decodeHeaders(std::move(response_headers), false); @@ -140,6 +149,8 @@ TEST_F(AsyncClientImplTest, Trailers) { TEST_F(AsyncClientImplTest, FailRequest) { EXPECT_CALL(stats_store_, counter("cluster.fake_cluster.upstream_rq_5xx")); EXPECT_CALL(stats_store_, counter("cluster.fake_cluster.upstream_rq_503")); + EXPECT_CALL(stats_store_, counter("cluster.fake_cluster.zone.from_az.to_az.upstream_rq_503")); + EXPECT_CALL(stats_store_, counter("cluster.fake_cluster.zone.from_az.to_az.upstream_rq_5xx")); EXPECT_CALL(stats_store_, counter("cluster.fake_cluster.internal.upstream_rq_5xx")); EXPECT_CALL(stats_store_, counter("cluster.fake_cluster.internal.upstream_rq_503")); @@ -153,7 +164,7 @@ TEST_F(AsyncClientImplTest, FailRequest) { EXPECT_CALL(stream_encoder_, encodeHeaders(HeaderMapEqualRef(ByRef(message_->headers())), true)); EXPECT_CALL(callbacks_, onFailure(Http::AsyncClient::FailureReason::Reset)); - AsyncClientImpl client(cluster_, *this, stats_store_, dispatcher_); + AsyncClientImpl client(cluster_, *this, stats_store_, dispatcher_, "from_az"); client.send(std::move(message_), callbacks_, Optional()); stream_encoder_.getStream().resetStream(StreamResetReason::RemoteReset); } @@ -169,7 +180,7 @@ TEST_F(AsyncClientImplTest, CancelRequest) { EXPECT_CALL(stream_encoder_, encodeHeaders(HeaderMapEqualRef(ByRef(message_->headers())), true)); EXPECT_CALL(stream_encoder_.stream_, resetStream(_)); - AsyncClientImpl client(cluster_, *this, stats_store_, dispatcher_); + AsyncClientImpl client(cluster_, *this, stats_store_, dispatcher_, "from_az"); AsyncClient::Request* request = client.send(std::move(message_), callbacks_, Optional()); request->cancel(); @@ -178,6 +189,8 @@ TEST_F(AsyncClientImplTest, CancelRequest) { TEST_F(AsyncClientImplTest, PoolFailure) { EXPECT_CALL(stats_store_, counter("cluster.fake_cluster.upstream_rq_5xx")); EXPECT_CALL(stats_store_, counter("cluster.fake_cluster.upstream_rq_503")); + EXPECT_CALL(stats_store_, counter("cluster.fake_cluster.zone.from_az.to_az.upstream_rq_5xx")); + EXPECT_CALL(stats_store_, counter("cluster.fake_cluster.zone.from_az.to_az.upstream_rq_503")); EXPECT_CALL(stats_store_, counter("cluster.fake_cluster.internal.upstream_rq_5xx")); EXPECT_CALL(stats_store_, counter("cluster.fake_cluster.internal.upstream_rq_503")); @@ -190,7 +203,7 @@ TEST_F(AsyncClientImplTest, PoolFailure) { })); EXPECT_CALL(callbacks_, onFailure(Http::AsyncClient::FailureReason::Reset)); - AsyncClientImpl client(cluster_, *this, stats_store_, dispatcher_); + AsyncClientImpl client(cluster_, *this, stats_store_, dispatcher_, "from_az"); EXPECT_EQ(nullptr, client.send(std::move(message_), callbacks_, Optional())); } @@ -198,6 +211,8 @@ TEST_F(AsyncClientImplTest, PoolFailure) { TEST_F(AsyncClientImplTest, RequestTimeout) { EXPECT_CALL(stats_store_, counter("cluster.fake_cluster.upstream_rq_5xx")); EXPECT_CALL(stats_store_, counter("cluster.fake_cluster.upstream_rq_504")); + EXPECT_CALL(stats_store_, counter("cluster.fake_cluster.zone.from_az.to_az.upstream_rq_5xx")); + EXPECT_CALL(stats_store_, counter("cluster.fake_cluster.zone.from_az.to_az.upstream_rq_504")); EXPECT_CALL(stats_store_, counter("cluster.fake_cluster.internal.upstream_rq_5xx")); EXPECT_CALL(stats_store_, counter("cluster.fake_cluster.internal.upstream_rq_504")); EXPECT_CALL(conn_pool_, newStream(_, _)) @@ -212,7 +227,7 @@ TEST_F(AsyncClientImplTest, RequestTimeout) { timer_ = new NiceMock(&dispatcher_); EXPECT_CALL(*timer_, enableTimer(std::chrono::milliseconds(40))); EXPECT_CALL(stream_encoder_.stream_, resetStream(_)); - AsyncClientImpl client(cluster_, *this, stats_store_, dispatcher_); + AsyncClientImpl client(cluster_, *this, stats_store_, dispatcher_, "from_az"); client.send(std::move(message_), callbacks_, std::chrono::milliseconds(40)); timer_->callback_(); @@ -232,7 +247,7 @@ TEST_F(AsyncClientImplTest, DisableTimer) { EXPECT_CALL(*timer_, enableTimer(std::chrono::milliseconds(200))); EXPECT_CALL(*timer_, disableTimer()); EXPECT_CALL(stream_encoder_.stream_, resetStream(_)); - AsyncClientImpl client(cluster_, *this, stats_store_, dispatcher_); + AsyncClientImpl client(cluster_, *this, stats_store_, dispatcher_, "from_az"); AsyncClient::Request* request = client.send(std::move(message_), callbacks_, std::chrono::milliseconds(200)); request->cancel(); From 598e8488810f71946392aced08ac88f9709b1d04 Mon Sep 17 00:00:00 2001 From: Roman Dzhabarov Date: Wed, 24 Aug 2016 13:24:18 -0700 Subject: [PATCH 03/10] docs. --- .../cluster_manager/cluster_stats.rst | 21 +++++++++++++++++-- 1 file changed, 19 insertions(+), 2 deletions(-) diff --git a/docs/configuration/cluster_manager/cluster_stats.rst b/docs/configuration/cluster_manager/cluster_stats.rst index a86c0a0308dd..7c97a24cd11d 100644 --- a/docs/configuration/cluster_manager/cluster_stats.rst +++ b/docs/configuration/cluster_manager/cluster_stats.rst @@ -85,8 +85,8 @@ are rooted at *cluster..* and contain the following statistics: :header: Name, Type, Description :widths: 1, 1, 2 - upstream_rq_<\*xx>, Counter, "Aggregate HTTP response codes (e.g., 2xx, 3xx, etc.)" - upstream_rq_<\*>, Counter, "Specific HTTP response codes (e.g., 201, 302, etc.)" + upstream_rq_<\*xx>, Counter, Aggregate HTTP response codes (e.g., 2xx, 3xx, etc.) + upstream_rq_<\*>, Counter, Specific HTTP response codes (e.g., 201, 302, etc.) upstream_rq_time, Timer, Request time milliseconds canary.upstream_rq_<\*xx>, Counter, Upstream canary aggregate HTTP response codes canary.upstream_rq_<\*>, Counter, Upstream canary specific HTTP response codes @@ -107,3 +107,20 @@ If alternate tree statistics are configured, they will be present in the *cluster...* namespace. The statistics produced are the same as documented in the dynamic HTTP statistics section :ref:`above `. + +.. _config_cluster_manager_cluster_per_az_stats: + +Per service zone dynamic HTTP statistics +---------------------------------------- + +If service zone is available for local service :option:`--service-zone` +and :ref:`upstream cluster `, +Envoy will track the following statistics in *cluster..zone...* namespace. + +.. csv-table:: + :header: Name, Type, Description + :widths: 1, 1, 2 + + upstream_rq_<\*xx>, Counter, Aggregate HTTP response codes (e.g., 2xx, 3xx, etc.) + upstream_rq_<\*>, Counter, Specific HTTP response codes (e.g., 201, 302, etc.) + upstream_rq_time, Timer, Request time milliseconds From b48975dcbe22018059d43aac4edb9de0f3ff91fa Mon Sep 17 00:00:00 2001 From: Roman Dzhabarov Date: Wed, 24 Aug 2016 13:54:32 -0700 Subject: [PATCH 04/10] Fixing docs format. --- docs/configuration/cluster_manager/cluster_stats.rst | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/docs/configuration/cluster_manager/cluster_stats.rst b/docs/configuration/cluster_manager/cluster_stats.rst index 7c97a24cd11d..43ad20cb56b7 100644 --- a/docs/configuration/cluster_manager/cluster_stats.rst +++ b/docs/configuration/cluster_manager/cluster_stats.rst @@ -85,8 +85,8 @@ are rooted at *cluster..* and contain the following statistics: :header: Name, Type, Description :widths: 1, 1, 2 - upstream_rq_<\*xx>, Counter, Aggregate HTTP response codes (e.g., 2xx, 3xx, etc.) - upstream_rq_<\*>, Counter, Specific HTTP response codes (e.g., 201, 302, etc.) + upstream_rq_<\*xx>, Counter, "Aggregate HTTP response codes (e.g., 2xx, 3xx, etc.)" + upstream_rq_<\*>, "Counter, Specific HTTP response codes (e.g., 201, 302, etc.)" upstream_rq_time, Timer, Request time milliseconds canary.upstream_rq_<\*xx>, Counter, Upstream canary aggregate HTTP response codes canary.upstream_rq_<\*>, Counter, Upstream canary specific HTTP response codes @@ -121,6 +121,6 @@ Envoy will track the following statistics in *cluster..zone.., Counter, Aggregate HTTP response codes (e.g., 2xx, 3xx, etc.) - upstream_rq_<\*>, Counter, Specific HTTP response codes (e.g., 201, 302, etc.) + upstream_rq_<\*xx>, Counter, "Aggregate HTTP response codes (e.g., 2xx, 3xx, etc.)" + upstream_rq_<\*>, Counter, "Specific HTTP response codes (e.g., 201, 302, etc.)" upstream_rq_time, Timer, Request time milliseconds From 2d8163b8bd126b00d49cf6ed956caa143d890235 Mon Sep 17 00:00:00 2001 From: Roman Dzhabarov Date: Wed, 24 Aug 2016 14:02:50 -0700 Subject: [PATCH 05/10] Change names for az. --- source/common/http/codes.cc | 14 +++++++------- source/common/http/codes.h | 8 ++++---- source/common/upstream/cluster_manager_impl.cc | 2 +- 3 files changed, 12 insertions(+), 12 deletions(-) diff --git a/source/common/http/codes.cc b/source/common/http/codes.cc index a60a5dd9c8bd..a0bbbe57d73d 100644 --- a/source/common/http/codes.cc +++ b/source/common/http/codes.cc @@ -50,11 +50,11 @@ void CodeUtility::chargeResponseStat(const ResponseStatInfo& info) { } // Handle per zone stats. - if (!info.from_az_.empty() && !info.to_az_.empty()) { - info.store_.counter(fmt::format("{}zone.{}.{}.upstream_rq_{}", info.prefix_, info.from_az_, - info.to_az_, group_string)).inc(); - info.store_.counter(fmt::format("{}zone.{}.{}.upstream_rq_{}", info.prefix_, info.from_az_, - info.to_az_, response_code)).inc(); + if (!info.from_zone_.empty() && !info.to_zone_.empty()) { + info.store_.counter(fmt::format("{}zone.{}.{}.upstream_rq_{}", info.prefix_, info.from_zone_, + info.to_zone_, group_string)).inc(); + info.store_.counter(fmt::format("{}zone.{}.{}.upstream_rq_{}", info.prefix_, info.from_zone_, + info.to_zone_, response_code)).inc(); } } @@ -81,9 +81,9 @@ void CodeUtility::chargeResponseTiming(const ResponseTimingInfo& info) { } // Handle per zone stats. - if (!info.from_az_.empty() && !info.to_az_.empty()) { + if (!info.from_zone_.empty() && !info.to_zone_.empty()) { info.store_.deliverTimingToSinks( - fmt::format("{}zone.{}.{}.upstream_rq_time", info.prefix_, info.from_az_, info.to_az_), + fmt::format("{}zone.{}.{}.upstream_rq_time", info.prefix_, info.from_zone_, info.to_zone_), ms); } } diff --git a/source/common/http/codes.h b/source/common/http/codes.h index 2dcd406a9922..f200ac4d330c 100644 --- a/source/common/http/codes.h +++ b/source/common/http/codes.h @@ -25,8 +25,8 @@ class CodeUtility { bool internal_request_; const std::string& request_vhost_name_; const std::string& request_vcluster_name_; - const std::string& from_az_; - const std::string& to_az_; + const std::string& from_zone_; + const std::string& to_zone_; }; /** @@ -44,8 +44,8 @@ class CodeUtility { bool internal_request_; const std::string& request_vhost_name_; const std::string& request_vcluster_name_; - const std::string& from_az_; - const std::string& to_az_; + const std::string& from_zone_; + const std::string& to_zone_; }; /** diff --git a/source/common/upstream/cluster_manager_impl.cc b/source/common/upstream/cluster_manager_impl.cc index b0359d654c7f..84c301601a70 100644 --- a/source/common/upstream/cluster_manager_impl.cc +++ b/source/common/upstream/cluster_manager_impl.cc @@ -41,7 +41,7 @@ ClusterManagerImpl::ClusterManagerImpl(const Json::Object& config, Stats::Store& } tls.set(thread_local_slot_, - [this, &stats, &runtime, &random, &local_zone_name](Event::Dispatcher& dispatcher) + [this, &stats, &runtime, &random, local_zone_name](Event::Dispatcher& dispatcher) -> ThreadLocal::ThreadLocalObjectPtr { return ThreadLocal::ThreadLocalObjectPtr{new ThreadLocalClusterManagerImpl( *this, dispatcher, runtime, random, local_zone_name)}; From d88098c963ec0f12de5e629bcdb540b06062ba26 Mon Sep 17 00:00:00 2001 From: Roman Dzhabarov Date: Wed, 24 Aug 2016 14:22:46 -0700 Subject: [PATCH 06/10] Fix comments. --- source/common/http/async_client_impl.cc | 17 ++++++++--------- source/common/http/async_client_impl.h | 2 ++ source/common/http/codes.cc | 6 +++--- source/common/router/router.cc | 18 ++++++++---------- source/common/router/router.h | 1 + 5 files changed, 22 insertions(+), 22 deletions(-) diff --git a/source/common/http/async_client_impl.cc b/source/common/http/async_client_impl.cc index c43661aba779..e39a105e1ba9 100644 --- a/source/common/http/async_client_impl.cc +++ b/source/common/http/async_client_impl.cc @@ -62,6 +62,10 @@ AsyncRequestImpl::AsyncRequestImpl(MessagePtr&& request, AsyncClientImpl& parent AsyncRequestImpl::~AsyncRequestImpl() { ASSERT(!stream_encoder_); } +const std::string& AsyncRequestImpl::upstreamZone() { + return upstream_host_ ? upstream_host_->zone() : EMPTY_STRING; +} + void AsyncRequestImpl::cancel() { ASSERT(stream_encoder_); stream_encoder_->resetStream(); @@ -76,10 +80,9 @@ void AsyncRequestImpl::decodeHeaders(HeaderMapPtr&& headers, bool end_stream) { -> void { log_debug(" '{}':'{}'", key.get(), value); }); #endif - const std::string& to_az = upstream_host_ ? upstream_host_->zone() : EMPTY_STRING; CodeUtility::ResponseStatInfo info{parent_.stats_store_, parent_.stat_prefix_, response_->headers(), true, EMPTY_STRING, EMPTY_STRING, - parent_.local_zone_name_, to_az}; + parent_.local_zone_name_, upstreamZone()}; CodeUtility::chargeResponseStat(info); if (end_stream) { @@ -113,12 +116,11 @@ void AsyncRequestImpl::decodeTrailers(HeaderMapPtr&& trailers) { } void AsyncRequestImpl::onComplete() { - const std::string& to_az = upstream_host_ ? upstream_host_->zone() : EMPTY_STRING; // TODO: Check host's canary status in addition to canary header. CodeUtility::ResponseTimingInfo info{ parent_.stats_store_, parent_.stat_prefix_, stream_encoder_->requestCompleteTime(), response_->headers().get(Headers::get().EnvoyUpstreamCanary) == "true", true, EMPTY_STRING, - EMPTY_STRING, parent_.local_zone_name_, to_az}; + EMPTY_STRING, parent_.local_zone_name_, upstreamZone()}; CodeUtility::chargeResponseTiming(info); callbacks_.onSuccess(std::move(response_)); @@ -126,21 +128,18 @@ void AsyncRequestImpl::onComplete() { } void AsyncRequestImpl::onResetStream(StreamResetReason) { - const std::string& to_az = upstream_host_ ? upstream_host_->zone() : EMPTY_STRING; - CodeUtility::ResponseStatInfo info{parent_.stats_store_, parent_.stat_prefix_, SERVICE_UNAVAILABLE_HEADER, true, EMPTY_STRING, EMPTY_STRING, - parent_.local_zone_name_, to_az}; + parent_.local_zone_name_, upstreamZone()}; CodeUtility::chargeResponseStat(info); callbacks_.onFailure(AsyncClient::FailureReason::Reset); cleanup(); } void AsyncRequestImpl::onRequestTimeout() { - const std::string& to_az = upstream_host_ ? upstream_host_->zone() : EMPTY_STRING; CodeUtility::ResponseStatInfo info{parent_.stats_store_, parent_.stat_prefix_, REQUEST_TIMEOUT_HEADER, true, EMPTY_STRING, EMPTY_STRING, - parent_.local_zone_name_, to_az}; + parent_.local_zone_name_, upstreamZone()}; CodeUtility::chargeResponseStat(info); parent_.cluster_.stats().upstream_rq_timeout_.inc(); stream_encoder_->resetStream(); diff --git a/source/common/http/async_client_impl.h b/source/common/http/async_client_impl.h index 715eb564d1cb..39e5fa5132a6 100644 --- a/source/common/http/async_client_impl.h +++ b/source/common/http/async_client_impl.h @@ -73,6 +73,8 @@ class AsyncRequestImpl final : public AsyncClient::Request, void cancel() override; private: + const std::string& upstreamZone(); + // Http::StreamDecoder void decodeHeaders(HeaderMapPtr&& headers, bool end_stream) override; void decodeData(const Buffer::Instance& data, bool end_stream) override; diff --git a/source/common/http/codes.cc b/source/common/http/codes.cc index a0bbbe57d73d..d74776e83ea4 100644 --- a/source/common/http/codes.cc +++ b/source/common/http/codes.cc @@ -82,9 +82,9 @@ void CodeUtility::chargeResponseTiming(const ResponseTimingInfo& info) { // Handle per zone stats. if (!info.from_zone_.empty() && !info.to_zone_.empty()) { - info.store_.deliverTimingToSinks( - fmt::format("{}zone.{}.{}.upstream_rq_time", info.prefix_, info.from_zone_, info.to_zone_), - ms); + info.store_.deliverTimingToSinks(fmt::format("{}zone.{}.{}.upstream_rq_time", info.prefix_, + info.from_zone_, info.to_zone_), + ms); } } } diff --git a/source/common/router/router.cc b/source/common/router/router.cc index df4122b0d85b..661f3ebeced9 100644 --- a/source/common/router/router.cc +++ b/source/common/router/router.cc @@ -89,15 +89,16 @@ Filter::~Filter() { ASSERT(!retry_state_); } +const std::string& Filter::upstreamZone() { + return upstream_host_ ? upstream_host_->zone() : EMPTY_STRING; +} + void Filter::chargeUpstreamCode(const Http::HeaderMap& response_headers) { if (!callbacks_->requestInfo().healthCheck()) { - const std::string& from_az = config_->service_zone_; - const std::string& to_az = upstream_host_ ? upstream_host_->zone() : EMPTY_STRING; - Http::CodeUtility::ResponseStatInfo info{ config_->stats_store_, stat_prefix_, response_headers, downstream_headers_->get(Http::Headers::get().EnvoyInternalRequest) == "true", - route_->virtualHostName(), request_vcluster_name_, from_az, to_az}; + route_->virtualHostName(), request_vcluster_name_, config_->service_zone_, upstreamZone()}; Http::CodeUtility::chargeResponseStat(info); @@ -105,7 +106,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", "", "", - from_az, to_az}; + config_->service_zone_, upstreamZone()}; Http::CodeUtility::chargeResponseStat(info); } @@ -413,16 +414,13 @@ void Filter::onUpstreamComplete() { upstream_request_->upstream_encoder_->resetStream(); } - const std::string& from_az = config_->service_zone_; - const std::string& to_az = upstream_host_ ? upstream_host_->zone() : EMPTY_STRING; - if (!callbacks_->requestInfo().healthCheck()) { Http::CodeUtility::ResponseTimingInfo info{ config_->stats_store_, stat_prefix_, upstream_request_->upstream_encoder_->requestCompleteTime(), upstream_request_->upstream_canary_, downstream_headers_->get(Http::Headers::get().EnvoyInternalRequest) == "true", - route_->virtualHostName(), request_vcluster_name_, from_az, to_az}; + route_->virtualHostName(), request_vcluster_name_, config_->service_zone_, upstreamZone()}; Http::CodeUtility::chargeResponseTiming(info); @@ -432,7 +430,7 @@ void Filter::onUpstreamComplete() { upstream_request_->upstream_encoder_->requestCompleteTime(), upstream_request_->upstream_canary_, downstream_headers_->get(Http::Headers::get().EnvoyInternalRequest) == "true", "", "", - from_az, to_az}; + config_->service_zone_, upstreamZone()}; Http::CodeUtility::chargeResponseTiming(info); } diff --git a/source/common/router/router.h b/source/common/router/router.h index 8776d08fd4a0..909a566e3dec 100644 --- a/source/common/router/router.h +++ b/source/common/router/router.h @@ -141,6 +141,7 @@ class Filter : Logger::Loggable, public Http::StreamDecoderF Http::AccessLog::FailureReason streamResetReasonToFailureReason(Http::StreamResetReason reset_reason); + const std::string& upstreamZone(); void chargeUpstreamCode(const Http::HeaderMap& response_headers); void chargeUpstreamCode(Http::Code code); void cleanup(); From e90acdc80ef0d3275069c4b53fd9ee028d0ee211 Mon Sep 17 00:00:00 2001 From: Roman Dzhabarov Date: Wed, 24 Aug 2016 14:26:53 -0700 Subject: [PATCH 07/10] minor fix in docs. --- docs/configuration/cluster_manager/cluster_stats.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/configuration/cluster_manager/cluster_stats.rst b/docs/configuration/cluster_manager/cluster_stats.rst index 43ad20cb56b7..fbd984c4afb2 100644 --- a/docs/configuration/cluster_manager/cluster_stats.rst +++ b/docs/configuration/cluster_manager/cluster_stats.rst @@ -86,7 +86,7 @@ are rooted at *cluster..* and contain the following statistics: :widths: 1, 1, 2 upstream_rq_<\*xx>, Counter, "Aggregate HTTP response codes (e.g., 2xx, 3xx, etc.)" - upstream_rq_<\*>, "Counter, Specific HTTP response codes (e.g., 201, 302, etc.)" + upstream_rq_<\*>, Counter, "Specific HTTP response codes (e.g., 201, 302, etc.)" upstream_rq_time, Timer, Request time milliseconds canary.upstream_rq_<\*xx>, Counter, Upstream canary aggregate HTTP response codes canary.upstream_rq_<\*>, Counter, Upstream canary specific HTTP response codes From 152e90fcb2865c66b266eaad88e32d6a02728292 Mon Sep 17 00:00:00 2001 From: Matt Klein Date: Wed, 24 Aug 2016 15:41:09 -0700 Subject: [PATCH 08/10] Update cluster_stats.rst --- docs/configuration/cluster_manager/cluster_stats.rst | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/configuration/cluster_manager/cluster_stats.rst b/docs/configuration/cluster_manager/cluster_stats.rst index fbd984c4afb2..001b138ceebc 100644 --- a/docs/configuration/cluster_manager/cluster_stats.rst +++ b/docs/configuration/cluster_manager/cluster_stats.rst @@ -113,8 +113,8 @@ the dynamic HTTP statistics section :ref:`above Per service zone dynamic HTTP statistics ---------------------------------------- -If service zone is available for local service :option:`--service-zone` -and :ref:`upstream cluster `, +If service zone is available for the local service via the :option:`--service-zone` option +and the :ref:`upstream cluster `, Envoy will track the following statistics in *cluster..zone...* namespace. .. csv-table:: From 9d06893528d40fc7c773b754a6e8c65d6fffaf7b Mon Sep 17 00:00:00 2001 From: Matt Klein Date: Wed, 24 Aug 2016 15:42:04 -0700 Subject: [PATCH 09/10] Update cluster_stats.rst --- docs/configuration/cluster_manager/cluster_stats.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/configuration/cluster_manager/cluster_stats.rst b/docs/configuration/cluster_manager/cluster_stats.rst index 001b138ceebc..cd2b81201632 100644 --- a/docs/configuration/cluster_manager/cluster_stats.rst +++ b/docs/configuration/cluster_manager/cluster_stats.rst @@ -113,7 +113,7 @@ the dynamic HTTP statistics section :ref:`above Per service zone dynamic HTTP statistics ---------------------------------------- -If service zone is available for the local service via the :option:`--service-zone` option +If the service zone is available for the local service (via the :option:`--service-zone`) and the :ref:`upstream cluster `, Envoy will track the following statistics in *cluster..zone...* namespace. From 889e8aae883119b52a6a30a6bd471eaae03113ad Mon Sep 17 00:00:00 2001 From: Matt Klein Date: Wed, 24 Aug 2016 15:42:28 -0700 Subject: [PATCH 10/10] Update cluster_stats.rst --- docs/configuration/cluster_manager/cluster_stats.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/configuration/cluster_manager/cluster_stats.rst b/docs/configuration/cluster_manager/cluster_stats.rst index cd2b81201632..4a6e69645a06 100644 --- a/docs/configuration/cluster_manager/cluster_stats.rst +++ b/docs/configuration/cluster_manager/cluster_stats.rst @@ -113,7 +113,7 @@ the dynamic HTTP statistics section :ref:`above Per service zone dynamic HTTP statistics ---------------------------------------- -If the service zone is available for the local service (via the :option:`--service-zone`) +If the service zone is available for the local service (via :option:`--service-zone`) and the :ref:`upstream cluster `, Envoy will track the following statistics in *cluster..zone...* namespace.