From cdf9ccd371317f49c78fa06795d6ba2d360ac40f Mon Sep 17 00:00:00 2001 From: Dimitry Kolyshev Date: Fri, 5 Jul 2024 16:19:27 +0300 Subject: [PATCH] all: partial revert slog logger usage --- internal/aghos/syslog.go | 6 ++-- internal/aghos/syslog_others.go | 13 ++++---- internal/aghos/syslog_windows.go | 23 +++++++------- internal/home/home.go | 18 +++++------ internal/home/log.go | 51 ++++---------------------------- internal/next/cmd/log.go | 6 +--- 6 files changed, 33 insertions(+), 84 deletions(-) diff --git a/internal/aghos/syslog.go b/internal/aghos/syslog.go index 5e58bdcb..a67a5635 100644 --- a/internal/aghos/syslog.go +++ b/internal/aghos/syslog.go @@ -1,8 +1,6 @@ package aghos -import "io" - -// ConfigureSyslog returns an output rerouted to syslog. -func ConfigureSyslog(serviceName string) (w io.Writer, err error) { +// ConfigureSyslog reroutes standard logger output to syslog. +func ConfigureSyslog(serviceName string) error { return configureSyslog(serviceName) } diff --git a/internal/aghos/syslog_others.go b/internal/aghos/syslog_others.go index 4242dd84..1659ae49 100644 --- a/internal/aghos/syslog_others.go +++ b/internal/aghos/syslog_others.go @@ -3,15 +3,16 @@ package aghos import ( - "io" "log/syslog" + + "github.com/AdguardTeam/golibs/log" ) -func configureSyslog(serviceName string) (w io.Writer, err error) { - w, err = syslog.New(syslog.LOG_NOTICE|syslog.LOG_USER, serviceName) +func configureSyslog(serviceName string) error { + w, err := syslog.New(syslog.LOG_NOTICE|syslog.LOG_USER, serviceName) if err != nil { - return nil, err + return err } - - return w, nil + log.SetOutput(w) + return nil } diff --git a/internal/aghos/syslog_windows.go b/internal/aghos/syslog_windows.go index 21de19c1..c8e86e78 100644 --- a/internal/aghos/syslog_windows.go +++ b/internal/aghos/syslog_windows.go @@ -3,9 +3,9 @@ package aghos import ( - "io" "strings" + "github.com/AdguardTeam/golibs/log" "golang.org/x/sys/windows" "golang.org/x/sys/windows/svc/eventlog" ) @@ -19,26 +19,23 @@ func (w *eventLogWriter) Write(b []byte) (int, error) { return len(b), w.el.Info(1, string(b)) } -func configureSyslog(serviceName string) (w io.Writer, err error) { - // Note that the eventlog src is the same as the service name. Otherwise, - // we will get "the description for event id cannot be found" warning in - // every log record. +func configureSyslog(serviceName string) error { + // Note that the eventlog src is the same as the service name + // Otherwise, we will get "the description for event id cannot be found" warning in every log record // Continue if we receive "registry key already exists" or if we get // ERROR_ACCESS_DENIED so that we can log without administrative permissions // for pre-existing eventlog sources. - err = eventlog.InstallAsEventCreate(serviceName, eventlog.Info|eventlog.Warning|eventlog.Error) - if err != nil { - if !strings.Contains(err.Error(), "registry key already exists") && - err != windows.ERROR_ACCESS_DENIED { - return nil, err + if err := eventlog.InstallAsEventCreate(serviceName, eventlog.Info|eventlog.Warning|eventlog.Error); err != nil { + if !strings.Contains(err.Error(), "registry key already exists") && err != windows.ERROR_ACCESS_DENIED { + return err } } - el, err := eventlog.Open(serviceName) if err != nil { - return nil, err + return err } - return &eventLogWriter{el: el}, nil + log.SetOutput(&eventLogWriter{el: el}) + return nil } diff --git a/internal/home/home.go b/internal/home/home.go index 0661fb49..74ca6583 100644 --- a/internal/home/home.go +++ b/internal/home/home.go @@ -39,7 +39,6 @@ import ( "github.com/AdguardTeam/golibs/errors" "github.com/AdguardTeam/golibs/hostsfile" "github.com/AdguardTeam/golibs/log" - "github.com/AdguardTeam/golibs/logutil/slogutil" "github.com/AdguardTeam/golibs/netutil" "github.com/AdguardTeam/golibs/osutil" ) @@ -559,15 +558,14 @@ func run(opts options, clientBuildFS fs.FS, done chan struct{}) { err = configureLogger(ls) fatalOnError(err) - // Configure slog logger. - l, err := initLogger(ls) - fatalOnError(err) + // TODO(a.garipov): Use slog everywhere. + l := initLogger(ls) // Print the first message after logger is configured. - l.Info(version.Full()) - l.Debug("current working directory is set", "work_dir", Context.workDir) + log.Info(version.Full()) + log.Debug("current working directory is %s", Context.workDir) if opts.runningAsService { - l.Info("AdGuard Home is running as a service") + log.Info("AdGuard Home is running as a service") } err = setupContext(opts, l) @@ -598,7 +596,7 @@ func run(opts options, clientBuildFS fs.FS, done chan struct{}) { } confPath := configFilePath() - l.Debug("using config path for updater", "path", confPath) + log.Debug("using config path %q for updater", confPath) upd := updater.NewUpdater(&updater.Config{ Client: config.Filtering.HTTPClient, @@ -640,7 +638,7 @@ func run(opts options, clientBuildFS fs.FS, done chan struct{}) { Context.tls, err = newTLSManager(config.TLS, config.DNS.ServePlainDNS) if err != nil { - l.Error("initializing tls", slogutil.KeyError, err) + log.Error("initializing tls: %s", err) onConfigModified() } @@ -664,7 +662,7 @@ func run(opts options, clientBuildFS fs.FS, done chan struct{}) { if Context.dhcpServer != nil { err = Context.dhcpServer.Start() if err != nil { - l.Error("starting dhcp server", slogutil.KeyError, err) + log.Error("starting dhcp server: %s", err) } } } diff --git a/internal/home/log.go b/internal/home/log.go index a83b9782..13b3aa53 100644 --- a/internal/home/log.go +++ b/internal/home/log.go @@ -3,7 +3,6 @@ package home import ( "cmp" "fmt" - "io" "log/slog" "path/filepath" "runtime" @@ -20,53 +19,16 @@ import ( const configSyslog = "syslog" // initLogger returns new [*slog.Logger] configured with the given settings. -func initLogger(ls *logSettings) (l *slog.Logger, err error) { +func initLogger(ls *logSettings) (l *slog.Logger) { if !ls.Enabled { - return slogutil.NewDiscardLogger(), nil - } - - w, err := logOutput(ls) - if err != nil { - return nil, fmt.Errorf("cannot initialize log output: %w", err) + return slogutil.NewDiscardLogger() } return slogutil.New(&slogutil.Config{ - Output: w, - Format: slogutil.FormatDefault, + Format: slogutil.FormatAdGuardLegacy, AddTimestamp: true, Verbose: ls.Verbose, - }), nil -} - -// logOutput returns a log output [io.Writer] configured with the settings. -func logOutput(ls *logSettings) (w io.Writer, err error) { - if ls.File == "" { - return nil, nil - } - - if ls.File == configSyslog { - // Use syslog where it is possible and eventlog on Windows. - w, err = aghos.ConfigureSyslog(serviceName) - if err != nil { - return nil, fmt.Errorf("cannot initialize syslog: %w", err) - } - - return w, nil - } - - logFilePath := ls.File - if !filepath.IsAbs(logFilePath) { - logFilePath = filepath.Join(Context.workDir, logFilePath) - } - - return &lumberjack.Logger{ - Filename: logFilePath, - Compress: ls.Compress, - LocalTime: ls.LocalTime, - MaxBackups: ls.MaxBackups, - MaxSize: ls.MaxSize, - MaxAge: ls.MaxAge, - }, nil + }) } // configureLogger configures logger level and output. @@ -89,14 +51,11 @@ func configureLogger(ls *logSettings) (err error) { if ls.File == configSyslog { // Use syslog where it is possible and eventlog on Windows. - var w io.Writer - w, err = aghos.ConfigureSyslog(serviceName) + err = aghos.ConfigureSyslog(serviceName) if err != nil { return fmt.Errorf("cannot initialize syslog: %w", err) } - log.SetOutput(w) - return nil } diff --git a/internal/next/cmd/log.go b/internal/next/cmd/log.go index 6c834082..3aa2a0e5 100644 --- a/internal/next/cmd/log.go +++ b/internal/next/cmd/log.go @@ -2,7 +2,6 @@ package cmd import ( "fmt" - "io" "os" "github.com/AdguardTeam/AdGuardHome/internal/aghos" @@ -23,13 +22,10 @@ func setLog(opts *options) (err error) { case "stderr": log.SetOutput(os.Stderr) case "syslog": - var w io.Writer - w, err = aghos.ConfigureSyslog(syslogServiceName) + err = aghos.ConfigureSyslog(syslogServiceName) if err != nil { return fmt.Errorf("initializing syslog: %w", err) } - - log.SetOutput(w) default: // TODO(a.garipov): Use the path. }