logging: Setup accept io.Writer instead of []io.Writer

Also accept a non-pointer Config, since the config is not modified
This commit is contained in:
Daniel Nephin 2020-08-19 12:09:35 -04:00
parent 2128f40fc2
commit 33c401a16e
4 changed files with 26 additions and 45 deletions

View File

@ -51,7 +51,7 @@ func NewBaseDeps(configLoader ConfigLoader, logOut io.Writer) (BaseDeps, error)
} }
// TODO: use logging.Config in RuntimeConfig instead of separate fields // TODO: use logging.Config in RuntimeConfig instead of separate fields
logConf := &logging.Config{ logConf := logging.Config{
LogLevel: cfg.LogLevel, LogLevel: cfg.LogLevel,
LogJSON: cfg.LogJSON, LogJSON: cfg.LogJSON,
Name: logging.Agent, Name: logging.Agent,
@ -62,7 +62,7 @@ func NewBaseDeps(configLoader ConfigLoader, logOut io.Writer) (BaseDeps, error)
LogRotateBytes: cfg.LogRotateBytes, LogRotateBytes: cfg.LogRotateBytes,
LogRotateMaxFiles: cfg.LogRotateMaxFiles, LogRotateMaxFiles: cfg.LogRotateMaxFiles,
} }
d.Logger, err = logging.Setup(logConf, []io.Writer{logOut}) d.Logger, err = logging.Setup(logConf, logOut)
if err != nil { if err != nil {
return d, err return d, err
} }

View File

@ -3,7 +3,6 @@ package proxy
import ( import (
"flag" "flag"
"fmt" "fmt"
"io"
"log" "log"
"net" "net"
"net/http" "net/http"
@ -132,7 +131,7 @@ func (c *cmd) Run(args []string) int {
} }
// Setup the log outputs // Setup the log outputs
logConfig := &logging.Config{ logConfig := logging.Config{
LogLevel: c.logLevel, LogLevel: c.logLevel,
Name: logging.Proxy, Name: logging.Proxy,
LogJSON: c.logJSON, LogJSON: c.logJSON,
@ -140,7 +139,7 @@ func (c *cmd) Run(args []string) int {
logGate := logging.GatedWriter{Writer: &cli.UiWriter{Ui: c.UI}} 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 { if err != nil {
c.UI.Error(err.Error()) c.UI.Error(err.Error())
return 1 return 1

View File

@ -52,34 +52,25 @@ var (
type LogSetupErrorFn func(string) 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. // Logs may be written to out, and optionally to syslog, and a file.
// * A GatedWriter is used to buffer logs until startup UI operations are func Setup(config Config, out io.Writer) (hclog.InterceptLogger, error) {
// 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) {
if !ValidateLogLevel(config.LogLevel) { if !ValidateLogLevel(config.LogLevel) {
return nil, fmt.Errorf("Invalid log level: %s. Valid log levels are: %v", return nil, fmt.Errorf("Invalid log level: %s. Valid log levels are: %v",
config.LogLevel, config.LogLevel,
allowedLogLevels) allowedLogLevels)
} }
// Set up syslog if it's enabled. writers := []io.Writer{out}
var syslog io.Writer
if config.EnableSyslog { if config.EnableSyslog {
retries := 12 retries := 12
delay := 5 * time.Second delay := 5 * time.Second
for i := 0; i <= retries; i++ { 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 { if err == nil {
syslog = &SyslogWrapper{l} writers = append(writers, syslog)
break 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 // Create a file logger if the user has specified the path to the log file
if config.LogFilePath != "" { if config.LogFilePath != "" {
dir, fileName := filepath.Split(config.LogFilePath) dir, fileName := filepath.Split(config.LogFilePath)

View File

@ -4,7 +4,6 @@ import (
"bytes" "bytes"
"encoding/json" "encoding/json"
"errors" "errors"
"io"
"os" "os"
"testing" "testing"
@ -15,9 +14,7 @@ import (
func TestLogger_SetupBasic(t *testing.T) { func TestLogger_SetupBasic(t *testing.T) {
t.Parallel() t.Parallel()
require := require.New(t) require := require.New(t)
cfg := &Config{ cfg := Config{LogLevel: "INFO"}
LogLevel: "INFO",
}
logger, err := Setup(cfg, nil) logger, err := Setup(cfg, nil)
require.NoError(err) require.NoError(err)
@ -26,7 +23,7 @@ func TestLogger_SetupBasic(t *testing.T) {
func TestLogger_SetupInvalidLogLevel(t *testing.T) { func TestLogger_SetupInvalidLogLevel(t *testing.T) {
t.Parallel() t.Parallel()
cfg := &Config{} cfg := Config{}
_, err := Setup(cfg, nil) _, err := Setup(cfg, nil)
testutil.RequireErrorContains(t, err, "Invalid log level") testutil.RequireErrorContains(t, err, "Invalid log level")
@ -61,7 +58,7 @@ func TestLogger_SetupLoggerErrorLevel(t *testing.T) {
require := require.New(t) require := require.New(t)
var buf bytes.Buffer var buf bytes.Buffer
logger, err := Setup(&cfg, []io.Writer{&buf}) logger, err := Setup(cfg, &buf)
require.NoError(err) require.NoError(err)
require.NotNil(logger) require.NotNil(logger)
@ -79,12 +76,10 @@ func TestLogger_SetupLoggerErrorLevel(t *testing.T) {
func TestLogger_SetupLoggerDebugLevel(t *testing.T) { func TestLogger_SetupLoggerDebugLevel(t *testing.T) {
t.Parallel() t.Parallel()
require := require.New(t) require := require.New(t)
cfg := &Config{ cfg := Config{LogLevel: "DEBUG"}
LogLevel: "DEBUG",
}
var buf bytes.Buffer var buf bytes.Buffer
logger, err := Setup(cfg, []io.Writer{&buf}) logger, err := Setup(cfg, &buf)
require.NoError(err) require.NoError(err)
require.NotNil(logger) require.NotNil(logger)
@ -100,13 +95,13 @@ func TestLogger_SetupLoggerDebugLevel(t *testing.T) {
func TestLogger_SetupLoggerWithName(t *testing.T) { func TestLogger_SetupLoggerWithName(t *testing.T) {
t.Parallel() t.Parallel()
require := require.New(t) require := require.New(t)
cfg := &Config{ cfg := Config{
LogLevel: "DEBUG", LogLevel: "DEBUG",
Name: "test-system", Name: "test-system",
} }
var buf bytes.Buffer var buf bytes.Buffer
logger, err := Setup(cfg, []io.Writer{&buf}) logger, err := Setup(cfg, &buf)
require.NoError(err) require.NoError(err)
require.NotNil(logger) require.NotNil(logger)
@ -118,14 +113,14 @@ func TestLogger_SetupLoggerWithName(t *testing.T) {
func TestLogger_SetupLoggerWithJSON(t *testing.T) { func TestLogger_SetupLoggerWithJSON(t *testing.T) {
t.Parallel() t.Parallel()
require := require.New(t) require := require.New(t)
cfg := &Config{ cfg := Config{
LogLevel: "DEBUG", LogLevel: "DEBUG",
LogJSON: true, LogJSON: true,
Name: "test-system", Name: "test-system",
} }
var buf bytes.Buffer var buf bytes.Buffer
logger, err := Setup(cfg, []io.Writer{&buf}) logger, err := Setup(cfg, &buf)
require.NoError(err) require.NoError(err)
require.NotNil(logger) require.NotNil(logger)
@ -146,13 +141,13 @@ func TestLogger_SetupLoggerWithValidLogPath(t *testing.T) {
tmpDir := testutil.TempDir(t, t.Name()) tmpDir := testutil.TempDir(t, t.Name())
cfg := &Config{ cfg := Config{
LogLevel: "INFO", LogLevel: "INFO",
LogFilePath: tmpDir + "/", LogFilePath: tmpDir + "/",
} }
var buf bytes.Buffer var buf bytes.Buffer
logger, err := Setup(cfg, []io.Writer{&buf}) logger, err := Setup(cfg, &buf)
require.NoError(err) require.NoError(err)
require.NotNil(logger) require.NotNil(logger)
} }
@ -161,13 +156,13 @@ func TestLogger_SetupLoggerWithInValidLogPath(t *testing.T) {
t.Parallel() t.Parallel()
require := require.New(t) require := require.New(t)
cfg := &Config{ cfg := Config{
LogLevel: "INFO", LogLevel: "INFO",
LogFilePath: "nonexistentdir/", LogFilePath: "nonexistentdir/",
} }
var buf bytes.Buffer var buf bytes.Buffer
logger, err := Setup(cfg, []io.Writer{&buf}) logger, err := Setup(cfg, &buf)
require.Error(err) require.Error(err)
require.True(errors.Is(err, os.ErrNotExist)) require.True(errors.Is(err, os.ErrNotExist))
require.Nil(logger) require.Nil(logger)
@ -182,13 +177,13 @@ func TestLogger_SetupLoggerWithInValidLogPathPermission(t *testing.T) {
os.Mkdir(tmpDir, 0000) os.Mkdir(tmpDir, 0000)
defer os.RemoveAll(tmpDir) defer os.RemoveAll(tmpDir)
cfg := &Config{ cfg := Config{
LogLevel: "INFO", LogLevel: "INFO",
LogFilePath: tmpDir + "/", LogFilePath: tmpDir + "/",
} }
var buf bytes.Buffer var buf bytes.Buffer
logger, err := Setup(cfg, []io.Writer{&buf}) logger, err := Setup(cfg, &buf)
require.Error(err) require.Error(err)
require.True(errors.Is(err, os.ErrPermission)) require.True(errors.Is(err, os.ErrPermission))
require.Nil(logger) require.Nil(logger)