-
Notifications
You must be signed in to change notification settings - Fork 113
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
Improve operator logging #477
Improve operator logging #477
Conversation
Set a TimeEncoder for zap logging system to have more speaking timestamp. from `1.6893201844680622e+09` to `2023-07-14T09:52:29.539392293Z` Signed-off-by: Andrea Panattoni <[email protected]>
In `renderDevicePluginConfigData()`, avoid logging the full ResourceList for every resource in the loop. Avoid logging in helper functions, as they can be invoked in loops. Signed-off-by: Andrea Panattoni <[email protected]>
Thanks for your PR,
To skip the vendors CIs use one of:
|
Pull Request Test Coverage Report for Build 5554171225
💛 - Coveralls |
lgtm |
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.
LGTM
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.
LGTM. @zeeke thanks for this improvement!
We are using a mix of different loggers. I see there is also |
we should definitely have a consistency with logging across different components in the operator. perhaps create an issue about it ? LGTM, merging this one. |
This PR improves the logging system of the operator:
a.
sigs.k8s.io/controller-runtime/pkg/log/zap
logging system needs a TimeEncoder to be set, so log lines change from:to
b. Avoid logging in helper functions
c.
renderDevicePluginConfigData()
was logging a growing ResourceList array in a loop, leading to excessive logging when the cluster has 20+ policies.