-
Notifications
You must be signed in to change notification settings - Fork 132
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
Use zap instead of klog #1606
Use zap instead of klog #1606
Conversation
5494e3a
to
e7787af
Compare
a4a6921
to
927a0ee
Compare
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.
/approve
LGTM label has been added. Git tree hash: 4a183fc0e06e9448bd1c76b0fcfc0095648e5f02
|
/approve |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: ahmedwaleedmalik, mfranczy, xrstf 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 |
/retest Review the full test history Silence the bot with an |
3 similar comments
/retest Review the full test history Silence the bot with an |
/retest Review the full test history Silence the bot with an |
/retest Review the full test history Silence the bot with an |
@xrstf: The following test failed, say
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. I understand the commands that are listed here. |
/hold @xrstf I'm not sure what's wrong with the VCD tests. You can create a ticket to fix these tests and then probably override them here on this PR.
|
/override pull-machine-controller-e2e-vmware-cloud-director |
@xrstf: Overrode contexts on behalf of xrstf: pull-machine-controller-e2e-vmware-cloud-director 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. |
* add logging bootstrapping * replace klog everywhere in the controllers
What this PR does / why we need it:
This PR replaces klog with Zap. Why?
There were a few log messages on level 3, which is somewhere between INFO and DEBUG. I made a gut choice for all of those, trying to make the logs useful but not too verbose.
klog still remains for the debug HTTP client (as it depends on numeric log levels, something we cannot do with Zap) and for the e2e tests (their logs are kind of boring and don't need Zap that much).
What type of PR is this?
/kind feature
Does this PR introduce a user-facing change? Then add your Release Note here:
Documentation: