-
Notifications
You must be signed in to change notification settings - Fork 73
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
Update log levels #2244
Update log levels #2244
Conversation
@clintyoshimura @Birdrock did y'all have any thoughts on the log level used by the new logErrorAndSetReadyStatus helper function? We considered moving the logs down to Info level given the additional error handling of the ready status in the api layer, but wanted to check with y'all first. |
59e21af
to
acc1da4
Compare
kpack-image-builder/controllers/imageprocessfetcher/image_process_fetcher.go
Outdated
Show resolved
Hide resolved
controllers/controllers/services/cfservicebinding_controller.go
Outdated
Show resolved
Hide resolved
- Use error log level for platform startup issues, debug log level for resource lifecycle events and info log level for everything else. - Attach an error to an info log entry as a "reason" if appropriate. - Default to info logs for errors in controllers that also set status condition - Make capitalization consistent for log messages (only capitalize kubernetes resource names) Co-authored-by: Matt Royal <mroyal@vmware.com> Co-authored-by: Dave Walter <walterda@vmware.com> Co-authored-by: Ashwin Krishna <krishnaas@vmware.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.
LGTM
Is there a related GitHub Issue?
#1926
What is this change about?
Audit and update all existing logs to an appropriate level. We generally did the following to logs:
Note that after rebasing on main there is a new helper function that is logging at the error level. We left the helper alone for now and will confirm with the team whether there is any issue with moving that down to info by default, potentially with a sibling helper function.Updated the new helper function to info log level like the rest of the controller logs returning errors back to the api.
Does this PR introduce a breaking change?
No
Acceptance Steps
Deploy Korifi normally and see many fewer noisy logs at the default log level.
Tag your pair, your PM, and/or team
@davewalter @matt-royal
cc @julian-hj for thoughts