-
-
Notifications
You must be signed in to change notification settings - Fork 3k
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
Improve logging #179
Improve logging #179
Conversation
Maybe we should add the different log levels to the help? |
@@ -105,7 +106,7 @@ func main() { | |||
err := CmdIpfs.Dispatch(os.Args[1:]) | |||
if err != nil { | |||
if len(err.Error()) > 0 { | |||
fmt.Fprintf(os.Stderr, "ipfs %s: %v\n", os.Args[1], err) | |||
log.Error("ipfs %s: %v\n", os.Args[1], err) |
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.
this is a case where we do want fmt.Fprintf(os.Stderr, ...
, because this should be output cleanly to the user in the cli (the logger has a complicated format, and may be redirected later on).
Think about this like git
or other tools. normal errors part of the use of the application should usually result in the user getting a simple error message. The logger is to signal what's going on under the hood, a logger.Error
is closer to an unexpected problem.
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.
ok makes sense
ipfs log merkledag debug - print debug output from the merkledag subsystem | ||
ipfs log * critical - change all subsystems to only log critical errors | ||
|
||
ipfs block is a utility command used to change the logging output of a running daemon.`, |
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.
UsageLine: "log <name> <level> ",
Short: "switch logging levels of a running daemon",
Long: `ipfs log <name> <level> - switch logging levels of a running daemon
<name> is a the subsystem logging identifier. Use * for all subsystems.
<level> is one of: debug, info, notice, warning, error, critical
ipfs log is a utility command used to change the logging output of a running daemon.
`
} | ||
} | ||
|
||
SetAllLoggers(lvl) |
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.
Was there a bug with how the logic was before (setting lvl := logging.ERROR
, and changing it if the env var, etc) ? That way SetAllLoggers
is only called once.
Ok, just a couple things to address, and this LGTM! |
Addressed :] |
@jbenet where do you get all these gifs from? I image you have a TB hard drive full of gifs sorted in some specific way, and maybe a helper program you can type keywords into and have it spit back a gif. Ill bet that was the motivation for IPFS, you didnt have enough local hard drive space for your gifs, so you needed a clever way to trick other people into storing them for you... very smart 👍 |
:D That would make for an awesome example app when the alpha is ready. |
Fix docstring and gx bump
This implements parts of #152.
Mainly the
IPFS_DEBUG
flag and theipfs log
command to change log levels of a running daemon. I chose add a explicitGetEnvBool()
to theutil
pkg.