backport: log-file-path setting does not work (#691)

cmd/tenderdash/commands/root.go

@@ -51,7 +51,7 @@ func RootCommand(conf *config.Config, logger log.Logger) *cobra.Command {
 			}
 			*conf = *pconf
 			config.EnsureRoot(conf.RootDir)
-			if err := log.OverrideWithNewLogger(logger, conf.LogFormat, conf.LogLevel); err != nil {
+			if err := log.OverrideWithNewLogger(logger, conf.LogFormat, conf.LogLevel, conf.LogFilePath); err != nil {
 				return err
 			}
 			if warning := pconf.DeprecatedFieldWarning(); warning != nil {

cmd/tenderdash/main.go

@@ -2,8 +2,6 @@ package main
 
 import (
 	"context"
-	"fmt"
-	"io"
 	"os"
 
 	"github.com/dashpay/tenderdash/cmd/tenderdash/commands"
@@ -23,11 +21,11 @@ func main() {
 		panic(err)
 	}
 
-	logger, stopFn, err := newLoggerFromConfig(conf)
+	logger, err := log.NewMultiLogger(conf.LogFormat, conf.LogLevel, conf.LogFilePath)
 	if err != nil {
 		panic(err)
 	}
-	defer stopFn()
+	defer logger.Close()
 
 	rcmd := commands.RootCommand(conf, logger)
 	rcmd.AddCommand(
@@ -64,33 +62,9 @@ func main() {
 	rcmd.AddCommand(commands.NewRunNodeCmd(nodeFunc, conf, logger))
 
 	if err := cli.RunWithTrace(ctx, rcmd); err != nil {
+		// os.Exit doesn't call defer functions, so we manually close the logger here
+		cancel()
+		_ = logger.Close()
 		os.Exit(2)
 	}
 }
-
-func newLoggerFromConfig(conf *config.Config) (log.Logger, func(), error) {
-	var (
-		writer    io.Writer = os.Stderr
-		closeFunc           = func() {}
-		err       error
-	)
-	if conf.LogFilePath != "" {
-		file, err := os.OpenFile(conf.LogFilePath, os.O_WRONLY|os.O_CREATE|os.O_APPEND, 0644)
-		if err != nil {
-			return nil, nil, fmt.Errorf("failed to create log writer: %w", err)
-		}
-		closeFunc = func() {
-			_ = file.Close()
-		}
-		writer = io.MultiWriter(writer, file)
-	}
-	writer, err = log.NewFormatter(conf.LogFormat, writer)
-	if err != nil {
-		return nil, nil, fmt.Errorf("failed to create log formatter: %w", err)
-	}
-	logger, err := log.NewLogger(conf.LogLevel, writer)
-	if err != nil {
-		return nil, nil, err
-	}
-	return logger, closeFunc, nil
-}

libs/log/default.go

@@ -7,13 +7,15 @@ import (
 	"strings"
 	"time"
 
+	"github.com/hashicorp/go-multierror"
 	"github.com/rs/zerolog"
 )
 
 var _ Logger = (*defaultLogger)(nil)
 
 type defaultLogger struct {
 	zerolog.Logger
+	closeFuncs []func() error
 }
 
 // NewDefaultLogger returns a default logger that can be used within Tendermint
@@ -25,29 +27,47 @@ type defaultLogger struct {
 // that in a generic interface, all logging methods accept a series of key/value
 // pair tuples, where the key must be a string.
 func NewDefaultLogger(format, level string) (Logger, error) {
-	var logWriter io.Writer
-	switch strings.ToLower(format) {
-	case LogFormatPlain, LogFormatText:
-		logWriter = zerolog.ConsoleWriter{
-			Out:        os.Stderr,
-			NoColor:    true,
-			TimeFormat: time.RFC3339Nano,
-			FormatLevel: func(i interface{}) string {
-				if ll, ok := i.(string); ok {
-					return strings.ToUpper(ll)
-				}
-				return "????"
-			},
-		}
-
-	case LogFormatJSON:
-		logWriter = os.Stderr
+	return NewMultiLogger(format, level, "")
+}
 
-	default:
-		return nil, fmt.Errorf("unsupported log format: %s", format)
+// NewMultiLogger creates a new logger that writes to os.Stderr and an additional log file if provided.
+// It takes in three parameters: format, level, and additionalLogPath.
+// The format parameter specifies the format of the log message.
+// The level parameter specifies the minimum log level to write.
+// The additionalLogPath parameter specifies the path to the additional log file.
+// If additionalLogPath is not empty, the logger writes to both os.Stderr and the additional log file.
+// The function returns a Logger interface and an error if any.
+//
+// See NewDefaultLogger for more details.
+func NewMultiLogger(format, level, additionalLogPath string) (Logger, error) {
+	var (
+		writer    io.Writer = os.Stderr
+		closeFunc           = func() error { return nil }
+		err       error
+	)
+	if additionalLogPath != "" {
+		file, err := os.OpenFile(additionalLogPath, os.O_WRONLY|os.O_CREATE|os.O_APPEND, 0644)
+		if err != nil {
+			return nil, fmt.Errorf("failed to create log writer: %w", err)
+		}
+		closeFunc = func() error {
+			return file.Close()
+		}
+		writer = io.MultiWriter(writer, file)
+	}
+	writer, err = NewFormatter(format, writer)
+	if err != nil {
+		_ = closeFunc()
+		return nil, fmt.Errorf("failed to create log formatter: %w", err)
+	}
+	logger, err := NewLogger(level, writer)
+	if err != nil {
+		_ = closeFunc()
+		return nil, err
 	}
+	logger.(*defaultLogger).closeFuncs = append(logger.(*defaultLogger).closeFuncs, closeFunc)
 
-	return NewLogger(level, logWriter)
+	return logger, nil
 }
 
 func NewLogger(level string, logWriter io.Writer) (Logger, error) {
@@ -59,7 +79,9 @@ func NewLogger(level string, logWriter io.Writer) (Logger, error) {
 	// make the writer thread-safe
 	logWriter = newSyncWriter(logWriter)
 
-	return &defaultLogger{Logger: zerolog.New(logWriter).Level(logLevel).With().Timestamp().Logger()}, nil
+	return &defaultLogger{
+		Logger: zerolog.New(logWriter).Level(logLevel).With().Timestamp().Logger(),
+	}, nil
 }
 
 func (l defaultLogger) Info(msg string, keyVals ...interface{}) {
@@ -81,26 +103,48 @@ func (l defaultLogger) Trace(msg string, keyVals ...interface{}) {
 func (l defaultLogger) With(keyVals ...interface{}) Logger {
 	return &defaultLogger{Logger: l.Logger.With().Fields(getLogFields(keyVals...)).Logger()}
 }
+func (l *defaultLogger) Close() (err error) {
+	if l == nil {
+		return nil
+	}
+	l.Debug("Closing logger")
+	for _, f := range l.closeFuncs {
+		if e := f(); e != nil {
+			err = multierror.Append(err, e)
+		}
+	}
+
+	l.closeFuncs = nil
+
+	return err
+}
 
 // OverrideWithNewLogger replaces an existing logger's internal with
 // a new logger, and makes it possible to reconfigure an existing
 // logger that has already been propagated to callers.
-func OverrideWithNewLogger(logger Logger, format, level string) error {
+func OverrideWithNewLogger(logger Logger, format, level, additionalLogFilePath string) error {
 	ol, ok := logger.(*defaultLogger)
 	if !ok {
 		return fmt.Errorf("logger %T cannot be overridden", logger)
 	}
 
-	newLogger, err := NewDefaultLogger(format, level)
+	newLogger, err := NewMultiLogger(format, level, additionalLogFilePath)
 	if err != nil {
 		return err
 	}
 	nl, ok := newLogger.(*defaultLogger)
 	if !ok {
+		newLogger.Close()
 		return fmt.Errorf("logger %T cannot be overridden by %T", logger, newLogger)
 	}
 
+	if err := ol.Close(); err != nil {
+		return err
+	}
+
 	ol.Logger = nl.Logger
+	ol.closeFuncs = nl.closeFuncs
+
 	return nil
 }
 

libs/log/logger.go

@@ -32,6 +32,8 @@ const (
 
 // Logger defines a generic logging interface compatible with Tendermint.
 type Logger interface {
+	io.Closer
+
 	Trace(msg string, keyVals ...interface{})
 	Debug(msg string, keyVals ...interface{})
 	Info(msg string, keyVals ...interface{})