Remove LogOutput from Client

agent/consul/client.go

@@ -3,7 +3,6 @@ package consul
 import (
 	"fmt"
 	"io"
-	"os"
 	"strconv"
 	"sync"
 	"sync/atomic"
@@ -89,58 +88,30 @@ 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) {
 	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
 	}
-
+	if flat.logger == nil {
-	// Ensure we have a log output
+		return nil, fmt.Errorf("logger is required")
-	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 connPool == nil {
 		connPool = &pool.ConnPool{
 			Server:          false,
 			SrcAddr:         config.RPCSrcAddr,
-			Logger:          logger.StandardLogger(&hclog.StandardLoggerOptions{InferLevels: true}),
+			Logger:          flat.logger.StandardLogger(&hclog.StandardLoggerOptions{InferLevels: true}),
 			MaxTime:         clientRPCConnMaxIdle,
 			MaxStreams:      clientMaxStreams,
 			TLSConfigurator: tlsConfigurator,
@@ -153,7 +124,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 +181,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")

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()
 	}
@@ -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,6 +479,18 @@ 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)
@@ -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)
 	}
@@ -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)
 	}