Add flag for setting log levels and log errors for debugging. #139
Conversation
| @@ -36,4 +43,5 @@ func Execute() { | |||
|
|
|||
| func init() { | |||
| rootCmd.Flags().BoolP("toggle", "t", false, "Help message for toggle") | |||
| rootCmd.PersistentFlags().StringVarP(&zarfLogLevel, "log-level", "l", "info", "Log level when runnning Zarf. Valid options are: debug, info, warn, error, fatal") | |||
There was a problem hiding this comment.
This sets the default logging level to info (which is what logrus would use if we didn't specify anything else). The way it is currently written would have this interaction:
./zarf init --log-level error -> Runs Zarf with the log level set to error
./zarf deploy. -> Runs Zarf with the log level set to info
Which is what I would expect it to do but I could see others expecting it to stay in error mode. Does anyone think this is something we might want to do?
There was a problem hiding this comment.
IMO the default log level should be the quieter one, with the ability to make it more noisy if you want
There was a problem hiding this comment.
info is the default the loggrus uses so it's what we're currently using right now. I can easily make it warn though.
YrrepNoj
force-pushed
the
feature/specifying-log-level
branch
from
acf2393 to
304dec6
Compare
|
So I do have concerns with just doing a generic log wrapper here, I don't see a compelling reason to make this change across all the files when you could just leave the I just tested that and confirmed that setting in root.cmd properly persists into other imports of logrus in the CLI without a wrapper. |
woah.. I don't know why I was so certain that setting the log level will only affect the logrus instance for that current package. |
I actually wasn't sure either and the docs aren't totally clear so had to test it myself.....and what you didn't isn't wrong, I just don't think we need to abstract the logger yet like that. |
|
random ?, does logrus.Debug give us any sort of stack trace as well? |
|
/test all |
Signed-off-by: Jeff McCoy <[email protected]>
Signed-off-by: Jeff McCoy <[email protected]>
Added a global flag (works for any subcommand) for setting the log level of logrus. This gets applied to a local utility logger that all the other packages will use. Having the local utility logger buys us the ability to set it once and forget about it instead of having to set the log level for every package. This also means we can have universal styling/formatting across all of our packages if that's something we care about in the future.
This PR also logs all errors to
Debugto make it easy to figure out what's happening when something happens to go wrong. I choseDebugmode since we probably don't want that filling the console all the time, but at the same time errors should be rare and maybe we want to know exactly what happened whenever it happens so I can also see an argument for usingErrormode instead..