-
Notifications
You must be signed in to change notification settings - Fork 4.7k
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.
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
Add stats for overload manager #4001
Conversation
Signed-off-by: Elisha Ziskind <eziskind@google.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for adding stats!
@@ -31,7 +33,9 @@ class ThresholdTriggerImpl : public OverloadAction::Trigger { | |||
|
|||
} // namespace | |||
|
|||
OverloadAction::OverloadAction(const envoy::config::overload::v2alpha::OverloadAction& config) { | |||
OverloadAction::OverloadAction(const envoy::config::overload::v2alpha::OverloadAction& config, | |||
Stats::Scope& stats_scope) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is the planned lifetime of of this overload action? I assume the manager will be global? Will actions be global also? Just trying to understand which scope this is going to use. Will it be the global scope? If so so we need an "overload." prefix? Will it be per-listener? (Probably not).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actions will be global like the manager so I think it makes sense to use global scope for stats. I'll add an "overload." prefix to the stats names.
Signed-off-by: Elisha Ziskind <eziskind@google.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, thanks for adding stats!
Add the following stats for monitoring the overload manager (issue #373):
Risk Level: low
Testing: unit tests
Signed-off-by: Elisha Ziskind eziskind@google.com