Change configuration logging verification and improve comments

Signed-off-by: Tristan Claverie <public@tclaverie.eu>

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 `yaml:"type"`
-	Type string
 
 	// The level of the logs to produce. Will output only this level and above.
-	//Yaml key is "level"
+	Level string `yaml:"level"`
-	Level logrus.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
 }
 
 // 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}
 	}

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.
+// Logrus hook which wraps another hook and filters log entries according to their level.
-// Dendrite supports multiples levels of logging at the same time, hence it is not
+// (Note that we cannot use solely logrus.SetLevel, because Dendrite supports multiple
-// possible to just use logrus.SetLevel to control that.
+// 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{