Add a LoggerWithMaxLevel interface #39
Conversation
Improves performance by using a fixed array of log levels instead of a map to avoid a hash lookup on each log call.
This interface is meant to be used by `Logger` implementations that
will discard messages above a certain log level. A prominent example
is the current logrus wrapper.
The interface is motivated by the need to sometimes skip resource
consuming string formatting for messages that the logger will discard.
Example:
if dlog.MaxLogLevel(ctx) >= dlog.LogLevelTrace {
// use a string buffer to build a trace message from some network
// package, then log it with
dlog.Trace(ctx, msg)
}
The max-level can also be usedful when an alternative logger must
be created for some third party module that implements its own logger
API.
dlog/logger.go
Outdated
| @@ -84,6 +84,19 @@ type OptimizedLogger interface { | |||
| UnformattedLogf(level LogLevel, format string, args ...interface{}) | |||
| } | |||
|
|
|||
| // LoggerWithMaxLevel can be implemented by loggers that define a maximum | |||
| // level that will be logged, i.e. if a logger defines a max-level of | |||
There was a problem hiding this comment.
nit: "i.e." is used where you might say "in other words"; "e.g." is for where you might say "for example".
dlog/logger.go
Outdated
| // LoggerWithMaxLevel can be implemented by loggers that define a maximum | ||
| // level that will be logged, i.e. if a logger defines a max-level of | ||
| // LogLevelInfo, then only LogLevelError, LogLevelWarn, and LogLevelInfo will | ||
| // be logged, and LogLevelDebug and LogLevelTrace will be discarded. |
There was a problem hiding this comment.
nit: since this comma is delimiting a set of comma-separated lists (LogLevelError, LogLevelWarn, and LogLevelInfo vs LogLevelDebug and LogLevelTrace), it should be a semicolon to indicate that it is higher precedence than the other commas.
| type LoggerWithMaxLevel interface { | ||
| // MaxLevel return the maximum loglevel that will be logged | ||
| MaxLevel() LogLevel | ||
| } |
There was a problem hiding this comment.
My only suggestion is that I'd consider including this in the OptimizedLogger interface rather than creating a separate interface. I'm not sure I would, but I'd consider it more deeply; and then include in the doc comment the reasoning for landing whichever way I did.
There was a problem hiding this comment.
(in other words: please document your rationale)
- Use semicolon to show comma priority - Use e.g. instead of i.e. - Add motivation for the extra interface to docs comment.
This interface is meant to be used by
Loggerimplementations thatwill discard messages above a certain log level. A prominent example
is the current logrus wrapper.
The interface is motivated by the need to sometimes skip resource
consuming string formatting for messages that the logger will discard.
Example:
The max-level can also be usedful when an alternative logger must
be created for some third party module that implements its own logger
API.