From e0ecc9e835ef56fae7ee58408ba91e29f000344d Mon Sep 17 00:00:00 2001 From: Jesse Duffield Date: Thu, 25 May 2023 18:18:35 +1000 Subject: [PATCH] Allow global logging when developing I'll be honest, for all I know logging should be global in general: it is a pain to pass a logger to any struct that needs it. But smart people on the internet tell me otherwise, and I do like the idea of not having any global variables lying around. Nonetheless, I often need to log things when locally debugging and that's a different kind of logging than the kind you would include in the actual released binary. For example if I want to log something from gocui, I would rather not have gocui depend on lazygit's logging setup. --- CONTRIBUTING.md | 23 +-------- pkg/app/app.go | 14 ++++++ pkg/app/entry_point.go | 9 +++- pkg/app/logging.go | 56 --------------------- pkg/config/app_config.go | 4 ++ pkg/logs/logs.go | 78 ++++++++++++++++++++--------- pkg/logs/{ => tail}/logs_default.go | 4 +- pkg/logs/{ => tail}/logs_windows.go | 12 ++--- pkg/logs/tail/tail.go | 28 +++++++++++ 9 files changed, 117 insertions(+), 111 deletions(-) delete mode 100644 pkg/app/logging.go rename pkg/logs/{ => tail}/logs_default.go (85%) rename pkg/logs/{ => tail}/logs_windows.go (78%) create mode 100644 pkg/logs/tail/tail.go diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 0bfee5686..bed0a124a 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -105,30 +105,11 @@ Boy that's a hard word to spell. Anyway, lazygit is translated into several lang The easiest way to debug lazygit is to have two terminal tabs open at once: one for running lazygit (via `go run main.go -debug` in the project root) and one for viewing lazygit's logs (which can be done via `go run main.go --logs` or just `lazygit --logs`). -From most places in the codebase you have access to a logger e.g. `gui.Log.Warn("blah")`. +From most places in the codebase you have access to a logger e.g. `gui.Log.Warn("blah")` or `self.c.Log.Warn("blah")`. If you find that the existing logs are too noisy, you can set the log level with e.g. `LOG_LEVEL=warn go run main.go -debug` and then only use `Warn` logs yourself. -If you need to log from code in the vendor directory (e.g. the `gocui` package), you won't have access to the logger, but you can easily add logging support by adding the following: - -```go -func newLogger() *logrus.Entry { - // REPLACE THE BELOW PATH WITH YOUR ACTUAL LOG PATH (YOU'LL SEE THIS PRINTED WHEN YOU RUN `lazygit --logs` - logPath := "/Users/jesseduffield/Library/Application Support/jesseduffield/lazygit/development.log" - file, err := os.OpenFile(logPath, os.O_CREATE|os.O_WRONLY|os.O_APPEND, 0666) - if err != nil { - panic("unable to log to file") - } - logger := logrus.New() - logger.SetLevel(logrus.WarnLevel) - logger.SetOutput(file) - return logger.WithFields(logrus.Fields{}) -} - -var Log = newLogger() -... -Log.Warn("blah") -``` +If you need to log from code in the vendor directory (e.g. the `gocui` package), you won't have access to the logger, but you can easily add logging support by setting the `LAZYGIT_LOG_PATH` environment variable and using `logs.Global.Warn("blah")`. This is a global logger that's only intended for development purposes. If you keep having to do some setup steps to reproduce an issue, read the Testing section below to see how to create an integration test by recording a lazygit session. It's pretty easy! diff --git a/pkg/app/app.go b/pkg/app/app.go index af34b7157..e77d2868c 100644 --- a/pkg/app/app.go +++ b/pkg/app/app.go @@ -10,6 +10,7 @@ import ( "strings" "github.com/go-errors/errors" + "github.com/sirupsen/logrus" "github.com/jesseduffield/generics/slices" appTypes "github.com/jesseduffield/lazygit/pkg/app/types" @@ -22,6 +23,7 @@ import ( "github.com/jesseduffield/lazygit/pkg/env" "github.com/jesseduffield/lazygit/pkg/gui" "github.com/jesseduffield/lazygit/pkg/i18n" + "github.com/jesseduffield/lazygit/pkg/logs" "github.com/jesseduffield/lazygit/pkg/updates" ) @@ -76,6 +78,18 @@ func NewCommon(config config.AppConfigurer) (*common.Common, error) { }, nil } +func newLogger(cfg config.AppConfigurer) *logrus.Entry { + if cfg.GetDebug() { + logPath, err := config.LogPath() + if err != nil { + log.Fatal(err) + } + return logs.NewDevelopmentLogger(logPath) + } else { + return logs.NewProductionLogger() + } +} + // NewApp bootstrap a new application func NewApp(config config.AppConfigurer, common *common.Common) (*App, error) { app := &App{ diff --git a/pkg/app/entry_point.go b/pkg/app/entry_point.go index 9519abb65..c8a39dc08 100644 --- a/pkg/app/entry_point.go +++ b/pkg/app/entry_point.go @@ -16,7 +16,7 @@ import ( "github.com/jesseduffield/lazygit/pkg/config" "github.com/jesseduffield/lazygit/pkg/env" integrationTypes "github.com/jesseduffield/lazygit/pkg/integration/types" - "github.com/jesseduffield/lazygit/pkg/logs" + "github.com/jesseduffield/lazygit/pkg/logs/tail" "github.com/jesseduffield/lazygit/pkg/secureexec" "github.com/jesseduffield/lazygit/pkg/utils" "github.com/samber/lo" @@ -106,7 +106,12 @@ func Start(buildInfo *BuildInfo, integrationTest integrationTypes.IntegrationTes } if cliArgs.TailLogs { - logs.TailLogs() + logPath, err := config.LogPath() + if err != nil { + log.Fatal(err.Error()) + } + + tail.TailLogs(logPath) os.Exit(0) } diff --git a/pkg/app/logging.go b/pkg/app/logging.go deleted file mode 100644 index 65bed7823..000000000 --- a/pkg/app/logging.go +++ /dev/null @@ -1,56 +0,0 @@ -package app - -import ( - "io" - "log" - "os" - - "github.com/jesseduffield/lazygit/pkg/config" - "github.com/sirupsen/logrus" -) - -func newLogger(config config.AppConfigurer) *logrus.Entry { - var log *logrus.Logger - if config.GetDebug() { - log = newDevelopmentLogger() - } else { - log = newProductionLogger() - } - - // highly recommended: tail -f development.log | humanlog - // https://github.com/aybabtme/humanlog - log.Formatter = &logrus.JSONFormatter{} - - return log.WithFields(logrus.Fields{}) -} - -func newProductionLogger() *logrus.Logger { - log := logrus.New() - log.Out = io.Discard - log.SetLevel(logrus.ErrorLevel) - return log -} - -func newDevelopmentLogger() *logrus.Logger { - logger := logrus.New() - logger.SetLevel(getLogLevel()) - logPath, err := config.LogPath() - if err != nil { - log.Fatal(err) - } - file, err := os.OpenFile(logPath, os.O_CREATE|os.O_WRONLY|os.O_APPEND, 0o666) - if err != nil { - log.Fatalf("Unable to log to log file: %v", err) - } - logger.SetOutput(file) - return logger -} - -func getLogLevel() logrus.Level { - strLevel := os.Getenv("LOG_LEVEL") - level, err := logrus.ParseLevel(strLevel) - if err != nil { - return logrus.DebugLevel - } - return level -} diff --git a/pkg/config/app_config.go b/pkg/config/app_config.go index 49ff32b04..27e3b1400 100644 --- a/pkg/config/app_config.go +++ b/pkg/config/app_config.go @@ -301,5 +301,9 @@ func getDefaultAppState() *AppState { } func LogPath() (string, error) { + if os.Getenv("LAZYGIT_LOG_PATH") != "" { + return os.Getenv("LAZYGIT_LOG_PATH"), nil + } + return configFilePath("development.log") } diff --git a/pkg/logs/logs.go b/pkg/logs/logs.go index a4fe94031..7ec1b91b4 100644 --- a/pkg/logs/logs.go +++ b/pkg/logs/logs.go @@ -1,34 +1,64 @@ package logs import ( - "fmt" + "io" "log" "os" - "github.com/aybabtme/humanlog" - "github.com/jesseduffield/lazygit/pkg/config" + "github.com/sirupsen/logrus" ) -// TailLogs lets us run `lazygit --logs` to print the logs produced by other lazygit processes. -// This makes for easier debugging. -func TailLogs() { - logFilePath, err := config.LogPath() - if err != nil { - log.Fatal(err) +// It's important that this package does not depend on any other package because we +// may want to import it from anywhere, and we don't want to create a circular dependency +// (because Go refuses to compile circular dependencies). + +// Global is a global logger that can be used anywhere in the app, for +// _development purposes only_. I want to avoid global variables when possible, +// so if you want to log something that's printed when the -debug flag is set, +// you'll need to ensure the struct you're working with has a logger field ( +// and most of them do). +// Global is only available if the LAZYGIT_LOG_PATH environment variable is set. +var Global *logrus.Entry + +func init() { + logPath := os.Getenv("LAZYGIT_LOG_PATH") + if logPath != "" { + Global = NewDevelopmentLogger(logPath) } - - fmt.Printf("Tailing log file %s\n\n", logFilePath) - - opts := humanlog.DefaultOptions - opts.Truncates = false - - _, err = os.Stat(logFilePath) - if err != nil { - if os.IsNotExist(err) { - log.Fatal("Log file does not exist. Run `lazygit --debug` first to create the log file") - } - log.Fatal(err) - } - - TailLogsForPlatform(logFilePath, opts) +} + +func NewProductionLogger() *logrus.Entry { + logger := logrus.New() + logger.Out = io.Discard + logger.SetLevel(logrus.ErrorLevel) + return formatted(logger) +} + +func NewDevelopmentLogger(logPath string) *logrus.Entry { + logger := logrus.New() + logger.SetLevel(getLogLevel()) + + file, err := os.OpenFile(logPath, os.O_CREATE|os.O_WRONLY|os.O_APPEND, 0o666) + if err != nil { + log.Fatalf("Unable to log to log file: %v", err) + } + logger.SetOutput(file) + return formatted(logger) +} + +func formatted(log *logrus.Logger) *logrus.Entry { + // highly recommended: tail -f development.log | humanlog + // https://github.com/aybabtme/humanlog + log.Formatter = &logrus.JSONFormatter{} + + return log.WithFields(logrus.Fields{}) +} + +func getLogLevel() logrus.Level { + strLevel := os.Getenv("LOG_LEVEL") + level, err := logrus.ParseLevel(strLevel) + if err != nil { + return logrus.DebugLevel + } + return level } diff --git a/pkg/logs/logs_default.go b/pkg/logs/tail/logs_default.go similarity index 85% rename from pkg/logs/logs_default.go rename to pkg/logs/tail/logs_default.go index b4474720c..87965a1e7 100644 --- a/pkg/logs/logs_default.go +++ b/pkg/logs/tail/logs_default.go @@ -1,7 +1,7 @@ //go:build !windows // +build !windows -package logs +package tail import ( "log" @@ -11,7 +11,7 @@ import ( "github.com/jesseduffield/lazygit/pkg/secureexec" ) -func TailLogsForPlatform(logFilePath string, opts *humanlog.HandlerOptions) { +func tailLogsForPlatform(logFilePath string, opts *humanlog.HandlerOptions) { cmd := secureexec.Command("tail", "-f", logFilePath) stdout, _ := cmd.StdoutPipe() diff --git a/pkg/logs/logs_windows.go b/pkg/logs/tail/logs_windows.go similarity index 78% rename from pkg/logs/logs_windows.go rename to pkg/logs/tail/logs_windows.go index 7fa17db26..88bf8581e 100644 --- a/pkg/logs/logs_windows.go +++ b/pkg/logs/tail/logs_windows.go @@ -1,7 +1,7 @@ //go:build windows // +build windows -package logs +package tail import ( "bufio" @@ -13,7 +13,7 @@ import ( "github.com/aybabtme/humanlog" ) -func TailLogsForPlatform(logFilePath string, opts *humanlog.HandlerOptions) { +func tailLogsForPlatform(logFilePath string, opts *humanlog.HandlerOptions) { var lastModified int64 = 0 var lastOffset int64 = 0 for { @@ -22,7 +22,7 @@ func TailLogsForPlatform(logFilePath string, opts *humanlog.HandlerOptions) { log.Fatal(err) } if stat.ModTime().Unix() > lastModified { - err = TailFrom(lastOffset, logFilePath, opts) + err = tailFrom(lastOffset, logFilePath, opts) if err != nil { log.Fatal(err) } @@ -32,7 +32,7 @@ func TailLogsForPlatform(logFilePath string, opts *humanlog.HandlerOptions) { } } -func OpenAndSeek(filepath string, offset int64) (*os.File, error) { +func openAndSeek(filepath string, offset int64) (*os.File, error) { file, err := os.Open(filepath) if err != nil { return nil, err @@ -46,8 +46,8 @@ func OpenAndSeek(filepath string, offset int64) (*os.File, error) { return file, nil } -func TailFrom(lastOffset int64, logFilePath string, opts *humanlog.HandlerOptions) error { - file, err := OpenAndSeek(logFilePath, lastOffset) +func tailFrom(lastOffset int64, logFilePath string, opts *humanlog.HandlerOptions) error { + file, err := openAndSeek(logFilePath, lastOffset) if err != nil { return err } diff --git a/pkg/logs/tail/tail.go b/pkg/logs/tail/tail.go new file mode 100644 index 000000000..b21bc21e4 --- /dev/null +++ b/pkg/logs/tail/tail.go @@ -0,0 +1,28 @@ +package tail + +import ( + "fmt" + "log" + "os" + + "github.com/aybabtme/humanlog" +) + +// TailLogs lets us run `lazygit --logs` to print the logs produced by other lazygit processes. +// This makes for easier debugging. +func TailLogs(logFilePath string) { + fmt.Printf("Tailing log file %s\n\n", logFilePath) + + opts := humanlog.DefaultOptions + opts.Truncates = false + + _, err := os.Stat(logFilePath) + if err != nil { + if os.IsNotExist(err) { + log.Fatal("Log file does not exist. Run `lazygit --debug` first to create the log file") + } + log.Fatal(err) + } + + tailLogsForPlatform(logFilePath, opts) +}