Skip to content
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

Configure klog to use the ACK runtime logger #126

Merged
merged 1 commit into from
Sep 22, 2023

Conversation

a-hilaly
Copy link
Member

This patch addresses an issue where the leaderElection feature was using
klog/v2 behind the scenes, which was set using a different logger than
the one we use in ACK. This inconsitancy caused the controller to have
two different log styles.

Fortunetly klog/v2 allow users to set their own loggers, and more
fortunetely go-logr is compatible with both klog and zap the
logger we use in ACK runtime.

Setting klog logger results in a more consistant and smooth logging
experience.

Signed-off-by: Amine Hilaly hilalyamine@gmail.com

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

This patch addresses an issue where the leaderElection feature was using
`klog/v2` behind the scenes, which was set using a different logger than
the one we use in ACK. This inconsitancy caused the controller to have
two different log styles.

Fortunetly `klog/v2` allow users to set their own loggers, and more
fortunetely `go-logr` is compatible with both `klog` and `zap` the
logger we use in ACK runtime.

Setting `klog` logger results in a more consistant and smooth logging
experience.

Signed-off-by: Amine <hilalyamine@gmail.com>
@ack-prow ack-prow bot added the approved label Sep 16, 2023
@a-hilaly a-hilaly changed the title Set klog logger using the ACK runtime logger Configure klog to use the ACK runtime logger Sep 19, 2023
@jljaco
Copy link

jljaco commented Sep 22, 2023

/lgtm

@ack-prow ack-prow bot added the lgtm Indicates that a PR is ready to be merged. label Sep 22, 2023
@ack-prow
Copy link

ack-prow bot commented Sep 22, 2023

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: a-hilaly, acornett21, jljaco

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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@ack-prow ack-prow bot merged commit 4fa0c97 into aws-controllers-k8s:main Sep 22, 2023
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved lgtm Indicates that a PR is ready to be merged.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants