Make logging configurable #605
Conversation
This PR adds two new build tags for enabling and disabling info and event logging. By default, no logging is enabled. Closes #556
…sue-556_make_badger_logging_configurable
There was a problem hiding this comment.
Reviewable status: 0 of 9 files reviewed, 3 unresolved discussions (waiting on @srfrog and @manishrjain)
debug/debug.go, line 18 at r1 (raw file):
package debug
Can move logger.go into this file. There's not much going on here, so better to consolidate all logic here.
debug/events.go, line 45 at r1 (raw file):
func NewEventLog(family, title string) EventLogger { if eventLogger != nil { return eventLogger
Can't return the same event logger. Each event logger tracks different events types.
I think you don't need to bring event logging into this fold, because it doesn't pollute the logs that a user typically sees. And even in /debug/events, it is a separate "Badger" family, which is clearly separate and labeled. Never heard anyone complain about this. So, let's remove event logging from this abstraction.
debug/logger.go, line 21 at r1 (raw file):
// Logger is implemented by any logging system that is used for standard logs. type Logger interface { Printf(string, ...interface{})
Have Infof, Warninf, and Errorf as the methods in the interface, and then you can provide a standard logger, which uses Printf internally to support all those, for convenience.
See: https://github.com/etcd-io/etcd/blob/master/raft/logger.go for reference.
updated Logger interface for error, info and warnings. renamed debug package to "log". added a default logger using Go log.
setting default logger at init.
There was a problem hiding this comment.
Reviewable status: 0 of 5 files reviewed, 3 unresolved discussions (waiting on @manishrjain)
debug/debug.go, line 18 at r1 (raw file):
Previously, manishrjain (Manish R Jain) wrote…
Can move logger.go into this file. There's not much going on here, so better to consolidate all logic here.
Done.
debug/events.go, line 45 at r1 (raw file):
Previously, manishrjain (Manish R Jain) wrote…
Can't return the same event logger. Each event logger tracks different events types.
I think you don't need to bring event logging into this fold, because it doesn't pollute the logs that a user typically sees. And even in
/debug/events, it is a separate "Badger" family, which is clearly separate and labeled. Never heard anyone complain about this. So, let's remove event logging from this abstraction.
Done.
debug/logger.go, line 21 at r1 (raw file):
Previously, manishrjain (Manish R Jain) wrote…
Have Infof, Warninf, and Errorf as the methods in the interface, and then you can provide a standard logger, which uses Printf internally to support all those, for convenience.
See: https://github.com/etcd-io/etcd/blob/master/raft/logger.go for reference.
Done.
There was a problem hiding this comment.
Reviewed 1 of 9 files at r1, 7 of 9 files at r2.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @srfrog)
db.go, line 903 at r2 (raw file):
} // Encounterd error. Retry indefinitely. Errorf("failure while flushing memtable to disk: %v. Retrying...\n", err)
Capital f in failure.
There was a problem hiding this comment.
Reviewable status: 4 of 5 files reviewed, 1 unresolved discussion (waiting on @manishrjain)
db.go, line 903 at r2 (raw file):
Previously, manishrjain (Manish R Jain) wrote…
Capital f in failure.
Done.
|
Just a quick note:
|
|
How does this PR make "default logging silent"? |
This PR makes default logging silent and toggeable.
Closes #556
This change is