From 33c401a16e36ce27900cacff3a7c07ed49b1b9d1 Mon Sep 17 00:00:00 2001 From: Daniel Nephin Date: Wed, 19 Aug 2020 12:09:35 -0400 Subject: [PATCH] logging: Setup accept io.Writer instead of []io.Writer Also accept a non-pointer Config, since the config is not modified --- agent/setup.go | 4 ++-- command/connect/proxy/proxy.go | 5 ++--- logging/logger.go | 27 +++++++------------------- logging/logger_test.go | 35 +++++++++++++++------------------- 4 files changed, 26 insertions(+), 45 deletions(-) diff --git a/agent/setup.go b/agent/setup.go index 9554ef7b5a..fcaac84766 100644 --- a/agent/setup.go +++ b/agent/setup.go @@ -51,7 +51,7 @@ func NewBaseDeps(configLoader ConfigLoader, logOut io.Writer) (BaseDeps, error) } // TODO: use logging.Config in RuntimeConfig instead of separate fields - logConf := &logging.Config{ + logConf := logging.Config{ LogLevel: cfg.LogLevel, LogJSON: cfg.LogJSON, Name: logging.Agent, @@ -62,7 +62,7 @@ func NewBaseDeps(configLoader ConfigLoader, logOut io.Writer) (BaseDeps, error) LogRotateBytes: cfg.LogRotateBytes, LogRotateMaxFiles: cfg.LogRotateMaxFiles, } - d.Logger, err = logging.Setup(logConf, []io.Writer{logOut}) + d.Logger, err = logging.Setup(logConf, logOut) if err != nil { return d, err } diff --git a/command/connect/proxy/proxy.go b/command/connect/proxy/proxy.go index 8bc5bbb888..028d39c930 100644 --- a/command/connect/proxy/proxy.go +++ b/command/connect/proxy/proxy.go @@ -3,7 +3,6 @@ package proxy import ( "flag" "fmt" - "io" "log" "net" "net/http" @@ -132,7 +131,7 @@ func (c *cmd) Run(args []string) int { } // Setup the log outputs - logConfig := &logging.Config{ + logConfig := logging.Config{ LogLevel: c.logLevel, Name: logging.Proxy, LogJSON: c.logJSON, @@ -140,7 +139,7 @@ func (c *cmd) Run(args []string) int { logGate := logging.GatedWriter{Writer: &cli.UiWriter{Ui: c.UI}} - logger, err := logging.Setup(logConfig, []io.Writer{&logGate}) + logger, err := logging.Setup(logConfig, &logGate) if err != nil { c.UI.Error(err.Error()) return 1 diff --git a/logging/logger.go b/logging/logger.go index e9988ffb07..a528c92796 100644 --- a/logging/logger.go +++ b/logging/logger.go @@ -52,34 +52,25 @@ var ( type LogSetupErrorFn func(string) -// Setup is used to perform setup of several logging objects: +// Setup logging from Config, and return an hclog Logger. // -// * A hclog.Logger is used to perform filtering by log level and write to io.Writer. -// * A GatedWriter is used to buffer logs until startup UI operations are -// complete. After this is flushed then logs flow directly to output -// destinations. -// * An io.Writer is provided as the sink for all logs to flow to. -// -// The provided ui object will get any log messages related to setting up -// logging itself, and will also be hooked up to the gated logger. The final bool -// parameter indicates if logging was set up successfully. -// TODO: accept a single io.Writer -func Setup(config *Config, writers []io.Writer) (hclog.InterceptLogger, error) { +// Logs may be written to out, and optionally to syslog, and a file. +func Setup(config Config, out io.Writer) (hclog.InterceptLogger, error) { if !ValidateLogLevel(config.LogLevel) { return nil, fmt.Errorf("Invalid log level: %s. Valid log levels are: %v", config.LogLevel, allowedLogLevels) } - // Set up syslog if it's enabled. - var syslog io.Writer + writers := []io.Writer{out} + if config.EnableSyslog { retries := 12 delay := 5 * time.Second for i := 0; i <= retries; i++ { - l, err := gsyslog.NewLogger(gsyslog.LOG_NOTICE, config.SyslogFacility, "consul") + syslog, err := gsyslog.NewLogger(gsyslog.LOG_NOTICE, config.SyslogFacility, "consul") if err == nil { - syslog = &SyslogWrapper{l} + writers = append(writers, syslog) break } @@ -92,10 +83,6 @@ func Setup(config *Config, writers []io.Writer) (hclog.InterceptLogger, error) { } } - if syslog != nil { - writers = append(writers, syslog) - } - // Create a file logger if the user has specified the path to the log file if config.LogFilePath != "" { dir, fileName := filepath.Split(config.LogFilePath) diff --git a/logging/logger_test.go b/logging/logger_test.go index f6ffc78802..c6bea09191 100644 --- a/logging/logger_test.go +++ b/logging/logger_test.go @@ -4,7 +4,6 @@ import ( "bytes" "encoding/json" "errors" - "io" "os" "testing" @@ -15,9 +14,7 @@ import ( func TestLogger_SetupBasic(t *testing.T) { t.Parallel() require := require.New(t) - cfg := &Config{ - LogLevel: "INFO", - } + cfg := Config{LogLevel: "INFO"} logger, err := Setup(cfg, nil) require.NoError(err) @@ -26,7 +23,7 @@ func TestLogger_SetupBasic(t *testing.T) { func TestLogger_SetupInvalidLogLevel(t *testing.T) { t.Parallel() - cfg := &Config{} + cfg := Config{} _, err := Setup(cfg, nil) testutil.RequireErrorContains(t, err, "Invalid log level") @@ -61,7 +58,7 @@ func TestLogger_SetupLoggerErrorLevel(t *testing.T) { require := require.New(t) var buf bytes.Buffer - logger, err := Setup(&cfg, []io.Writer{&buf}) + logger, err := Setup(cfg, &buf) require.NoError(err) require.NotNil(logger) @@ -79,12 +76,10 @@ func TestLogger_SetupLoggerErrorLevel(t *testing.T) { func TestLogger_SetupLoggerDebugLevel(t *testing.T) { t.Parallel() require := require.New(t) - cfg := &Config{ - LogLevel: "DEBUG", - } + cfg := Config{LogLevel: "DEBUG"} var buf bytes.Buffer - logger, err := Setup(cfg, []io.Writer{&buf}) + logger, err := Setup(cfg, &buf) require.NoError(err) require.NotNil(logger) @@ -100,13 +95,13 @@ func TestLogger_SetupLoggerDebugLevel(t *testing.T) { func TestLogger_SetupLoggerWithName(t *testing.T) { t.Parallel() require := require.New(t) - cfg := &Config{ + cfg := Config{ LogLevel: "DEBUG", Name: "test-system", } var buf bytes.Buffer - logger, err := Setup(cfg, []io.Writer{&buf}) + logger, err := Setup(cfg, &buf) require.NoError(err) require.NotNil(logger) @@ -118,14 +113,14 @@ func TestLogger_SetupLoggerWithName(t *testing.T) { func TestLogger_SetupLoggerWithJSON(t *testing.T) { t.Parallel() require := require.New(t) - cfg := &Config{ + cfg := Config{ LogLevel: "DEBUG", LogJSON: true, Name: "test-system", } var buf bytes.Buffer - logger, err := Setup(cfg, []io.Writer{&buf}) + logger, err := Setup(cfg, &buf) require.NoError(err) require.NotNil(logger) @@ -146,13 +141,13 @@ func TestLogger_SetupLoggerWithValidLogPath(t *testing.T) { tmpDir := testutil.TempDir(t, t.Name()) - cfg := &Config{ + cfg := Config{ LogLevel: "INFO", LogFilePath: tmpDir + "/", } var buf bytes.Buffer - logger, err := Setup(cfg, []io.Writer{&buf}) + logger, err := Setup(cfg, &buf) require.NoError(err) require.NotNil(logger) } @@ -161,13 +156,13 @@ func TestLogger_SetupLoggerWithInValidLogPath(t *testing.T) { t.Parallel() require := require.New(t) - cfg := &Config{ + cfg := Config{ LogLevel: "INFO", LogFilePath: "nonexistentdir/", } var buf bytes.Buffer - logger, err := Setup(cfg, []io.Writer{&buf}) + logger, err := Setup(cfg, &buf) require.Error(err) require.True(errors.Is(err, os.ErrNotExist)) require.Nil(logger) @@ -182,13 +177,13 @@ func TestLogger_SetupLoggerWithInValidLogPathPermission(t *testing.T) { os.Mkdir(tmpDir, 0000) defer os.RemoveAll(tmpDir) - cfg := &Config{ + cfg := Config{ LogLevel: "INFO", LogFilePath: tmpDir + "/", } var buf bytes.Buffer - logger, err := Setup(cfg, []io.Writer{&buf}) + logger, err := Setup(cfg, &buf) require.Error(err) require.True(errors.Is(err, os.ErrPermission)) require.Nil(logger)