-
Notifications
You must be signed in to change notification settings - Fork 3.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: make logger easily replaceable #15358
Changes from 6 commits
3f9a2b8
7859355
75ffa82
693e07a
d6014be
b133e57
452feed
cce9776
698c9a3
68ed68b
4847096
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,5 +1,5 @@ | ||
--- | ||
sidebar_position: 0 | ||
sidebar_position: 1 | ||
--- | ||
|
||
# Requests for Comments | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -10,7 +10,6 @@ import ( | |
"os/signal" | ||
"path" | ||
"path/filepath" | ||
"strconv" | ||
"strings" | ||
"syscall" | ||
"time" | ||
|
@@ -51,20 +50,11 @@ type Context struct { | |
Logger log.Logger | ||
} | ||
|
||
// ErrorCode contains the exit code for server exit. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This was not used. |
||
type ErrorCode struct { | ||
Code int | ||
} | ||
|
||
func (e ErrorCode) Error() string { | ||
return strconv.Itoa(e.Code) | ||
} | ||
|
||
func NewDefaultContext() *Context { | ||
return NewContext( | ||
viper.New(), | ||
cmtcfg.DefaultConfig(), | ||
log.NewLogger(os.Stdout), // TODO(mr): update NewDefaultContext to accept log destination. | ||
log.NewLogger(os.Stdout), | ||
) | ||
} | ||
|
||
|
@@ -106,7 +96,26 @@ func bindFlags(basename string, cmd *cobra.Command, v *viper.Viper) (err error) | |
return err | ||
} | ||
|
||
// InterceptConfigsPreRunHandler performs a pre-run function for the root daemon | ||
// InterceptConfigsPreRunHandler is identical to CreateServerContextFromConfig | ||
// except it also sets the server context on the command and the server logger. | ||
func InterceptConfigsPreRunHandler(cmd *cobra.Command, customAppConfigTemplate string, customAppConfig interface{}, cmtConfig *cmtcfg.Config) error { | ||
serverCtx, err := CreateServerContextFromConfig(cmd, customAppConfigTemplate, customAppConfig, cmtConfig) | ||
if err != nil { | ||
return err | ||
} | ||
|
||
// overwrite default server logger | ||
logger, err := CreateSDKLogger(serverCtx, cmd.OutOrStdout()) | ||
if err != nil { | ||
return err | ||
} | ||
serverCtx.Logger = logger.With(log.ModuleKey, "server") | ||
|
||
// set server context | ||
return SetCmdServerContext(cmd, serverCtx) | ||
} | ||
|
||
// CreateServerContextFromConfig performs a pre-run function for the root daemon | ||
// application command. It will create a Viper literal and a default server | ||
// Context. The server CometBFT configuration will either be read and parsed | ||
// or created and saved to disk, where the server Context is updated to reflect | ||
|
@@ -116,25 +125,25 @@ func bindFlags(basename string, cmd *cobra.Command, v *viper.Viper) (err error) | |
// is used to read and parse the application configuration. Command handlers can | ||
// fetch the server Context to get the CometBFT configuration or to get access | ||
// to Viper. | ||
func InterceptConfigsPreRunHandler(cmd *cobra.Command, customAppConfigTemplate string, customAppConfig interface{}, cmtConfig *cmtcfg.Config) error { | ||
func CreateServerContextFromConfig(cmd *cobra.Command, customAppConfigTemplate string, customAppConfig interface{}, cmtConfig *cmtcfg.Config) (*Context, error) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do you think @tac0turtle this conveys enough what the function does? (which is more than creating a server context, as it calls I feel like maybe There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Actually, I even think, this could be named There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Updated to |
||
serverCtx := NewDefaultContext() | ||
|
||
// Get the executable name and configure the viper instance so that environmental | ||
// variables are checked based off that name. The underscore character is used | ||
// as a separator. | ||
executableName, err := os.Executable() | ||
if err != nil { | ||
return err | ||
return nil, err | ||
} | ||
|
||
basename := path.Base(executableName) | ||
|
||
// configure the viper instance | ||
if err := serverCtx.Viper.BindPFlags(cmd.Flags()); err != nil { | ||
return err | ||
return nil, err | ||
} | ||
if err := serverCtx.Viper.BindPFlags(cmd.PersistentFlags()); err != nil { | ||
return err | ||
return nil, err | ||
} | ||
|
||
serverCtx.Viper.SetEnvPrefix(basename) | ||
|
@@ -144,48 +153,52 @@ func InterceptConfigsPreRunHandler(cmd *cobra.Command, customAppConfigTemplate s | |
// intercept configuration files, using both Viper instances separately | ||
config, err := interceptConfigs(serverCtx.Viper, customAppConfigTemplate, customAppConfig, cmtConfig) | ||
if err != nil { | ||
return err | ||
return nil, err | ||
} | ||
|
||
// return value is a CometBFT configuration object | ||
serverCtx.Config = config | ||
if err = bindFlags(basename, cmd, serverCtx.Viper); err != nil { | ||
return err | ||
return nil, err | ||
} | ||
|
||
return serverCtx, nil | ||
} | ||
|
||
// CreateSDKLogger creates a the default SDK logger. | ||
// It reads the log level and format from the server context. | ||
func CreateSDKLogger(ctx *Context, out io.Writer) (log.Logger, error) { | ||
var logger log.Logger | ||
if serverCtx.Viper.GetString(flags.FlagLogFormat) == cmtcfg.LogFormatJSON { | ||
zl := zerolog.New(cmd.OutOrStdout()).With().Timestamp().Logger() | ||
if ctx.Viper.GetString(flags.FlagLogFormat) == cmtcfg.LogFormatJSON { | ||
zl := zerolog.New(out).With().Timestamp().Logger() | ||
logger = log.NewCustomLogger(zl) | ||
} else { | ||
logger = log.NewLogger(cmd.OutOrStdout()) | ||
logger = log.NewLogger(out) | ||
} | ||
|
||
// set filter level or keys for the logger if any | ||
logLvlStr := serverCtx.Viper.GetString(flags.FlagLogLevel) | ||
logLvlStr := ctx.Viper.GetString(flags.FlagLogLevel) | ||
logLvl, err := zerolog.ParseLevel(logLvlStr) | ||
if err != nil { | ||
// If the log level is not a valid zerolog level, then we try to parse it as a key filter. | ||
filterFunc, err := log.ParseLogLevel(logLvlStr, zerolog.InfoLevel.String()) | ||
if err != nil { | ||
return fmt.Errorf("failed to parse log level (%s): %w", logLvlStr, err) | ||
return nil, fmt.Errorf("failed to parse log level (%s): %w", logLvlStr, err) | ||
} | ||
|
||
logger = log.FilterKeys(logger, filterFunc) | ||
} else { | ||
zl := logger.Impl().(*zerolog.Logger) | ||
// Check if the CometBFT flag for trace logging is set if it is then setup a tracing logger in this app as well. | ||
// Note it overrides log level passed in `log_levels`. | ||
if serverCtx.Viper.GetBool(cmtcli.TraceFlag) { | ||
if ctx.Viper.GetBool(cmtcli.TraceFlag) { | ||
logger = log.NewCustomLogger(zl.Level(zerolog.TraceLevel)) | ||
} else { | ||
logger = log.NewCustomLogger(zl.Level(logLvl)) | ||
} | ||
} | ||
|
||
serverCtx.Logger = logger.With("module", "server") | ||
|
||
return SetCmdServerContext(cmd, serverCtx) | ||
return logger, nil | ||
} | ||
|
||
// GetServerContextFromCmd returns a Context from a command or an empty Context | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,23 +1,14 @@ | ||
package main | ||
|
||
import ( | ||
"os" | ||
|
||
"cosmossdk.io/simapp" | ||
"cosmossdk.io/simapp/simd/cmd" | ||
"github.com/cosmos/cosmos-sdk/server" | ||
svrcmd "github.com/cosmos/cosmos-sdk/server/cmd" | ||
) | ||
|
||
func main() { | ||
rootCmd := cmd.NewRootCmd() | ||
if err := svrcmd.Execute(rootCmd, "", simapp.DefaultNodeHome); err != nil { | ||
switch e := err.(type) { | ||
case server.ErrorCode: | ||
os.Exit(e.Code) | ||
|
||
default: | ||
os.Exit(1) | ||
} | ||
panic(err) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Does this keep the exit code of 1? If not, please revert. |
||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
unrelated: #15354 (comment)