Remove LogOutput from Agent

Now that it is no longer used, we can remove this unnecessary field. This is a pre-step in cleanup up RuntimeConfig->Consul.Config, which is a pre-step to adding a gRPCHandler component to Server for streaming.

Removing this field also allows us to remove one of the return values from logging.Setup.

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)
 

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))
 	}
 

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

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, nil
-	return logger, logOutput, nil
 }

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)