Proposal of type Logger interface instead of glog implementation
Initiative
The existing implementation of the logger is [glog]https://github.com/golang/glog/tree/master which is a concrete implementation that outputs logs to local output. However, in my scenario, I want to use an existing app-level logger that logs into a remote log sink.
I assume that many solutions are to actually let glog write logs locally, then have another agent (e.g. fluentd) installed on the server which can forward the local log into a remote log sink so you can decouple the complexity of writing a log and store the log, which is an understandable and well-known practice. However, I would like to still propose a generic logger interface so we can include the scenario where some applications still can implement an app-level logger that directly writes logs to the remote sink.
What is the new Logger looking like
to minimize the impact , i would like to propose a type Logger Interface which has the same common interfaces which glog can also satisfy
package logger
type Logger interface {
SetLevel(level Level)
Infof(format string, args ...interface{})
Exitf(format string, args ...interface{}
....
}
var defaultLogger *Logger
// invoked during the server initialization where you pass your concrete implementation
func SetDefaultLogger(logger *Logger) { defaultLogger = logger }
func Infof(format string, args ...interface{}) { defaultLogger.Infof(format, args...) }
func Exitf(format string, args ...interface{} {...}
then in a custom logger file, write the implementation of your own logger, which should be initialized during the server startup and become a package where different parts of the prebid server can directly import and invoke
e.g
package server
log "github.com/prebid/prebid-server/logger"
var (
mainListener net.Listener
main server = newMainServer(cfg, handler)
)
go shutdownAfterSignals(mainServer, stopMain, done)
if mainListener, err = newTCPListener(mainServer.Addr, metrics); err != nil {
log.Errorf("Error listening for TCP connections on %s: %v for main server", mainServer.Addr, err)
return
}
go runServer(mainServer, "Main", mainListener)
...
Let me know if my proposal makes sense or if you have a different design, happy to discuss it here
Looks good to me. Your proposal looks perfectly in-line for Go programs. I look forward to consolidating all glog references to a single location, as it will allow us to offer other options in the future.
This design reminds be a lot of this implementation.
Ideally, the logger instance would be passed throughout the program via dependency injection. I recognize this isn't how the program works today and it would be an enormous effort. I agree it's appropriate to use logger.Infof etc.. as a drop in replacement.
Discussed with the PBs-Go team. You're good to proceed with development. We'll review more specifics in the eventual PR.
@SyntaxNode resume the development of this part of the code starting next week
related to #4085