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

Use non-global logger to avoid unexpected global log discarding #312

Merged
merged 1 commit into from
Feb 28, 2022

Conversation

imjasonh
Copy link
Contributor

Reported in GoogleContainerTools/kaniko#1955

Changes in #309 made it possible for callers depending on this code as a Go library to redirect log output to ioutil.Discard to avoid spurious log statements.

Unfortunately, because in that change the logger being used was the shared global logger.StandardLogger(), redirecting its output to ioutil.Discard to quiet ecr-login also disabled all logging using the shared global logger (like Kaniko does). 🤦‍♂️

Signed-off-by: Jason Hall [email protected]

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

ecr-login/ecr.go Outdated Show resolved Hide resolved
@kzys
Copy link
Contributor

kzys commented Feb 28, 2022

Can you squash the two commits into one?

writes to the requested output , instead of changing the behavior of the
standard logrus logger.

Rename WithLogOutput to WithLogger to make this new behavior clearer.

Use non-global logger to avoid unexpected global log discarding

Reported in GoogleContainerTools/kaniko#1955

Changes in
awslabs#309 made it
possible for callers depending on this code as a Go library to redirect
log output to ioutil.Discard to avoid spurious log statements.

Unfortunately, because in that change the logger being used was the
shared global logger.StandardLogger(), redirecting its output to
ioutil.Discard to quiet ecr-login also disabled all logging using the
shared global logger (like Kaniko does).

Signed-off-by: Jason Hall <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants