Skip to content

Additional counters for mixer policy check #2118

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
duderino merged 4 commits into from
Feb 16, 2019
Merged

Additional counters for mixer policy check #2118

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
f278700
Coalesce all memory for policy check requests and telemetry reports i…
Feb 14, 2019
32de45c
A few small fixups for the code review.
Feb 15, 2019
3aa4a8d
Added finer-grained counters to mixer policy check
Feb 15, 2019
c3d2667
Merge remote-tracking branch 'upstream/release-1.1' into jblatt_8224_…
Feb 15, 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
77 changes: 62 additions & 15 deletions include/istio/mixerclient/client.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
71 changes: 32 additions & 39 deletions src/envoy/utils/stats.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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_);

@JimmyCYJ JimmyCYJ Feb 15, 2019

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need trailing underscore "". It should be new_stats.total_check_calls instead of new_stats.total_check_calls, no?

@JimmyCYJ JimmyCYJ Feb 15, 2019

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see, you have updated stats names with "_". Please ignore my comment.

@duderino duderino Feb 15, 2019

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@JimmyCYJ thank you for jumping on this PR! appreciate it :)

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;
Expand Down
34 changes: 26 additions & 8 deletions src/envoy/utils/stats.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
Loading