From 01d8d085e81d79ba0fe16ce8773e91529afdb346 Mon Sep 17 00:00:00 2001 From: Abin Simon Date: Fri, 13 Jan 2023 13:14:55 +0530 Subject: [PATCH] Write logs to disk by default (#2082) ## Description This changes the behavior of logs and writes it to disk instead of `stdout` by default. It also adds a new flag `--log-file` to specify the filename for the log file. ## Does this PR need a docs update or release note? - [x] :white_check_mark: Yes, it's included - [ ] :clock1: Yes, but in a later PR - [ ] :no_entry: No ## Type of change - [x] :sunflower: Feature - [ ] :bug: Bugfix - [ ] :world_map: Documentation - [ ] :robot: Test - [ ] :computer: CI/Deployment - [ ] :broom: Tech Debt/Cleanup ## Issue(s) * fixes https://github.com/alcionai/corso/issues/2076 ## Test Plan - [x] :muscle: Manual - [ ] :zap: Unit test - [ ] :green_heart: E2E --- CHANGELOG.md | 1 + src/cli/cli.go | 6 +- src/pkg/logger/logger.go | 82 +++++++++++++++++------ src/pkg/logger/logpath_darwin.go | 10 +++ src/pkg/logger/logpath_windows.go | 9 +++ src/pkg/logger/logpath_xdg.go | 17 +++++ website/docs/setup/configuration.md | 14 ++++ website/docs/support/bugs-and-features.md | 3 +- website/styles/Vocab/Base/accept.txt | 4 +- 9 files changed, 123 insertions(+), 23 deletions(-) create mode 100644 src/pkg/logger/logpath_darwin.go create mode 100644 src/pkg/logger/logpath_windows.go create mode 100644 src/pkg/logger/logpath_xdg.go diff --git a/CHANGELOG.md b/CHANGELOG.md index 650b773c54..3a38d85541 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - The selectors Reduce() process will only include details that match the DiscreteOwner, if one is specified. - New selector constructors will automatically set the DiscreteOwner if given a single-item slice. +- Write logs to disk by default ([#2082](https://github.com/alcionai/corso/pull/2082)) ### Fixed diff --git a/src/cli/cli.go b/src/cli/cli.go index c9b2a333e0..be5059809e 100644 --- a/src/cli/cli.go +++ b/src/cli/cli.go @@ -64,7 +64,7 @@ func BuildCommandTree(cmd *cobra.Command) { cmd.Flags().BoolP("version", "v", false, "current version info") cmd.PersistentPostRunE = config.InitFunc() config.AddConfigFlags(cmd) - logger.AddLogLevelFlag(cmd) + logger.AddLoggingFlags(cmd) observe.AddProgressBarFlags(cmd) print.AddOutputFlag(cmd) options.AddGlobalOperationFlags(cmd) @@ -91,7 +91,9 @@ func Handle() { BuildCommandTree(corsoCmd) - ctx, log := logger.Seed(ctx, logger.PreloadLogLevel()) + loglevel, logfile := logger.PreloadLoggingFlags() + ctx, log := logger.Seed(ctx, loglevel, logfile) + defer func() { _ = log.Sync() // flush all logs in the buffer }() diff --git a/src/pkg/logger/logger.go b/src/pkg/logger/logger.go index bd30fcbd90..bead584b57 100644 --- a/src/pkg/logger/logger.go +++ b/src/pkg/logger/logger.go @@ -3,6 +3,8 @@ package logger import ( "context" "os" + "path/filepath" + "time" "github.com/spf13/cobra" "github.com/spf13/pflag" @@ -10,6 +12,9 @@ import ( "go.uber.org/zap/zapcore" ) +// Default location for writing logs, initialized in platform specific files +var userLogsDir string + var ( logCore *zapcore.Core loggerton *zap.SugaredLogger @@ -17,6 +22,9 @@ var ( // logging level flag llFlag = "info" + // logging file flags + lfFlag = "" + DebugAPI bool readableOutput bool ) @@ -34,17 +42,26 @@ const ( const ( debugAPIFN = "debug-api-calls" logLevelFN = "log-level" + logFileFN = "log-file" readableLogsFN = "readable-logs" ) -// adds the persistent flag --log-level to the provided command. -// defaults to "info". +// Returns the default location for writing logs +func defaultLogLocation() string { + return filepath.Join(userLogsDir, "corso", "logs", time.Now().UTC().Format("2006-01-02T15-04-05Z")+".log") +} + +// adds the persistent flag --log-level and --log-file to the provided command. +// defaults to "info" and the default log location. // This is a hack for help displays. Due to seeding the context, we also // need to parse the log level before we execute the command. -func AddLogLevelFlag(cmd *cobra.Command) { +func AddLoggingFlags(cmd *cobra.Command) { fs := cmd.PersistentFlags() fs.StringVar(&llFlag, logLevelFN, "info", "set the log level to debug|info|warn|error") + // The default provided here is only for help info + fs.StringVar(&lfFlag, logFileFN, "corso-.log", "location for writing logs, use '-' for stdout") + fs.Bool(debugAPIFN, false, "add non-2xx request/response errors to logging") fs.Bool( @@ -54,13 +71,17 @@ func AddLogLevelFlag(cmd *cobra.Command) { fs.MarkHidden(readableLogsFN) } -// Due to races between the lazy evaluation of flags in cobra and the need to init logging -// behavior in a ctx, log-level gets pre-processed manually here using pflags. The canonical -// AddLogLevelFlag() ensures the flag is displayed as part of the help/usage output. -func PreloadLogLevel() string { +// Due to races between the lazy evaluation of flags in cobra and the +// need to init logging behavior in a ctx, log-level and log-file gets +// pre-processed manually here using pflags. The canonical +// AddLogLevelFlag() and AddLogFileFlag() ensures the flags are +// displayed as part of the help/usage output. +func PreloadLoggingFlags() (string, string) { + dlf := defaultLogLocation() fs := pflag.NewFlagSet("seed-logger", pflag.ContinueOnError) fs.ParseErrorsWhitelist.UnknownFlags = true fs.String(logLevelFN, "info", "set the log level to debug|info|warn|error") + fs.String(logFileFN, dlf, "location for writing logs") fs.BoolVar(&DebugAPI, debugAPIFN, false, "add non-2xx request/response errors to logging") fs.BoolVar(&readableOutput, readableLogsFN, false, "minimizes log output: removes the file and date, colors the level") // prevents overriding the corso/cobra help processor @@ -68,20 +89,40 @@ func PreloadLogLevel() string { // parse the os args list to find the log level flag if err := fs.Parse(os.Args[1:]); err != nil { - return "info" + return "info", dlf } // retrieve the user's preferred log level // automatically defaults to "info" levelString, err := fs.GetString(logLevelFN) if err != nil { - return "info" + return "info", dlf + } + + // retrieve the user's preferred log file location + // automatically defaults to default log location + logfile, err := fs.GetString(logFileFN) + if err != nil { + return "info", dlf + } + + if logfile == "-" { + logfile = "stdout" + } + + if logfile != "stdout" && logfile != "stderr" { + logdir := filepath.Dir(logfile) + + err := os.MkdirAll(logdir, 0o755) + if err != nil { + return "info", "stderr" + } } - return levelString + return levelString, logfile } -func genLogger(level logLevel) (*zapcore.Core, *zap.SugaredLogger) { +func genLogger(level logLevel, logfile string) (*zapcore.Core, *zap.SugaredLogger) { // when testing, ensure debug logging matches the test.v setting for _, arg := range os.Args { if arg == `--test.v=true` { @@ -136,20 +177,23 @@ func genLogger(level logLevel) (*zapcore.Core, *zap.SugaredLogger) { cfg.EncoderConfig.EncodeLevel = zapcore.CapitalColorLevelEncoder } + cfg.OutputPaths = []string{logfile} lgr, err = cfg.Build(opts...) } else { - lgr, err = zap.NewProduction() + cfg := zap.NewProductionConfig() + cfg.OutputPaths = []string{logfile} + lgr, err = cfg.Build() } // fall back to the core config if the default creation fails if err != nil { - lgr = zap.New(*logCore) + lgr = zap.New(core) } return &core, lgr.Sugar() } -func singleton(level logLevel) *zap.SugaredLogger { +func singleton(level logLevel, logfile string) *zap.SugaredLogger { if loggerton != nil { return loggerton } @@ -161,7 +205,7 @@ func singleton(level logLevel) *zap.SugaredLogger { return loggerton } - logCore, loggerton = genLogger(level) + logCore, loggerton = genLogger(level, logfile) return loggerton } @@ -178,12 +222,12 @@ const ctxKey loggingKey = "corsoLogger" // It also parses the command line for flag values prior to executing // cobra. This early parsing is necessary since logging depends on // a seeded context prior to cobra evaluating flags. -func Seed(ctx context.Context, lvl string) (context.Context, *zap.SugaredLogger) { +func Seed(ctx context.Context, lvl, logfile string) (context.Context, *zap.SugaredLogger) { if len(lvl) == 0 { lvl = "info" } - zsl := singleton(levelOf(lvl)) + zsl := singleton(levelOf(lvl), logfile) return Set(ctx, zsl), zsl } @@ -192,7 +236,7 @@ func Seed(ctx context.Context, lvl string) (context.Context, *zap.SugaredLogger) func SeedLevel(ctx context.Context, level logLevel) (context.Context, *zap.SugaredLogger) { l := ctx.Value(ctxKey) if l == nil { - zsl := singleton(level) + zsl := singleton(level, defaultLogLocation()) return Set(ctx, zsl), zsl } @@ -212,7 +256,7 @@ func Set(ctx context.Context, logger *zap.SugaredLogger) context.Context { func Ctx(ctx context.Context) *zap.SugaredLogger { l := ctx.Value(ctxKey) if l == nil { - return singleton(levelOf(llFlag)) + return singleton(levelOf(llFlag), defaultLogLocation()) } return l.(*zap.SugaredLogger) diff --git a/src/pkg/logger/logpath_darwin.go b/src/pkg/logger/logpath_darwin.go new file mode 100644 index 0000000000..1c2ea862cc --- /dev/null +++ b/src/pkg/logger/logpath_darwin.go @@ -0,0 +1,10 @@ +package logger + +import ( + "os" + "path/filepath" +) + +func init() { + userLogsDir = filepath.Join(os.Getenv("HOME"), "Library", "Logs") +} diff --git a/src/pkg/logger/logpath_windows.go b/src/pkg/logger/logpath_windows.go new file mode 100644 index 0000000000..dfa046ba73 --- /dev/null +++ b/src/pkg/logger/logpath_windows.go @@ -0,0 +1,9 @@ +package logger + +import ( + "os" +) + +func init() { + userLogsDir = os.Getenv("LOCALAPPDATA") +} diff --git a/src/pkg/logger/logpath_xdg.go b/src/pkg/logger/logpath_xdg.go new file mode 100644 index 0000000000..fe10003389 --- /dev/null +++ b/src/pkg/logger/logpath_xdg.go @@ -0,0 +1,17 @@ +//go:build !windows && !darwin +// +build !windows,!darwin + +package logger + +import ( + "os" + "path/filepath" +) + +func init() { + if os.Getenv("XDG_CACHE_HOME") != "" { + userLogsDir = os.Getenv("XDG_CACHE_HOME") + } else { + userLogsDir = filepath.Join(os.Getenv("HOME"), ".cache") + } +} diff --git a/website/docs/setup/configuration.md b/website/docs/setup/configuration.md index 4a07d8f21e..cee7c79884 100644 --- a/website/docs/setup/configuration.md +++ b/website/docs/setup/configuration.md @@ -126,3 +126,17 @@ directory within the container. + +## Log Files + +The location of log files varies by operating system: + +* On Linux - `~/.cache/corso/logs/.log` +* On macOS - `~/Library/Logs/corso/logs/.log` +* On Windows - `%LocalAppData%\corso/logs/.log` + +Log file location can be overridden by setting the `--log-file` flag. + +:::info +You can use `stdout` or `stderr` as the `--log-file` location to redirect the logs to "stdout" and "stderr" respectively. +::: diff --git a/website/docs/support/bugs-and-features.md b/website/docs/support/bugs-and-features.md index 1e3bfff122..ac637655ae 100644 --- a/website/docs/support/bugs-and-features.md +++ b/website/docs/support/bugs-and-features.md @@ -4,4 +4,5 @@ You can learn more about the Corso roadmap and how to interpret it [here](https: If you run into a bug or have feature requests, please file a [GitHub issue](https://github.com/alcionai/corso/issues/) and attach the `bug` or `enhancement` label to the issue. When filing bugs, please run Corso with `--log-level debug` -and add the logs to the bug report. +and add the logs to the bug report. You can find more information about where logs are stored in the +[log files](../../setup/configuration/#log-files) section in setup docs. diff --git a/website/styles/Vocab/Base/accept.txt b/website/styles/Vocab/Base/accept.txt index 7adf5969f2..eae43d149e 100644 --- a/website/styles/Vocab/Base/accept.txt +++ b/website/styles/Vocab/Base/accept.txt @@ -34,4 +34,6 @@ Gitlab cyberattack Atlassian SLAs -runbooks \ No newline at end of file +runbooks +stdout +stderr \ No newline at end of file