prebid-server icon indicating copy to clipboard operation
prebid-server copied to clipboard

Proposal of type Logger interface instead of glog implementation

Open zhongshixi-blockthrough opened this issue 2 years ago • 4 comments

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

zhongshixi-blockthrough avatar Jul 19 '23 19:07 zhongshixi-blockthrough

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.

SyntaxNode avatar Jul 25 '23 22:07 SyntaxNode

Discussed with the PBs-Go team. You're good to proceed with development. We'll review more specifics in the eventual PR.

SyntaxNode avatar Jul 25 '23 22:07 SyntaxNode

@SyntaxNode resume the development of this part of the code starting next week

zhongshixi avatar Nov 22 '24 00:11 zhongshixi

related to #4085

justadreamer avatar Apr 18 '25 09:04 justadreamer