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

feat: allow log level to be set to any level #1345

Conversation

matthewmcneely
Copy link
Contributor

When using watch tower for a convenient way to reload freshly built images from the local Docker registry (with --no-pull and a poll interval of one second), the "Session done" log message spams the logs: level=info msg="Session done" Failed=0 Scanned=3 Updated=0 notify=no.

Since this is really debug-oriented messaging, seems appropriate.

@piksel
Copy link
Member

piksel commented Aug 18, 2022

Why do you consider the result of a session "debug" information? What is it that you are looking for in the logs if the summary of a run is "too much"?

@matthewmcneely
Copy link
Contributor Author

It's a fine line sure, I get your point. In my case I'm trying to use watchtower to check the local docker registry every second for new images. So the logs are getting full of "Session done" messages.

It would be fine if I could actually control the general watchtower logging level that's going to stderr (I'd just set it to WARN). Seems that I can change the notification logging level, but that does not effect general logging it seems, only the mail, slack etc notification system.

@simskij
Copy link
Member

simskij commented Aug 23, 2022

This is way too much of an edge case to warrant changing the defaults. Having it as INFO likely is what most users would expect.

@matthewmcneely
Copy link
Contributor Author

@simskij OK, understood.

Am I correct that there is no way (flag, env var) to configure the default logging? I understand I can change the Notification logging level that sends out emails, slack messages, etc.—but not the 'console' logging.

For my use case (checking local registry every second), that is what I first attempted to do.

@simskij
Copy link
Member

simskij commented Aug 24, 2022

@simskij OK, understood.

Am I correct that there is no way (flag, env var) to configure the default logging? I understand I can change the Notification logging level that sends out emails, slack messages, etc.—but not the 'console' logging.

For my use case (checking local registry every second), that is what I first attempted to do.

That is kind of correct. You can switch to tracing or debugging, but that really is the opposite of what you're looking for. I would suggest extending the command line arguments with a flag to set the level to a less permissive one, following the same pattern as in

watchtower/cmd/root.go

Lines 90 to 95 in 1465768

if enabled, _ := f.GetBool("debug"); enabled {
log.SetLevel(log.DebugLevel)
}
if enabled, _ := f.GetBool("trace"); enabled {
log.SetLevel(log.TraceLevel)
}
. This should be a fairly trivial change only requiring you to make a couple of minor additions in a few files.

@piksel
Copy link
Member

piksel commented Sep 2, 2022

Agreed. Using the new flag handling patterns, the --debug and --trace flags should be aliases for --log-level debug etc.

@codecov
Copy link

codecov bot commented Sep 5, 2022

Codecov Report

Merging #1345 (d34bd83) into main (626bd54) will increase coverage by 0.12%.
The diff coverage is 82.35%.

@@            Coverage Diff             @@
##             main    #1345      +/-   ##
==========================================
+ Coverage   64.85%   64.98%   +0.12%     
==========================================
  Files          23       23              
  Lines        2302     2319      +17     
==========================================
+ Hits         1493     1507      +14     
- Misses        712      714       +2     
- Partials       97       98       +1     
Impacted Files Coverage Δ
internal/flags/flags.go 85.71% <82.35%> (-0.12%) ⬇️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@piksel piksel changed the title Change session done stats to 'debug' level feat: allow log level to be set to any level Sep 6, 2022
matthewmcneely and others added 3 commits September 6, 2022 12:53
When using watch tower for a convenient way to reload freshly built images from the local Docker registry (with --no-pull and a poll interval of one second), the "Session done" log message spams the logs: `level=info msg="Session done" Failed=0 Scanned=3 Updated=0 notify=no`.

Since this is really debug-oriented messaging, seems appropriate.
@piksel piksel force-pushed the feature/change-session-ended-stats-to-debug-level branch from c3787b2 to 5653b76 Compare September 6, 2022 10:54
@piksel piksel requested a review from simskij September 6, 2022 15:55
@piksel piksel merged commit 0fddbfb into containrrr:main Sep 17, 2022
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.

None yet

3 participants