Merge pull request #9233 from ellemouton/moveLoggingCompressorOption

build+config: move file logger specific options to `logging.file`

Author: guggero

build/config.go

@@ -1,34 +1,42 @@
-//go:build !dev
-// +build !dev
-
 package build
 
-import "github.com/btcsuite/btclog/v2"
+import (
+	"fmt"
+
+	"github.com/btcsuite/btclog/v2"
+)
 
 const (
 	callSiteOff   = "off"
 	callSiteShort = "short"
 	callSiteLong  = "long"
+
+	defaultLogCompressor = Gzip
+
+	// DefaultMaxLogFiles is the default maximum number of log files to
+	// keep.
+	DefaultMaxLogFiles = 10
+
+	// DefaultMaxLogFileSize is the default maximum log file size in MB.
+	DefaultMaxLogFileSize = 20
 )
 
 // LogConfig holds logging configuration options.
 //
 //nolint:lll
 type LogConfig struct {
-	Console *LoggerConfig `group:"console" namespace:"console" description:"The logger writing to stdout and stderr."`
-	File    *LoggerConfig `group:"file" namespace:"file" description:"The logger writing to LND's standard log file."`
+	Console *consoleLoggerCfg `group:"console" namespace:"console" description:"The logger writing to stdout and stderr."`
+	File    *FileLoggerConfig `group:"file" namespace:"file" description:"The logger writing to LND's standard log file."`
 }
 
-// DefaultLogConfig returns the default logging config options.
-func DefaultLogConfig() *LogConfig {
-	return &LogConfig{
-		Console: &LoggerConfig{
-			CallSite: callSiteOff,
-		},
-		File: &LoggerConfig{
-			CallSite: callSiteOff,
-		},
+// Validate validates the LogConfig struct values.
+func (c *LogConfig) Validate() error {
+	if !SupportedLogCompressor(c.File.Compressor) {
+		return fmt.Errorf("invalid log compressor: %v",
+			c.File.Compressor)
 	}
+
+	return nil
 }
 
 // LoggerConfig holds options for a particular logger.
@@ -40,6 +48,21 @@ type LoggerConfig struct {
 	CallSite     string `long:"call-site" description:"Include the call-site of each log line." choice:"off" choice:"short" choice:"long"`
 }
 
+// DefaultLogConfig returns the default logging config options.
+func DefaultLogConfig() *LogConfig {
+	return &LogConfig{
+		Console: defaultConsoleLoggerCfg(),
+		File: &FileLoggerConfig{
+			Compressor:     defaultLogCompressor,
+			MaxLogFiles:    DefaultMaxLogFiles,
+			MaxLogFileSize: DefaultMaxLogFileSize,
+			LoggerConfig: LoggerConfig{
+				CallSite: callSiteOff,
+			},
+		},
+	}
+}
+
 // HandlerOptions returns the set of btclog.HandlerOptions that the state of the
 // config struct translates to.
 func (cfg *LoggerConfig) HandlerOptions() []btclog.HandlerOption {
@@ -50,6 +73,7 @@ func (cfg *LoggerConfig) HandlerOptions() []btclog.HandlerOption {
 		// to 7 here.
 		btclog.WithCallSiteSkipDepth(7),
 	}
+
 	if cfg.NoTimestamps {
 		opts = append(opts, btclog.WithNoTimestamp())
 	}
@@ -63,3 +87,13 @@ func (cfg *LoggerConfig) HandlerOptions() []btclog.HandlerOption {
 
 	return opts
 }
+
+// FileLoggerConfig extends LoggerConfig with specific log file options.
+//
+//nolint:lll
+type FileLoggerConfig struct {
+	LoggerConfig
+	Compressor     string `long:"compressor" description:"Compression algorithm to use when rotating logs." choice:"gzip" choice:"zstd"`
+	MaxLogFiles    int    `long:"max-files" description:"Maximum logfiles to keep (0 for no rotation)"`
+	MaxLogFileSize int    `long:"max-file-size" description:"Maximum logfile size in MB"`
+}

build/config_dev.go

@@ -17,68 +17,8 @@ const (
 	faintSeq = "2"
 	esc      = '\x1b'
 	csi      = string(esc) + "["
-
-	callSiteOff   = "off"
-	callSiteShort = "short"
-	callSiteLong  = "long"
 )
 
-// LogConfig holds logging configuration options.
-//
-//nolint:lll
-type LogConfig struct {
-	Console *consoleLoggerCfg `group:"console" namespace:"console" description:"The logger writing to stdout and stderr."`
-	File    *LoggerConfig     `group:"file" namespace:"file" description:"The logger writing to LND's standard log file."`
-}
-
-// DefaultLogConfig returns the default logging config options.
-func DefaultLogConfig() *LogConfig {
-	return &LogConfig{
-		Console: &consoleLoggerCfg{
-			LoggerConfig: LoggerConfig{
-				CallSite: callSiteShort,
-			},
-		},
-		File: &LoggerConfig{
-			CallSite: callSiteOff,
-		},
-	}
-}
-
-// LoggerConfig holds options for a particular logger.
-//
-//nolint:lll
-type LoggerConfig struct {
-	Disable      bool   `long:"disable" description:"Disable this logger."`
-	NoTimestamps bool   `long:"no-timestamps" description:"Omit timestamps from log lines."`
-	CallSite     string `long:"call-site" description:"Include the call-site of each log line." choice:"off" choice:"short" choice:"long"`
-}
-
-// HandlerOptions returns the set of btclog.HandlerOptions that the state of the
-// config struct translates to.
-func (cfg *LoggerConfig) HandlerOptions() []btclog.HandlerOption {
-	opts := []btclog.HandlerOption{
-		// The default skip depth used by the logging library is 6 but
-		// since we wrap the logging handlers with another level of
-		// abstraction with the handlerSet, we increase the skip depth
-		// to 7 here.
-		btclog.WithCallSiteSkipDepth(7),
-	}
-
-	if cfg.NoTimestamps {
-		opts = append(opts, btclog.WithNoTimestamp())
-	}
-
-	switch cfg.CallSite {
-	case callSiteShort:
-		opts = append(opts, btclog.WithCallerFlags(btclog.Lshortfile))
-	case callSiteLong:
-		opts = append(opts, btclog.WithCallerFlags(btclog.Llongfile))
-	}
-
-	return opts
-}
-
 // consoleLoggerCfg extends the LoggerConfig struct by adding a Color option
 // which is only available for a console logger.
 //
@@ -88,6 +28,16 @@ type consoleLoggerCfg struct {
 	Style bool `long:"style" description:"If set, the output will be styled with color and fonts"`
 }
 
+// defaultConsoleLoggerCfg returns the default consoleLoggerCfg for the dev
+// console logger.
+func defaultConsoleLoggerCfg() *consoleLoggerCfg {
+	return &consoleLoggerCfg{
+		LoggerConfig: LoggerConfig{
+			CallSite: callSiteShort,
+		},
+	}
+}
+
 // HandlerOptions returns the set of btclog.HandlerOptions that the state of the
 // config struct translates to.
 func (cfg *consoleLoggerCfg) HandlerOptions() []btclog.HandlerOption {

build/config_prod.go

@@ -0,0 +1,22 @@
+//go:build !dev
+// +build !dev
+
+package build
+
+// consoleLoggerCfg embeds the LoggerConfig struct along with any extensions
+// specific to a production deployment.
+//
+//nolint:lll
+type consoleLoggerCfg struct {
+	LoggerConfig
+}
+
+// defaultConsoleLoggerCfg returns the default consoleLoggerCfg for the prod
+// console logger.
+func defaultConsoleLoggerCfg() *consoleLoggerCfg {
+	return &consoleLoggerCfg{
+		LoggerConfig: LoggerConfig{
+			CallSite: callSiteOff,
+		},
+	}
+}

build/log.go

@@ -52,9 +52,9 @@ var logCompressors = map[string]string{
 	Zstd: "zst",
 }
 
-// SuportedLogCompressor returns whether or not logCompressor is a supported
+// SupportedLogCompressor returns whether or not logCompressor is a supported
 // compression algorithm for log files.
-func SuportedLogCompressor(logCompressor string) bool {
+func SupportedLogCompressor(logCompressor string) bool {
 	_, ok := logCompressors[logCompressor]
 
 	return ok

build/logrotator.go

@@ -31,8 +31,8 @@ func NewRotatingLogWriter() *RotatingLogWriter {
 // InitLogRotator initializes the log file rotator to write logs to logFile and
 // create roll files in the same directory. It should be called as early on
 // startup and possible and must be closed on shutdown by calling `Close`.
-func (r *RotatingLogWriter) InitLogRotator(logFile, logCompressor string,
-	maxLogFileSize int, maxLogFiles int) error {
+func (r *RotatingLogWriter) InitLogRotator(cfg *FileLoggerConfig,
+	logFile string) error {
 
 	logDir, _ := filepath.Split(logFile)
 	err := os.MkdirAll(logDir, 0700)
@@ -41,19 +41,19 @@ func (r *RotatingLogWriter) InitLogRotator(logFile, logCompressor string,
 	}
 
 	r.rotator, err = rotator.New(
-		logFile, int64(maxLogFileSize*1024), false, maxLogFiles,
+		logFile, int64(cfg.MaxLogFileSize*1024), false, cfg.MaxLogFiles,
 	)
 	if err != nil {
 		return fmt.Errorf("failed to create file rotator: %w", err)
 	}
 
 	// Reject unknown compressors.
-	if !SuportedLogCompressor(logCompressor) {
-		return fmt.Errorf("unknown log compressor: %v", logCompressor)
+	if !SupportedLogCompressor(cfg.Compressor) {
+		return fmt.Errorf("unknown log compressor: %v", cfg.Compressor)
 	}
 
 	var c rotator.Compressor
-	switch logCompressor {
+	switch cfg.Compressor {
 	case Gzip:
 		c = gzip.NewWriter(nil)
 
@@ -66,7 +66,7 @@ func (r *RotatingLogWriter) InitLogRotator(logFile, logCompressor string,
 	}
 
 	// Apply the compressor and its file suffix to the log rotator.
-	r.rotator.SetCompressor(c, logCompressors[logCompressor])
+	r.rotator.SetCompressor(c, logCompressors[cfg.Compressor])
 
 	// Run rotator as a goroutine now but make sure we catch any errors
 	// that happen in case something with the rotation goes wrong during

config.go

@@ -59,7 +59,6 @@ const (
 	defaultLogLevel           = "info"
 	defaultLogDirname         = "logs"
 	defaultLogFilename        = "lnd.log"
-	defaultLogCompressor      = build.Gzip
 	defaultRPCPort            = 10009
 	defaultRESTPort           = 8080
 	defaultPeerPort           = 9735
@@ -72,8 +71,6 @@ const (
 	defaultChanEnableTimeout             = 19 * time.Minute
 	defaultChanDisableTimeout            = 20 * time.Minute
 	defaultHeightHintCacheQueryDisable   = false
-	defaultMaxLogFiles                   = 3
-	defaultMaxLogFileSize                = 10
 	defaultMinBackoff                    = time.Second
 	defaultMaxBackoff                    = time.Hour
 	defaultLetsEncryptDirname            = "letsencrypt"
@@ -316,9 +313,8 @@ type Config struct {
 	ReadMacPath     string        `long:"readonlymacaroonpath" description:"Path to write the read-only macaroon for lnd's RPC and REST services if it doesn't exist"`
 	InvoiceMacPath  string        `long:"invoicemacaroonpath" description:"Path to the invoice-only macaroon for lnd's RPC and REST services if it doesn't exist"`
 	LogDir          string        `long:"logdir" description:"Directory to log output."`
-	LogCompressor   string        `long:"logcompressor" description:"Compression algorithm to use when rotating logs." choice:"gzip" choice:"zstd"`
-	MaxLogFiles     int           `long:"maxlogfiles" description:"Maximum logfiles to keep (0 for no rotation)"`
-	MaxLogFileSize  int           `long:"maxlogfilesize" description:"Maximum logfile size in MB"`
+	MaxLogFiles     int           `long:"maxlogfiles" description:"Maximum logfiles to keep (0 for no rotation). DEPRECATED: use --logging.file.max-files instead" hidden:"true"`
+	MaxLogFileSize  int           `long:"maxlogfilesize" description:"Maximum logfile size in MB. DEPRECATED: use --logging.file.max-file-size instead" hidden:"true"`
 	AcceptorTimeout time.Duration `long:"acceptortimeout" description:"Time after which an RPCAcceptor will time out and return false if it hasn't yet received a response"`
 
 	LetsEncryptDir    string `long:"letsencryptdir" description:"The directory to store Let's Encrypt certificates within"`
@@ -564,9 +560,8 @@ func DefaultConfig() Config {
 		LetsEncryptDir:    defaultLetsEncryptDir,
 		LetsEncryptListen: defaultLetsEncryptListen,
 		LogDir:            defaultLogDir,
-		LogCompressor:     defaultLogCompressor,
-		MaxLogFiles:       defaultMaxLogFiles,
-		MaxLogFileSize:    defaultMaxLogFileSize,
+		MaxLogFiles:       build.DefaultMaxLogFiles,
+		MaxLogFileSize:    build.DefaultMaxLogFileSize,
 		AcceptorTimeout:   defaultAcceptorTimeout,
 		WSPingInterval:    lnrpc.DefaultPingInterval,
 		WSPongWait:        lnrpc.DefaultPongWait,
@@ -1403,9 +1398,8 @@ func ValidateConfig(cfg Config, interceptor signal.Interceptor, fileParser,
 		lncfg.NormalizeNetwork(cfg.ActiveNetParams.Name),
 	)
 
-	if !build.SuportedLogCompressor(cfg.LogCompressor) {
-		return nil, mkErr("invalid log compressor: %v",
-			cfg.LogCompressor)
+	if err := cfg.LogConfig.Validate(); err != nil {
+		return nil, mkErr("error validating logging config: %w", err)
 	}
 
 	cfg.SubLogMgr = build.NewSubLoggerManager(build.NewDefaultLogHandlers(
@@ -1421,9 +1415,31 @@ func ValidateConfig(cfg Config, interceptor signal.Interceptor, fileParser,
 			cfg.SubLogMgr.SupportedSubsystems())
 		os.Exit(0)
 	}
+
+	if cfg.MaxLogFiles != build.DefaultMaxLogFiles {
+		if cfg.LogConfig.File.MaxLogFiles !=
+			build.DefaultMaxLogFiles {
+
+			return nil, mkErr("cannot set both maxlogfiles and "+
+				"logging.file.max-files", err)
+		}
+
+		cfg.LogConfig.File.MaxLogFiles = cfg.MaxLogFiles
+	}
+	if cfg.MaxLogFileSize != build.DefaultMaxLogFileSize {
+		if cfg.LogConfig.File.MaxLogFileSize !=
+			build.DefaultMaxLogFileSize {
+
+			return nil, mkErr("cannot set both maxlogfilesize and "+
+				"logging.file.max-file-size", err)
+		}
+
+		cfg.LogConfig.File.MaxLogFileSize = cfg.MaxLogFileSize
+	}
+
 	err = cfg.LogRotator.InitLogRotator(
+		cfg.LogConfig.File,
 		filepath.Join(cfg.LogDir, defaultLogFilename),
-		cfg.LogCompressor, cfg.MaxLogFileSize, cfg.MaxLogFiles,
 	)
 	if err != nil {
 		str := "log rotation setup failed: %v"

docs/release-notes/release-notes-0.19.0.md

@@ -57,7 +57,9 @@
 # Improvements
 ## Functional Updates
 
-* [Allow](https://github.com/lightningnetwork/lnd/pull/9017) the compression of logs during rotation with ZSTD via the `logcompressor` startup argument.
+* [Allow](https://github.com/lightningnetwork/lnd/pull/9017) the compression of 
+  logs during rotation with ZSTD via the `logging.file.compressor` startup 
+  argument.
 
 * The SCB file now [contains more data][https://github.com/lightningnetwork/lnd/pull/8183]
   that enable a last resort rescue for certain cases where the peer is no longer
@@ -88,6 +90,14 @@
   Finally, the new `--logging.console.style` option can be used under the `dev` 
   build tag to add styling to console logging.
 
+* [Add max files and max file size](https://github.com/lightningnetwork/lnd/pull/9233) 
+  options to the `logging` config namespace under new `--logging.file.max-files` 
+  and `--logging.files.max-file-size` options. The old options (`--maxlogfiles` 
+  and `--maxlogfilesize`) will still work but deprecation notices have been 
+  added and they will be removed in a future release. The defaults values for 
+  these options have also been increased from max 3 log files to 10 and from 
+  max 10 MB to 20 MB. 
+ 
 ## Breaking Changes
 ## Performance Improvements