Remove logrus (#50829)

* Remove all direct logrus usage from teleport module

The only remaining use of logrus is from integrations, which is
unfortunately imported by teleport.e, and prevents logrus from
being moved to an indirect dependency. The logrus formatter and
initialization of the logrus logger will remain in place until
integrations is using slog. To prevent any accidental inclusions
of logrus within the teleport module the depguard rules have been
updated to prohibit importing logrus. The rules also include
prohibit a few common log packages that tools like gopls might
automatically import.

* Refactor logger initialization

Consolidates configuring of global loggers to a single function.
This is mainly to facilitate configuring the logger for
teleport scp, but will also allow us to remove the copy of logger
initialization that currently exists in integrations/lib/logger.

* fix: document ValidateFields

* fix: remove copied yaml tags

* fix: update file path comment

.golangci.yml

@@ -99,10 +99,6 @@ linters-settings:
             desc: 'use "github.com/google/uuid" instead'
           - pkg: github.com/pborman/uuid
             desc: 'use "github.com/google/uuid" instead'
-          - pkg: github.com/siddontang/go-log/log
-            desc: 'use "github.com/sirupsen/logrus" instead'
-          - pkg: github.com/siddontang/go/log
-            desc: 'use "github.com/sirupsen/logrus" instead'
           - pkg: github.com/tj/assert
             desc: 'use "github.com/stretchr/testify/assert" instead'
           - pkg: go.uber.org/atomic
@@ -117,16 +113,29 @@ linters-settings:
             desc: 'use "github.com/gravitational/teleport/lib/msgraph" instead'
           - pkg: github.com/cloudflare/cfssl
             desc: 'use "crypto" or "x/crypto" instead'
-      # Prevent logrus from being imported by api and e. Once everything in teleport has been converted
-      # to use log/slog this should be moved into the main block above.
-      logrus:
+      # Prevent importing any additional logging libraries.
+      logging:
         files:
-          - '**/api/**'
-          - '**/e/**'
-          - '**/lib/srv/**'
+          # Integrations are still allowed to use logrus becuase they haven't
+          # been converted to slog yet. Once they use slog, remove this exception.
+          - '!**/integrations/**'
+          # The log package still contains the logrus formatter consumed by the integrations.
+          # Remove this exception when said formatter is deleted.
+          - '!**/lib/utils/log/**'
+          - '!**/lib/utils/cli.go'
         deny:
           - pkg: github.com/sirupsen/logrus
             desc: 'use "log/slog" instead'
+          - pkg: github.com/siddontang/go-log/log
+            desc: 'use "log/slog" instead'
+          - pkg: github.com/siddontang/go/log
+            desc: 'use "log/slog" instead'
+          - pkg: github.com/mailgun/log
+            desc: 'use "log/slog" instead'
+          - pkg: github.com/saferwall/pe/log
+            desc: 'use "log/slog" instead'
+          - pkg: golang.org/x/exp/slog
+            desc: 'use "log/slog" instead'
       # Prevent importing internal packages in client tools or packages containing
       # common interfaces consumed by them that are known to bloat binaries or break builds
       # because they only support a single platform.

integration/helpers/instance.go

@@ -40,7 +40,6 @@ import (
 	"github.com/gorilla/websocket"
 	"github.com/gravitational/trace"
 	"github.com/jonboulle/clockwork"
-	"github.com/sirupsen/logrus"
 	"github.com/stretchr/testify/require"
 	"golang.org/x/crypto/ssh"
 
@@ -327,10 +326,6 @@ type InstanceConfig struct {
 	Priv []byte
 	// Pub is SSH public key of the instance
 	Pub []byte
-	// Log specifies the logger
-	// Deprecated: Use Logger instead
-	// TODO(tross): Delete when e is updated
-	Log utils.Logger
 	// Logger specifies the logger
 	Logger *slog.Logger
 	// Ports is a collection of instance ports.
@@ -354,10 +349,6 @@ func NewInstance(t *testing.T, cfg InstanceConfig) *TeleInstance {
 		cfg.Listeners = StandardListenerSetup(t, &cfg.Fds)
 	}
 
-	if cfg.Log == nil {
-		cfg.Log = logrus.New()
-	}
-
 	if cfg.Logger == nil {
 		cfg.Logger = slog.New(logutils.DiscardHandler{})
 	}

integration/hostuser_test.go

@@ -637,7 +637,7 @@ func TestRootLoginAsHostUser(t *testing.T) {
 		NodeName:    Host,
 		Priv:        privateKey,
 		Pub:         publicKey,
-		Log:         utils.NewLoggerForTests(),
+		Logger:      utils.NewSlogLoggerForTests(),
 	})
 
 	// Create a user that can create a host user.
@@ -735,7 +735,7 @@ func TestRootStaticHostUsers(t *testing.T) {
 		NodeName:    Host,
 		Priv:        privateKey,
 		Pub:         publicKey,
-		Log:         utils.NewLoggerForTests(),
+		Logger:      utils.NewSlogLoggerForTests(),
 	})
 
 	require.NoError(t, instance.Create(t, nil, false, nil))

integrations/lib/logger/logger.go

@@ -34,15 +34,6 @@ import (
 	logutils "github.com/gravitational/teleport/lib/utils/log"
 )
 
-// These values are meant to be kept in sync with teleport/lib/config.
-// (We avoid importing that package here because integrations must not require CGo)
-const (
-	// logFileDefaultMode is the preferred permissions mode for log file.
-	logFileDefaultMode fs.FileMode = 0o644
-	// logFileDefaultFlag is the preferred flags set to log file.
-	logFileDefaultFlag = os.O_WRONLY | os.O_CREATE | os.O_APPEND
-)
-
 type Config struct {
 	Output   string `toml:"output"`
 	Severity string `toml:"severity"`
@@ -108,8 +99,16 @@ func Setup(conf Config) error {
 }
 
 // NewSLogLogger builds a slog.Logger from the logger.Config.
-// TODO: this code is adapted from `config.applyLogConfig`, we'll want to deduplicate the logic next time we refactor the logging setup
+// TODO(tross): Defer logging initialization to logutils.Initialize and use the
+// global slog loggers once integrations has been updated to use slog.
 func (conf Config) NewSLogLogger() (*slog.Logger, error) {
+	const (
+		// logFileDefaultMode is the preferred permissions mode for log file.
+		logFileDefaultMode fs.FileMode = 0o644
+		// logFileDefaultFlag is the preferred flags set to log file.
+		logFileDefaultFlag = os.O_WRONLY | os.O_CREATE | os.O_APPEND
+	)
+
 	var w io.Writer
 	switch conf.Output {
 	case "":
@@ -120,7 +119,7 @@ func (conf Config) NewSLogLogger() (*slog.Logger, error) {
 		w = logutils.NewSharedWriter(os.Stdout)
 	case teleport.Syslog:
 		w = os.Stderr
-		sw, err := utils.NewSyslogWriter()
+		sw, err := logutils.NewSyslogWriter()
 		if err != nil {
 			slog.Default().ErrorContext(context.Background(), "Failed to switch logging to syslog", "error", err)
 			break

lib/client/api.go

@@ -35,6 +35,7 @@ import (
 	"slices"
 	"strconv"
 	"strings"
+	"sync"
 	"sync/atomic"
 	"time"
 	"unicode/utf8"
@@ -2850,6 +2851,21 @@ type execResult struct {
 	exitStatus int
 }
 
+// sharedWriter is an [io.Writer] implementation that protects
+// writes with a mutex. This allows a single [io.Writer] to be shared
+// by both logrus and slog without their output clobbering each other.
+type sharedWriter struct {
+	mu sync.Mutex
+	io.Writer
+}
+
+func (s *sharedWriter) Write(p []byte) (int, error) {
+	s.mu.Lock()
+	defer s.mu.Unlock()
+
+	return s.Writer.Write(p)
+}
+
 // runCommandOnNodes executes a given bash command on a bunch of remote nodes.
 func (tc *TeleportClient) runCommandOnNodes(ctx context.Context, clt *ClusterClient, nodes []TargetNode, command []string) error {
 	cluster := clt.ClusterName()
@@ -2909,10 +2925,10 @@ func (tc *TeleportClient) runCommandOnNodes(ctx context.Context, clt *ClusterCli
 		}
 	}
 
-	stdout := logutils.NewSharedWriter(tc.Stdout)
+	stdout := &sharedWriter{Writer: tc.Stdout}
 	stderr := stdout
 	if tc.Stdout != tc.Stderr {
-		stderr = logutils.NewSharedWriter(tc.Stderr)
+		stderr = &sharedWriter{Writer: tc.Stderr}
 	}
 
 	for _, node := range nodes {

lib/config/configuration.go

@@ -27,7 +27,6 @@ import (
 	"crypto/x509"
 	"errors"
 	"io"
-	"io/fs"
 	"log/slog"
 	"maps"
 	"net"
@@ -72,13 +71,6 @@ import (
 	logutils "github.com/gravitational/teleport/lib/utils/log"
 )
 
-const (
-	// logFileDefaultMode is the preferred permissions mode for log file.
-	logFileDefaultMode fs.FileMode = 0o644
-	// logFileDefaultFlag is the preferred flags set to log file.
-	logFileDefaultFlag = os.O_WRONLY | os.O_CREATE | os.O_APPEND
-)
-
 // CommandLineFlags stores command line flag values, it's a much simplified subset
 // of Teleport configuration (which is fully expressed via YAML config file)
 type CommandLineFlags struct {
@@ -789,79 +781,22 @@ func applyAuthOrProxyAddress(fc *FileConfig, cfg *servicecfg.Config) error {
 }
 
 func applyLogConfig(loggerConfig Log, cfg *servicecfg.Config) error {
-	// TODO: this code is copied in the access plugin logging setup `logger.Config.NewSLogLogger`
-	// We'll want to deduplicate the logic next time we refactor the logging setup
-	var w io.Writer
 	switch loggerConfig.Output {
-	case "":
-		w = os.Stderr
-	case "stderr", "error", "2":
-		w = os.Stderr
-		cfg.Console = io.Discard // disable console printing
-	case "stdout", "out", "1":
-		w = os.Stdout
+	case "stderr", "error", "2", "stdout", "out", "1":
 		cfg.Console = io.Discard // disable console printing
-	case teleport.Syslog:
-		var err error
-		w, err = utils.NewSyslogWriter()
-		if err != nil {
-			slog.ErrorContext(context.Background(), "Failed to switch logging to syslog", "error", err)
-			break
-		}
-	default:
-		// Assume this is a file path.
-		sharedWriter, err := logutils.NewFileSharedWriter(loggerConfig.Output, logFileDefaultFlag, logFileDefaultMode)
-		if err != nil {
-			return trace.Wrap(err, "failed to init the log file shared writer")
-		}
-		w = logutils.NewWriterFinalizer[*logutils.FileSharedWriter](sharedWriter)
-		if err := sharedWriter.RunWatcherReopen(context.Background()); err != nil {
-			return trace.Wrap(err)
-		}
-	}
-
-	level := new(slog.LevelVar)
-	switch strings.ToLower(loggerConfig.Severity) {
-	case "", "info":
-		level.Set(slog.LevelInfo)
-	case "err", "error":
-		level.Set(slog.LevelError)
-	case teleport.DebugLevel:
-		level.Set(slog.LevelDebug)
-	case "warn", "warning":
-		level.Set(slog.LevelWarn)
-	case "trace":
-		level.Set(logutils.TraceLevel)
-	default:
-		return trace.BadParameter("unsupported logger severity: %q", loggerConfig.Severity)
 	}
 
-	configuredFields, err := logutils.ValidateFields(loggerConfig.Format.ExtraFields)
+	logger, level, err := logutils.Initialize(logutils.Config{
+		Output:       loggerConfig.Output,
+		Severity:     loggerConfig.Severity,
+		Format:       loggerConfig.Format.Output,
+		ExtraFields:  loggerConfig.Format.ExtraFields,
+		EnableColors: utils.IsTerminal(os.Stderr),
+	})
 	if err != nil {
 		return trace.Wrap(err)
 	}
 
-	var logger *slog.Logger
-	switch strings.ToLower(loggerConfig.Format.Output) {
-	case "":
-		fallthrough // not set. defaults to 'text'
-	case "text":
-		logger = slog.New(logutils.NewSlogTextHandler(w, logutils.SlogTextHandlerConfig{
-			Level:            level,
-			EnableColors:     utils.IsTerminal(os.Stderr),
-			ConfiguredFields: configuredFields,
-		}))
-		slog.SetDefault(logger)
-	case "json":
-		logger = slog.New(logutils.NewSlogJSONHandler(w, logutils.SlogJSONHandlerConfig{
-			Level:            level,
-			ConfiguredFields: configuredFields,
-		}))
-		slog.SetDefault(logger)
-	default:
-		return trace.BadParameter("unsupported log output format : %q", loggerConfig.Format.Output)
-	}
-
 	cfg.Logger = logger
 	cfg.LoggerLevel = level
 	return nil

lib/service/service.go

@@ -56,7 +56,6 @@ import (
 	"github.com/jonboulle/clockwork"
 	"github.com/prometheus/client_golang/prometheus/promhttp"
 	"github.com/quic-go/quic-go"
-	"github.com/sirupsen/logrus"
 	"go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc"
 	"go.opentelemetry.io/otel/attribute"
 	"golang.org/x/crypto/acme"
@@ -992,10 +991,6 @@ func NewTeleport(cfg *servicecfg.Config) (*TeleportProcess, error) {
 	}
 
 	processID := fmt.Sprintf("%v", nextProcessID())
-	cfg.Log = utils.WrapLogger(cfg.Log.WithFields(logrus.Fields{
-		teleport.ComponentKey: teleport.Component(teleport.ComponentProcess, processID),
-		"pid":                 fmt.Sprintf("%v.%v", os.Getpid(), processID),
-	}))
 	cfg.Logger = cfg.Logger.With(
 		teleport.ComponentKey, teleport.Component(teleport.ComponentProcess, processID),
 		"pid", fmt.Sprintf("%v.%v", os.Getpid(), processID),

lib/service/service_test.go

@@ -948,7 +948,6 @@ func TestTeleportProcess_reconnectToAuth(t *testing.T) {
 	cfg.Testing.ConnectFailureC = make(chan time.Duration, 5)
 	cfg.Testing.ClientTimeout = time.Millisecond
 	cfg.InstanceMetadataClient = imds.NewDisabledIMDSClient()
-	cfg.Log = utils.NewLoggerForTests()
 	cfg.Logger = utils.NewSlogLoggerForTests()
 	process, err := NewTeleport(cfg)
 	require.NoError(t, err)

lib/service/servicecfg/config.go

@@ -34,7 +34,6 @@ import (
 	"github.com/ghodss/yaml"
 	"github.com/gravitational/trace"
 	"github.com/jonboulle/clockwork"
-	"github.com/sirupsen/logrus"
 	"golang.org/x/crypto/ssh"
 
 	"github.com/gravitational/teleport"
@@ -52,7 +51,6 @@ import (
 	"github.com/gravitational/teleport/lib/sshca"
 	usagereporter "github.com/gravitational/teleport/lib/usagereporter/teleport"
 	"github.com/gravitational/teleport/lib/utils"
-	logutils "github.com/gravitational/teleport/lib/utils/log"
 )
 
 // Config contains the configuration for all services that Teleport can run.
@@ -223,10 +221,6 @@ type Config struct {
 	// Kube is a Kubernetes API gateway using Teleport client identities.
 	Kube KubeConfig
 
-	// Log optionally specifies the logger.
-	// Deprecated: use Logger instead.
-	Log utils.Logger
-
 	// Logger outputs messages using slog. The underlying handler respects
 	// the user supplied logging config.
 	Logger *slog.Logger
@@ -518,10 +512,6 @@ func ApplyDefaults(cfg *Config) {
 
 	cfg.Version = defaults.TeleportConfigVersionV1
 
-	if cfg.Log == nil {
-		cfg.Log = utils.NewLogger()
-	}
-
 	if cfg.Logger == nil {
 		cfg.Logger = slog.Default()
 	}
@@ -698,10 +688,6 @@ func applyDefaults(cfg *Config) {
 		cfg.Console = io.Discard
 	}
 
-	if cfg.Log == nil {
-		cfg.Log = logrus.StandardLogger()
-	}
-
 	if cfg.Logger == nil {
 		cfg.Logger = slog.Default()
 	}
@@ -799,7 +785,6 @@ func verifyEnabledService(cfg *Config) error {
 // If called after `config.ApplyFileConfig` or `config.Configure` it will also
 // change the global loggers.
 func (c *Config) SetLogLevel(level slog.Level) {
-	c.Log.SetLevel(logutils.SlogLevelToLogrusLevel(level))
 	c.LoggerLevel.Set(level)
 }