diff --git a/include/istio/mixerclient/client.h b/include/istio/mixerclient/client.h index 0d904856084..7c49ceffcc4 100644 --- a/include/istio/mixerclient/client.h +++ b/include/istio/mixerclient/client.h @@ -52,24 +52,71 @@ struct MixerClientOptions { // The statistics recorded by mixerclient library. struct Statistics { - // Total number of check calls. - uint64_t total_check_calls; - // Total number of remote check calls. - uint64_t total_remote_check_calls; - // Total number of remote check calls that blocking origin requests. - uint64_t total_blocking_remote_check_calls; - - // Total number of quota calls. - uint64_t total_quota_calls; - // Total number of remote quota calls. - uint64_t total_remote_quota_calls; - // Total number of remote quota calls that blocking origin requests. - uint64_t total_blocking_remote_quota_calls; + // + // Policy check counters. + // + // total_check_calls = total_check_hits + total_check_misses + // total_check_hits = total_check_hit_accepts + total_check_hit_denies + // total_remote_check_calls = total_check_misses + // total_remote_check_calls >= total_remote_check_accepts + + // total_remote_check_denies + // ^ Transport errors are responsible for the >= + // + + uint64_t total_check_calls_{0}; // 1.0 + uint64_t total_check_cache_hits_{0}; // 1.1 + uint64_t total_check_cache_misses_{0}; // 1.1 + uint64_t total_check_cache_hit_accepts_{0}; // 1.1 + uint64_t total_check_cache_hit_denies_{0}; // 1.1 + uint64_t total_remote_check_calls_{0}; // 1.0 + uint64_t total_remote_check_accepts_{0}; // 1.1 + uint64_t total_remote_check_denies_{0}; // 1.1 + + // + // Quota check counters + // + // total_quota_calls = total_quota_hits + total_quota_misses + // total_quota_hits = total_quota_hit_accepts + total_quota_hit_denies + // total_remote_quota_calls = total_quota_misses + + // total_remote_quota_prefetch_calls total_remote_quota_calls >= + // total_remote_quota_accepts + total_remote_quota_denies + // ^ Transport errors are responsible for the >= + // + + uint64_t total_quota_calls_{0}; // 1.0 + uint64_t total_quota_cache_hits_{0}; // 1.1 + uint64_t total_quota_cache_misses_{0}; // 1.1 + uint64_t total_quota_cache_hit_accepts_{0}; // 1.1 + uint64_t total_quota_cache_hit_denies_{0}; // 1.1 + uint64_t total_remote_quota_calls_{0}; // 1.0 + uint64_t total_remote_quota_accepts_{0}; // 1.1 + uint64_t total_remote_quota_denies_{0}; // 1.1 + uint64_t total_remote_quota_prefetch_calls_{0}; // 1.1 + + // + // Counters for upstream requests to Mixer. + // + // total_remote_calls = SUM(total_remote_call_successes, ..., + // total_remote_call_other_errors) Total transport errors would be + // (total_remote_calls - total_remote_call_successes). + // + + uint64_t total_remote_calls_{0}; // 1.1 + uint64_t total_remote_call_successes_{0}; // 1.1 + uint64_t total_remote_call_timeouts_{0}; // 1.1 + uint64_t total_remote_call_send_errors_{0}; // 1.1 + uint64_t total_remote_call_other_errors_{0}; // 1.1 + uint64_t total_remote_call_retries_{0}; // 1.1 + uint64_t total_remote_call_cancellations_{0}; // 1.1 + + // + // Telemetry report counters + // // Total number of report calls. - uint64_t total_report_calls; + uint64_t total_report_calls_{0}; // Total number of remote report calls. - uint64_t total_remote_report_calls; + uint64_t total_remote_report_calls_{0}; }; class MixerClient { diff --git a/src/envoy/utils/stats.cc b/src/envoy/utils/stats.cc index cb1211c5c34..eec09c69efb 100644 --- a/src/envoy/utils/stats.cc +++ b/src/envoy/utils/stats.cc @@ -53,47 +53,40 @@ void MixerStatsObject::OnTimer() { timer_->enableTimer(std::chrono::milliseconds(stats_update_interval_)); } +#define CHECK_AND_UPDATE_STATS(NAME) \ + if (new_stats.NAME > old_stats_.NAME) { \ + stats_.NAME.add(new_stats.NAME - old_stats_.NAME); \ + } + void MixerStatsObject::CheckAndUpdateStats( const ::istio::mixerclient::Statistics& new_stats) { - if (new_stats.total_check_calls > old_stats_.total_check_calls) { - stats_.total_check_calls_.add(new_stats.total_check_calls - - old_stats_.total_check_calls); - } - if (new_stats.total_remote_check_calls > - old_stats_.total_remote_check_calls) { - stats_.total_remote_check_calls_.add(new_stats.total_remote_check_calls - - old_stats_.total_remote_check_calls); - } - if (new_stats.total_blocking_remote_check_calls > - old_stats_.total_blocking_remote_check_calls) { - stats_.total_blocking_remote_check_calls_.add( - new_stats.total_blocking_remote_check_calls - - old_stats_.total_blocking_remote_check_calls); - } - if (new_stats.total_quota_calls > old_stats_.total_quota_calls) { - stats_.total_quota_calls_.add(new_stats.total_quota_calls - - old_stats_.total_quota_calls); - } - if (new_stats.total_remote_quota_calls > - old_stats_.total_remote_quota_calls) { - stats_.total_remote_quota_calls_.add(new_stats.total_remote_quota_calls - - old_stats_.total_remote_quota_calls); - } - if (new_stats.total_blocking_remote_quota_calls > - old_stats_.total_blocking_remote_quota_calls) { - stats_.total_blocking_remote_quota_calls_.add( - new_stats.total_blocking_remote_quota_calls - - old_stats_.total_blocking_remote_quota_calls); - } - if (new_stats.total_report_calls > old_stats_.total_report_calls) { - stats_.total_report_calls_.add(new_stats.total_report_calls - - old_stats_.total_report_calls); - } - if (new_stats.total_remote_report_calls > - old_stats_.total_remote_report_calls) { - stats_.total_remote_report_calls_.add(new_stats.total_remote_report_calls - - old_stats_.total_remote_report_calls); - } + CHECK_AND_UPDATE_STATS(total_check_calls_); + CHECK_AND_UPDATE_STATS(total_check_cache_hits_); + CHECK_AND_UPDATE_STATS(total_check_cache_misses_); + CHECK_AND_UPDATE_STATS(total_check_cache_hit_accepts_); + CHECK_AND_UPDATE_STATS(total_check_cache_hit_denies_); + CHECK_AND_UPDATE_STATS(total_remote_check_calls_); + CHECK_AND_UPDATE_STATS(total_remote_check_accepts_); + CHECK_AND_UPDATE_STATS(total_remote_check_denies_); + CHECK_AND_UPDATE_STATS(total_quota_calls_); + CHECK_AND_UPDATE_STATS(total_quota_cache_hits_); + CHECK_AND_UPDATE_STATS(total_quota_cache_misses_); + CHECK_AND_UPDATE_STATS(total_quota_cache_hit_accepts_); + CHECK_AND_UPDATE_STATS(total_quota_cache_hit_denies_); + CHECK_AND_UPDATE_STATS(total_remote_quota_calls_); + CHECK_AND_UPDATE_STATS(total_remote_quota_accepts_); + CHECK_AND_UPDATE_STATS(total_remote_quota_denies_); + CHECK_AND_UPDATE_STATS(total_remote_quota_prefetch_calls_); + CHECK_AND_UPDATE_STATS(total_remote_calls_); + CHECK_AND_UPDATE_STATS(total_remote_call_successes_); + CHECK_AND_UPDATE_STATS(total_remote_call_timeouts_); + CHECK_AND_UPDATE_STATS(total_remote_call_send_errors_); + CHECK_AND_UPDATE_STATS(total_remote_call_other_errors_); + CHECK_AND_UPDATE_STATS(total_remote_call_retries_); + CHECK_AND_UPDATE_STATS(total_remote_call_cancellations_); + + CHECK_AND_UPDATE_STATS(total_report_calls_); + CHECK_AND_UPDATE_STATS(total_remote_report_calls_); // Copy new_stats to old_stats_ for next stats update. old_stats_ = new_stats; diff --git a/src/envoy/utils/stats.h b/src/envoy/utils/stats.h index 7b7e63ff712..b9fc3096159 100644 --- a/src/envoy/utils/stats.h +++ b/src/envoy/utils/stats.h @@ -27,14 +27,32 @@ namespace Utils { * All mixer filter stats. @see stats_macros.h */ // clang-format off -#define ALL_MIXER_FILTER_STATS(COUNTER) \ - COUNTER(total_check_calls) \ - COUNTER(total_remote_check_calls) \ - COUNTER(total_blocking_remote_check_calls) \ - COUNTER(total_quota_calls) \ - COUNTER(total_remote_quota_calls) \ - COUNTER(total_blocking_remote_quota_calls) \ - COUNTER(total_report_calls) \ +#define ALL_MIXER_FILTER_STATS(COUNTER) \ + COUNTER(total_check_calls) \ + COUNTER(total_check_cache_hits) \ + COUNTER(total_check_cache_misses) \ + COUNTER(total_check_cache_hit_accepts) \ + COUNTER(total_check_cache_hit_denies) \ + COUNTER(total_remote_check_calls) \ + COUNTER(total_remote_check_accepts) \ + COUNTER(total_remote_check_denies) \ + COUNTER(total_quota_calls) \ + COUNTER(total_quota_cache_hits) \ + COUNTER(total_quota_cache_misses) \ + COUNTER(total_quota_cache_hit_accepts) \ + COUNTER(total_quota_cache_hit_denies) \ + COUNTER(total_remote_quota_calls) \ + COUNTER(total_remote_quota_accepts) \ + COUNTER(total_remote_quota_denies) \ + COUNTER(total_remote_quota_prefetch_calls) \ + COUNTER(total_remote_calls) \ + COUNTER(total_remote_call_successes) \ + COUNTER(total_remote_call_timeouts) \ + COUNTER(total_remote_call_send_errors) \ + COUNTER(total_remote_call_other_errors) \ + COUNTER(total_remote_call_retries) \ + COUNTER(total_remote_call_cancellations) \ + COUNTER(total_report_calls) \ COUNTER(total_remote_report_calls) // clang-format on diff --git a/src/istio/mixerclient/client_impl.cc b/src/istio/mixerclient/client_impl.cc index f4498ac7a8e..25d527a34ee 100644 --- a/src/istio/mixerclient/client_impl.cc +++ b/src/istio/mixerclient/client_impl.cc @@ -24,10 +24,42 @@ using ::istio::mixer::v1::CheckRequest; using ::istio::mixer::v1::CheckResponse; using ::istio::mixer::v1::ReportRequest; using ::istio::mixer::v1::ReportResponse; +using ::istio::mixerclient::CheckContextSharedPtr; +using ::istio::mixerclient::SharedAttributesSharedPtr; namespace istio { namespace mixerclient { +static ::google::protobuf::StringPiece TIMEOUT_MESSAGE( + "upstream request timeout"); +static ::google::protobuf::StringPiece SEND_ERROR_MESSAGE( + "upstream connect error or disconnect/reset before headers"); + +enum class TransportResult { + SUCCESS, // Response received + SEND_ERROR, // Cannot connect to peer or send request to peer. + RESPONSE_TIMEOUT, // Connected to peer and sent request, but didn't receive a + // response in time. + OTHER // Something else went wrong +}; + +TransportResult TransportStatus(const Status &status) { + if (status.ok()) { + return TransportResult::SUCCESS; + } + + if (Code::UNAVAILABLE == status.error_code()) { + if (TIMEOUT_MESSAGE == status.error_message()) { + return TransportResult::RESPONSE_TIMEOUT; + } + if (SEND_ERROR_MESSAGE == status.error_message()) { + return TransportResult::SEND_ERROR; + } + } + + return TransportResult::OTHER; +} + MixerClientImpl::MixerClientImpl(const MixerClientOptions &options) : options_(options) { check_cache_ = @@ -41,13 +73,6 @@ MixerClientImpl::MixerClientImpl(const MixerClientOptions &options) if (options_.env.uuid_generate_func) { deduplication_id_base_ = options_.env.uuid_generate_func(); } - - total_check_calls_ = 0; - total_remote_check_calls_ = 0; - total_blocking_remote_check_calls_ = 0; - total_quota_calls_ = 0; - total_remote_quota_calls_ = 0; - total_blocking_remote_quota_calls_ = 0; } MixerClientImpl::~MixerClientImpl() {} @@ -62,36 +87,63 @@ CancelFunc MixerClientImpl::Check(CheckContextSharedPtr &context, context->checkPolicyCache(*check_cache_); ++total_check_calls_; - if (context->policyCacheHit() && - (!context->policyStatus().ok() || !context->quotaCheckRequired())) { - // - // If the policy cache denies the request, immediately fail the request + if (context->policyCacheHit()) { + ++total_check_cache_hits_; + + if (!context->policyStatus().ok()) { + // + // If the policy cache denies the request, immediately fail the request + // + ++total_check_cache_hit_denies_; + context->setFinalStatus(context->policyStatus()); + on_done(*context); + return nullptr; + } + // // If policy cache accepts the request and a quota check is not required, // immediately accept the request. // - context->setFinalStatus(context->policyStatus()); - on_done(*context); - return nullptr; + ++total_check_cache_hit_accepts_; + if (!context->quotaCheckRequired()) { + context->setFinalStatus(context->policyStatus()); + on_done(*context); + return nullptr; + } + } else { + ++total_check_cache_misses_; } if (context->quotaCheckRequired()) { context->checkQuotaCache(*quota_cache_); ++total_quota_calls_; - if (context->quotaCacheHit() && context->policyCacheHit()) { - // - // If both policy and quota caches are hit, we can call the completion - // handler now. However sometimes the quota cache's prefetch - // implementation will still need to send a request to the Mixer server - // in the background. - // - context->setFinalStatus(context->quotaStatus()); - on_done(*context); - on_done = nullptr; - if (!context->remoteQuotaRequestRequired()) { - return nullptr; + if (context->quotaCacheHit()) { + ++total_quota_cache_hits_; + if (context->quotaStatus().ok()) { + ++total_quota_cache_hit_accepts_; + } else { + ++total_quota_cache_hit_denies_; + } + + if (context->policyCacheHit()) { + // + // If both policy and quota caches are hit, we can call the completion + // handler now. However sometimes the quota cache's prefetch + // implementation will still need to send a request to the Mixer server + // in the background. + // + context->setFinalStatus(context->quotaStatus()); + on_done(*context); + on_done = nullptr; + if (!context->remoteQuotaRequestRequired()) { + return nullptr; + } else { + ++total_remote_quota_prefetch_calls_; + } } + } else { + ++total_quota_cache_misses_; } } @@ -105,24 +157,42 @@ CancelFunc MixerClientImpl::Check(CheckContextSharedPtr &context, } // - // We are going to make a remote call now. + // Classify and track reason for remote request // - ++total_remote_check_calls_; + ++total_remote_calls_; - if (context->quotaCheckRequired()) { - ++total_remote_quota_calls_; + if (!context->policyCacheHit()) { + ++total_remote_check_calls_; } - if (on_done) { - ++total_blocking_remote_check_calls_; - if (context->quotaCheckRequired()) { - ++total_blocking_remote_quota_calls_; - } + if (context->remoteQuotaRequestRequired()) { + ++total_remote_quota_calls_; } return transport(context->request(), context->response(), [this, context, on_done](const Status &status) { + // + // Classify and track transport errors + // + + TransportResult result = TransportStatus(status); + + switch (result) { + case TransportResult::SUCCESS: + ++total_remote_call_successes_; + break; + case TransportResult::RESPONSE_TIMEOUT: + ++total_remote_call_timeouts_; + break; + case TransportResult::SEND_ERROR: + ++total_remote_call_send_errors_; + break; + case TransportResult::OTHER: + ++total_remote_call_other_errors_; + break; + } + // // Update caches. This has the side-effect of updating // status, so track those too @@ -130,10 +200,22 @@ CancelFunc MixerClientImpl::Check(CheckContextSharedPtr &context, if (!context->policyCacheHit()) { context->updatePolicyCache(status, *context->response()); + + if (context->policyStatus().ok()) { + ++total_remote_check_accepts_; + } else { + ++total_remote_check_denies_; + } } if (context->quotaCheckRequired()) { context->updateQuotaCache(status, *context->response()); + + if (context->quotaStatus().ok()) { + ++total_remote_quota_accepts_; + } else { + ++total_remote_quota_denies_; + } } // @@ -142,7 +224,7 @@ CancelFunc MixerClientImpl::Check(CheckContextSharedPtr &context, // the final status is not Status::OK // - if (!status.ok()) { + if (result != TransportResult::SUCCESS) { if (context->networkFailOpen()) { context->setFinalStatus(Status::OK); } else { @@ -171,14 +253,33 @@ void MixerClientImpl::Report(const SharedAttributesSharedPtr &attributes) { } void MixerClientImpl::GetStatistics(Statistics *stat) const { - stat->total_check_calls = total_check_calls_; - stat->total_remote_check_calls = total_remote_check_calls_; - stat->total_blocking_remote_check_calls = total_blocking_remote_check_calls_; - stat->total_quota_calls = total_quota_calls_; - stat->total_remote_quota_calls = total_remote_quota_calls_; - stat->total_blocking_remote_quota_calls = total_blocking_remote_quota_calls_; - stat->total_report_calls = report_batch_->total_report_calls(); - stat->total_remote_report_calls = report_batch_->total_remote_report_calls(); + stat->total_check_calls_ = total_check_calls_; + stat->total_check_cache_hits_ = total_check_cache_hits_; + stat->total_check_cache_misses_ = total_check_cache_misses_; + stat->total_check_cache_hit_accepts_ = total_check_cache_hit_accepts_; + stat->total_check_cache_hit_denies_ = total_check_cache_hit_denies_; + stat->total_remote_check_calls_ = total_remote_check_calls_; + stat->total_remote_check_accepts_ = total_remote_check_accepts_; + stat->total_remote_check_denies_ = total_remote_check_denies_; + stat->total_quota_calls_ = total_quota_calls_; + stat->total_quota_cache_hits_ = total_quota_cache_hits_; + stat->total_quota_cache_misses_ = total_quota_cache_misses_; + stat->total_quota_cache_hit_accepts_ = total_quota_cache_hit_accepts_; + stat->total_quota_cache_hit_denies_ = total_quota_cache_hit_denies_; + stat->total_remote_quota_calls_ = total_remote_quota_calls_; + stat->total_remote_quota_accepts_ = total_remote_quota_accepts_; + stat->total_remote_quota_denies_ = total_remote_quota_denies_; + stat->total_remote_quota_prefetch_calls_ = total_remote_quota_prefetch_calls_; + stat->total_remote_calls_ = total_remote_calls_; + stat->total_remote_call_successes_ = total_remote_call_successes_; + stat->total_remote_call_timeouts_ = total_remote_call_timeouts_; + stat->total_remote_call_send_errors_ = total_remote_call_send_errors_; + stat->total_remote_call_other_errors_ = total_remote_call_other_errors_; + stat->total_remote_call_retries_ = total_remote_call_retries_; + stat->total_remote_call_cancellations_ = total_remote_call_cancellations_; + + stat->total_report_calls_ = report_batch_->total_report_calls(); + stat->total_remote_report_calls_ = report_batch_->total_remote_report_calls(); } // Creates a MixerClient object. diff --git a/src/istio/mixerclient/client_impl.h b/src/istio/mixerclient/client_impl.h index 3cc6d2892f1..35c1ff4b5d3 100644 --- a/src/istio/mixerclient/client_impl.h +++ b/src/istio/mixerclient/client_impl.h @@ -38,6 +38,7 @@ class MixerClientImpl : public MixerClient { CancelFunc Check(istio::mixerclient::CheckContextSharedPtr& context, TransportCheckFunc transport, CheckDoneFunc on_done) override; + void Report(const SharedAttributesSharedPtr& attributes) override; void GetStatistics(Statistics* stat) const override; @@ -60,17 +61,66 @@ class MixerClientImpl : public MixerClient { std::string deduplication_id_base_; std::atomic deduplication_id_; - // Atomic objects for recording statistics. - // check cache miss rate: - // total_blocking_remote_check_calls_ / total_check_calls_. - // quota cache miss rate: - // total_blocking_remote_quota_calls_ / total_quota_calls_. - std::atomic_int_fast64_t total_check_calls_; - std::atomic_int_fast64_t total_remote_check_calls_; - std::atomic_int_fast64_t total_blocking_remote_check_calls_; - std::atomic_int_fast64_t total_quota_calls_; - std::atomic_int_fast64_t total_remote_quota_calls_; - std::atomic_int_fast64_t total_blocking_remote_quota_calls_; + // + // Policy check counters. + // + // total_check_calls = total_check_hits + total_check_misses + // total_check_hits = total_check_hit_accepts + total_check_hit_denies + // total_remote_check_calls = total_check_misses + // total_remote_check_calls >= total_remote_check_accepts + + // total_remote_check_denies + // ^ Transport errors are responsible for the >= + // + + std::atomic total_check_calls_{0}; // 1.0 + std::atomic total_check_cache_hits_{0}; // 1.1 + std::atomic total_check_cache_misses_{0}; // 1.1 + std::atomic total_check_cache_hit_accepts_{0}; // 1.1 + std::atomic total_check_cache_hit_denies_{0}; // 1.1 + std::atomic total_remote_check_calls_{0}; // 1.0 + std::atomic total_remote_check_accepts_{0}; // 1.1 + std::atomic total_remote_check_denies_{0}; // 1.1 + + // + // Quota check counters + // + // total_quota_calls = total_quota_hits + total_quota_misses + // total_quota_hits >= total_quota_hit_accepts + total_quota_hit_denies + // ^ we will neither accept or deny from the quota cache if the policy + // cache is missed + // total_remote_quota_calls = total_quota_misses + total_quota_hit_denies + // ^ we will neither accept or deny from the quota cache if the policy + // cache is missed + // total_remote_quota_calls >= total_remote_quota_accepts + + // total_remote_quota_denies + // ^ Transport errors are responsible for the >= + // + + std::atomic total_quota_calls_{0}; // 1.0 + std::atomic total_quota_cache_hits_{0}; // 1.1 + std::atomic total_quota_cache_misses_{0}; // 1.1 + std::atomic total_quota_cache_hit_accepts_{0}; // 1.1 + std::atomic total_quota_cache_hit_denies_{0}; // 1.1 + std::atomic total_remote_quota_calls_{0}; // 1.0 + std::atomic total_remote_quota_accepts_{0}; // 1.1 + std::atomic total_remote_quota_denies_{0}; // 1.1 + std::atomic total_remote_quota_prefetch_calls_{0}; // 1.1 + + // + // Counters for upstream requests to Mixer. + // + // total_remote_calls = SUM(total_remote_call_successes, ..., + // total_remote_call_other_errors) Total transport errors would be + // (total_remote_calls - total_remote_call_successes). + // + + std::atomic total_remote_calls_{0}; // 1.1 + std::atomic total_remote_call_successes_{0}; // 1.1 + std::atomic total_remote_call_timeouts_{0}; // 1.1 + std::atomic total_remote_call_send_errors_{0}; // 1.1 + std::atomic total_remote_call_other_errors_{0}; // 1.1 + std::atomic total_remote_call_retries_{0}; // 1.1 + std::atomic total_remote_call_cancellations_{0}; // 1.1 GOOGLE_DISALLOW_EVIL_CONSTRUCTORS(MixerClientImpl); }; diff --git a/src/istio/mixerclient/client_impl_test.cc b/src/istio/mixerclient/client_impl_test.cc index f984c17b32f..e34d1357f3f 100644 --- a/src/istio/mixerclient/client_impl_test.cc +++ b/src/istio/mixerclient/client_impl_test.cc @@ -67,6 +67,66 @@ class MixerClientImplTest : public ::testing::Test { client_ = CreateMixerClient(options); } + void CheckStatisticsInvariants(const Statistics& stats) { + // + // Policy check counters. + // + // total_check_calls = total_check_hits + total_check_misses + // total_check_hits = total_check_hit_accepts + total_check_hit_denies + // total_remote_check_calls = total_check_misses + // total_remote_check_calls >= total_remote_check_accepts + + // total_remote_check_denies + // ^ Transport errors are responsible for the >= + // + + EXPECT_EQ(stats.total_check_calls_, + stats.total_check_cache_hits_ + stats.total_check_cache_misses_); + EXPECT_EQ(stats.total_check_cache_hits_, + stats.total_check_cache_hit_accepts_ + + stats.total_check_cache_hit_denies_); + EXPECT_EQ(stats.total_remote_check_calls_, stats.total_check_cache_misses_); + EXPECT_GE( + stats.total_remote_check_calls_, + stats.total_remote_check_accepts_ + stats.total_remote_check_denies_); + + // + // Quota check counters + // + // total_quota_calls = total_quota_hits + total_quota_misses + // total_quota_hits = total_quota_hit_accepts + total_quota_hit_denies + // total_remote_quota_calls = total_quota_misses + + // total_remote_quota_prefetch_calls total_remote_quota_calls >= + // total_remote_quota_accepts + total_remote_quota_denies + // ^ Transport errors are responsible for the >= + // + + EXPECT_EQ(stats.total_quota_calls_, + stats.total_quota_cache_hits_ + stats.total_quota_cache_misses_); + EXPECT_EQ(stats.total_quota_cache_hits_, + stats.total_quota_cache_hit_accepts_ + + stats.total_quota_cache_hit_denies_); + EXPECT_EQ(stats.total_remote_quota_calls_, + stats.total_quota_cache_misses_ + + stats.total_remote_quota_prefetch_calls_); + EXPECT_GE( + stats.total_remote_quota_calls_, + stats.total_remote_quota_accepts_ + stats.total_remote_quota_denies_); + + // + // Counters for upstream requests to Mixer. + // + // total_remote_calls = SUM(total_remote_call_successes, ..., + // total_remote_call_other_errors) Total transport errors would be + // (total_remote_calls - total_remote_call_successes). + // + + EXPECT_EQ(stats.total_remote_calls_, + stats.total_remote_call_successes_ + + stats.total_remote_call_timeouts_ + + stats.total_remote_call_send_errors_ + + stats.total_remote_call_other_errors_); + } + CheckContextSharedPtr CreateContext(int quota_request) { bool fail_open{false}; istio::mixerclient::SharedAttributesSharedPtr attributes{ @@ -112,14 +172,33 @@ TEST_F(MixerClientImplTest, TestSuccessCheck) { Statistics stat; client_->GetStatistics(&stat); - EXPECT_EQ(stat.total_check_calls, 11); - // The first check call is a remote blocking check call. - EXPECT_EQ(stat.total_remote_check_calls, 1); - EXPECT_EQ(stat.total_blocking_remote_check_calls, 1); + CheckStatisticsInvariants(stat); + + EXPECT_EQ(stat.total_check_calls_, 11); + // The first check call misses the policy cache, the rest hit and are accepted + EXPECT_EQ(stat.total_check_cache_hits_, 10); + EXPECT_EQ(stat.total_check_cache_misses_, 1); + EXPECT_EQ(stat.total_check_cache_hit_accepts_, 10); + EXPECT_EQ(stat.total_check_cache_hit_denies_, 0); + EXPECT_EQ(stat.total_remote_check_calls_, 1); + EXPECT_EQ(stat.total_remote_check_accepts_, 1); + EXPECT_EQ(stat.total_remote_check_denies_, 0); // Empty quota does not trigger any quota call. - EXPECT_EQ(stat.total_quota_calls, 0); - EXPECT_EQ(stat.total_remote_quota_calls, 0); - EXPECT_EQ(stat.total_blocking_remote_quota_calls, 0); + EXPECT_EQ(stat.total_quota_calls_, 0); + EXPECT_EQ(stat.total_quota_cache_hits_, 0); + EXPECT_EQ(stat.total_quota_cache_misses_, 0); + EXPECT_EQ(stat.total_quota_cache_hit_accepts_, 0); + EXPECT_EQ(stat.total_quota_cache_hit_denies_, 0); + EXPECT_EQ(stat.total_remote_quota_calls_, 0); + EXPECT_EQ(stat.total_remote_quota_accepts_, 0); + EXPECT_EQ(stat.total_remote_quota_denies_, 0); + EXPECT_EQ(stat.total_remote_quota_prefetch_calls_, 0); + // Only one remote call and it succeeds + EXPECT_EQ(stat.total_remote_calls_, 1); + EXPECT_EQ(stat.total_remote_call_successes_, 1); + EXPECT_EQ(stat.total_remote_call_timeouts_, 0); + EXPECT_EQ(stat.total_remote_call_send_errors_, 0); + EXPECT_EQ(stat.total_remote_call_other_errors_, 0); } TEST_F(MixerClientImplTest, TestPerRequestTransport) { @@ -155,14 +234,33 @@ TEST_F(MixerClientImplTest, TestPerRequestTransport) { Statistics stat; client_->GetStatistics(&stat); - EXPECT_EQ(stat.total_check_calls, 11); - // The first check call is a remote blocking check call. - EXPECT_EQ(stat.total_remote_check_calls, 1); - EXPECT_EQ(stat.total_blocking_remote_check_calls, 1); + CheckStatisticsInvariants(stat); + + EXPECT_EQ(stat.total_check_calls_, 11); + // The first check call misses the policy cache, the rest hit and are accepted + EXPECT_EQ(stat.total_check_cache_hits_, 10); + EXPECT_EQ(stat.total_check_cache_misses_, 1); + EXPECT_EQ(stat.total_check_cache_hit_accepts_, 10); + EXPECT_EQ(stat.total_check_cache_hit_denies_, 0); + EXPECT_EQ(stat.total_remote_check_calls_, 1); + EXPECT_EQ(stat.total_remote_check_accepts_, 1); + EXPECT_EQ(stat.total_remote_check_denies_, 0); // Empty quota does not trigger any quota call. - EXPECT_EQ(stat.total_quota_calls, 0); - EXPECT_EQ(stat.total_remote_quota_calls, 0); - EXPECT_EQ(stat.total_blocking_remote_quota_calls, 0); + EXPECT_EQ(stat.total_quota_calls_, 0); + EXPECT_EQ(stat.total_quota_cache_hits_, 0); + EXPECT_EQ(stat.total_quota_cache_misses_, 0); + EXPECT_EQ(stat.total_quota_cache_hit_accepts_, 0); + EXPECT_EQ(stat.total_quota_cache_hit_denies_, 0); + EXPECT_EQ(stat.total_remote_quota_calls_, 0); + EXPECT_EQ(stat.total_remote_quota_accepts_, 0); + EXPECT_EQ(stat.total_remote_quota_denies_, 0); + EXPECT_EQ(stat.total_remote_quota_prefetch_calls_, 0); + // Only one remote call and it succeeds + EXPECT_EQ(stat.total_remote_calls_, 1); + EXPECT_EQ(stat.total_remote_call_successes_, 1); + EXPECT_EQ(stat.total_remote_call_timeouts_, 0); + EXPECT_EQ(stat.total_remote_call_send_errors_, 0); + EXPECT_EQ(stat.total_remote_call_other_errors_, 0); } TEST_F(MixerClientImplTest, TestNoCheckCache) { @@ -203,13 +301,35 @@ TEST_F(MixerClientImplTest, TestNoCheckCache) { EXPECT_EQ(call_counts, 11); Statistics stat; client_->GetStatistics(&stat); - // Because there is no check cache, we make remote blocking call every time. - EXPECT_EQ(stat.total_check_calls, 11); - EXPECT_EQ(stat.total_remote_check_calls, 11); - EXPECT_EQ(stat.total_blocking_remote_check_calls, 11); - EXPECT_EQ(stat.total_quota_calls, 11); - EXPECT_EQ(stat.total_remote_quota_calls, 11); - EXPECT_EQ(stat.total_blocking_remote_quota_calls, 11); + CheckStatisticsInvariants(stat); + + EXPECT_EQ(stat.total_check_calls_, 11); + EXPECT_EQ(stat.total_check_cache_hits_, 0); + EXPECT_EQ(stat.total_check_cache_misses_, 11); + EXPECT_EQ(stat.total_check_cache_hit_accepts_, 0); + EXPECT_EQ(stat.total_check_cache_hit_denies_, 0); + EXPECT_EQ(stat.total_remote_check_calls_, 11); + EXPECT_EQ(stat.total_remote_check_accepts_, 11); + EXPECT_EQ(stat.total_remote_check_denies_, 0); + // + // The current quota cache impl forces a cache miss whenever the check cache + // is missed. + // + EXPECT_EQ(stat.total_quota_calls_, 11); + EXPECT_EQ(stat.total_quota_cache_hits_, 0); + EXPECT_EQ(stat.total_quota_cache_misses_, 11); + EXPECT_EQ(stat.total_quota_cache_hit_accepts_, 0); + EXPECT_EQ(stat.total_quota_cache_hit_denies_, 0); + EXPECT_EQ(stat.total_remote_quota_calls_, 11); + EXPECT_EQ(stat.total_remote_quota_accepts_, 11); + EXPECT_EQ(stat.total_remote_quota_denies_, 0); + EXPECT_EQ(stat.total_remote_quota_prefetch_calls_, 0); + // And all remote quota calls succeed + EXPECT_EQ(stat.total_remote_calls_, 11); + EXPECT_EQ(stat.total_remote_call_successes_, 11); + EXPECT_EQ(stat.total_remote_call_timeouts_, 0); + EXPECT_EQ(stat.total_remote_call_send_errors_, 0); + EXPECT_EQ(stat.total_remote_call_other_errors_, 0); } TEST_F(MixerClientImplTest, TestNoQuotaCache) { @@ -252,13 +372,32 @@ TEST_F(MixerClientImplTest, TestNoQuotaCache) { EXPECT_EQ(call_counts, 11); Statistics stat; client_->GetStatistics(&stat); - // Because there is no quota cache, we make remote blocking call every time. - EXPECT_EQ(stat.total_check_calls, 11); - EXPECT_EQ(stat.total_remote_check_calls, 11); - EXPECT_EQ(stat.total_blocking_remote_check_calls, 11); - EXPECT_EQ(stat.total_quota_calls, 11); - EXPECT_EQ(stat.total_remote_quota_calls, 11); - EXPECT_EQ(stat.total_blocking_remote_quota_calls, 11); + CheckStatisticsInvariants(stat); + + EXPECT_EQ(stat.total_check_calls_, 11); + // The first check call misses the policy cache, the rest hit and are accepted + EXPECT_EQ(stat.total_check_cache_hits_, 10); + EXPECT_EQ(stat.total_check_cache_misses_, 1); + EXPECT_EQ(stat.total_check_cache_hit_accepts_, 10); + EXPECT_EQ(stat.total_check_cache_hit_denies_, 0); + EXPECT_EQ(stat.total_remote_check_calls_, 1); + EXPECT_EQ(stat.total_remote_check_accepts_, 1); + EXPECT_EQ(stat.total_remote_check_denies_, 0); + EXPECT_EQ(stat.total_quota_calls_, 11); + EXPECT_EQ(stat.total_quota_cache_hits_, 0); + EXPECT_EQ(stat.total_quota_cache_misses_, 11); + EXPECT_EQ(stat.total_quota_cache_hit_accepts_, 0); + EXPECT_EQ(stat.total_quota_cache_hit_denies_, 0); + EXPECT_EQ(stat.total_remote_quota_calls_, 11); + EXPECT_EQ(stat.total_remote_quota_accepts_, 11); + EXPECT_EQ(stat.total_remote_quota_denies_, 0); + EXPECT_EQ(stat.total_remote_quota_prefetch_calls_, 0); + // And all remote quota calls succeed + EXPECT_EQ(stat.total_remote_calls_, 11); + EXPECT_EQ(stat.total_remote_call_successes_, 11); + EXPECT_EQ(stat.total_remote_call_timeouts_, 0); + EXPECT_EQ(stat.total_remote_call_send_errors_, 0); + EXPECT_EQ(stat.total_remote_call_other_errors_, 0); } TEST_F(MixerClientImplTest, TestSuccessCheckAndQuota) { @@ -308,13 +447,33 @@ TEST_F(MixerClientImplTest, TestSuccessCheckAndQuota) { EXPECT_EQ(call_counts, 1 + expected_prefetchs); Statistics stat; client_->GetStatistics(&stat); - - EXPECT_EQ(stat.total_check_calls, 101); - EXPECT_EQ(stat.total_remote_check_calls, 1 + expected_prefetchs); - EXPECT_EQ(stat.total_blocking_remote_check_calls, 1); - EXPECT_EQ(stat.total_quota_calls, 101); - EXPECT_EQ(stat.total_remote_quota_calls, 1 + expected_prefetchs); - EXPECT_EQ(stat.total_blocking_remote_quota_calls, 1); + CheckStatisticsInvariants(stat); + + EXPECT_EQ(stat.total_check_calls_, 101); + // The first check call misses the policy cache, the rest hit and are accepted + EXPECT_EQ(stat.total_check_cache_hits_, 100); + EXPECT_EQ(stat.total_check_cache_misses_, 1); + EXPECT_EQ(stat.total_check_cache_hit_accepts_, 100); + EXPECT_EQ(stat.total_check_cache_hit_denies_, 0); + EXPECT_EQ(stat.total_remote_check_calls_, 1); + EXPECT_EQ(stat.total_remote_check_accepts_, 1); + EXPECT_EQ(stat.total_remote_check_denies_, 0); + // Quota cache is always hit because of the quota prefetch mechanism. + EXPECT_EQ(stat.total_quota_calls_, 101); + EXPECT_EQ(stat.total_quota_cache_hits_, 100); + EXPECT_EQ(stat.total_quota_cache_misses_, 1); + EXPECT_EQ(stat.total_quota_cache_hit_accepts_, 100); + EXPECT_EQ(stat.total_quota_cache_hit_denies_, 0); + EXPECT_EQ(stat.total_remote_quota_calls_, 1 + expected_prefetchs); + EXPECT_EQ(stat.total_remote_quota_accepts_, 1 + expected_prefetchs); + EXPECT_EQ(stat.total_remote_quota_denies_, 0); + EXPECT_EQ(stat.total_remote_quota_prefetch_calls_, expected_prefetchs); + // And all remote quota calls succeed + EXPECT_EQ(stat.total_remote_calls_, 1 + expected_prefetchs); + EXPECT_EQ(stat.total_remote_call_successes_, 1 + expected_prefetchs); + EXPECT_EQ(stat.total_remote_call_timeouts_, 0); + EXPECT_EQ(stat.total_remote_call_send_errors_, 0); + EXPECT_EQ(stat.total_remote_call_other_errors_, 0); } TEST_F(MixerClientImplTest, TestFailedCheckAndQuota) { @@ -351,14 +510,34 @@ TEST_F(MixerClientImplTest, TestFailedCheckAndQuota) { } Statistics stat; client_->GetStatistics(&stat); + CheckStatisticsInvariants(stat); + + EXPECT_EQ(stat.total_check_calls_, 11); // The first call is a remote blocking call, which returns failed precondition // in check response. Following calls only make check cache calls and return. - EXPECT_EQ(stat.total_check_calls, 11); - EXPECT_EQ(stat.total_remote_check_calls, 1); - EXPECT_EQ(stat.total_blocking_remote_check_calls, 1); - EXPECT_EQ(stat.total_quota_calls, 1); - EXPECT_EQ(stat.total_remote_quota_calls, 1); - EXPECT_EQ(stat.total_blocking_remote_quota_calls, 1); + EXPECT_EQ(stat.total_check_cache_hits_, 10); + EXPECT_EQ(stat.total_check_cache_misses_, 1); + EXPECT_EQ(stat.total_check_cache_hit_accepts_, 0); + EXPECT_EQ(stat.total_check_cache_hit_denies_, 10); + EXPECT_EQ(stat.total_remote_check_calls_, 1); + EXPECT_EQ(stat.total_remote_check_accepts_, 0); + EXPECT_EQ(stat.total_remote_check_denies_, 1); + // If the check cache denies the request, the quota cache never sees it. + EXPECT_EQ(stat.total_quota_calls_, 1); + EXPECT_EQ(stat.total_quota_cache_hits_, 0); + EXPECT_EQ(stat.total_quota_cache_misses_, 1); + EXPECT_EQ(stat.total_quota_cache_hit_accepts_, 0); + EXPECT_EQ(stat.total_quota_cache_hit_denies_, 0); + EXPECT_EQ(stat.total_remote_quota_calls_, 1); + EXPECT_EQ(stat.total_remote_quota_accepts_, 1); + EXPECT_EQ(stat.total_remote_quota_denies_, 0); + EXPECT_EQ(stat.total_remote_quota_prefetch_calls_, 0); + // Only one remote call and it succeeds at the transport level + EXPECT_EQ(stat.total_remote_calls_, 1); + EXPECT_EQ(stat.total_remote_call_successes_, 1); + EXPECT_EQ(stat.total_remote_call_timeouts_, 0); + EXPECT_EQ(stat.total_remote_call_send_errors_, 0); + EXPECT_EQ(stat.total_remote_call_other_errors_, 0); } } // namespace