Skip to content

stats: Capture all StatNames needed in the hystrix filter at construction time. #7046

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
mattklein123 merged 5 commits into from
May 28, 2019
Merged

stats: Capture all StatNames needed in the hystrix filter at construction time. #7046

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
6647f6e
Use StatName in hystrix implementation, and test both real and fake S…
jmarantz May 22, 2019
c71f10c
format
jmarantz May 22, 2019
bb598c8
Merge branch 'master' into hystrix-with-stat-names
jmarantz May 23, 2019
e8090f6
Rename template calss name to be more spelled out.
jmarantz May 23, 2019
de44859
Declare StatName member variables as const.
jmarantz May 27, 2019
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
24 changes: 14 additions & 10 deletions source/extensions/stat_sinks/hystrix/hystrix.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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();

Expand All @@ -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();
Expand Down Expand Up @@ -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");
Expand Down Expand Up @@ -372,8 +376,8 @@ void HystrixSink::flush(Stats::Source& source) {
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;
Expand Down
6 changes: 6 additions & 0 deletions source/extensions/stat_sinks/hystrix/hystrix.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,12 @@ class HystrixSink : public Stats::Sink, public Logger::Loggable<Logger::Id::hyst
Stats::StatNamePool stat_name_pool_;
Stats::StatName cluster_name_;
Stats::StatName cluster_upstream_rq_time_;
Stats::StatName membership_total_;
Comment thread
jmarantz marked this conversation as resolved.
Outdated
Stats::StatName retry_upstream_rq_4xx_;
Stats::StatName retry_upstream_rq_5xx_;
Stats::StatName upstream_rq_2xx_;
Stats::StatName upstream_rq_4xx_;
Stats::StatName upstream_rq_5xx_;
};

typedef std::unique_ptr<HystrixSink> HystrixSinkPtr;
Expand Down
64 changes: 42 additions & 22 deletions test/common/http/codes_speed_test.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,14 @@
namespace Envoy {
namespace Http {

class CodeUtilitySpeedTest {
template <class SymTab> class CodeUtilitySpeedTest {
Comment thread
jmarantz marked this conversation as resolved.
Outdated
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(),
Expand All @@ -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_;
SymTab symbol_table_;
Stats::IsolatedStoreImpl global_store_;
Stats::IsolatedStoreImpl cluster_scope_;
Http::CodeStatsImpl code_stats_;
Stats::StatNamePool pool_;
Stats::StatName from_az_;
Stats::StatName prefix_;
Stats::StatName req_vcluster_name_;
Stats::StatName test_cluster_;
Stats::StatName test_vhost_;
Stats::StatName to_az_;
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<Envoy::Stats::FakeSymbolTableImpl> 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<Envoy::Stats::FakeSymbolTableImpl> context;

for (auto _ : state) {
context.responseTiming();
}
}
BENCHMARK(BM_ResponseTiming);
BENCHMARK(BM_ResponseTimingFakeSymtab);

static void BM_AddResponsesRealSymtab(benchmark::State& state) {
Envoy::Http::CodeUtilitySpeedTest<Envoy::Stats::SymbolTableImpl> context;

for (auto _ : state) {
context.addResponses();
}
}
BENCHMARK(BM_AddResponsesRealSymtab);

static void BM_ResponseTimingRealSymtab(benchmark::State& state) {
Envoy::Http::CodeUtilitySpeedTest<Envoy::Stats::SymbolTableImpl> 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) {
Expand Down