-
-
Notifications
You must be signed in to change notification settings - Fork 92
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
Support custom logger for plugins #115
Comments
Good point. Enforcing the lib package to use a particular logger wasn't a good idea, I have planned to make it right but completely forgot. Maybe all we need here is a single method logger interface like I do in go-pkgz/auth (and injected like this) ? this way we still keep the common logging format and the common logging presentation, but the user can inject any custom logger with a single |
I don't like the idea of an interface with a single method. Every user who wants leveled logging has to write Another option is to add 2 interfaces:
type Plugin struct {
// ...
log AdvancedLogger
}
func (p *Plugin) SetLogger(log Logger) {
if advanced, ok := log.(AdvancedLogger); ok {
p.log = advanced
return
}
p.log = advancedLogger{log}
}
// advancedLogger implements AdvancedLogger interface for Logger
//
// TODO: not sure about the naming
type advancedLogger struct {
log Logger
}
func (l advancedLogger) Tracef(format string, args ...interface{}) { l.log.Logf("TRACE "+format, args...) }
// And so on... However I think there are only 2 types of users:
So, most users will use |
I would rather keep the logging interface closer to stdlib and not to particular leveled loggers. I think the adopter from Lelveled interface to Logf based interface can be provided as a part of lib/logging package, in a similar way as the example above provides Std adopter. |
I noticed that
github.com/umputun/reproxy/lib
uses the global logger provided bygithub.com/go-pkgz/lgr
. It can be inconvenient because log records can have different formats (especially when usinggithub.com/sirupsen/logrus
orgithub.com/uber-go/zap
).I think it makes sense to support custom logger.
Logger
declaration can look like this (these methods cover all levels ofgithub.com/go-pkgz/lgr
):If custom logger (
Plugin.Log
) is not set,github.com/go-pkgz/lgr
will be used (via a wrapper).I can work on this, but first, I would like to hear your opinion.
The text was updated successfully, but these errors were encountered: