From 1f941bbec73ea22487db0c8ed28d583e9c5d61b9 Mon Sep 17 00:00:00 2001 From: Julien Robert Date: Mon, 14 Oct 2024 08:05:47 +0200 Subject: [PATCH] fix(server/v2): use only one logger for app and server (#22241) --- server/v2/commands.go | 8 ++--- server/v2/server.go | 9 +++--- server/v2/server_test.go | 7 +++-- server/v2/util.go | 52 +++++++++++++++++++++----------- simapp/v2/simdv2/cmd/commands.go | 8 +---- simapp/v2/simdv2/cmd/config.go | 2 +- simapp/v2/simdv2/cmd/testnet.go | 3 +- 7 files changed, 50 insertions(+), 39 deletions(-) diff --git a/server/v2/commands.go b/server/v2/commands.go index 0357fe180d9..86f8e400467 100644 --- a/server/v2/commands.go +++ b/server/v2/commands.go @@ -13,7 +13,6 @@ import ( "github.com/spf13/viper" "cosmossdk.io/core/transaction" - "cosmossdk.io/log" ) // Execute executes the root command of an application. @@ -37,7 +36,6 @@ func Execute(rootCmd *cobra.Command, envPrefix, defaultHome string) error { func AddCommands[T transaction.Tx]( rootCmd *cobra.Command, newApp AppCreator[T], - logger log.Logger, globalServerCfg ServerConfig, components ...ServerComponent[T], ) error { @@ -45,7 +43,7 @@ func AddCommands[T transaction.Tx]( return errors.New("no components provided") } - server := NewServer(logger, globalServerCfg, components...) + server := NewServer(globalServerCfg, components...) originalPersistentPreRunE := rootCmd.PersistentPreRunE rootCmd.PersistentPreRunE = func(cmd *cobra.Command, args []string) error { // set the default command outputs @@ -170,12 +168,12 @@ func configHandle[T transaction.Tx](s *Server[T], cmd *cobra.Command) error { return err } - log, err := NewLogger(v, cmd.OutOrStdout()) + logger, err := NewLogger(v, cmd.OutOrStdout()) if err != nil { return err } - return SetCmdServerContext(cmd, v, log) + return SetCmdServerContext(cmd, v, logger) } // findSubCommand finds a sub-command of the provided command whose Use diff --git a/server/v2/server.go b/server/v2/server.go index a628fbdf100..486f5c0ceec 100644 --- a/server/v2/server.go +++ b/server/v2/server.go @@ -63,18 +63,15 @@ var _ ServerComponent[transaction.Tx] = (*Server[transaction.Tx])(nil) // Server is the top-level server component which contains all other server components. type Server[T transaction.Tx] struct { - logger log.Logger components []ServerComponent[T] config ServerConfig } func NewServer[T transaction.Tx]( - logger log.Logger, config ServerConfig, components ...ServerComponent[T], ) *Server[T] { return &Server[T]{ - logger: logger, config: config, components: components, } @@ -86,7 +83,8 @@ func (s *Server[T]) Name() string { // Start starts all components concurrently. func (s *Server[T]) Start(ctx context.Context) error { - s.logger.Info("starting servers...") + logger := GetLoggerFromContext(ctx) + logger.With(log.ModuleKey, s.Name()).Info("starting servers...") g, ctx := errgroup.WithContext(ctx) for _, mod := range s.components { @@ -106,7 +104,8 @@ func (s *Server[T]) Start(ctx context.Context) error { // Stop stops all components concurrently. func (s *Server[T]) Stop(ctx context.Context) error { - s.logger.Info("stopping servers...") + logger := GetLoggerFromContext(ctx) + logger.With(log.ModuleKey, s.Name()).Info("stopping servers...") g, ctx := errgroup.WithContext(ctx) for _, mod := range s.components { diff --git a/server/v2/server_test.go b/server/v2/server_test.go index b67fd209e1b..c35dea1fc62 100644 --- a/server/v2/server_test.go +++ b/server/v2/server_test.go @@ -65,6 +65,10 @@ func TestServer(t *testing.T) { cfg := v.AllSettings() logger := log.NewLogger(os.Stdout) + + ctx, err := serverv2.SetServerContext(context.Background(), v, logger) + require.NoError(t, err) + grpcServer := grpc.New[transaction.Tx]() err = grpcServer.Init(&mockApp[transaction.Tx]{}, cfg, logger) require.NoError(t, err) @@ -76,7 +80,6 @@ func TestServer(t *testing.T) { mockServer := &mockServer{name: "mock-server-1", ch: make(chan string, 100)} server := serverv2.NewServer( - logger, serverv2.DefaultServerConfig(), grpcServer, storeServer, @@ -97,7 +100,7 @@ func TestServer(t *testing.T) { require.Equal(t, v.GetString(grpcServer.Name()+".address"), grpc.DefaultConfig().Address) // start empty - ctx, cancelFn := context.WithCancel(context.TODO()) + ctx, cancelFn := context.WithCancel(ctx) go func() { // wait 5sec and cancel context <-time.After(5 * time.Second) diff --git a/server/v2/util.go b/server/v2/util.go index 72335621b44..d02ea30125e 100644 --- a/server/v2/util.go +++ b/server/v2/util.go @@ -13,44 +13,62 @@ import ( "cosmossdk.io/log" ) +// SetServerContext sets the logger and viper in the context. +// The server manager expects the logger and viper to be set in the context. +func SetServerContext(ctx context.Context, viper *viper.Viper, logger log.Logger) (context.Context, error) { + if ctx == nil { + ctx = context.Background() + } + + ctx = context.WithValue(ctx, corectx.LoggerContextKey, logger) + ctx = context.WithValue(ctx, corectx.ViperContextKey, viper) + return ctx, nil +} + // SetCmdServerContext sets a command's Context value to the provided argument. +// The server manager expects the logger and viper to be set in the context. // If the context has not been set, set the given context as the default. func SetCmdServerContext(cmd *cobra.Command, viper *viper.Viper, logger log.Logger) error { - var cmdCtx context.Context - if cmd.Context() == nil { - cmdCtx = context.Background() - } else { - cmdCtx = cmd.Context() + ctx, err := SetServerContext(cmd.Context(), viper, logger) + if err != nil { + return err } - - cmdCtx = context.WithValue(cmdCtx, corectx.LoggerContextKey, logger) - cmdCtx = context.WithValue(cmdCtx, corectx.ViperContextKey, viper) - cmd.SetContext(cmdCtx) - + cmd.SetContext(ctx) return nil } -func GetViperFromCmd(cmd *cobra.Command) *viper.Viper { - value := cmd.Context().Value(corectx.ViperContextKey) +// GetViperFromContext returns the viper instance from the context. +func GetViperFromContext(ctx context.Context) *viper.Viper { + value := ctx.Value(corectx.ViperContextKey) v, ok := value.(*viper.Viper) if !ok { - panic(fmt.Sprintf("incorrect viper type %T: expected *viper.Viper. Have you forgot to set the viper in the command context?", value)) + panic(fmt.Sprintf("incorrect viper type %T: expected *viper.Viper. Have you forgot to set the viper in the context?", value)) } return v } -func GetLoggerFromCmd(cmd *cobra.Command) log.Logger { - v := cmd.Context().Value(corectx.LoggerContextKey) +// GetViperFromCmd returns the viper instance from the command context. +func GetViperFromCmd(cmd *cobra.Command) *viper.Viper { + return GetViperFromContext(cmd.Context()) +} + +// GetLoggerFromContext returns the logger instance from the context. +func GetLoggerFromContext(ctx context.Context) log.Logger { + v := ctx.Value(corectx.LoggerContextKey) logger, ok := v.(log.Logger) if !ok { - panic(fmt.Sprintf("incorrect logger type %T: expected log.Logger. Have you forgot to set the logger in the command context?", v)) + panic(fmt.Sprintf("incorrect logger type %T: expected log.Logger. Have you forgot to set the logger in the context?", v)) } return logger } +// GetLoggerFromCmd returns the logger instance from the command context. +func GetLoggerFromCmd(cmd *cobra.Command) log.Logger { + return GetLoggerFromContext(cmd.Context()) +} + // ExternalIP https://stackoverflow.com/questions/23558425/how-do-i-get-the-local-ip-address-in-go -// TODO there must be a better way to get external IP func ExternalIP() (string, error) { ifaces, err := net.Interfaces() if err != nil { diff --git a/simapp/v2/simdv2/cmd/commands.go b/simapp/v2/simdv2/cmd/commands.go index 3fcbed114d9..8d4186be8d1 100644 --- a/simapp/v2/simdv2/cmd/commands.go +++ b/simapp/v2/simdv2/cmd/commands.go @@ -55,11 +55,6 @@ func initRootCmd[T transaction.Tx]( NewTestnetCmd(moduleManager), ) - logger, err := serverv2.NewLogger(viper.New(), rootCmd.OutOrStdout()) - if err != nil { - panic(fmt.Sprintf("failed to create logger: %v", err)) - } - // add keybase, auxiliary RPC, query, genesis, and tx child commands rootCmd.AddCommand( genesisCommand(moduleManager), @@ -70,10 +65,9 @@ func initRootCmd[T transaction.Tx]( ) // wire server commands - if err = serverv2.AddCommands( + if err := serverv2.AddCommands( rootCmd, newApp, - logger, initServerConfig(), cometbft.New( &genericTxDecoder[T]{txConfig}, diff --git a/simapp/v2/simdv2/cmd/config.go b/simapp/v2/simdv2/cmd/config.go index 51a7adb178e..16cb19db93e 100644 --- a/simapp/v2/simdv2/cmd/config.go +++ b/simapp/v2/simdv2/cmd/config.go @@ -79,7 +79,7 @@ func initCometConfig() cometbft.CfgOption { cfg := cmtcfg.DefaultConfig() // display only warn logs by default except for p2p and state - cfg.LogLevel = "*:warn,p2p:info,state:info" + cfg.LogLevel = "*:warn,server:info,p2p:info,state:info" // increase block timeout cfg.Consensus.TimeoutCommit = 5 * time.Second // overwrite default pprof listen address diff --git a/simapp/v2/simdv2/cmd/testnet.go b/simapp/v2/simdv2/cmd/testnet.go index f030db8e595..7b71e98b588 100644 --- a/simapp/v2/simdv2/cmd/testnet.go +++ b/simapp/v2/simdv2/cmd/testnet.go @@ -15,7 +15,6 @@ import ( "github.com/spf13/pflag" "cosmossdk.io/core/transaction" - "cosmossdk.io/log" "cosmossdk.io/math" "cosmossdk.io/math/unsafe" runtimev2 "cosmossdk.io/runtime/v2" @@ -344,7 +343,7 @@ func initTestnetFiles[T transaction.Tx]( ) storeServer := store.New[T]() grpcServer := grpc.New[T](grpc.OverwriteDefaultConfig(grpcConfig)) - server := serverv2.NewServer[T](log.NewNopLogger(), serverCfg, cometServer, grpcServer, storeServer) + server := serverv2.NewServer[T](serverCfg, cometServer, grpcServer, storeServer) err = server.WriteConfig(filepath.Join(nodeDir, "config")) if err != nil { return err