diff --git a/source/extensions/stat_sinks/hystrix/hystrix.cc b/source/extensions/stat_sinks/hystrix/hystrix.cc index c14fcf8150ac9..6a5c32dac3122 100644 --- a/source/extensions/stat_sinks/hystrix/hystrix.cc +++ b/source/extensions/stat_sinks/hystrix/hystrix.cc @@ -84,7 +84,6 @@ uint64_t HystrixSink::getRollingValue(RollingWindow rolling_window) { void HystrixSink::updateRollingWindowMap(const Upstream::ClusterInfo& cluster_info, ClusterStatsCache& cluster_stats_cache) { - const std::string cluster_name = cluster_info.name(); Upstream::ClusterStats& cluster_stats = cluster_info.stats(); Stats::Scope& cluster_stats_scope = cluster_info.statsScope(); @@ -99,15 +98,15 @@ void HystrixSink::updateRollingWindowMap(const Upstream::ClusterInfo& cluster_in // (alternative: each request including the retries counted as 1) // since timeouts are 504 (or 408), deduce them from here ("-" sign). // Timeout retries were not counted here anyway. - uint64_t errors = cluster_stats_scope.counter("upstream_rq_5xx").value() + - cluster_stats_scope.counter("retry.upstream_rq_5xx").value() + - cluster_stats_scope.counter("upstream_rq_4xx").value() + - cluster_stats_scope.counter("retry.upstream_rq_4xx").value() - + uint64_t errors = cluster_stats_scope.counterFromStatName(upstream_rq_5xx_).value() + + cluster_stats_scope.counterFromStatName(retry_upstream_rq_5xx_).value() + + cluster_stats_scope.counterFromStatName(upstream_rq_4xx_).value() + + cluster_stats_scope.counterFromStatName(retry_upstream_rq_4xx_).value() - cluster_stats.upstream_rq_timeout_.value(); pushNewValue(cluster_stats_cache.errors_, errors); - uint64_t success = cluster_stats_scope.counter("upstream_rq_2xx").value(); + uint64_t success = cluster_stats_scope.counterFromStatName(upstream_rq_2xx_).value(); pushNewValue(cluster_stats_cache.success_, success); uint64_t rejected = cluster_stats.upstream_rq_pending_overflow_.value(); @@ -270,8 +269,13 @@ HystrixSink::HystrixSink(Server::Instance& server, const uint64_t num_buckets) : server_(server), current_index_(num_buckets > 0 ? num_buckets : DEFAULT_NUM_BUCKETS), window_size_(current_index_ + 1), stat_name_pool_(server.stats().symbolTable()), cluster_name_(stat_name_pool_.add(Config::TagNames::get().CLUSTER_NAME)), - cluster_upstream_rq_time_(stat_name_pool_.add("cluster.upstream_rq_time")) { - + cluster_upstream_rq_time_(stat_name_pool_.add("cluster.upstream_rq_time")), + membership_total_(stat_name_pool_.add("membership_total")), + retry_upstream_rq_4xx_(stat_name_pool_.add("retry.upstream_rq_4xx")), + retry_upstream_rq_5xx_(stat_name_pool_.add("retry.upstream_rq_5xx")), + upstream_rq_2xx_(stat_name_pool_.add("upstream_rq_2xx")), + upstream_rq_4xx_(stat_name_pool_.add("upstream_rq_4xx")), + upstream_rq_5xx_(stat_name_pool_.add("upstream_rq_5xx")) { Server::Admin& admin = server_.admin(); ENVOY_LOG(debug, "adding hystrix_event_stream endpoint to enable connection to hystrix dashboard"); @@ -373,8 +377,8 @@ void HystrixSink::flush(Stats::MetricSnapshot& snapshot) { addClusterStatsToStream( *cluster_stats_cache_ptr, cluster_info->name(), cluster_info->resourceManager(Upstream::ResourcePriority::Default).pendingRequests().max(), - cluster_info->statsScope().gauge("membership_total").value(), server_.statsFlushInterval(), - time_histograms[cluster_info->name()], ss); + cluster_info->statsScope().gaugeFromStatName(membership_total_).value(), + server_.statsFlushInterval(), time_histograms[cluster_info->name()], ss); } Buffer::OwnedImpl data; diff --git a/source/extensions/stat_sinks/hystrix/hystrix.h b/source/extensions/stat_sinks/hystrix/hystrix.h index 44234669f7782..31c1df700b02d 100644 --- a/source/extensions/stat_sinks/hystrix/hystrix.h +++ b/source/extensions/stat_sinks/hystrix/hystrix.h @@ -159,8 +159,14 @@ class HystrixSink : public Stats::Sink, public Logger::Loggable HystrixSinkPtr; diff --git a/test/common/http/codes_speed_test.cc b/test/common/http/codes_speed_test.cc index 7883b97ac4ccb..b49b86cf0fdfe 100644 --- a/test/common/http/codes_speed_test.cc +++ b/test/common/http/codes_speed_test.cc @@ -17,11 +17,14 @@ namespace Envoy { namespace Http { -class CodeUtilitySpeedTest { +template class CodeUtilitySpeedTest { public: CodeUtilitySpeedTest() : global_store_(symbol_table_), cluster_scope_(symbol_table_), code_stats_(symbol_table_), - pool_(symbol_table_), prefix_(pool_.add("prefix")) {} + pool_(symbol_table_), from_az_(pool_.add("from_az")), prefix_(pool_.add("prefix")), + req_vcluster_name_(pool_.add("req_vcluster_name")), + test_cluster_(pool_.add("test-cluster")), test_vhost_(pool_.add("test-vhost")), + to_az_(pool_.add("to_az")), vhost_name_(pool_.add("vhost_name")) {} void addResponse(uint64_t code, bool canary, bool internal_request, Stats::StatName request_vhost_name = Stats::StatName(), @@ -44,53 +47,70 @@ class CodeUtilitySpeedTest { addResponse(300, false, false); Stats::StatName empty_stat_name; addResponse(500, true, false); - addResponse(200, false, false, pool_.add("test-vhost"), pool_.add("test-cluster")); - addResponse(200, false, false, empty_stat_name, empty_stat_name, pool_.add("from_az"), - pool_.add("to_az")); + addResponse(200, false, false, test_vhost_, test_cluster_); + addResponse(200, false, false, empty_stat_name, empty_stat_name, from_az_, to_az_); } void responseTiming() { - Http::CodeStats::ResponseTimingInfo info{global_store_, - cluster_scope_, - prefix_, - std::chrono::milliseconds(5), - true, - true, - pool_.add("vhost_name"), - pool_.add("req_vcluster_name"), - pool_.add("from_az"), - pool_.add("to_az")}; + Http::CodeStats::ResponseTimingInfo info{ + global_store_, cluster_scope_, prefix_, std::chrono::milliseconds(5), + true, true, vhost_name_, req_vcluster_name_, + from_az_, to_az_}; code_stats_.chargeResponseTiming(info); } - Stats::SymbolTableImpl symbol_table_; + SymbolTableClass symbol_table_; Stats::IsolatedStoreImpl global_store_; Stats::IsolatedStoreImpl cluster_scope_; Http::CodeStatsImpl code_stats_; Stats::StatNamePool pool_; - Stats::StatName prefix_; + const Stats::StatName from_az_; + const Stats::StatName prefix_; + const Stats::StatName req_vcluster_name_; + const Stats::StatName test_cluster_; + const Stats::StatName test_vhost_; + const Stats::StatName to_az_; + const Stats::StatName vhost_name_; }; } // namespace Http } // namespace Envoy -static void BM_AddResponses(benchmark::State& state) { - Envoy::Http::CodeUtilitySpeedTest context; +static void BM_AddResponsesFakeSymtab(benchmark::State& state) { + Envoy::Http::CodeUtilitySpeedTest context; for (auto _ : state) { context.addResponses(); } } -BENCHMARK(BM_AddResponses); +BENCHMARK(BM_AddResponsesFakeSymtab); -static void BM_ResponseTiming(benchmark::State& state) { - Envoy::Http::CodeUtilitySpeedTest context; +static void BM_ResponseTimingFakeSymtab(benchmark::State& state) { + Envoy::Http::CodeUtilitySpeedTest context; for (auto _ : state) { context.responseTiming(); } } -BENCHMARK(BM_ResponseTiming); +BENCHMARK(BM_ResponseTimingFakeSymtab); + +static void BM_AddResponsesRealSymtab(benchmark::State& state) { + Envoy::Http::CodeUtilitySpeedTest context; + + for (auto _ : state) { + context.addResponses(); + } +} +BENCHMARK(BM_AddResponsesRealSymtab); + +static void BM_ResponseTimingRealSymtab(benchmark::State& state) { + Envoy::Http::CodeUtilitySpeedTest context; + + for (auto _ : state) { + context.responseTiming(); + } +} +BENCHMARK(BM_ResponseTimingRealSymtab); // Boilerplate main(), which discovers benchmarks in the same file and runs them. int main(int argc, char** argv) {