diff --git a/pkg/logflags/logflags.go b/pkg/logflags/logflags.go index 430cd566..233e9bb9 100644 --- a/pkg/logflags/logflags.go +++ b/pkg/logflags/logflags.go @@ -29,17 +29,20 @@ var minidump = false var logOut io.WriteCloser -func makeLogger(flag bool, fields logrus.Fields) *logrus.Entry { - logger := logrus.New().WithFields(fields) - logger.Logger.Formatter = &textFormatter{} +func makeLogger(flag bool, fields Fields) Logger { + if lf := loggerFactory; lf != nil { + return lf(flag, fields, logOut) + } + logger := logrus.New().WithFields(logrus.Fields(fields)) + logger.Logger.Formatter = DefaultFormatter() if logOut != nil { logger.Logger.Out = logOut } - logger.Logger.Level = logrus.DebugLevel - if !flag { - logger.Logger.Level = logrus.ErrorLevel + logger.Logger.Level = logrus.ErrorLevel + if flag { + logger.Logger.Level = logrus.DebugLevel } - return logger + return &logrusLogger{logger} } // Any returns true if any logging is enabled. @@ -54,8 +57,8 @@ func GdbWire() bool { } // GdbWireLogger returns a configured logger for the gdbserial wire protocol. -func GdbWireLogger() *logrus.Entry { - return makeLogger(gdbWire, logrus.Fields{"layer": "gdbconn"}) +func GdbWireLogger() Logger { + return makeLogger(gdbWire, Fields{"layer": "gdbconn"}) } // Debugger returns true if the debugger package should log. @@ -64,8 +67,8 @@ func Debugger() bool { } // DebuggerLogger returns a logger for the debugger package. -func DebuggerLogger() *logrus.Entry { - return makeLogger(debugger, logrus.Fields{"layer": "debugger"}) +func DebuggerLogger() Logger { + return makeLogger(debugger, Fields{"layer": "debugger"}) } // LLDBServerOutput returns true if the output of the LLDB server should be @@ -80,14 +83,24 @@ func DebugLineErrors() bool { return debugLineErrors } +// DebugLineLogger returns a logger for the dwarf/line package. +func DebugLineLogger() Logger { + return makeLogger(debugLineErrors, Fields{"layer": "dwarf-line"}) +} + // RPC returns true if RPC messages should be logged. func RPC() bool { return rpc } // RPCLogger returns a logger for RPC messages. -func RPCLogger() *logrus.Entry { - return makeLogger(rpc, logrus.Fields{"layer": "rpc"}) +func RPCLogger() Logger { + return rpcLogger(rpc) +} + +// rpcLogger returns a logger for RPC messages set to a specific minimal log level. +func rpcLogger(flag bool) Logger { + return makeLogger(flag, Fields{"layer": "rpc"}) } // DAP returns true if dap package should log. @@ -96,8 +109,8 @@ func DAP() bool { } // DAPLogger returns a logger for dap package. -func DAPLogger() *logrus.Entry { - return makeLogger(dap, logrus.Fields{"layer": "dap"}) +func DAPLogger() Logger { + return makeLogger(dap, Fields{"layer": "dap"}) } // FnCall returns true if the function call protocol should be logged. @@ -105,8 +118,8 @@ func FnCall() bool { return fnCall } -func FnCallLogger() *logrus.Entry { - return makeLogger(fnCall, logrus.Fields{"layer": "proc", "kind": "fncall"}) +func FnCallLogger() Logger { + return makeLogger(fnCall, Fields{"layer": "proc", "kind": "fncall"}) } // Minidump returns true if the minidump loader should be logged. @@ -114,8 +127,8 @@ func Minidump() bool { return minidump } -func MinidumpLogger() *logrus.Entry { - return makeLogger(minidump, logrus.Fields{"layer": "core", "kind": "minidump"}) +func MinidumpLogger() Logger { + return makeLogger(minidump, Fields{"layer": "core", "kind": "minidump"}) } // WriteDAPListeningMessage writes the "DAP server listening" message in dap mode. @@ -139,8 +152,7 @@ func writeListeningMessage(server string, addr net.Addr) { if tcpAddr == nil || tcpAddr.IP.IsLoopback() { return } - logger := RPCLogger() - logger.Logger.Level = logrus.WarnLevel + logger := rpcLogger(true) logger.Warnln("Listening for remote connections (connections are not authenticated nor encrypted)") } @@ -217,12 +229,17 @@ func Close() { } } -// textFormatter is a simplified version of logrus.TextFormatter that +// DefaultFormatter provides a simplified version of logrus.TextFormatter that // doesn't make logs unreadable when they are output to a text file or to a // terminal that doesn't support colors. -type textFormatter struct { +func DefaultFormatter() logrus.Formatter { + return textFormatterInstance } +type textFormatter struct{} + +var textFormatterInstance = &textFormatter{} + func (f *textFormatter) Format(entry *logrus.Entry) ([]byte, error) { var b *bytes.Buffer if entry.Buffer != nil { diff --git a/pkg/logflags/logflags_test.go b/pkg/logflags/logflags_test.go new file mode 100644 index 00000000..d2392df1 --- /dev/null +++ b/pkg/logflags/logflags_test.go @@ -0,0 +1,117 @@ +package logflags + +import ( + "bytes" + "io" + "reflect" + "testing" + + "github.com/sirupsen/logrus" +) + +func TestMakeLogger_usingLoggerFactory(t *testing.T) { + if loggerFactory != nil { + t.Fatalf("expected loggerFactory to be nil; but was <%v>", loggerFactory) + } + defer func() { + loggerFactory = nil + }() + if logOut != nil { + t.Fatalf("expected logOut to be nil; but was <%v>", logOut) + } + logOut = &bufferWriter{} + defer func() { + logOut = nil + }() + + expectedLogger := &logrusLogger{} + SetLoggerFactory(func(flag bool, fields Fields, out io.Writer) Logger { + if flag != true { + t.Fatalf("expected flag to be <%v>; but was <%v>", true, flag) + } + if len(fields) != 1 || fields["foo"] != "bar" { + t.Fatalf("expected fields to be {'foo':'bar'}; but was <%v>", fields) + } + if out != logOut { + t.Fatalf("expected out to be <%v>; but was <%v>", logOut, out) + } + return expectedLogger + }) + + actual := makeLogger(true, Fields{"foo": "bar"}) + if actual != expectedLogger { + t.Fatalf("expected actual to <%v>; but was <%v>", expectedLogger, actual) + } +} + +func TestMakeLogger_usingDefaultBehavior(t *testing.T) { + if loggerFactory != nil { + t.Fatalf("expected loggerFactory to be nil; but was <%v>", loggerFactory) + } + if logOut != nil { + t.Fatalf("expected logOut to be nil; but was <%v>", logOut) + } + logOut = &bufferWriter{} + defer func() { + logOut = nil + }() + + actual := makeLogger(false, Fields{"foo": "bar"}) + + actualEntry, expectedType := actual.(*logrusLogger) + if !expectedType { + t.Fatalf("expected actual to be of type <%v>; but was <%v>", reflect.TypeOf((*logrus.Entry)(nil)), reflect.TypeOf(actualEntry)) + } + if actualEntry.Entry.Logger.Level != logrus.ErrorLevel { + t.Fatalf("expected actualEntry.Entry.Logger.Level to be <%v>; but was <%v>", logrus.ErrorLevel, actualEntry.Logger.Level) + } + if actualEntry.Entry.Logger.Out != logOut { + t.Fatalf("expected actualEntry.Entry.Logger.Out to be <%v>; but was <%v>", logOut, actualEntry.Logger.Out) + } + if actualEntry.Entry.Logger.Formatter != textFormatterInstance { + t.Fatalf("expected actualEntry.Entry.Logger.Formatter to be <%v>; but was <%v>", textFormatterInstance, actualEntry.Logger.Formatter) + } + if len(actualEntry.Entry.Data) != 1 || actualEntry.Entry.Data["foo"] != "bar" { + t.Fatalf("expected actualEntry.Entry.Data to be {'foo':'bar'}; but was <%v>", actualEntry.Data) + } +} + +func TestMakeLogger_usingDefaultBehaviorAndFlagged(t *testing.T) { + if loggerFactory != nil { + t.Fatalf("expected loggerFactory to be nil; but was <%v>", loggerFactory) + } + if logOut != nil { + t.Fatalf("expected logOut to be nil; but was <%v>", logOut) + } + logOut = &bufferWriter{} + defer func() { + logOut = nil + }() + + actual := makeLogger(true, Fields{"foo": "bar"}) + + actualEntry, expectedType := actual.(*logrusLogger) + if !expectedType { + t.Fatalf("expected actual to be of type <%v>; but was <%v>", reflect.TypeOf((*logrus.Entry)(nil)), reflect.TypeOf(actualEntry)) + } + if actualEntry.Entry.Logger.Level != logrus.DebugLevel { + t.Fatalf("expected actualEntry.Entry.Logger.Level to be <%v>; but was <%v>", logrus.DebugLevel, actualEntry.Logger.Level) + } + if actualEntry.Entry.Logger.Out != logOut { + t.Fatalf("expected actualEntry.Entry.Logger.Out to be <%v>; but was <%v>", logOut, actualEntry.Logger.Out) + } + if actualEntry.Entry.Logger.Formatter != textFormatterInstance { + t.Fatalf("expected actualEntry.Entry.Logger.Formatter to be <%v>; but was <%v>", textFormatterInstance, actualEntry.Logger.Formatter) + } + if len(actualEntry.Entry.Data) != 1 || actualEntry.Entry.Data["foo"] != "bar" { + t.Fatalf("expected actualEntry.Entry.Data to be {'foo':'bar'}; but was <%v>", actualEntry.Data) + } +} + +type bufferWriter struct { + bytes.Buffer +} + +func (bw bufferWriter) Close() error { + return nil +} diff --git a/pkg/logflags/logger.go b/pkg/logflags/logger.go new file mode 100644 index 00000000..14fd07ea --- /dev/null +++ b/pkg/logflags/logger.go @@ -0,0 +1,77 @@ +package logflags + +import ( + "github.com/sirupsen/logrus" + "io" +) + +// Logger represents a generic interface for logging inside of +// Delve codebase. +type Logger interface { + // WithField returns a new Logger enriched with the given field. + WithField(key string, value interface{}) Logger + // WithFields returns a new Logger enriched with the given fields. + WithFields(fields Fields) Logger + // WithError returns a new Logger enriched with the given error. + WithError(err error) Logger + + Debugf(format string, args ...interface{}) + Infof(format string, args ...interface{}) + Printf(format string, args ...interface{}) + Warnf(format string, args ...interface{}) + Warningf(format string, args ...interface{}) + Errorf(format string, args ...interface{}) + Fatalf(format string, args ...interface{}) + Panicf(format string, args ...interface{}) + + Debug(args ...interface{}) + Info(args ...interface{}) + Print(args ...interface{}) + Warn(args ...interface{}) + Warning(args ...interface{}) + Error(args ...interface{}) + Fatal(args ...interface{}) + Panic(args ...interface{}) + + Debugln(args ...interface{}) + Infoln(args ...interface{}) + Println(args ...interface{}) + Warnln(args ...interface{}) + Warningln(args ...interface{}) + Errorln(args ...interface{}) + Fatalln(args ...interface{}) + Panicln(args ...interface{}) +} + +// LoggerFactory is used to create new Logger instances. +// SetLoggerFactory can be used to configure it. +// +// The given parameters fields and out can be both be nil. +type LoggerFactory func(flag bool, fields Fields, out io.Writer) Logger + +var loggerFactory LoggerFactory + +// SetLoggerFactory will ensure that every Logger created by this package, will be now created +// by the given LoggerFactory. Default behavior will be a logrus based Logger instance using DefaultFormatter. +func SetLoggerFactory(lf LoggerFactory) { + loggerFactory = lf +} + +// Fields type wraps many fields for Logger +type Fields map[string]interface{} + +type logrusLogger struct { + *logrus.Entry +} + +func (l *logrusLogger) WithField(key string, value interface{}) Logger { + return &logrusLogger{l.Entry.WithField(key, value)} +} + +func (l *logrusLogger) WithFields(fields Fields) Logger { + return &logrusLogger{l.Entry.WithFields(logrus.Fields(fields))} +} + +func (l *logrusLogger) WithError(err error) Logger { + return &logrusLogger{l.Entry.WithError(err)} +} diff --git a/pkg/proc/bininfo.go b/pkg/proc/bininfo.go index dd92fe58..950bdda1 100644 --- a/pkg/proc/bininfo.go +++ b/pkg/proc/bininfo.go @@ -34,7 +34,6 @@ import ( "github.com/go-delve/delve/pkg/logflags" "github.com/go-delve/delve/pkg/proc/debuginfod" "github.com/hashicorp/golang-lru/simplelru" - "github.com/sirupsen/logrus" ) const ( @@ -111,7 +110,7 @@ type BinaryInfo struct { // Go 1.17 register ABI is enabled. regabi bool - logger *logrus.Entry + logger logflags.Logger } var ( @@ -944,7 +943,7 @@ func (image *Image) Close() error { return err2 } -func (image *Image) setLoadError(logger *logrus.Entry, fmtstr string, args ...interface{}) { +func (image *Image) setLoadError(logger logflags.Logger, fmtstr string, args ...interface{}) { image.loadErrMu.Lock() image.loadErr = fmt.Errorf(fmtstr, args...) image.loadErrMu.Unlock() @@ -1586,7 +1585,7 @@ func (bi *BinaryInfo) setGStructOffsetElf(image *Image, exe *elf.File, wg *sync. } } -func getSymbol(image *Image, logger *logrus.Entry, exe *elf.File, name string) *elf.Symbol { +func getSymbol(image *Image, logger logflags.Logger, exe *elf.File, name string) *elf.Symbol { symbols, err := exe.Symbols() if err != nil { image.setLoadError(logger, "could not parse ELF symbols: %v", err) @@ -2069,11 +2068,7 @@ func (bi *BinaryInfo) loadDebugInfoMaps(image *Image, debugInfoBytes, debugLineB if hasLineInfo && lineInfoOffset >= 0 && lineInfoOffset < int64(len(debugLineBytes)) { var logfn func(string, ...interface{}) if logflags.DebugLineErrors() { - logger := logrus.New().WithFields(logrus.Fields{"layer": "dwarf-line"}) - logger.Logger.Level = logrus.DebugLevel - logfn = func(fmt string, args ...interface{}) { - logger.Printf(fmt, args) - } + logfn = logflags.DebugLineLogger().Printf } cu.lineInfo = line.Parse(compdir, bytes.NewBuffer(debugLineBytes[lineInfoOffset:]), image.debugLineStr, logfn, image.StaticBase, bi.GOOS == "windows", bi.Arch.PtrSize()) } diff --git a/pkg/proc/gdbserial/gdbserver_conn.go b/pkg/proc/gdbserial/gdbserver_conn.go index 0449ed14..f6bde758 100644 --- a/pkg/proc/gdbserial/gdbserver_conn.go +++ b/pkg/proc/gdbserial/gdbserver_conn.go @@ -17,7 +17,6 @@ import ( "github.com/go-delve/delve/pkg/logflags" "github.com/go-delve/delve/pkg/proc" - "github.com/sirupsen/logrus" ) type gdbConn struct { @@ -49,7 +48,7 @@ type gdbConn struct { useXcmd bool // forces writeMemory to use the 'X' command - log *logrus.Entry + log logflags.Logger } var ErrTooManyAttempts = errors.New("too many transmit attempts") diff --git a/service/dap/server.go b/service/dap/server.go index d0df77b9..e505bab7 100644 --- a/service/dap/server.go +++ b/service/dap/server.go @@ -43,8 +43,6 @@ import ( "github.com/go-delve/delve/service/debugger" "github.com/go-delve/delve/service/internal/sameuser" "github.com/google/go-dap" - - "github.com/sirupsen/logrus" ) // Server implements a DAP server that can accept a single client for @@ -167,7 +165,7 @@ type Config struct { *service.Config // log is used for structured logging. - log *logrus.Entry + log logflags.Logger // StopTriggered is closed when the server is Stop()-ed. // Can be used to safeguard against duplicate shutdown sequences. StopTriggered chan struct{} diff --git a/service/debugger/debugger.go b/service/debugger/debugger.go index 746d1674..66dbf285 100644 --- a/service/debugger/debugger.go +++ b/service/debugger/debugger.go @@ -31,7 +31,6 @@ import ( "github.com/go-delve/delve/pkg/proc/gdbserial" "github.com/go-delve/delve/pkg/proc/native" "github.com/go-delve/delve/service/api" - "github.com/sirupsen/logrus" ) var ( @@ -70,7 +69,7 @@ type Debugger struct { targetMutex sync.Mutex target *proc.TargetGroup - log *logrus.Entry + log logflags.Logger running bool runningMutex sync.Mutex diff --git a/service/rpccommon/server.go b/service/rpccommon/server.go index a6889417..d7916cb5 100644 --- a/service/rpccommon/server.go +++ b/service/rpccommon/server.go @@ -25,7 +25,6 @@ import ( "github.com/go-delve/delve/service/internal/sameuser" "github.com/go-delve/delve/service/rpc1" "github.com/go-delve/delve/service/rpc2" - "github.com/sirupsen/logrus" ) // ServerImpl implements a JSON-RPC server that can switch between two @@ -45,7 +44,7 @@ type ServerImpl struct { s2 *rpc2.RPCServer // maps of served methods, one for each supported API. methodMaps []map[string]*methodType - log *logrus.Entry + log logflags.Logger } type RPCCallback struct { @@ -216,7 +215,7 @@ func isExportedOrBuiltinType(t reflect.Type) bool { // // func (rcvr ReceiverType) Method(in InputType, out *ReplyType) error // func (rcvr ReceiverType) Method(in InputType, cb service.RPCCallback) -func suitableMethods(rcvr interface{}, methods map[string]*methodType, log *logrus.Entry) { +func suitableMethods(rcvr interface{}, methods map[string]*methodType, log logflags.Logger) { typ := reflect.TypeOf(rcvr) rcvrv := reflect.ValueOf(rcvr) sname := reflect.Indirect(rcvrv).Type().Name()