Pull request: 6093-log-conf

Updates #6093.

Squashed commit of the following:

commit f3478b9ad30a025b2a4044ab4ca54517568c833d
Author: Dimitry Kolyshev <dkolyshev@adguard.com>
Date:   Fri Aug 11 15:27:51 2023 +0300

    home: imp code

commit 07b51d83eb1491f33ef959406495f3154aadd774
Author: Dimitry Kolyshev <dkolyshev@adguard.com>
Date:   Fri Aug 11 15:18:00 2023 +0300

    all: imp docs

commit 4892c8673af738d9f1b2abd5b1da854f60439bcf
Merge: 7cb376c95 c54635e8a
Author: Dimitry Kolyshev <dkolyshev@adguard.com>
Date:   Fri Aug 11 13:57:02 2023 +0300

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

commit 7cb376c953222437aba67120ab14f74341260b20
Author: Dimitry Kolyshev <dkolyshev@adguard.com>
Date:   Fri Aug 11 12:48:26 2023 +0300

    all: docs

commit 89f0d46bd99cebde234e56db6867fdf371d8191b
Merge: 67c776763 94cf50a53
Author: Dimitry Kolyshev <dkolyshev@adguard.com>
Date:   Fri Aug 11 12:46:26 2023 +0300

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

commit 67c7767631da510bcb05856a6556fb6aff172fda
Author: Dimitry Kolyshev <dkolyshev@adguard.com>
Date:   Fri Aug 11 12:39:26 2023 +0300

    home: log conf

CHANGELOG.md

@@ -64,6 +64,7 @@ In this release, the schema version has changed from 24 to 25.
 
 ### Fixed
 
+- File log configuration, such as `max_size`, being ignored ([#6093]). 
 - Panic on using a single-slash filtering rule.
 - Panic on shutting down while DNS requests are in process of filtering
   ([#5948]).
@@ -72,6 +73,7 @@ In this release, the schema version has changed from 24 to 25.
 [#5948]: https://github.com/AdguardTeam/AdGuardHome/issues/5948
 [#6020]: https://github.com/AdguardTeam/AdGuardHome/issues/6020
 [#6053]: https://github.com/AdguardTeam/AdGuardHome/issues/6053
+[#6093]: https://github.com/AdguardTeam/AdGuardHome/issues/6093
 
 <!--
 NOTE: Add new changes ABOVE THIS COMMENT.

internal/home/config.go

@@ -431,25 +431,6 @@ func (c *configuration) getConfigFilename() string {
 	return configFile
 }
 
-// readLogSettings reads logging settings from the config file.  We do it in a
-// separate method in order to configure logger before the actual configuration
-// is parsed and applied.
-func readLogSettings() (ls *logSettings) {
-	conf := &configuration{}
-
-	yamlFile, err := readConfigFile()
-	if err != nil {
-		return &logSettings{}
-	}
-
-	err = yaml.Unmarshal(yamlFile, conf)
-	if err != nil {
-		log.Error("Couldn't get logging settings from the configuration: %s", err)
-	}
-
-	return &conf.Log
-}
-
 // validateBindHosts returns error if any of binding hosts from configuration is
 // not a valid IP address.
 func validateBindHosts(conf *configuration) (err error) {

internal/home/home.go

@@ -37,12 +37,6 @@ import (
 	"github.com/AdguardTeam/golibs/netutil"
 	"github.com/AdguardTeam/golibs/stringutil"
 	"golang.org/x/exp/slices"
-	"gopkg.in/natefinch/lumberjack.v2"
-)
-
-const (
-	// Used in config to indicate that syslog or eventlog (win) should be used for logger output
-	configSyslog = "syslog"
 )
 
 // Global context
@@ -747,79 +741,6 @@ func initWorkingDir(opts options) (err error) {
 	return nil
 }
 
-// configureLogger configures logger level and output.
-func configureLogger(opts options) (err error) {
-	ls := getLogSettings(opts)
-
-	// Configure logger level.
-	if ls.Verbose {
-		log.SetLevel(log.DEBUG)
-	}
-
-	// Make sure that we see the microseconds in logs, as networking stuff can
-	// happen pretty quickly.
-	log.SetFlags(log.LstdFlags | log.Lmicroseconds)
-
-	// Write logs to stdout by default.
-	if ls.File == "" {
-		return nil
-	}
-
-	if ls.File == configSyslog {
-		// Use syslog where it is possible and eventlog on Windows.
-		err = aghos.ConfigureSyslog(serviceName)
-		if err != nil {
-			return fmt.Errorf("cannot initialize syslog: %w", err)
-		}
-
-		return nil
-	}
-
-	logFilePath := ls.File
-	if !filepath.IsAbs(logFilePath) {
-		logFilePath = filepath.Join(Context.workDir, logFilePath)
-	}
-
-	log.SetOutput(&lumberjack.Logger{
-		Filename:   logFilePath,
-		Compress:   ls.Compress,
-		LocalTime:  ls.LocalTime,
-		MaxBackups: ls.MaxBackups,
-		MaxSize:    ls.MaxSize,
-		MaxAge:     ls.MaxAge,
-	})
-
-	return nil
-}
-
-// 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 || configLogSettings.Verbose {
-		ls.Verbose = true
-	}
-
-	ls.File = stringutil.Coalesce(opts.logFile, configLogSettings.File, ls.File)
-
-	// Handle default log settings overrides.
-	ls.Compress = configLogSettings.Compress
-	ls.LocalTime = configLogSettings.LocalTime
-	ls.MaxBackups = configLogSettings.MaxBackups
-	ls.MaxSize = configLogSettings.MaxSize
-	ls.MaxAge = configLogSettings.MaxAge
-
-	if opts.runningAsService && ls.File == "" && runtime.GOOS == "windows" {
-		// When running as a Windows service, use eventlog by default if
-		// nothing else is configured.  Otherwise, we'll lose the log output.
-		ls.File = configSyslog
-	}
-
-	return ls
-}
-
 // cleanup stops and resets all the modules.
 func cleanup(ctx context.Context) {
 	log.Info("stopping AdGuard Home")

internal/home/log.go

@@ -0,0 +1,106 @@
+package home
+
+import (
+	"fmt"
+	"path/filepath"
+	"runtime"
+
+	"github.com/AdguardTeam/AdGuardHome/internal/aghos"
+	"github.com/AdguardTeam/golibs/log"
+	"github.com/AdguardTeam/golibs/stringutil"
+	"gopkg.in/natefinch/lumberjack.v2"
+	"gopkg.in/yaml.v3"
+)
+
+// configSyslog is used to indicate that syslog or eventlog (win) should be used
+// for logger output.
+const configSyslog = "syslog"
+
+// configureLogger configures logger level and output.
+func configureLogger(opts options) (err error) {
+	ls := getLogSettings(opts)
+
+	// Configure logger level.
+	if ls.Verbose {
+		log.SetLevel(log.DEBUG)
+	}
+
+	// Make sure that we see the microseconds in logs, as networking stuff can
+	// happen pretty quickly.
+	log.SetFlags(log.LstdFlags | log.Lmicroseconds)
+
+	// Write logs to stdout by default.
+	if ls.File == "" {
+		return nil
+	}
+
+	if ls.File == configSyslog {
+		// Use syslog where it is possible and eventlog on Windows.
+		err = aghos.ConfigureSyslog(serviceName)
+		if err != nil {
+			return fmt.Errorf("cannot initialize syslog: %w", err)
+		}
+
+		return nil
+	}
+
+	logFilePath := ls.File
+	if !filepath.IsAbs(logFilePath) {
+		logFilePath = filepath.Join(Context.workDir, logFilePath)
+	}
+
+	log.SetOutput(&lumberjack.Logger{
+		Filename:   logFilePath,
+		Compress:   ls.Compress,
+		LocalTime:  ls.LocalTime,
+		MaxBackups: ls.MaxBackups,
+		MaxSize:    ls.MaxSize,
+		MaxAge:     ls.MaxAge,
+	})
+
+	return nil
+}
+
+// getLogSettings returns a log settings object properly initialized from opts.
+func getLogSettings(opts options) (ls *logSettings) {
+	configLogSettings := config.Log
+
+	ls = readLogSettings()
+	if ls == nil {
+		// Use default log settings.
+		ls = &configLogSettings
+	}
+
+	// Command-line arguments can override config settings.
+	if opts.verbose {
+		ls.Verbose = true
+	}
+	ls.File = stringutil.Coalesce(opts.logFile, ls.File)
+
+	if opts.runningAsService && ls.File == "" && runtime.GOOS == "windows" {
+		// When running as a Windows service, use eventlog by default if
+		// nothing else is configured.  Otherwise, we'll lose the log output.
+		ls.File = configSyslog
+	}
+
+	return ls
+}
+
+// readLogSettings reads logging settings from the config file.  We do it in a
+// separate method in order to configure logger before the actual configuration
+// is parsed and applied.
+func readLogSettings() (ls *logSettings) {
+	conf := &configuration{}
+
+	yamlFile, err := readConfigFile()
+	if err != nil {
+		return nil
+	}
+
+	err = yaml.Unmarshal(yamlFile, conf)
+	if err != nil {
+		log.Error("Couldn't get logging settings from the configuration: %s", err)
+	}
+
+	return &conf.Log
+}