From b2cd007d8dd550cd6e61c00349021d73ae28eeb8 Mon Sep 17 00:00:00 2001 From: Anna Shaleva Date: Fri, 7 Oct 2022 14:51:51 +0300 Subject: [PATCH] cli: move handleLoggingParams to 'options' package It will be reused by other CLI packages. --- cli/options/options.go | 89 ++++++++++++++++++++++++++++++++++++++ cli/server/server.go | 91 ++------------------------------------- cli/server/server_test.go | 26 +++++------ 3 files changed, 102 insertions(+), 104 deletions(-) diff --git a/cli/options/options.go b/cli/options/options.go index b2bcedc13..7fe705992 100644 --- a/cli/options/options.go +++ b/cli/options/options.go @@ -6,16 +6,23 @@ package options import ( "context" "errors" + "fmt" + "net/url" + "os" + "runtime" "strconv" "time" "github.com/nspcc-dev/neo-go/pkg/config" "github.com/nspcc-dev/neo-go/pkg/config/netmode" "github.com/nspcc-dev/neo-go/pkg/core/transaction" + "github.com/nspcc-dev/neo-go/pkg/io" "github.com/nspcc-dev/neo-go/pkg/rpcclient" "github.com/nspcc-dev/neo-go/pkg/rpcclient/invoker" "github.com/nspcc-dev/neo-go/pkg/util" "github.com/urfave/cli" + "go.uber.org/zap" + "go.uber.org/zap/zapcore" ) // DefaultTimeout is the default timeout used for RPC requests. @@ -151,3 +158,85 @@ func GetConfigFromContext(ctx *cli.Context) (config.Config, error) { } return config.Load(configPath, GetNetwork(ctx)) } + +var ( + // _winfileSinkRegistered denotes whether zap has registered + // user-supplied factory for all sinks with `winfile`-prefixed scheme. + _winfileSinkRegistered bool + _winfileSinkCloser func() error +) + +// HandleLoggingParams reads logging parameters. +// If a user selected debug level -- function enables it. +// If logPath is configured -- function creates a dir and a file for logging. +// If logPath is configured on Windows -- function returns closer to be +// able to close sink for the opened log output file. +func HandleLoggingParams(debug bool, cfg config.ApplicationConfiguration) (*zap.Logger, func() error, error) { + level := zapcore.InfoLevel + if debug { + level = zapcore.DebugLevel + } + + cc := zap.NewProductionConfig() + cc.DisableCaller = true + cc.DisableStacktrace = true + cc.EncoderConfig.EncodeDuration = zapcore.StringDurationEncoder + cc.EncoderConfig.EncodeLevel = zapcore.CapitalLevelEncoder + cc.EncoderConfig.EncodeTime = zapcore.ISO8601TimeEncoder + cc.Encoding = "console" + cc.Level = zap.NewAtomicLevelAt(level) + cc.Sampling = nil + + if logPath := cfg.LogPath; logPath != "" { + if err := io.MakeDirForFile(logPath, "logger"); err != nil { + return nil, nil, err + } + + if runtime.GOOS == "windows" { + if !_winfileSinkRegistered { + // See https://github.com/uber-go/zap/issues/621. + err := zap.RegisterSink("winfile", func(u *url.URL) (zap.Sink, error) { + if u.User != nil { + return nil, fmt.Errorf("user and password not allowed with file URLs: got %v", u) + } + if u.Fragment != "" { + return nil, fmt.Errorf("fragments not allowed with file URLs: got %v", u) + } + if u.RawQuery != "" { + return nil, fmt.Errorf("query parameters not allowed with file URLs: got %v", u) + } + // Error messages are better if we check hostname and port separately. + if u.Port() != "" { + return nil, fmt.Errorf("ports not allowed with file URLs: got %v", u) + } + if hn := u.Hostname(); hn != "" && hn != "localhost" { + return nil, fmt.Errorf("file URLs must leave host empty or use localhost: got %v", u) + } + switch u.Path { + case "stdout": + return os.Stdout, nil + case "stderr": + return os.Stderr, nil + } + f, err := os.OpenFile(u.Path[1:], // Remove leading slash left after url.Parse. + os.O_WRONLY|os.O_APPEND|os.O_CREATE, 0644) + _winfileSinkCloser = func() error { + _winfileSinkCloser = nil + return f.Close() + } + return f, err + }) + if err != nil { + return nil, nil, fmt.Errorf("failed to register windows-specific sinc: %w", err) + } + _winfileSinkRegistered = true + } + logPath = "winfile:///" + logPath + } + + cc.OutputPaths = []string{logPath} + } + + log, err := cc.Build() + return log, _winfileSinkCloser, err +} diff --git a/cli/server/server.go b/cli/server/server.go index 88d5ff790..827e3ae77 100644 --- a/cli/server/server.go +++ b/cli/server/server.go @@ -4,10 +4,8 @@ import ( "context" "errors" "fmt" - "net/url" "os" "os/signal" - "runtime" "syscall" "time" @@ -31,14 +29,6 @@ import ( "github.com/nspcc-dev/neo-go/pkg/services/stateroot" "github.com/urfave/cli" "go.uber.org/zap" - "go.uber.org/zap/zapcore" -) - -var ( - // _winfileSinkRegistered denotes whether zap has registered - // user-supplied factory for all sinks with `winfile`-prefixed scheme. - _winfileSinkRegistered bool - _winfileSinkCloser func() error ) // NewCommands returns 'node' command. @@ -126,81 +116,6 @@ func newGraceContext() context.Context { return ctx } -// handleLoggingParams reads logging parameters. -// If a user selected debug level -- function enables it. -// If logPath is configured -- function creates a dir and a file for logging. -// If logPath is configured on Windows -- function returns closer to be -// able to close sink for the opened log output file. -func handleLoggingParams(ctx *cli.Context, cfg config.ApplicationConfiguration) (*zap.Logger, func() error, error) { - level := zapcore.InfoLevel - if ctx.Bool("debug") { - level = zapcore.DebugLevel - } - - cc := zap.NewProductionConfig() - cc.DisableCaller = true - cc.DisableStacktrace = true - cc.EncoderConfig.EncodeDuration = zapcore.StringDurationEncoder - cc.EncoderConfig.EncodeLevel = zapcore.CapitalLevelEncoder - cc.EncoderConfig.EncodeTime = zapcore.ISO8601TimeEncoder - cc.Encoding = "console" - cc.Level = zap.NewAtomicLevelAt(level) - cc.Sampling = nil - - if logPath := cfg.LogPath; logPath != "" { - if err := io.MakeDirForFile(logPath, "logger"); err != nil { - return nil, nil, err - } - - if runtime.GOOS == "windows" { - if !_winfileSinkRegistered { - // See https://github.com/uber-go/zap/issues/621. - err := zap.RegisterSink("winfile", func(u *url.URL) (zap.Sink, error) { - if u.User != nil { - return nil, fmt.Errorf("user and password not allowed with file URLs: got %v", u) - } - if u.Fragment != "" { - return nil, fmt.Errorf("fragments not allowed with file URLs: got %v", u) - } - if u.RawQuery != "" { - return nil, fmt.Errorf("query parameters not allowed with file URLs: got %v", u) - } - // Error messages are better if we check hostname and port separately. - if u.Port() != "" { - return nil, fmt.Errorf("ports not allowed with file URLs: got %v", u) - } - if hn := u.Hostname(); hn != "" && hn != "localhost" { - return nil, fmt.Errorf("file URLs must leave host empty or use localhost: got %v", u) - } - switch u.Path { - case "stdout": - return os.Stdout, nil - case "stderr": - return os.Stderr, nil - } - f, err := os.OpenFile(u.Path[1:], // Remove leading slash left after url.Parse. - os.O_WRONLY|os.O_APPEND|os.O_CREATE, 0644) - _winfileSinkCloser = func() error { - _winfileSinkCloser = nil - return f.Close() - } - return f, err - }) - if err != nil { - return nil, nil, fmt.Errorf("failed to register windows-specific sinc: %w", err) - } - _winfileSinkRegistered = true - } - logPath = "winfile:///" + logPath - } - - cc.OutputPaths = []string{logPath} - } - - log, err := cc.Build() - return log, _winfileSinkCloser, err -} - func initBCWithMetrics(cfg config.Config, log *zap.Logger) (*core.Blockchain, *metrics.Service, *metrics.Service, error) { chain, err := initBlockChain(cfg, log) if err != nil { @@ -225,7 +140,7 @@ func dumpDB(ctx *cli.Context) error { if err != nil { return cli.NewExitError(err, 1) } - log, logCloser, err := handleLoggingParams(ctx, cfg.ApplicationConfiguration) + log, logCloser, err := options.HandleLoggingParams(ctx.Bool("debug"), cfg.ApplicationConfiguration) if err != nil { return cli.NewExitError(err, 1) } @@ -278,7 +193,7 @@ func restoreDB(ctx *cli.Context) error { if err != nil { return err } - log, logCloser, err := handleLoggingParams(ctx, cfg.ApplicationConfiguration) + log, logCloser, err := options.HandleLoggingParams(ctx.Bool("debug"), cfg.ApplicationConfiguration) if err != nil { return cli.NewExitError(err, 1) } @@ -464,7 +379,7 @@ func startServer(ctx *cli.Context) error { if err != nil { return cli.NewExitError(err, 1) } - log, logCloser, err := handleLoggingParams(ctx, cfg.ApplicationConfiguration) + log, logCloser, err := options.HandleLoggingParams(ctx.Bool("debug"), cfg.ApplicationConfiguration) if err != nil { return cli.NewExitError(err, 1) } diff --git a/cli/server/server_test.go b/cli/server/server_test.go index 36d751cb1..d311db08e 100644 --- a/cli/server/server_test.go +++ b/cli/server/server_test.go @@ -7,13 +7,14 @@ import ( "path/filepath" "testing" + "go.uber.org/zap/zapcore" + "github.com/nspcc-dev/neo-go/cli/options" "github.com/nspcc-dev/neo-go/pkg/config" "github.com/nspcc-dev/neo-go/pkg/config/netmode" "github.com/nspcc-dev/neo-go/pkg/core/storage/dbconfig" "github.com/stretchr/testify/require" "github.com/urfave/cli" - "go.uber.org/zap" "gopkg.in/yaml.v3" ) @@ -45,49 +46,42 @@ func TestHandleLoggingParams(t *testing.T) { t.Run("logdir is a file", func(t *testing.T) { logfile := filepath.Join(d, "logdir") require.NoError(t, os.WriteFile(logfile, []byte{1, 2, 3}, os.ModePerm)) - set := flag.NewFlagSet("flagSet", flag.ExitOnError) - ctx := cli.NewContext(cli.NewApp(), set, nil) cfg := config.ApplicationConfiguration{ LogPath: filepath.Join(logfile, "file.log"), } - _, closer, err := handleLoggingParams(ctx, cfg) + _, closer, err := options.HandleLoggingParams(false, cfg) require.Error(t, err) require.Nil(t, closer) }) t.Run("default", func(t *testing.T) { - set := flag.NewFlagSet("flagSet", flag.ExitOnError) - ctx := cli.NewContext(cli.NewApp(), set, nil) cfg := config.ApplicationConfiguration{ LogPath: testLog, } - logger, closer, err := handleLoggingParams(ctx, cfg) + logger, closer, err := options.HandleLoggingParams(false, cfg) require.NoError(t, err) t.Cleanup(func() { if closer != nil { require.NoError(t, closer()) } }) - require.True(t, logger.Core().Enabled(zap.InfoLevel)) - require.False(t, logger.Core().Enabled(zap.DebugLevel)) + require.True(t, logger.Core().Enabled(zapcore.InfoLevel)) + require.False(t, logger.Core().Enabled(zapcore.DebugLevel)) }) t.Run("debug", func(t *testing.T) { - set := flag.NewFlagSet("flagSet", flag.ExitOnError) - set.Bool("debug", true, "") - ctx := cli.NewContext(cli.NewApp(), set, nil) cfg := config.ApplicationConfiguration{ LogPath: testLog, } - logger, closer, err := handleLoggingParams(ctx, cfg) + logger, closer, err := options.HandleLoggingParams(true, cfg) require.NoError(t, err) t.Cleanup(func() { if closer != nil { require.NoError(t, closer()) } }) - require.True(t, logger.Core().Enabled(zap.InfoLevel)) - require.True(t, logger.Core().Enabled(zap.DebugLevel)) + require.True(t, logger.Core().Enabled(zapcore.InfoLevel)) + require.True(t, logger.Core().Enabled(zapcore.DebugLevel)) }) } @@ -104,7 +98,7 @@ func TestInitBCWithMetrics(t *testing.T) { ctx := cli.NewContext(cli.NewApp(), set, nil) cfg, err := options.GetConfigFromContext(ctx) require.NoError(t, err) - logger, closer, err := handleLoggingParams(ctx, cfg.ApplicationConfiguration) + logger, closer, err := options.HandleLoggingParams(true, cfg.ApplicationConfiguration) require.NoError(t, err) t.Cleanup(func() { if closer != nil {