dlib
dlib copied to clipboard
Add SetMaxLogLevel method to LoggerWithMaxLevel interface
Follow up to #39
Propose to add SetMaxLogLevel
to LoggerWithMaxLevel
interface to complete the leveled logging add-in interface for loggers that support it or add leveled logging for logger implementations that don't natively support it.
The main motivation is that there are situations where the loglevel wants to be set dynamically at runtime (ex. from an environment variable) but do not want to be dependent on a particular logging implementation or has to reference a global logger
An example is in emissary where logrus.Logger is exposed globally from the package
var logrusLogger *logrus.Logger
...
// Setting global state
func SetLogLevel(lvl logrus.Level) {
logrusLogger.SetLevel(lvl)
}
// Getting package global state
func GetLogLevel() logrus.Level {
return logrusLogger.GetLevel()
}
With this new method to the interface we can set the log level for the logger passed in through the context without being dependent on a particular implementation and without pulling the logger out of the context which can lead to drift.
logger := dlog.WrapLogrus(logrusLogger).
ctx := dlog.WithLogger(context.Background(), logger)
// later on possibly in separate package
doSomething(ctx) {
switch os.Getenv("LOGLEVEL") {
case "WARN":
dlog.SetMaxLogLevel(ctx, dlog.LogLevelWarn)
case "ERROR":
dlog.SetMaxLogLevel(ctx, dlog.LogLevelError)
default:
dlog.SetMaxLogLevel(ctx, dlog.LogLevelInfo)
}
...
}
I think this offers a cleaner api if we're passing loggers through context.
I can see the benefit of making it scoped instead of modifying the global logger.
I went back and forth whether it should be a separate interface. It seems weird to me to be able to get a level but not set one. I guess a more versatile approach would be:
interface LoggerWithMaxLevel {
MaxLevel
}
interface MaxLevelSetter {
SetMaxLevel
}
interface LeveledLogger{
LoggerWithMaxLevel
MaxLevelSetter
}
and leave it to the logger implementation to decide which to implement. But I didn't think it was strictly necessary at this time