-
Notifications
You must be signed in to change notification settings - Fork 552
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
Make logging for creating/deleting AWS resources consistent #3144
Conversation
Some resources had only delete log while other resources create/delete logs were missing.
/test? |
@pydctw: This issue is currently awaiting triage. If CAPA/CAPI contributors determines this is a relevant issue, they will accept it by applying the The Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
@pydctw: The following commands are available to trigger required jobs:
The following commands are available to trigger optional jobs:
Use
In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
/test pull-cluster-api-provider-aws-e2e |
See manager.log for an updated output log. |
I think there is a systematic way you have in mind in this PR: logging as info() only at the end of a method and logging v2.info() elsewhere. Do you want to track this or maybe add documentation as part of this PR? |
This is really good observation, as I have recently got feedback that the logs are inconsistent. As @sedefsavas mentioned, I also think we should have documentation in place so that if we add new services/behavior, then this documentation might help follow consistency. |
I actually didn't have any systematic way in mind 🙂 What I did was
But 100% agree that it will be great to have a guidance on what log level to use across the repo. Will open an issue for discussion. Thank you for the suggestion, @sedefsavas and @Ankitasw. |
Created an issue for consistent logging as a follow up: #3152 |
I looked too hard for seeing consistency then =) /lgtm |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: sedefsavas The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
What type of PR is this?
/kind cleanup
What this PR does / why we need it:
Make logging for creating/deleting AWS resources consistent. See #3143 for a detailed description.
Which issue(s) this PR fixes (optional, in
fixes #<issue number>(, fixes #<issue_number>, ...)
format, will close the issue(s) when PR gets merged):Fixes #3143
Special notes for your reviewer:
Added missing logs, formatted some logs and moved unimportant logs to level 2.
Checklist:
Release note: