From 89ce8bcb1fdbf7090bf88a32c71f9d688078c00a Mon Sep 17 00:00:00 2001 From: Tristan Claverie Date: Tue, 19 Dec 2017 17:43:48 +0100 Subject: [PATCH] Change configuration logging verification and improve comments Signed-off-by: Tristan Claverie --- .../dendrite/common/config/config.go | 50 +++++-------------- .../matrix-org/dendrite/common/log.go | 31 ++++++++---- 2 files changed, 34 insertions(+), 47 deletions(-) diff --git a/src/github.com/matrix-org/dendrite/common/config/config.go b/src/github.com/matrix-org/dendrite/common/config/config.go index 3cd9d09bd..e890d0f3f 100644 --- a/src/github.com/matrix-org/dendrite/common/config/config.go +++ b/src/github.com/matrix-org/dendrite/common/config/config.go @@ -207,7 +207,7 @@ type Dendrite struct { } // The config for logging informations. Each hook will be added to logrus. - Logging []Hook `yaml:"logging"` + Logging []LogrusHook `yaml:"logging"` // Any information derived from the configuration options for later use. Derived struct { @@ -252,50 +252,18 @@ type ThumbnailSize struct { ResizeMethod string `yaml:"method,omitempty"` } -// Hook represents a single logrus hook. At this point, only parsing and +// LogrusHook represents a single logrus hook. At this point, only parsing and // verification of the proper values for type and level are done. // Validity/integrity checks on the parameters are done when configuring logrus. -type Hook struct { +type LogrusHook struct { // The type of hook, currently only "file" is supported. - // Yaml key is "type" - Type string + Type string `yaml:"type"` // The level of the logs to produce. Will output only this level and above. - //Yaml key is "level" - Level logrus.Level + Level string `yaml:"level"` // The parameters for this hook. - // Yaml key is "params" - Params map[string]interface{} -} - -// UnmarshalYAML performs type coercion for a logrus hook. Additionally, -// it ensures the type is one supported. -func (hook *Hook) UnmarshalYAML(unmarshaler func(interface{}) error) error { - var tmp struct { - Type string `yaml:"type"` - Level string `yaml:"level"` - Params map[string]interface{} `yaml:"params"` - } - - if err := unmarshaler(&tmp); err != nil { - return err - } - - if tmp.Type != "file" { - return fmt.Errorf("Unknown value for %q: %s, want one of [file]", "logging.type", tmp.Type) - } - - level, err := logrus.ParseLevel(tmp.Level) - if err != nil { - return fmt.Errorf("Unknown value for %q: %s, want one of [debug,info,warn,error,fatal,panic]", "logging.level", tmp.Level) - } - - hook.Type = tmp.Type - hook.Level = level - hook.Params = tmp.Params - - return nil + Params map[string]interface{} `yaml:"params"` } // Load a yaml config file for a server run as multiple processes. @@ -444,6 +412,7 @@ func (e Error) Error() string { // check returns an error type containing all errors found within the config // file. +// nolint: gocyclo func (config *Dendrite) check(monolithic bool) error { var problems []string @@ -521,6 +490,11 @@ func (config *Dendrite) check(monolithic bool) error { checkNotEmpty("listen.room_server", string(config.Listen.RoomServer)) } + for _, logrusHook := range config.Logging { + checkNotEmpty("logging.type", string(logrusHook.Type)) + checkNotEmpty("logging.level", string(logrusHook.Level)) + } + if problems != nil { return Error{problems} } diff --git a/src/github.com/matrix-org/dendrite/common/log.go b/src/github.com/matrix-org/dendrite/common/log.go index d7ea6942b..8c21968af 100644 --- a/src/github.com/matrix-org/dendrite/common/log.go +++ b/src/github.com/matrix-org/dendrite/common/log.go @@ -33,15 +33,15 @@ func (f utcFormatter) Format(entry *logrus.Entry) ([]byte, error) { return f.Formatter.Format(entry) } -// Wrapper hook which allows to filter entries according to their level. -// Dendrite supports multiples levels of logging at the same time, hence it is not -// possible to just use logrus.SetLevel to control that. +// Logrus hook which wraps another hook and filters log entries according to their level. +// (Note that we cannot use solely logrus.SetLevel, because Dendrite supports multiple +// levels of logging at the same time.) type logLevelHook struct { level logrus.Level logrus.Hook } -// All the levels supported by this hook. +// Levels returns all the levels supported by this hook. func (h *logLevelHook) Levels() []logrus.Level { levels := make([]logrus.Level, 0) @@ -70,14 +70,27 @@ func SetupStdLogging() { // SetupHookLogging configures the logging hooks defined in the configuration. // If something fails here it means that the logging was improperly configured, // so we just exit with the error -func SetupHookLogging(hooks []config.Hook, componentName string) { +func SetupHookLogging(hooks []config.LogrusHook, componentName string) { for _, hook := range hooks { + + // Ensures we received a proper logging level + level, err := logrus.ParseLevel(hook.Level) + if err != nil { + logrus.Fatalf("Unrecognized logging level %s: %q", hook.Level, err) + } + + // Perform a first filter on the logs according to the lowest level of all + // (Ex: If we have hook for info and above, prevent logrus from processing debug logs) + if logrus.GetLevel() < level { + logrus.SetLevel(level) + } + switch hook.Type { case "file": checkFileHookParams(hook.Params) - setupFileHook(hook, componentName) + setupFileHook(hook, level, componentName) default: - logrus.Fatalf("Unrecognized hook type: %s", hook.Type) + logrus.Fatalf("Unrecognized logging hook type: %s", hook.Type) } } } @@ -95,7 +108,7 @@ func checkFileHookParams(params map[string]interface{}) { } // Add a new FSHook to the logger. Each component will log in its own file -func setupFileHook(hook config.Hook, componentName string) { +func setupFileHook(hook config.LogrusHook, level logrus.Level, componentName string) { dirPath := (hook.Params["path"]).(string) fullPath := filepath.Join(dirPath, componentName+".log") @@ -104,7 +117,7 @@ func setupFileHook(hook config.Hook, componentName string) { } logrus.AddHook(&logLevelHook{ - hook.Level, + level, dugong.NewFSHook( fullPath, &utcFormatter{