migrate all bar gitlab-shell to new slog labkit setup

This change utilises the slog.SetDefault(logger) call to
ensure that any subsequent log emissions using slog will use the
LabKit configured setup.

This is really quite useful as it allows us to avoid having to
plumb through the logger into every component within our system.

Whilst this would have been a nicer approach when it comes to testing,
pragmatically, that would have involved a significant code change.

I've tried to walk through that process with other MRs but end up with
a few hundred lines of a delta which I feel is much too risky to
roll out alongside a logger change.

Migrating to this first, and then cleaning up the service to follow
cleaner architectural patterns feels like a less risky approach.

Author: e_forbes

cmd/gitlab-shell-authorized-keys-check/main.go

@@ -54,8 +54,10 @@ func execute(readWriter *readwriter.ReadWriter) (int, error) {
 		return exitCodeFailure, fmt.Errorf("failed to read config, exiting")
 	}
 
-	logCloser := logger.Configure(config)
-	defer func() { _ = logCloser.Close() }()
+	logCloser := logger.ConfigureLogger(config)
+	if logCloser != nil {
+		defer logCloser.Close() //nolint:errcheck
+	}
 
 	cmd, err := cmd.New(os.Args[1:], config, readWriter)
 	if err != nil {

cmd/gitlab-shell-authorized-principals-check/main.go

@@ -46,8 +46,10 @@ func run() int {
 		return 1
 	}
 
-	logCloser := logger.Configure(config)
-	defer logCloser.Close() //nolint:errcheck
+	logCloser := logger.ConfigureLogger(config)
+	if logCloser != nil {
+		defer logCloser.Close() //nolint:errcheck
+	}
 
 	cmd, err := cmd.New(os.Args[1:], config, readWriter)
 	if err != nil {

cmd/gitlab-shell-check/main.go

@@ -51,8 +51,10 @@ func run() int {
 		return code
 	}
 
-	logCloser := logger.Configure(config)
-	defer logCloser.Close() //nolint:errcheck
+	logCloser := logger.ConfigureLogger(config)
+	if logCloser != nil {
+		defer logCloser.Close() //nolint:errcheck
+	}
 
 	cmd, err := checkCmd.New(config, readWriter)
 	if code := exitOnError(err, "Failed to create command"); code != 0 {

cmd/gitlab-sshd/main.go

@@ -59,26 +59,19 @@ func main() {
 			))
 		}
 	}
-	log, logCloser, err := logger.ConfigureLogger(cfg)
-	if err != nil {
-		log.ErrorContext(ctx, "failed to log to file, reverting to stderr", slog.String(fields.ErrorMessage, err.Error()))
-	} else {
-		// nolint
-		defer func() {
-			if err = logCloser.Close(); err != nil {
-				log.ErrorContext(ctx, "failed to close log file", slog.String(fields.ErrorMessage, err.Error()))
-			}
-		}()
+	logCloser := logger.ConfigureLogger(cfg)
+	if logCloser != nil {
+		defer logCloser.Close() //nolint:errcheck
 	}
-	log.InfoContext(ctx, "gitlab-sshd starting up...")
+	slog.InfoContext(ctx, "gitlab-sshd starting up...")
 
 	overrideConfigFromEnvironment(cfg)
 	if err := isConfigSane(cfg); err != nil {
 		ctx = v2log.WithFields(ctx, slog.String(fields.ErrorMessage, err.Error()))
 		if *configDir == "" {
-			log.ErrorContext(ctx, "no config-dir provided, using only environment variables")
+			slog.ErrorContext(ctx, "no config-dir provided, using only environment variables")
 		} else {
-			log.ErrorContext(ctx, "configuration error")
+			slog.ErrorContext(ctx, "configuration error")
 		}
 	}
 
@@ -90,14 +83,14 @@ func main() {
 
 	server, err := sshd.NewServer(cfg)
 	if err != nil {
-		log.ErrorContext(ctx, "Failed to start Gitlab built-in sshd", slog.String(
+		slog.ErrorContext(ctx, "Failed to start Gitlab built-in sshd", slog.String(
 			fields.ErrorMessage, err.Error(),
 		))
 	}
 
 	// Startup monitoring endpoint.
 	if cfg.Server.WebListen != "" {
-		startupMonitoringEndpoint(ctx, cfg, server, log)
+		startupMonitoringEndpoint(ctx, cfg, server)
 	}
 
 	ctx, cancel := context.WithCancel(ctx)
@@ -106,10 +99,10 @@ func main() {
 	done := make(chan os.Signal, 1)
 	signal.Notify(done, syscall.SIGINT, syscall.SIGTERM)
 
-	gracefulShutdown(ctx, done, cfg, server, cancel, log)
+	gracefulShutdown(ctx, done, cfg, server, cancel)
 
 	if err := server.ListenAndServe(ctx); err != nil {
-		log.ErrorContext(ctx, "GitLab built-in sshd failed to listen for new connections",
+		slog.ErrorContext(ctx, "GitLab built-in sshd failed to listen for new connections",
 			slog.String(fields.ErrorMessage, err.Error()))
 	}
 }
@@ -120,18 +113,17 @@ func gracefulShutdown(
 	cfg *config.Config,
 	server *sshd.Server,
 	cancel context.CancelFunc,
-	log *slog.Logger,
 ) {
 	go func() {
 		sig := <-done
 		signal.Reset(syscall.SIGINT, syscall.SIGTERM)
 
 		gracePeriod := time.Duration(cfg.Server.GracePeriod)
-		log.InfoContext(ctx, fmt.Sprintf("Shutdown initiated with grace period: %f", gracePeriod.Seconds()),
+		slog.InfoContext(ctx, fmt.Sprintf("Shutdown initiated with grace period: %f", gracePeriod.Seconds()),
 			slog.String("signal", sig.String()))
 
 		if err := server.Shutdown(); err != nil {
-			log.ErrorContext(ctx, "Error shutting down the server", slog.String(
+			slog.ErrorContext(ctx, "Error shutting down the server", slog.String(
 				fields.ErrorMessage, err.Error()))
 		}
 		<-time.After(gracePeriod)
@@ -140,15 +132,15 @@ func gracefulShutdown(
 	}()
 }
 
-func startupMonitoringEndpoint(ctx context.Context, cfg *config.Config, server *sshd.Server, log *slog.Logger) {
+func startupMonitoringEndpoint(ctx context.Context, cfg *config.Config, server *sshd.Server) {
 	go func() {
 		err := monitoring.Start(
 			monitoring.WithListenerAddress(cfg.Server.WebListen),
 			monitoring.WithBuildInformation(Version, BuildTime),
 			monitoring.WithServeMux(server.MonitoringServeMux()),
 		)
 
-		log.ErrorContext(ctx, "monitoring service raised an error", slog.String(
+		slog.ErrorContext(ctx, "monitoring service raised an error", slog.String(
 			fields.ErrorMessage, err.Error(),
 		))
 		panic(err)

internal/command/command.go

@@ -6,13 +6,16 @@ package command
 import (
 	"context"
 	"fmt"
+	"log/slog"
 	"os"
 	"path"
 	"strings"
 
 	"gitlab.com/gitlab-org/gitlab-shell/v14/internal/config"
 	"gitlab.com/gitlab-org/labkit/correlation"
+	"gitlab.com/gitlab-org/labkit/fields"
 	"gitlab.com/gitlab-org/labkit/tracing"
+	"gitlab.com/gitlab-org/labkit/v2/log"
 )
 
 // Command is the interface that all gitlab-shell commands must implement.
@@ -82,6 +85,7 @@ func Setup(serviceName string, config *config.Config) (context.Context, func())
 	if correlationID == "" {
 		correlationID := correlation.SafeRandomID()
 		ctx = correlation.ContextWithCorrelation(ctx, correlationID)
+		ctx = log.WithFields(ctx, slog.String(fields.CorrelationID, correlationID))
 	}
 
 	return ctx, func() {

internal/logger/logger.go

@@ -18,14 +18,20 @@ import (
 // ConfigureLogger - gitlab-sshd's log output can be configured to text as per the documentation:
 // https://docs.gitlab.com/omnibus/settings/logs/#json-logging
 // This is currently controlled by the GITLAB_LOG_FORMAT environment variable.
-func ConfigureLogger(cfg *config.Config) (*slog.Logger, io.Closer, error) {
+func ConfigureLogger(cfg *config.Config) io.Closer {
 	logConfig := &v2log.Config{
 		LogLevel: parseLogLevel(cfg.LogLevel),
 	}
 	if gitlabLogFormat := os.Getenv("GITLAB_LOG_FORMAT"); gitlabLogFormat == "text" {
 		logConfig.UseTextFormat = true
 	}
-	return v2log.NewWithFile(cfg.LogFile, logConfig)
+	logger, logCloser, err := v2log.NewWithFile(cfg.LogFile, logConfig)
+	slog.SetDefault(logger)
+	if err != nil {
+		return nil
+	}
+
+	return logCloser
 }
 
 func parseLogLevel(level string) slog.Level {

internal/logger/logger_test.go

@@ -2,6 +2,7 @@ package logger
 
 import (
 	"io"
+	"log/slog"
 	"os"
 	"regexp"
 	"testing"
@@ -108,11 +109,12 @@ func TestConfigureLabkitV2Log(t *testing.T) {
 		LogLevel:  "debug",
 	}
 
-	logger, closer, err := ConfigureLogger(&config)
-	require.NoError(t, err)
-	defer MustClose(t, closer)
-	logger.Info("this is a test")
-	logger.Debug("debug log message")
+	closer := ConfigureLogger(&config)
+	if closer != nil {
+		defer MustClose(t, closer)
+	}
+	slog.Info("this is a test")
+	slog.Debug("debug log message")
 
 	data, err := os.ReadFile(tmpFile)
 	dataStr := string(data)