From 4ee54156ac23005a04130dfdaad7cbf4090f6b90 Mon Sep 17 00:00:00 2001 From: Daniel Mancia <21249320+dmanc@users.noreply.github.com> Date: Tue, 30 Jan 2024 15:36:39 -0800 Subject: [PATCH 1/3] Use logfmt instead of terminal format --- common/logging/logging.go | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/common/logging/logging.go b/common/logging/logging.go index cb4da3d150..cfb03a9e2b 100644 --- a/common/logging/logging.go +++ b/common/logging/logging.go @@ -32,13 +32,7 @@ func GetLogger(cfg Config) (common.Logger, error) { } logger := &Logger{Logger: log.New()} - // This is required to print locations of log calls - // This was recently added in this PR: https://github.com/ethereum/go-ethereum/pull/28069/files - // where the default behavior was changed to not print origins - // This was due to it being very expensive to compute origins - // We should evaluate enabling/disabling this based on the flag - log.PrintOrigins(true) - stdh := log.StreamHandler(os.Stdout, log.TerminalFormat(false)) + stdh := log.StreamHandler(os.Stdout, log.LogfmtFormat()) stdHandler := log.CallerFileHandler(log.LvlFilterHandler(stdLevel, stdh)) if cfg.Path != "" { fh, err := log.FileHandler(cfg.Path, log.LogfmtFormat()) From 8517344981c7af2173142d9d0b4e71fc6c8c2d12 Mon Sep 17 00:00:00 2001 From: Daniel Mancia <21249320+dmanc@users.noreply.github.com> Date: Tue, 30 Jan 2024 18:35:32 -0800 Subject: [PATCH 2/3] Update config so that it defaults to previous behavior --- common/logging/cli.go | 40 +++++++++++++++++++++++++++++---------- common/logging/logging.go | 39 ++++++++++++++++++++++++++++++++++++-- 2 files changed, 67 insertions(+), 12 deletions(-) diff --git a/common/logging/cli.go b/common/logging/cli.go index a055b460df..c4313469d5 100644 --- a/common/logging/cli.go +++ b/common/logging/cli.go @@ -6,16 +6,20 @@ import ( ) const ( - PathFlagName = "log.path" - FileLevelFlagName = "log.level-file" - StdLevelFlagName = "log.level-std" + PathFlagName = "log.path" + FileFormatFlagName = "log.format-file" + FileLevelFlagName = "log.level-file" + StdFormatFlagName = "log.format-std" + StdLevelFlagName = "log.level-std" ) type Config struct { - Path string - Prefix string - FileLevel string - StdLevel string + Path string + Prefix string + FileLevel string + FileFormat string + StdLevel string + StdFormat string } func CLIFlags(envPrefix string, flagPrefix string) []cli.Flag { @@ -38,21 +42,37 @@ func CLIFlags(envPrefix string, flagPrefix string) []cli.Flag { Value: "", EnvVar: common.PrefixEnvVar(envPrefix, "LOG_PATH"), }, + cli.StringFlag{ + Name: common.PrefixFlag(flagPrefix, StdFormatFlagName), + Usage: "Format of the log messages. Accepted options are 'terminal', 'json', and 'logfmt'", + Value: "terminal", + EnvVar: common.PrefixEnvVar(envPrefix, "STD_LOG_FORMAT"), + }, + cli.StringFlag{ + Name: common.PrefixFlag(flagPrefix, FileFormatFlagName), + Usage: "Format of the log messages. Accepted options are 'terminal', 'json', and 'logfmt'", + Value: "logfmt", + EnvVar: common.PrefixEnvVar(envPrefix, "FILE_LOG_FORMAT"), + }, } } func DefaultCLIConfig() Config { return Config{ - Path: "", - FileLevel: "debug", - StdLevel: "debug", + Path: "", + FileFormat: "logfmt", + FileLevel: "debug", + StdFormat: "terminal", + StdLevel: "debug", } } func ReadCLIConfig(ctx *cli.Context, flagPrefix string) Config { cfg := DefaultCLIConfig() + cfg.StdFormat = ctx.GlobalString(common.PrefixFlag(flagPrefix, StdFormatFlagName)) cfg.StdLevel = ctx.GlobalString(common.PrefixFlag(flagPrefix, StdLevelFlagName)) cfg.FileLevel = ctx.GlobalString(common.PrefixFlag(flagPrefix, FileLevelFlagName)) + cfg.FileFormat = ctx.GlobalString(common.PrefixFlag(flagPrefix, FileFormatFlagName)) cfg.Path = ctx.GlobalString(common.PrefixFlag(flagPrefix, PathFlagName)) return cfg } diff --git a/common/logging/logging.go b/common/logging/logging.go index cfb03a9e2b..171b9736fe 100644 --- a/common/logging/logging.go +++ b/common/logging/logging.go @@ -20,6 +20,32 @@ func (l *Logger) SetHandler(h log.Handler) { l.Logger.SetHandler(h) } +func getStreamHandlerFromFormat(format string) (log.Handler, error) { + switch format { + case "terminal": + return log.StreamHandler(os.Stdout, log.TerminalFormat(false)), nil + case "json": + return log.StreamHandler(os.Stdout, log.JSONFormat()), nil + case "logfmt": + return log.StreamHandler(os.Stdout, log.LogfmtFormat()), nil + default: + return nil, fmt.Errorf("invalid log format: %s", format) + } +} + +func getFileHandlerFromFormat(path string, format string) (log.Handler, error) { + switch format { + case "terminal": + return log.FileHandler(path, log.TerminalFormat(false)) + case "json": + return log.FileHandler(path, log.JSONFormat()) + case "logfmt": + return log.FileHandler(path, log.LogfmtFormat()) + default: + return nil, fmt.Errorf("invalid log format: %s", format) + } +} + // GetLogger returns a logger with the specified configuration. func GetLogger(cfg Config) (common.Logger, error) { fileLevel, err := log.LvlFromString(cfg.FileLevel) @@ -32,10 +58,19 @@ func GetLogger(cfg Config) (common.Logger, error) { } logger := &Logger{Logger: log.New()} - stdh := log.StreamHandler(os.Stdout, log.LogfmtFormat()) + // This is required to print locations of log calls + // This was recently added in this PR: https://github.com/ethereum/go-ethereum/pull/28069/files + // where the default behavior was changed to not print origins + // This was due to it being very expensive to compute origins + // We should evaluate enabling/disabling this based on the flag + log.PrintOrigins(true) + stdh, err := getStreamHandlerFromFormat(cfg.StdFormat) + if err != nil { + return nil, err + } stdHandler := log.CallerFileHandler(log.LvlFilterHandler(stdLevel, stdh)) if cfg.Path != "" { - fh, err := log.FileHandler(cfg.Path, log.LogfmtFormat()) + fh, err := getFileHandlerFromFormat(cfg.Path, cfg.FileFormat) if err != nil { return nil, err } From 0bc5fa4ecf05ac7617a43325dcb698c0c74b0792 Mon Sep 17 00:00:00 2001 From: Daniel Mancia <21249320+dmanc@users.noreply.github.com> Date: Tue, 30 Jan 2024 19:43:00 -0800 Subject: [PATCH 3/3] Fix unit tests --- core/indexer/state_test.go | 6 ++++-- core/thegraph/state_integration_test.go | 6 ++++-- core/thegraph/state_test.go | 12 ++++++++---- 3 files changed, 16 insertions(+), 8 deletions(-) diff --git a/core/indexer/state_test.go b/core/indexer/state_test.go index 71df476414..139c8400ea 100644 --- a/core/indexer/state_test.go +++ b/core/indexer/state_test.go @@ -144,8 +144,10 @@ var _ = Describe("Indexer", func() { } logger, err := logging.GetLogger(logging.Config{ - StdLevel: "debug", - FileLevel: "debug", + StdFormat: "terminal", + StdLevel: "debug", + FileFormat: "logfmt", + FileLevel: "debug", }) Expect(err).ToNot(HaveOccurred()) diff --git a/core/thegraph/state_integration_test.go b/core/thegraph/state_integration_test.go index 5968e48184..772a5b3a24 100644 --- a/core/thegraph/state_integration_test.go +++ b/core/thegraph/state_integration_test.go @@ -81,8 +81,10 @@ func TestIndexerIntegration(t *testing.T) { defer teardown() logger, err := logging.GetLogger(logging.Config{ - StdLevel: "debug", - FileLevel: "debug", + StdFormat: "terminal", + StdLevel: "debug", + FileFormat: "logfmt", + FileLevel: "debug", }) assert.NoError(t, err) diff --git a/core/thegraph/state_test.go b/core/thegraph/state_test.go index c78d1134aa..fa11ed27f1 100644 --- a/core/thegraph/state_test.go +++ b/core/thegraph/state_test.go @@ -42,8 +42,10 @@ func (m *mockChainState) GetOperatorStateByOperator(ctx context.Context, blockNu func TestIndexedChainState_GetIndexedOperatorState(t *testing.T) { logger, err := logging.GetLogger(logging.Config{ - StdLevel: "debug", - FileLevel: "debug", + StdFormat: "terminal", + StdLevel: "debug", + FileFormat: "logfmt", + FileLevel: "debug", }) assert.NoError(t, err) @@ -104,8 +106,10 @@ func TestIndexedChainState_GetIndexedOperatorState(t *testing.T) { func TestIndexedChainState_GetIndexedOperatorInfoByOperatorId(t *testing.T) { logger, err := logging.GetLogger(logging.Config{ - StdLevel: "debug", - FileLevel: "debug", + StdFormat: "terminal", + StdLevel: "debug", + FileFormat: "logfmt", + FileLevel: "debug", }) assert.NoError(t, err)