Pull request: log-yaml-conf

Updates #4897.

Squashed commit of the following:

commit 8a961157c9930bf4859ce2209e5016ce94987e12
Author: Dimitry Kolyshev <dkolyshev@adguard.com>
Date:   Wed Jul 5 10:13:24 2023 +0400

    home: imp code

commit 509c07eed06311d773bc3e34b3ca28d9f14186fe
Author: Dimitry Kolyshev <dkolyshev@adguard.com>
Date:   Tue Jul 4 17:33:31 2023 +0400

    all: fix

commit f032e28f98552f238721491c998fca2d7d4b9802
Merge: ee0113435 c46516475
Author: Dimitry Kolyshev <dkolyshev@adguard.com>
Date:   Tue Jul 4 17:31:29 2023 +0400

    Merge remote-tracking branch 'origin/master' into log-yaml-conf

    # Conflicts:
    #	CHANGELOG.md

commit ee011343512e82d4e21cb402759d0284523ba02a
Author: Dimitry Kolyshev <dkolyshev@adguard.com>
Date:   Tue Jul 4 12:31:42 2023 +0400

    all: changelog

commit 07f4c4a244b1b6200d3056cde5ebced6254084a7
Merge: 2042c0753 97af062f7
Author: Dimitry Kolyshev <dkolyshev@adguard.com>
Date:   Tue Jul 4 12:25:21 2023 +0400

    Merge remote-tracking branch 'origin/master' into log-yaml-conf

commit 2042c0753ec29de6045c3f1de6d075cb93d6ec27
Merge: a1d3a5130 8004b135b
Author: Dimitry Kolyshev <dkolyshev@adguard.com>
Date:   Mon Jul 3 16:25:26 2023 +0400

    Merge remote-tracking branch 'origin/master' into log-yaml-conf

commit a1d3a51307e80f9e509bd6f3bee1a7b17bf1ffe6
Author: Dimitry Kolyshev <dkolyshev@adguard.com>
Date:   Fri Jun 30 11:11:32 2023 +0400

    home: imp code

commit 2392a3b02620b8c38e88afb4d75988be85fe1338
Merge: 4224fbed7 39f5c50ac
Author: Dimitry Kolyshev <dkolyshev@adguard.com>
Date:   Thu Jun 29 16:46:47 2023 +0400

    home: imp code

commit 4224fbed7113e94bee44d0ab0272e8302a8086f3
Author: Dimitry Kolyshev <dkolyshev@adguard.com>
Date:   Thu Jun 29 16:39:35 2023 +0400

    home: imp code

commit 5ce708cc50bed83e64f062e599fc8b6143d0d44d
Author: Dimitry Kolyshev <dkolyshev@adguard.com>
Date:   Thu Jun 29 12:48:42 2023 +0400

    all: docs

commit 4b6d898a888818410f59b843c3ca1a685aafec82
Author: Dimitry Kolyshev <dkolyshev@adguard.com>
Date:   Thu Jun 29 12:20:54 2023 +0400

    home: imp code

commit 431b44eda71f488c747c3efa4da0a6c222b1cf06
Author: Dimitry Kolyshev <dkolyshev@adguard.com>
Date:   Thu Jun 29 12:03:17 2023 +0400

    home: imp tests

commit 53a5c31060018c37953beb27d80c46f92bbe14af
Author: Dimitry Kolyshev <dkolyshev@adguard.com>
Date:   Wed Jun 28 17:40:49 2023 +0400

    home: imp docs

commit bfa57d9f21e3326baafd3a52e91d54396d8e03fa
Author: Dimitry Kolyshev <dkolyshev@adguard.com>
Date:   Wed Jun 28 15:49:40 2023 +0400

    home: log conf

commit 49e06dca9dc2ceb2647b7e36dac145ccf78a6c3f
Author: Dimitry Kolyshev <dkolyshev@adguard.com>
Date:   Wed Jun 28 15:41:02 2023 +0400

    home: log conf

commit 9be432dea7cec8ae0c0d3ee1f73c58c76376d07e
Author: Dimitry Kolyshev <dkolyshev@adguard.com>
Date:   Wed Jun 28 10:45:01 2023 +0400

    home: log conf

CHANGELOG.md

@@ -23,6 +23,40 @@ See also the [v0.107.34 GitHub milestone][ms-v0.107.34].
 NOTE: Add new changes BELOW THIS COMMENT.
 -->
 
+### Changed
+
+#### Configuration Changes
+
+In this release, the schema version has changed from 23 to 24.
+
+- Properties starting with `log_`, and `verbose` property, which used to set up
+  logging are now moved to the new object `log` containing new properties `file`,
+  `max_backups`, `max_size`, `max_age`, `compress`, `local_time`, and `verbose`:
+
+  ```yaml
+  # BEFORE:
+  'log_file': ""
+  'log_max_backups': 0
+  'log_max_size': 100
+  'log_max_age': 3
+  'log_compress': false
+  'log_localtime': false
+  'verbose': false
+
+  # AFTER:
+  'log':
+    'file': ""
+    'max_backups': 0
+    'max_size': 100
+    'max_age': 3
+    'compress': false
+    'local_time': false
+    'verbose': false
+  ```
+
+  To rollback this change, remove the new object `log`, set back `log_` and
+  `verbose` properties and change the `schema_version` back to `23`.
+
 ### Fixed
 
 - Excessive RAM and CPU consumption by Safe Browsing and Parental Control

internal/home/config.go

@@ -30,32 +30,30 @@ import (
 const dataDir = "data"
 
 // logSettings are the logging settings part of the configuration file.
-//
-// TODO(a.garipov): Put them into a separate object.
 type logSettings struct {
 	// File is the path to the log file.  If empty, logs are written to stdout.
 	// If "syslog", logs are written to syslog.
-	File string `yaml:"log_file"`
+	File string `yaml:"file"`
 
 	// MaxBackups is the maximum number of old log files to retain.
 	//
 	// NOTE: MaxAge may still cause them to get deleted.
-	MaxBackups int `yaml:"log_max_backups"`
+	MaxBackups int `yaml:"max_backups"`
 
 	// MaxSize is the maximum size of the log file before it gets rotated, in
 	// megabytes.  The default value is 100 MB.
-	MaxSize int `yaml:"log_max_size"`
+	MaxSize int `yaml:"max_size"`
 
 	// MaxAge is the maximum duration for retaining old log files, in days.
-	MaxAge int `yaml:"log_max_age"`
+	MaxAge int `yaml:"max_age"`
 
 	// Compress determines, if the rotated log files should be compressed using
 	// gzip.
-	Compress bool `yaml:"log_compress"`
+	Compress bool `yaml:"compress"`
 
 	// LocalTime determines, if the time used for formatting the timestamps in
 	// is the computer's local time.
-	LocalTime bool `yaml:"log_localtime"`
+	LocalTime bool `yaml:"local_time"`
 
 	// Verbose determines, if verbose (aka debug) logging is enabled.
 	Verbose bool `yaml:"verbose"`
@@ -142,7 +140,8 @@ type configuration struct {
 	// Keep this field sorted to ensure consistent ordering.
 	Clients *clientsConfig `yaml:"clients"`
 
-	logSettings `yaml:",inline"`
+	// Log is a block with log configuration settings.
+	Log logSettings `yaml:"log"`
 
 	OSConfig *osConfig `yaml:"os"`
 
@@ -390,7 +389,7 @@ var config = &configuration{
 			HostsFile: true,
 		},
 	},
-	logSettings: logSettings{
+	Log: logSettings{
 		Compress:   false,
 		LocalTime:  false,
 		MaxBackups: 0,
@@ -421,19 +420,19 @@ func (c *configuration) getConfigFilename() string {
 // separate method in order to configure logger before the actual configuration
 // is parsed and applied.
 func readLogSettings() (ls *logSettings) {
-	ls = &logSettings{}
+	conf := &configuration{}
 
 	yamlFile, err := readConfigFile()
 	if err != nil {
-		return ls
+		return &logSettings{}
 	}
 
-	err = yaml.Unmarshal(yamlFile, ls)
+	err = yaml.Unmarshal(yamlFile, conf)
 	if err != nil {
 		log.Error("Couldn't get logging settings from the configuration: %s", err)
 	}
 
-	return ls
+	return &conf.Log
 }
 
 // validateBindHosts returns error if any of binding hosts from configuration is

internal/home/home.go

@@ -829,20 +829,21 @@ func configureLogger(opts options) (err error) {
 // getLogSettings returns a log settings object properly initialized from opts.
 func getLogSettings(opts options) (ls *logSettings) {
 	ls = readLogSettings()
+	configLogSettings := config.Log
 
 	// Command-line arguments can override config settings.
-	if opts.verbose || config.Verbose {
+	if opts.verbose || configLogSettings.Verbose {
 		ls.Verbose = true
 	}
 
-	ls.File = stringutil.Coalesce(opts.logFile, config.File, ls.File)
+	ls.File = stringutil.Coalesce(opts.logFile, configLogSettings.File, ls.File)
 
 	// Handle default log settings overrides.
-	ls.Compress = config.Compress
+	ls.Compress = configLogSettings.Compress
-	ls.LocalTime = config.LocalTime
+	ls.LocalTime = configLogSettings.LocalTime
-	ls.MaxBackups = config.MaxBackups
+	ls.MaxBackups = configLogSettings.MaxBackups
-	ls.MaxSize = config.MaxSize
+	ls.MaxSize = configLogSettings.MaxSize
-	ls.MaxAge = config.MaxAge
+	ls.MaxAge = configLogSettings.MaxAge
 
 	if opts.runningAsService && ls.File == "" && runtime.GOOS == "windows" {
 		// When running as a Windows service, use eventlog by default if

internal/home/upgrade.go

@@ -23,7 +23,7 @@ import (
 )
 
 // currentSchemaVersion is the current schema version.
-const currentSchemaVersion = 23
+const currentSchemaVersion = 24
 
 // These aliases are provided for convenience.
 type (
@@ -98,6 +98,7 @@ func upgradeConfigSchema(oldVersion int, diskConf yobj) (err error) {
 		upgradeSchema20to21,
 		upgradeSchema21to22,
 		upgradeSchema22to23,
+		upgradeSchema23to24,
 	}
 
 	n := 0
@@ -1325,6 +1326,110 @@ func upgradeSchema22to23(diskConf yobj) (err error) {
 	return nil
 }
 
+// upgradeSchema23to24 performs the following changes:
+//
+//	# BEFORE:
+//	'log_file': ""
+//	'log_max_backups': 0
+//	'log_max_size': 100
+//	'log_max_age': 3
+//	'log_compress': false
+//	'log_localtime': false
+//	'verbose': false
+//
+//	# AFTER:
+//	'log':
+//	  'file': ""
+//	  'max_backups': 0
+//	  'max_size': 100
+//	  'max_age': 3
+//	  'compress': false
+//	  'local_time': false
+//	  'verbose': false
+func upgradeSchema23to24(diskConf yobj) (err error) {
+	log.Printf("Upgrade yaml: 23 to 24")
+	diskConf["schema_version"] = 24
+
+	logObj := yobj{}
+	err = coalesceError(
+		moveField[string](diskConf, logObj, "log_file", "file"),
+		moveField[int](diskConf, logObj, "log_max_backups", "max_backups"),
+		moveField[int](diskConf, logObj, "log_max_size", "max_size"),
+		moveField[int](diskConf, logObj, "log_max_age", "max_age"),
+		moveField[bool](diskConf, logObj, "log_compress", "compress"),
+		moveField[bool](diskConf, logObj, "log_localtime", "local_time"),
+		moveField[bool](diskConf, logObj, "verbose", "verbose"),
+	)
+	if err != nil {
+		// Don't wrap the error, because it's informative enough as is.
+		return err
+	}
+
+	if len(logObj) != 0 {
+		diskConf["log"] = logObj
+	}
+
+	delete(diskConf, "log_file")
+	delete(diskConf, "log_max_backups")
+	delete(diskConf, "log_max_size")
+	delete(diskConf, "log_max_age")
+	delete(diskConf, "log_compress")
+	delete(diskConf, "log_localtime")
+	delete(diskConf, "verbose")
+
+	return nil
+}
+
+// moveField gets field value for key from diskConf, and then set this value
+// in newConf for newKey.
+func moveField[T any](diskConf, newConf yobj, key, newKey string) (err error) {
+	ok, newVal, err := fieldValue[T](diskConf, key)
+	if !ok {
+		return err
+	}
+
+	switch v := newVal.(type) {
+	case int, bool, string:
+		newConf[newKey] = v
+	default:
+		return fmt.Errorf("invalid type of %s: %T", key, newVal)
+	}
+
+	return nil
+}
+
+// fieldValue returns the value of type T for key in diskConf object.
+func fieldValue[T any](diskConf yobj, key string) (ok bool, field any, err error) {
+	fieldVal, ok := diskConf[key]
+	if !ok {
+		return false, new(T), nil
+	}
+
+	f, ok := fieldVal.(T)
+	if !ok {
+		return false, nil, fmt.Errorf("unexpected type of %s: %T", key, fieldVal)
+	}
+
+	return true, f, nil
+}
+
+// coalesceError returns the first non-nil error.  It is named after function
+// COALESCE in SQL.  If all errors are nil, it returns nil.
+//
+// TODO(a.garipov): Consider a similar helper to group errors together to show
+// as many errors as possible.
+//
+// TODO(a.garipov): Think of ways to merge with [aghalg.Coalesce].
+func coalesceError(errors ...error) (res error) {
+	for _, err := range errors {
+		if err != nil {
+			return err
+		}
+	}
+
+	return nil
+}
+
 // TODO(a.garipov): Replace with log.Output when we port it to our logging
 // package.
 func funcName() string {

internal/home/upgrade_test.go

@@ -1306,3 +1306,76 @@ func TestUpgradeSchema22to23(t *testing.T) {
 		})
 	}
 }
+
+func TestUpgradeSchema23to24(t *testing.T) {
+	const newSchemaVer = 24
+
+	testCases := []struct {
+		in         yobj
+		want       yobj
+		name       string
+		wantErrMsg string
+	}{{
+		name: "empty",
+		in:   yobj{},
+		want: yobj{
+			"schema_version": newSchemaVer,
+		},
+		wantErrMsg: "",
+	}, {
+		name: "ok",
+		in: yobj{
+			"log_file":        "/test/path.log",
+			"log_max_backups": 1,
+			"log_max_size":    2,
+			"log_max_age":     3,
+			"log_compress":    true,
+			"log_localtime":   true,
+			"verbose":         true,
+		},
+		want: yobj{
+			"log": yobj{
+				"file":        "/test/path.log",
+				"max_backups": 1,
+				"max_size":    2,
+				"max_age":     3,
+				"compress":    true,
+				"local_time":  true,
+				"verbose":     true,
+			},
+			"schema_version": newSchemaVer,
+		},
+		wantErrMsg: "",
+	}, {
+		name: "invalid",
+		in: yobj{
+			"log_file":        "/test/path.log",
+			"log_max_backups": 1,
+			"log_max_size":    2,
+			"log_max_age":     3,
+			"log_compress":    "",
+			"log_localtime":   true,
+			"verbose":         true,
+		},
+		want: yobj{
+			"log_file":        "/test/path.log",
+			"log_max_backups": 1,
+			"log_max_size":    2,
+			"log_max_age":     3,
+			"log_compress":    "",
+			"log_localtime":   true,
+			"verbose":         true,
+			"schema_version":  newSchemaVer,
+		},
+		wantErrMsg: "unexpected type of log_compress: string",
+	}}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			err := upgradeSchema23to24(tc.in)
+			testutil.AssertErrorMsg(t, tc.wantErrMsg, err)
+
+			assert.Equal(t, tc.want, tc.in)
+		})
+	}
+}