diff --git a/agent/acl_test.go b/agent/acl_test.go index f8bfdb3d17..3d25b965a8 100644 --- a/agent/acl_test.go +++ b/agent/acl_test.go @@ -37,10 +37,6 @@ type TestACLAgent struct { // when Shutdown() is called. Config *config.RuntimeConfig - // LogOutput is the sink for the logs. If nil, logs are written - // to os.Stderr. - LogOutput io.Writer - // DataDir is the data directory which is used when Config.DataDir // is not set. It is created automatically and removed when // Shutdown() is called. @@ -59,11 +55,10 @@ func NewTestACLAgent(t *testing.T, name string, hcl string, resolveAuthz authzRe a := &TestACLAgent{Name: name, HCL: hcl, resolveAuthzFn: resolveAuthz, resolveIdentFn: resolveIdent} dataDir := `data_dir = "acl-agent"` - logOutput := testutil.NewLogBuffer(t) logger := hclog.NewInterceptLogger(&hclog.LoggerOptions{ Name: a.Name, Level: hclog.Debug, - Output: logOutput, + Output: testutil.NewLogBuffer(t), }) opts := []AgentOption{ @@ -82,7 +77,6 @@ func NewTestACLAgent(t *testing.T, name string, hcl string, resolveAuthz authzRe a.Config = agent.GetConfig() a.Agent = agent - agent.LogOutput = logOutput agent.logger = logger agent.MemSink = metrics.NewInmemSink(1*time.Second, time.Minute) diff --git a/agent/agent.go b/agent/agent.go index 195c3d0edd..74440efd6e 100644 --- a/agent/agent.go +++ b/agent/agent.go @@ -174,15 +174,6 @@ type Agent struct { // Used for writing our logs logger hclog.InterceptLogger - // LogOutput is a Writer which is used when creating dependencies that - // require logging. Note that this LogOutput is not used by the agent logger, - // so setting this field does not result in the agent logs being written to - // LogOutput. - // FIXME: refactor so that: dependencies accept an hclog.Logger, - // or LogOutput is part of RuntimeConfig, or change Agent.logger to be - // a new type with an Out() io.Writer method which returns this value. - LogOutput io.Writer - // In-memory sink used for collecting metrics MemSink *metrics.InmemSink @@ -477,14 +468,11 @@ func New(options ...AgentOption) (*Agent, error) { LogRotateMaxFiles: config.LogRotateMaxFiles, } - logger, logOutput, err := logging.Setup(logConf, flat.writers) + a.logger, err = logging.Setup(logConf, flat.writers) if err != nil { return nil, err } - a.logger = logger - a.LogOutput = logOutput - grpclog.SetLoggerV2(logging.NewGRPCLogger(logConf, a.logger)) } @@ -591,16 +579,10 @@ func (a *Agent) initializeConnectionPool() error { rpcSrcAddr = &net.TCPAddr{IP: a.config.RPCBindAddr.IP} } - // Ensure we have a log output for the connection pool. - logOutput := a.LogOutput - if logOutput == nil { - logOutput = os.Stderr - } - pool := &pool.ConnPool{ Server: a.config.ServerMode, SrcAddr: rpcSrcAddr, - LogOutput: logOutput, + Logger: a.logger.StandardLogger(&hclog.StandardLoggerOptions{InferLevels: true}), TLSConfigurator: a.tlsConfigurator, Datacenter: a.config.Datacenter, } @@ -713,13 +695,13 @@ func (a *Agent) Start(ctx context.Context) error { // Setup either the client or the server. if c.ServerMode { - server, err := consul.NewServerWithOptions(consulCfg, options...) + server, err := consul.NewServer(consulCfg, options...) if err != nil { return fmt.Errorf("Failed to start Consul server: %v", err) } a.delegate = server } else { - client, err := consul.NewClientWithOptions(consulCfg, options...) + client, err := consul.NewClient(consulCfg, options...) if err != nil { return fmt.Errorf("Failed to start Consul client: %v", err) } @@ -1263,7 +1245,7 @@ func (a *Agent) reloadWatches(cfg *config.RuntimeConfig) error { httpConfig := wp.Exempt["http_handler_config"].(*watch.HttpHandlerConfig) wp.Handler = makeHTTPWatchHandler(a.logger, httpConfig) } - wp.LogOutput = a.LogOutput + wp.Logger = a.logger.Named("watch") addr := config.Address if config.Scheme == "https" { @@ -1523,10 +1505,6 @@ func (a *Agent) consulConfig() (*consul.Config, error) { } } - // Setup the loggers - base.LogLevel = a.config.LogLevel - base.LogOutput = a.LogOutput - // This will set up the LAN keyring, as well as the WAN and any segments // for servers. if err := a.setupKeyrings(base); err != nil { diff --git a/agent/consul/client.go b/agent/consul/client.go index 3a17740330..8d3786a421 100644 --- a/agent/consul/client.go +++ b/agent/consul/client.go @@ -3,7 +3,6 @@ package consul import ( "fmt" "io" - "os" "strconv" "sync" "sync/atomic" @@ -89,58 +88,31 @@ type Client struct { tlsConfigurator *tlsutil.Configurator } -// NewClient is used to construct a new Consul client from the configuration, -// potentially returning an error. -// NewClient only used to help setting up a client for testing. Normal code -// exercises NewClientLogger. -func NewClient(config *Config) (*Client, error) { - c, err := tlsutil.NewConfigurator(config.ToTLSUtilConfig(), nil) - if err != nil { - return nil, err - } - return NewClientLogger(config, nil, c) -} - -func NewClientWithOptions(config *Config, options ...ConsulOption) (*Client, error) { +// NewClient creates and returns a Client +func NewClient(config *Config, options ...ConsulOption) (*Client, error) { flat := flattenConsulOptions(options) - logger := flat.logger tlsConfigurator := flat.tlsConfigurator connPool := flat.connPool - // Check the protocol version if err := config.CheckProtocolVersion(); err != nil { return nil, err } - - // Check for a data directory! if config.DataDir == "" { return nil, fmt.Errorf("Config must provide a DataDir") } - - // Sanity check the ACLs if err := config.CheckACL(); err != nil { return nil, err } - - // Ensure we have a log output - if config.LogOutput == nil { - config.LogOutput = os.Stderr - } - - // Create a logger - if logger == nil { - logger = hclog.NewInterceptLogger(&hclog.LoggerOptions{ - Level: hclog.Debug, - Output: config.LogOutput, - }) + if flat.logger == nil { + return nil, fmt.Errorf("logger is required") } if connPool == nil { connPool = &pool.ConnPool{ Server: false, SrcAddr: config.RPCSrcAddr, - LogOutput: config.LogOutput, + Logger: flat.logger.StandardLogger(&hclog.StandardLoggerOptions{InferLevels: true}), MaxTime: clientRPCConnMaxIdle, MaxStreams: clientMaxStreams, TLSConfigurator: tlsConfigurator, @@ -153,7 +125,7 @@ func NewClientWithOptions(config *Config, options ...ConsulOption) (*Client, err config: config, connPool: connPool, eventCh: make(chan serf.Event, serfEventBacklog), - logger: logger.NamedIntercept(logging.ConsulClient), + logger: flat.logger.NamedIntercept(logging.ConsulClient), shutdownCh: make(chan struct{}), tlsConfigurator: tlsConfigurator, } @@ -210,10 +182,6 @@ func NewClientWithOptions(config *Config, options ...ConsulOption) (*Client, err return c, nil } -func NewClientLogger(config *Config, logger hclog.InterceptLogger, tlsConfigurator *tlsutil.Configurator) (*Client, error) { - return NewClientWithOptions(config, WithLogger(logger), WithTLSConfigurator(tlsConfigurator)) -} - // Shutdown is used to shutdown the client func (c *Client) Shutdown() error { c.logger.Info("shutting down client") diff --git a/agent/consul/client_test.go b/agent/consul/client_test.go index 9adbb17fa4..4ec005ecd3 100644 --- a/agent/consul/client_test.go +++ b/agent/consul/client_test.go @@ -48,8 +48,6 @@ func testClientConfig(t *testing.T) (string, *Config) { config.SerfLANConfig.MemberlistConfig.ProbeTimeout = 200 * time.Millisecond config.SerfLANConfig.MemberlistConfig.ProbeInterval = time.Second config.SerfLANConfig.MemberlistConfig.GossipInterval = 100 * time.Millisecond - config.LogOutput = testutil.NewLogBuffer(t) - return dir, config } @@ -72,15 +70,10 @@ func testClientWithConfigWithErr(t *testing.T, cb func(c *Config)) (string, *Cli if cb != nil { cb(config) } - w := config.LogOutput - if w == nil { - w = os.Stderr - } - logger := hclog.NewInterceptLogger(&hclog.LoggerOptions{ Name: config.NodeName, Level: hclog.Debug, - Output: w, + Output: testutil.NewLogBuffer(t), }) tlsConf, err := tlsutil.NewConfigurator(config.ToTLSUtilConfig(), logger) @@ -88,7 +81,7 @@ func testClientWithConfigWithErr(t *testing.T, cb func(c *Config)) (string, *Cli t.Fatalf("err: %v", err) } - client, err := NewClientLogger(config, logger, tlsConf) + client, err := NewClient(config, WithLogger(logger), WithTLSConfigurator(tlsConf)) if err != nil { config.NotifyShutdown() } @@ -445,7 +438,7 @@ func TestClient_RPC_TLS(t *testing.T) { conf1.VerifyIncoming = true conf1.VerifyOutgoing = true configureTLS(conf1) - s1, err := NewServer(conf1) + s1, err := newServer(t, conf1) if err != nil { t.Fatalf("err: %v", err) } @@ -456,7 +449,7 @@ func TestClient_RPC_TLS(t *testing.T) { defer conf2.NotifyShutdown() conf2.VerifyOutgoing = true configureTLS(conf2) - c1, err := NewClient(conf2) + c1, err := newClient(t, conf2) if err != nil { t.Fatalf("err: %v", err) } @@ -486,10 +479,22 @@ func TestClient_RPC_TLS(t *testing.T) { }) } +func newClient(t *testing.T, config *Config) (*Client, error) { + c, err := tlsutil.NewConfigurator(config.ToTLSUtilConfig(), nil) + if err != nil { + return nil, err + } + logger := hclog.NewInterceptLogger(&hclog.LoggerOptions{ + Level: hclog.Debug, + Output: testutil.NewLogBuffer(t), + }) + return NewClient(config, WithLogger(logger), WithTLSConfigurator(c)) +} + func TestClient_RPC_RateLimit(t *testing.T) { t.Parallel() dir1, conf1 := testServerConfig(t) - s1, err := NewServer(conf1) + s1, err := newServer(t, conf1) if err != nil { t.Fatalf("err: %v", err) } @@ -501,7 +506,7 @@ func TestClient_RPC_RateLimit(t *testing.T) { defer conf2.NotifyShutdown() conf2.RPCRate = 2 conf2.RPCMaxBurst = 2 - c1, err := NewClient(conf2) + c1, err := newClient(t, conf2) if err != nil { t.Fatalf("err: %v", err) } @@ -569,7 +574,7 @@ func TestClient_SnapshotRPC_RateLimit(t *testing.T) { defer conf1.NotifyShutdown() conf1.RPCRate = 2 conf1.RPCMaxBurst = 2 - c1, err := NewClient(conf1) + c1, err := newClient(t, conf1) if err != nil { t.Fatalf("err: %v", err) } @@ -601,7 +606,7 @@ func TestClient_SnapshotRPC_TLS(t *testing.T) { conf1.VerifyIncoming = true conf1.VerifyOutgoing = true configureTLS(conf1) - s1, err := NewServer(conf1) + s1, err := newServer(t, conf1) if err != nil { t.Fatalf("err: %v", err) } @@ -612,7 +617,7 @@ func TestClient_SnapshotRPC_TLS(t *testing.T) { defer conf2.NotifyShutdown() conf2.VerifyOutgoing = true configureTLS(conf2) - c1, err := NewClient(conf2) + c1, err := newClient(t, conf2) if err != nil { t.Fatalf("err: %v", err) } diff --git a/agent/consul/config.go b/agent/consul/config.go index 9498865057..ebfca98249 100644 --- a/agent/consul/config.go +++ b/agent/consul/config.go @@ -2,7 +2,6 @@ package consul import ( "fmt" - "io" "net" "os" "time" @@ -161,13 +160,6 @@ type Config struct { // leader election. ReconcileInterval time.Duration - // LogLevel is the level of the logs to write. Defaults to "INFO". - LogLevel string - - // LogOutput is the location to write logs to. If this is not set, - // logs will go to stderr. - LogOutput io.Writer - // ProtocolVersion is the protocol version to speak. This must be between // ProtocolVersionMin and ProtocolVersionMax. ProtocolVersion uint8 diff --git a/agent/consul/leader_test.go b/agent/consul/leader_test.go index 365e50ec25..ed34091197 100644 --- a/agent/consul/leader_test.go +++ b/agent/consul/leader_test.go @@ -10,10 +10,13 @@ import ( "time" "github.com/hashicorp/consul/agent/structs" + "github.com/hashicorp/consul/agent/token" "github.com/hashicorp/consul/api" "github.com/hashicorp/consul/sdk/testutil" "github.com/hashicorp/consul/sdk/testutil/retry" "github.com/hashicorp/consul/testrpc" + "github.com/hashicorp/consul/tlsutil" + "github.com/hashicorp/go-hclog" msgpackrpc "github.com/hashicorp/net-rpc-msgpackrpc" "github.com/hashicorp/serf/serf" "github.com/stretchr/testify/require" @@ -1283,24 +1286,39 @@ func TestLeader_ConfigEntryBootstrap_Fail(t *testing.T) { } }() - dir1, s1 := testServerWithConfig(t, func(c *Config) { - c.LogOutput = io.MultiWriter(pw, testutil.NewLogBuffer(t)) - c.Build = "1.6.0" - c.ConfigEntryBootstrap = []structs.ConfigEntry{ - &structs.ServiceSplitterConfigEntry{ - Kind: structs.ServiceSplitter, - Name: "web", - Splits: []structs.ServiceSplit{ - {Weight: 100, Service: "web"}, - }, + dir, config := testServerConfig(t) + defer os.RemoveAll(dir) + config.Build = "1.6.0" + config.ConfigEntryBootstrap = []structs.ConfigEntry{ + &structs.ServiceSplitterConfigEntry{ + Kind: structs.ServiceSplitter, + Name: "web", + Splits: []structs.ServiceSplit{ + {Weight: 100, Service: "web"}, }, - } - }) - defer os.RemoveAll(dir1) - defer s1.Shutdown() + }, + } - result := <-ch - require.Empty(t, result) + logger := hclog.NewInterceptLogger(&hclog.LoggerOptions{ + Name: config.NodeName, + Level: hclog.Debug, + Output: io.MultiWriter(pw, testutil.NewLogBuffer(t)), + }) + tlsConf, err := tlsutil.NewConfigurator(config.ToTLSUtilConfig(), logger) + require.NoError(t, err) + srv, err := NewServer(config, + WithLogger(logger), + WithTokenStore(new(token.Store)), + WithTLSConfigurator(tlsConf)) + require.NoError(t, err) + defer srv.Shutdown() + + select { + case result := <-ch: + require.Empty(t, result) + case <-time.After(time.Second): + t.Fatal("timeout waiting for a result from tailing logs") + } } func TestLeader_ACLLegacyReplication(t *testing.T) { diff --git a/agent/consul/rpc.go b/agent/consul/rpc.go index a73f613be8..4ab54a8b9e 100644 --- a/agent/consul/rpc.go +++ b/agent/consul/rpc.go @@ -295,7 +295,10 @@ func (s *Server) handleNativeTLS(conn net.Conn) { func (s *Server) handleMultiplexV2(conn net.Conn) { defer conn.Close() conf := yamux.DefaultConfig() - conf.LogOutput = s.config.LogOutput + // override the default because LogOutput conflicts with Logger + conf.LogOutput = nil + // TODO: should this be created once and cached? + conf.Logger = s.logger.StandardLogger(&hclog.StandardLoggerOptions{InferLevels: true}) server, _ := yamux.Server(conn, conf) for { sub, err := server.Accept() diff --git a/agent/consul/server.go b/agent/consul/server.go index ba492326bd..5ba45d71ce 100644 --- a/agent/consul/server.go +++ b/agent/consul/server.go @@ -313,28 +313,9 @@ type Server struct { EnterpriseServer } -// NewServer is only used to help setting up a server for testing. Normal code -// exercises NewServerLogger. -func NewServer(config *Config) (*Server, error) { - c, err := tlsutil.NewConfigurator(config.ToTLSUtilConfig(), nil) - if err != nil { - return nil, err - } - return NewServerLogger(config, nil, new(token.Store), c) -} - -// NewServerLogger is used to construct a new Consul server from the -// configuration, potentially returning an error -func NewServerLogger(config *Config, logger hclog.InterceptLogger, tokens *token.Store, tlsConfigurator *tlsutil.Configurator) (*Server, error) { - return NewServerWithOptions(config, - WithLogger(logger), - WithTokenStore(tokens), - WithTLSConfigurator(tlsConfigurator)) -} - -// NewServerWithOptions is used to construct a new Consul server from the configuration -// and extra options, potentially returning an error -func NewServerWithOptions(config *Config, options ...ConsulOption) (*Server, error) { +// NewServer is used to construct a new Consul server from the configuration +// and extra options, potentially returning an error. +func NewServer(config *Config, options ...ConsulOption) (*Server, error) { flat := flattenConsulOptions(options) logger := flat.logger @@ -342,31 +323,17 @@ func NewServerWithOptions(config *Config, options ...ConsulOption) (*Server, err tlsConfigurator := flat.tlsConfigurator connPool := flat.connPool - // Check the protocol version. if err := config.CheckProtocolVersion(); err != nil { return nil, err } - - // Check for a data directory. if config.DataDir == "" && !config.DevMode { return nil, fmt.Errorf("Config must provide a DataDir") } - - // Sanity check the ACLs. if err := config.CheckACL(); err != nil { return nil, err } - - // Ensure we have a log output and create a logger. - if config.LogOutput == nil { - config.LogOutput = os.Stderr - } - if logger == nil { - logger = hclog.NewInterceptLogger(&hclog.LoggerOptions{ - Level: hclog.Debug, - Output: config.LogOutput, - }) + return nil, fmt.Errorf("logger is required") } // Check if TLS is enabled @@ -400,7 +367,7 @@ func NewServerWithOptions(config *Config, options ...ConsulOption) (*Server, err connPool = &pool.ConnPool{ Server: true, SrcAddr: config.RPCSrcAddr, - LogOutput: config.LogOutput, + Logger: logger.StandardLogger(&hclog.StandardLoggerOptions{InferLevels: true}), MaxTime: serverRPCCache, MaxStreams: serverMaxStreams, TLSConfigurator: tlsConfigurator, @@ -735,7 +702,7 @@ func (s *Server) setupRaft() error { log = cacheStore // Create the snapshot store. - snapshots, err := raft.NewFileSnapshotStore(path, snapshotsRetained, s.config.LogOutput) + snapshots, err := raft.NewFileSnapshotStoreWithLogger(path, snapshotsRetained, s.logger.Named("snapshot")) if err != nil { return err } diff --git a/agent/consul/server_test.go b/agent/consul/server_test.go index 99632a499c..18d61618f0 100644 --- a/agent/consul/server_test.go +++ b/agent/consul/server_test.go @@ -146,7 +146,6 @@ func testServerConfig(t *testing.T) (string, *Config) { config.Bootstrap = true config.Datacenter = "dc1" config.DataDir = dir - config.LogOutput = testutil.NewLogBuffer(t) // bind the rpc server to a random port. config.RPCAdvertise will be // set to the listen address unless it was set in the configuration. @@ -258,18 +257,18 @@ func testServerDCExpectNonVoter(t *testing.T, dc string, expect int) (string, *S func testServerWithConfig(t *testing.T, cb func(*Config)) (string, *Server) { var dir string - var config *Config var srv *Server - var err error // Retry added to avoid cases where bind addr is already in use retry.RunWith(retry.ThreeTimes(), t, func(r *retry.R) { + var config *Config dir, config = testServerConfig(t) if cb != nil { cb(config) } - srv, err = newServer(config) + var err error + srv, err = newServer(t, config) if err != nil { config.NotifyShutdown() os.RemoveAll(dir) @@ -295,7 +294,7 @@ func testACLServerWithConfig(t *testing.T, cb func(*Config), initReplicationToke return dir, srv, codec } -func newServer(c *Config) (*Server, error) { +func newServer(t *testing.T, c *Config) (*Server, error) { // chain server up notification oldNotify := c.NotifyListen up := make(chan struct{}) @@ -306,21 +305,19 @@ func newServer(c *Config) (*Server, error) { } } - // start server - w := c.LogOutput - if w == nil { - w = os.Stderr - } logger := hclog.NewInterceptLogger(&hclog.LoggerOptions{ Name: c.NodeName, Level: hclog.Debug, - Output: w, + Output: testutil.NewLogBuffer(t), }) tlsConf, err := tlsutil.NewConfigurator(c.ToTLSUtilConfig(), logger) if err != nil { return nil, err } - srv, err := NewServerLogger(c, logger, new(token.Store), tlsConf) + srv, err := NewServer(c, + WithLogger(logger), + WithTokenStore(new(token.Store)), + WithTLSConfigurator(tlsConf)) if err != nil { return nil, err } @@ -1088,7 +1085,7 @@ func TestServer_JoinLAN_TLS(t *testing.T) { conf1.VerifyIncoming = true conf1.VerifyOutgoing = true configureTLS(conf1) - s1, err := newServer(conf1) + s1, err := newServer(t, conf1) if err != nil { t.Fatalf("err: %v", err) } @@ -1101,7 +1098,7 @@ func TestServer_JoinLAN_TLS(t *testing.T) { conf2.VerifyIncoming = true conf2.VerifyOutgoing = true configureTLS(conf2) - s2, err := newServer(conf2) + s2, err := newServer(t, conf2) if err != nil { t.Fatalf("err: %v", err) } @@ -1486,7 +1483,7 @@ func TestServer_RPC_RateLimit(t *testing.T) { dir1, conf1 := testServerConfig(t) conf1.RPCRate = 2 conf1.RPCMaxBurst = 2 - s1, err := NewServer(conf1) + s1, err := newServer(t, conf1) if err != nil { t.Fatalf("err: %v", err) } @@ -1512,7 +1509,11 @@ func TestServer_CALogging(t *testing.T) { c, err := tlsutil.NewConfigurator(conf1.ToTLSUtilConfig(), logger) require.NoError(t, err) - s1, err := NewServerLogger(conf1, logger, new(token.Store), c) + + s1, err := NewServer(conf1, + WithLogger(logger), + WithTokenStore(new(token.Store)), + WithTLSConfigurator(c)) if err != nil { t.Fatalf("err: %v", err) } diff --git a/agent/pool/pool.go b/agent/pool/pool.go index 1aca440345..d081dfe174 100644 --- a/agent/pool/pool.go +++ b/agent/pool/pool.go @@ -4,7 +4,7 @@ import ( "container/list" "crypto/tls" "fmt" - "io" + "log" "net" "net/rpc" "sync" @@ -126,8 +126,9 @@ type ConnPool struct { // SrcAddr is the source address for outgoing connections. SrcAddr *net.TCPAddr - // LogOutput is used to control logging - LogOutput io.Writer + // Logger passed to yamux + // TODO: consider refactoring to accept a full yamux.Config instead of a logger + Logger *log.Logger // The maximum time to keep a connection open MaxTime time.Duration @@ -454,9 +455,10 @@ func (p *ConnPool) getNewConn(dc string, nodeName string, addr net.Addr) (*Conn, return nil, err } - // Setup the logger conf := yamux.DefaultConfig() - conf.LogOutput = p.LogOutput + // override the default because LogOutput conflicts with Logger. + conf.LogOutput = nil + conf.Logger = p.Logger // Create a multiplexed session session, err := yamux.Client(conn, conf) diff --git a/api/watch/plan.go b/api/watch/plan.go index 6cf11157ca..f3b7981fb0 100644 --- a/api/watch/plan.go +++ b/api/watch/plan.go @@ -31,8 +31,10 @@ func (p *Plan) Run(address string) error { // Run is used to run a watch plan func (p *Plan) RunWithConfig(address string, conf *consulapi.Config) error { - // Create the logger - logger := newWatchLogger(p.LogOutput) + logger := p.Logger + if logger == nil { + logger = newWatchLogger(p.LogOutput) + } // Setup the client p.address = address diff --git a/api/watch/watch.go b/api/watch/watch.go index 09ee15edd2..1dce252911 100644 --- a/api/watch/watch.go +++ b/api/watch/watch.go @@ -8,6 +8,7 @@ import ( "time" consulapi "github.com/hashicorp/consul/api" + "github.com/hashicorp/go-hclog" "github.com/mitchellh/mapstructure" ) @@ -29,7 +30,10 @@ type Plan struct { // on index param. To support hash based watches, set HybridHandler instead. Handler HandlerFunc HybridHandler HybridHandlerFunc - LogOutput io.Writer + + Logger hclog.Logger + // Deprecated: use Logger + LogOutput io.Writer address string client *consulapi.Client diff --git a/command/connect/proxy/proxy.go b/command/connect/proxy/proxy.go index 658c820172..8bc5bbb888 100644 --- a/command/connect/proxy/proxy.go +++ b/command/connect/proxy/proxy.go @@ -140,7 +140,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, []io.Writer{&logGate}) if err != nil { c.UI.Error(err.Error()) return 1 diff --git a/logging/logger.go b/logging/logger.go index ec1dcc613d..bd530581cc 100644 --- a/logging/logger.go +++ b/logging/logger.go @@ -63,9 +63,9 @@ type LogSetupErrorFn func(string) // 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. -func Setup(config *Config, writers []io.Writer) (hclog.InterceptLogger, io.Writer, error) { +func Setup(config *Config, writers []io.Writer) (hclog.InterceptLogger, error) { if !ValidateLogLevel(config.LogLevel) { - return nil, 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, allowedLogLevels) } @@ -84,7 +84,7 @@ func Setup(config *Config, writers []io.Writer) (hclog.InterceptLogger, io.Write if i == retries { timeout := time.Duration(retries) * delay - return nil, nil, fmt.Errorf("Syslog setup did not succeed within timeout (%s).", timeout.String()) + return nil, fmt.Errorf("Syslog setup did not succeed within timeout (%s).", timeout.String()) } time.Sleep(delay) @@ -121,19 +121,16 @@ func Setup(config *Config, writers []io.Writer) (hclog.InterceptLogger, io.Write MaxFiles: config.LogRotateMaxFiles, } if err := logFile.openNew(); err != nil { - return nil, nil, fmt.Errorf("Failed to setup logging: %w", err) + return nil, fmt.Errorf("Failed to setup logging: %w", err) } writers = append(writers, logFile) } - logOutput := io.MultiWriter(writers...) - logger := hclog.NewInterceptLogger(&hclog.LoggerOptions{ Level: LevelFromString(config.LogLevel), Name: config.Name, - Output: logOutput, + Output: io.MultiWriter(writers...), JSONFormat: config.LogJSON, }) - - return logger, logOutput, nil + return logger, nil } diff --git a/logging/logger_test.go b/logging/logger_test.go index eed4029e46..d7eb9b04dd 100644 --- a/logging/logger_test.go +++ b/logging/logger_test.go @@ -19,9 +19,8 @@ func TestLogger_SetupBasic(t *testing.T) { LogLevel: "INFO", } - logger, writer, err := Setup(cfg, nil) + logger, err := Setup(cfg, nil) require.NoError(err) - require.NotNil(writer) require.NotNil(logger) } @@ -29,7 +28,7 @@ func TestLogger_SetupInvalidLogLevel(t *testing.T) { t.Parallel() cfg := &Config{} - _, _, err := Setup(cfg, nil) + _, err := Setup(cfg, nil) testutil.RequireErrorContains(t, err, "Invalid log level") } @@ -62,7 +61,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, []io.Writer{&buf}) require.NoError(err) require.NotNil(logger) @@ -85,7 +84,7 @@ func TestLogger_SetupLoggerDebugLevel(t *testing.T) { } var buf bytes.Buffer - logger, _, err := Setup(cfg, []io.Writer{&buf}) + logger, err := Setup(cfg, []io.Writer{&buf}) require.NoError(err) require.NotNil(logger) @@ -107,7 +106,7 @@ func TestLogger_SetupLoggerWithName(t *testing.T) { } var buf bytes.Buffer - logger, _, err := Setup(cfg, []io.Writer{&buf}) + logger, err := Setup(cfg, []io.Writer{&buf}) require.NoError(err) require.NotNil(logger) @@ -126,7 +125,7 @@ func TestLogger_SetupLoggerWithJSON(t *testing.T) { } var buf bytes.Buffer - logger, _, err := Setup(cfg, []io.Writer{&buf}) + logger, err := Setup(cfg, []io.Writer{&buf}) require.NoError(err) require.NotNil(logger) @@ -154,7 +153,7 @@ func TestLogger_SetupLoggerWithValidLogPath(t *testing.T) { } var buf bytes.Buffer - logger, _, err := Setup(cfg, []io.Writer{&buf}) + logger, err := Setup(cfg, []io.Writer{&buf}) require.NoError(err) require.NotNil(logger) } @@ -169,7 +168,7 @@ func TestLogger_SetupLoggerWithInValidLogPath(t *testing.T) { } var buf bytes.Buffer - logger, _, err := Setup(cfg, []io.Writer{&buf}) + logger, err := Setup(cfg, []io.Writer{&buf}) require.Error(err) require.True(errors.Is(err, os.ErrNotExist)) require.Nil(logger) @@ -190,7 +189,7 @@ func TestLogger_SetupLoggerWithInValidLogPathPermission(t *testing.T) { } var buf bytes.Buffer - logger, _, err := Setup(cfg, []io.Writer{&buf}) + logger, err := Setup(cfg, []io.Writer{&buf}) require.Error(err) require.True(errors.Is(err, os.ErrPermission)) require.Nil(logger)