From bd92941a21c63d5a43cdb44abd593f557e1130ae Mon Sep 17 00:00:00 2001 From: sudeep Date: Thu, 9 Mar 2023 14:05:52 +0530 Subject: [PATCH 1/3] use lumberjack logger for file logging (with rotation) move DirectoryFlag (cmd/utils -> internal/debug), where it can be used for the new log location option use command line flag to set lumberjack folder location moving the directory customflag to internal/flags fix log folder creation logic reverts deletion of interface implementation assertions adds category to logLocationFlag revert flags.go file rename; respond to other PR comments adds other flags related to logger file rotation --- .gitignore | 1 + go.mod | 1 + go.sum | 3 ++ internal/debug/flags.go | 88 ++++++++++++++++++++++++++++++++++++----- 4 files changed, 83 insertions(+), 10 deletions(-) diff --git a/.gitignore b/.gitignore index 1ee8b83022ef..e24e1d167709 100644 --- a/.gitignore +++ b/.gitignore @@ -47,3 +47,4 @@ profile.cov /dashboard/assets/package-lock.json **/yarn-error.log +logs/ \ No newline at end of file diff --git a/go.mod b/go.mod index 95c80c6fd8b1..1bf77ef7ef3d 100644 --- a/go.mod +++ b/go.mod @@ -65,6 +65,7 @@ require ( golang.org/x/text v0.8.0 golang.org/x/time v0.0.0-20220922220347-f3bd1da661af golang.org/x/tools v0.7.0 + gopkg.in/natefinch/lumberjack.v2 v2.0.0 gopkg.in/natefinch/npipe.v2 v2.0.0-20160621034901-c1b8fa8bdcce ) diff --git a/go.sum b/go.sum index 4c134a277221..0c230148a446 100644 --- a/go.sum +++ b/go.sum @@ -7,6 +7,7 @@ github.com/Azure/azure-sdk-for-go/sdk/internal v0.8.3/go.mod h1:KLF4gFr6DcKFZwSu github.com/Azure/azure-sdk-for-go/sdk/storage/azblob v0.3.0 h1:Px2UA+2RvSSvv+RvJNuUB6n7rs5Wsel4dXLe90Um2n4= github.com/Azure/azure-sdk-for-go/sdk/storage/azblob v0.3.0/go.mod h1:tPaiy8S5bQ+S5sOiDlINkp7+Ef339+Nz5L5XO+cnOHo= github.com/BurntSushi/toml v0.3.1/go.mod h1:xHWCNGjB5oqiDr8zfno3MHue2Ht5sIBksp03qcyfWMU= +github.com/BurntSushi/toml v1.2.0 h1:Rt8g24XnyGTyglgET/PRUNlrUeu9F5L+7FilkXfZgs0= github.com/CloudyKit/fastprinter v0.0.0-20200109182630-33d98a066a53/go.mod h1:+3IMCy2vIlbG1XG/0ggNQv0SvxCAIpPM5b1nCz56Xno= github.com/CloudyKit/jet/v3 v3.0.0/go.mod h1:HKQPgSJmdK8hdoAbKUUWajkHyHo4RaU5rMdUywE7VMo= github.com/DataDog/zstd v1.5.2 h1:vUG4lAyuPCXO0TLbXvPv7EB7cNK1QV/luu55UHLrrn8= @@ -616,6 +617,8 @@ gopkg.in/go-playground/assert.v1 v1.2.1/go.mod h1:9RXL0bg/zibRAgZUYszZSwO/z8Y/a8 gopkg.in/go-playground/validator.v8 v8.18.2/go.mod h1:RX2a/7Ha8BgOhfk7j780h4/u/RRjR0eouCJSH80/M2Y= gopkg.in/ini.v1 v1.51.1/go.mod h1:pNLf8WUiyNEtQjuu5G5vTm06TEv9tsIgeAvK8hOrP4k= gopkg.in/mgo.v2 v2.0.0-20180705113604-9856a29383ce/go.mod h1:yeKp02qBN3iKW1OzL3MGk2IdtZzaj7SFntXj72NppTA= +gopkg.in/natefinch/lumberjack.v2 v2.0.0 h1:1Lc07Kr7qY4U2YPouBjpCLxpiyxIVoxqXgkXLknAOE8= +gopkg.in/natefinch/lumberjack.v2 v2.0.0/go.mod h1:l0ndWWf7gzL7RNwBG7wST/UCcT4T24xpD6X8LsfU/+k= gopkg.in/natefinch/npipe.v2 v2.0.0-20160621034901-c1b8fa8bdcce h1:+JknDZhAj8YMt7GC73Ei8pv4MzjDUNPHgQWJdtMAaDU= gopkg.in/natefinch/npipe.v2 v2.0.0-20160621034901-c1b8fa8bdcce/go.mod h1:5AcXVHNjg+BDxry382+8OKon8SEWiKktQR07RKPsv1c= gopkg.in/tomb.v1 v1.0.0-20141024135613-dd632973f1e7 h1:uRGJdciOHaEIrze2W8Q3AKkepLTh2hOroT7a+7czfdQ= diff --git a/internal/debug/flags.go b/internal/debug/flags.go index 068817d532a9..b77184551826 100644 --- a/internal/debug/flags.go +++ b/internal/debug/flags.go @@ -20,8 +20,10 @@ import ( "fmt" "io" "net/http" - _ "net/http/pprof" // nolint: gosec + _ "net/http/pprof" "os" + "path" + "path/filepath" "runtime" "github.com/ethereum/go-ethereum/internal/flags" @@ -32,6 +34,7 @@ import ( "github.com/mattn/go-colorable" "github.com/mattn/go-isatty" "github.com/urfave/cli/v2" + "gopkg.in/natefinch/lumberjack.v2" ) var Memsize memsizeui.Handler @@ -76,6 +79,36 @@ var ( Usage: "Prepends log messages with call-site location (file and line number)", Category: flags.LoggingCategory, } + logLocationFlag = &flags.DirectoryFlag{ + Name: "log.folder", + Usage: "Location where the log files should be placed", + Value: flags.DirectoryString("./logs/"), + Category: flags.LoggingCategory, + } + logMaxSizeMBsFlag = &cli.IntFlag{ + Name: "log.maxsize", + Usage: "Maximum size in MBs of a single log file", + Value: 100, + Category: flags.LoggingCategory, + } + logMaxBackupsFlag = &cli.IntFlag{ + Name: "log.maxbackups", + Usage: "Maximum number of log files to retain", + Value: 10, + Category: flags.LoggingCategory, + } + logMaxAgeFlag = &cli.IntFlag{ + Name: "log.maxage", + Usage: "Maximum number of days to retain a log file", + Value: 30, + Category: flags.LoggingCategory, + } + logCompressFlag = &cli.BoolFlag{ + Name: "log.compress", + Usage: "Compress the log files", + Value: false, + Category: flags.LoggingCategory, + } pprofFlag = &cli.BoolFlag{ Name: "pprof", Usage: "Enable the pprof HTTP server", @@ -125,6 +158,11 @@ var Flags = []cli.Flag{ logFileFlag, backtraceAtFlag, debugFlag, + logLocationFlag, + logMaxSizeMBsFlag, + logMaxBackupsFlag, + logMaxAgeFlag, + logCompressFlag, pprofFlag, pprofAddrFlag, pprofPortFlag, @@ -148,24 +186,24 @@ func init() { // Setup initializes profiling and logging based on the CLI flags. // It should be called as early as possible in the program. func Setup(ctx *cli.Context) error { - logFile := ctx.String(logFileFlag.Name) - useColor := logFile == "" && os.Getenv("TERM") != "dumb" && (isatty.IsTerminal(os.Stderr.Fd()) || isatty.IsCygwinTerminal(os.Stderr.Fd())) - + output := io.Writer(os.Stderr) var logfmt log.Format + ttyHasColor := (isatty.IsTerminal(os.Stderr.Fd()) || isatty.IsCygwinTerminal(os.Stderr.Fd())) && os.Getenv("TERM") != "dumb" + switch ctx.String(logFormatFlag.Name) { case "json": logfmt = log.JSONFormat() case "logfmt": logfmt = log.LogfmtFormat() case "terminal": - logfmt = log.TerminalFormat(useColor) + logfmt = log.TerminalFormat(ttyHasColor) case "": // Retain backwards compatibility with `--log.json` flag if `--log.format` not set if ctx.Bool(logjsonFlag.Name) { defer log.Warn("The flag '--log.json' is deprecated, please use '--log.format=json' instead") logfmt = log.JSONFormat() } else { - logfmt = log.TerminalFormat(useColor) + logfmt = log.TerminalFormat(ttyHasColor) } default: // Unknown log format specified @@ -179,13 +217,29 @@ func Setup(ctx *cli.Context) error { return err } } else { - output := io.Writer(os.Stderr) - if useColor { + if ttyHasColor { output = colorable.NewColorableStderr() } - logOutputStream = log.StreamHandler(output, logfmt) + logfmt = log.TerminalFormat(ttyHasColor) } - glogger.SetHandler(logOutputStream) + + stdHandler := log.StreamHandler(output, logfmt) + ostream := stdHandler + if folder := ctx.String(logLocationFlag.Name); folder != "" { + if err := validateLogLocation(folder); err != nil { + return fmt.Errorf("failed to initiatilize file logger: %v", err) + } + lumberjackHandler := log.StreamHandler(&lumberjack.Logger{ + Filename: path.Join(folder, "log.log"), + MaxSize: ctx.Int(logMaxSizeMBsFlag.Name), + MaxBackups: ctx.Int(logMaxBackupsFlag.Name), + MaxAge: ctx.Int(logMaxAgeFlag.Name), + Compress: ctx.Bool(logCompressFlag.Name), + }, logfmt) + ostream = log.MultiHandler(lumberjackHandler, stdHandler) + } + + glogger.SetHandler(ostream) // logging verbosity := ctx.Int(verbosityFlag.Name) @@ -263,3 +317,17 @@ func Exit() { closer.Close() } } + +func validateLogLocation(path string) error { + if err := os.MkdirAll(path, os.ModePerm); err != nil { + return fmt.Errorf("error creating the directory: %w", err) + } + // Check if the path is writable by trying to create a temporary file + tmp := filepath.Join(path, "tmp") + if f, err := os.Create(tmp); err != nil { + return err + } else { + f.Close() + } + return os.Remove(tmp) +} From 87aae7d72f924e97649683e8a7f409073870f6e6 Mon Sep 17 00:00:00 2001 From: Martin Holst Swende Date: Thu, 30 Mar 2023 11:49:53 +0200 Subject: [PATCH 2/3] cmd/geth: make log rotation option explicit, combine log.file flag internal: post-rebase fixups internal: fix cli-param redeclaration sq --- internal/debug/flags.go | 97 +++++++++++++++++++++-------------------- 1 file changed, 50 insertions(+), 47 deletions(-) diff --git a/internal/debug/flags.go b/internal/debug/flags.go index b77184551826..7e008cd6fa3f 100644 --- a/internal/debug/flags.go +++ b/internal/debug/flags.go @@ -22,7 +22,6 @@ import ( "net/http" _ "net/http/pprof" "os" - "path" "path/filepath" "runtime" @@ -79,11 +78,9 @@ var ( Usage: "Prepends log messages with call-site location (file and line number)", Category: flags.LoggingCategory, } - logLocationFlag = &flags.DirectoryFlag{ - Name: "log.folder", - Usage: "Location where the log files should be placed", - Value: flags.DirectoryString("./logs/"), - Category: flags.LoggingCategory, + logRotateFlag = &cli.BoolFlag{ + Name: "log.rotate", + Usage: "Enables log file rotation", } logMaxSizeMBsFlag = &cli.IntFlag{ Name: "log.maxsize", @@ -153,12 +150,12 @@ var ( var Flags = []cli.Flag{ verbosityFlag, vmoduleFlag, + backtraceAtFlag, + debugFlag, logjsonFlag, logFormatFlag, logFileFlag, - backtraceAtFlag, - debugFlag, - logLocationFlag, + logRotateFlag, logMaxSizeMBsFlag, logMaxBackupsFlag, logMaxAgeFlag, @@ -186,59 +183,65 @@ func init() { // Setup initializes profiling and logging based on the CLI flags. // It should be called as early as possible in the program. func Setup(ctx *cli.Context) error { - output := io.Writer(os.Stderr) - var logfmt log.Format - ttyHasColor := (isatty.IsTerminal(os.Stderr.Fd()) || isatty.IsCygwinTerminal(os.Stderr.Fd())) && os.Getenv("TERM") != "dumb" - - switch ctx.String(logFormatFlag.Name) { - case "json": + var ( + logfmt log.Format + output = io.Writer(os.Stderr) + logFmtFlag = ctx.String(logFormatFlag.Name) + ) + switch { + case ctx.Bool(logjsonFlag.Name): + // Retain backwards compatibility with `--log.json` flag if `--log.format` not set + defer log.Warn("The flag '--log.json' is deprecated, please use '--log.format=json' instead") + logfmt = log.JSONFormat() + case logFmtFlag == "json": logfmt = log.JSONFormat() - case "logfmt": + case logFmtFlag == "logfmt": logfmt = log.LogfmtFormat() - case "terminal": - logfmt = log.TerminalFormat(ttyHasColor) - case "": - // Retain backwards compatibility with `--log.json` flag if `--log.format` not set - if ctx.Bool(logjsonFlag.Name) { - defer log.Warn("The flag '--log.json' is deprecated, please use '--log.format=json' instead") - logfmt = log.JSONFormat() - } else { - logfmt = log.TerminalFormat(ttyHasColor) + case logFmtFlag == "", logFmtFlag == "terminal": + useColor := (isatty.IsTerminal(os.Stderr.Fd()) || isatty.IsCygwinTerminal(os.Stderr.Fd())) && os.Getenv("TERM") != "dumb" + if useColor { + output = colorable.NewColorableStderr() } + logfmt = log.TerminalFormat(useColor) default: // Unknown log format specified return fmt.Errorf("unknown log format: %v", ctx.String(logFormatFlag.Name)) } - - if logFile != "" { - var err error - logOutputStream, err = log.FileHandler(logFile, logfmt) - if err != nil { - return err - } - } else { - if ttyHasColor { - output = colorable.NewColorableStderr() + var ( + stdHandler = log.StreamHandler(output, logfmt) + ostream = stdHandler + logFile = ctx.String(logFileFlag.Name) + rotation = ctx.Bool(logRotateFlag.Name) + ) + if len(logFile) > 0 { + if err := validateLogLocation(filepath.Dir(logFile)); err != nil { + return fmt.Errorf("failed to initiatilize file logger: %v", err) } - logfmt = log.TerminalFormat(ttyHasColor) } - - stdHandler := log.StreamHandler(output, logfmt) - ostream := stdHandler - if folder := ctx.String(logLocationFlag.Name); folder != "" { - if err := validateLogLocation(folder); err != nil { - return fmt.Errorf("failed to initiatilize file logger: %v", err) + if rotation { + // Lumberjack uses -lumberjack.log in is.TempDir() if empty. + // so typically /tmp/geth-lumberjack.log on linux + if len(logFile) > 0 { + log.Info("Rotating file logging configured", "location", logFile) + } else { + log.Info("Rotating file logging configured", "location", + filepath.Join(os.TempDir(), "geth-lumberjack.log")) } - lumberjackHandler := log.StreamHandler(&lumberjack.Logger{ - Filename: path.Join(folder, "log.log"), + ostream = log.MultiHandler(log.StreamHandler(&lumberjack.Logger{ + Filename: logFile, MaxSize: ctx.Int(logMaxSizeMBsFlag.Name), MaxBackups: ctx.Int(logMaxBackupsFlag.Name), MaxAge: ctx.Int(logMaxAgeFlag.Name), Compress: ctx.Bool(logCompressFlag.Name), - }, logfmt) - ostream = log.MultiHandler(lumberjackHandler, stdHandler) + }, logfmt), stdHandler) + } else if logFile != "" { + if logOutputStream, err := log.FileHandler(logFile, logfmt); err != nil { + return err + } else { + ostream = log.MultiHandler(logOutputStream, stdHandler) + log.Info("File logging configured", "location", logFile) + } } - glogger.SetHandler(ostream) // logging From 9bdb091c3a18d7c4160cb197c9908842eb4b5366 Mon Sep 17 00:00:00 2001 From: Martin Holst Swende Date: Mon, 3 Apr 2023 09:24:06 +0200 Subject: [PATCH 3/3] internal/debug: print log-info on non-default options --- internal/debug/flags.go | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/internal/debug/flags.go b/internal/debug/flags.go index 7e008cd6fa3f..3c8b569b4f38 100644 --- a/internal/debug/flags.go +++ b/internal/debug/flags.go @@ -218,14 +218,19 @@ func Setup(ctx *cli.Context) error { return fmt.Errorf("failed to initiatilize file logger: %v", err) } } + context := []interface{}{"rotate", rotation} + if len(logFmtFlag) > 0 { + context = append(context, "format", logFmtFlag) + } else { + context = append(context, "format", "terminal") + } if rotation { // Lumberjack uses -lumberjack.log in is.TempDir() if empty. // so typically /tmp/geth-lumberjack.log on linux if len(logFile) > 0 { - log.Info("Rotating file logging configured", "location", logFile) + context = append(context, "location", logFile) } else { - log.Info("Rotating file logging configured", "location", - filepath.Join(os.TempDir(), "geth-lumberjack.log")) + context = append(context, "location", filepath.Join(os.TempDir(), "geth-lumberjack.log")) } ostream = log.MultiHandler(log.StreamHandler(&lumberjack.Logger{ Filename: logFile, @@ -239,7 +244,7 @@ func Setup(ctx *cli.Context) error { return err } else { ostream = log.MultiHandler(logOutputStream, stdHandler) - log.Info("File logging configured", "location", logFile) + context = append(context, "location", logFile) } } glogger.SetHandler(ostream) @@ -293,6 +298,9 @@ func Setup(ctx *cli.Context) error { // It cannot be imported because it will cause a cyclical dependency. StartPProf(address, !ctx.IsSet("metrics.addr")) } + if len(logFile) > 0 || rotation { + log.Info("Logging configured", context...) + } return nil }