Merge pull request #46489 from thaJeztah/less_logrus

remove more direct uses of logrus

cmd/dockerd/daemon.go

@@ -916,15 +916,19 @@ func systemContainerdRunning(honorXDG bool) (string, bool, error) {
 func configureDaemonLogs(conf *config.Config) {
 	switch log.OutputFormat(conf.LogFormat) {
 	case log.JSONFormat:
-		logrus.SetFormatter(&logrus.JSONFormatter{
-			TimestampFormat: log.RFC3339NanoFixed,
-		})
+		if err := log.SetFormat(log.JSONFormat); err != nil {
+			panic(err.Error())
+		}
 	case log.TextFormat, "":
-		logrus.SetFormatter(&logrus.TextFormatter{
-			TimestampFormat: log.RFC3339NanoFixed,
-			DisableColors:   conf.RawLogs,
-			FullTimestamp:   true,
-		})
+		if err := log.SetFormat(log.TextFormat); err != nil {
+			panic(err.Error())
+		}
+		if conf.RawLogs {
+			// FIXME(thaJeztah): this needs a better solution: containerd doesn't allow disabling colors, and this code is depending on internal knowledge of "log.SetFormat"
+			if l, ok := log.L.Logger.Formatter.(*logrus.TextFormatter); ok {
+				l.DisableColors = true
+			}
+		}
 	default:
 		panic("unsupported log format " + conf.LogFormat)
 	}

cmd/dockerd/daemon_test.go

@@ -6,7 +6,6 @@ import (
 	"github.com/containerd/containerd/log"
 	"github.com/docker/docker/daemon/config"
 	"github.com/google/go-cmp/cmp/cmpopts"
-	"github.com/sirupsen/logrus"
 	"github.com/spf13/pflag"
 	"gotest.tools/v3/assert"
 	is "gotest.tools/v3/assert/cmp"
@@ -221,8 +220,7 @@ func TestConfigureDaemonLogs(t *testing.T) {
 
 	conf.LogLevel = "warn"
 	configureDaemonLogs(conf)
-	// TODO (thaJeztah): add more aliases in log package
-	assert.Check(t, is.Equal(logrus.WarnLevel, log.GetLevel()))
+	assert.Check(t, is.Equal(log.WarnLevel, log.GetLevel()))
 }
 
 func TestCDISpecDirs(t *testing.T) {

cmd/dockerd/docker.go

@@ -13,7 +13,6 @@ import (
 	"github.com/docker/docker/pkg/rootless"
 	"github.com/moby/buildkit/util/apicaps"
 	"github.com/moby/term"
-	"github.com/sirupsen/logrus"
 	"github.com/spf13/cobra"
 )
 
@@ -82,23 +81,22 @@ func main() {
 	// Fixes https://github.com/docker/docker/issues/19728
 	signal.Ignore(syscall.SIGPIPE)
 
-	// initial log formatting; this setting is updated after the daemon configuration is loaded.
-	logrus.SetFormatter(&logrus.TextFormatter{
-		TimestampFormat: log.RFC3339NanoFixed,
-		FullTimestamp:   true,
-	})
-
 	// Set terminal emulation based on platform as required.
 	_, stdout, stderr := term.StdStreams()
-
-	initLogging(stdout, stderr)
-	configureGRPCLog()
-
 	onError := func(err error) {
 		fmt.Fprintf(stderr, "%s\n", err)
 		os.Exit(1)
 	}
 
+	// initial log formatting; this setting is updated after the daemon configuration is loaded.
+	err := log.SetFormat(log.TextFormat)
+	if err != nil {
+		onError(err)
+	}
+
+	initLogging(stdout, stderr)
+	configureGRPCLog()
+
 	cmd, err := newDaemonCommand()
 	if err != nil {
 		onError(err)

cmd/dockerd/docker_unix.go

@@ -5,7 +5,7 @@ package main
 import (
 	"io"
 
-	"github.com/sirupsen/logrus"
+	"github.com/containerd/containerd/log"
 )
 
 func runDaemon(opts *daemonOptions) error {
@@ -14,5 +14,5 @@ func runDaemon(opts *daemonOptions) error {
 }
 
 func initLogging(_, stderr io.Writer) {
-	logrus.SetOutput(stderr)
+	log.L.Logger.SetOutput(stderr)
 }

cmd/dockerd/docker_windows.go

@@ -5,7 +5,7 @@ import (
 	"path/filepath"
 
 	"github.com/Microsoft/go-winio/pkg/etwlogrus"
-	"github.com/sirupsen/logrus"
+	"github.com/containerd/containerd/log"
 )
 
 func runDaemon(opts *daemonOptions) error {
@@ -41,13 +41,13 @@ func runDaemon(opts *daemonOptions) error {
 func initLogging(stdout, _ io.Writer) {
 	// Maybe there is a historic reason why on non-Windows, stderr is used
 	// for output. However, on Windows it makes no sense and there is no need.
-	logrus.SetOutput(stdout)
+	log.L.Logger.SetOutput(stdout)
 
 	// Provider ID: {6996f090-c5de-5082-a81e-5841acc3a635}
 	// Hook isn't closed explicitly, as it will exist until process exit.
 	// GUID is generated based on name - see Microsoft/go-winio/tools/etw-provider-gen.
 	if hook, err := etwlogrus.NewHook("Moby"); err == nil {
-		logrus.AddHook(hook)
+		log.L.Logger.AddHook(hook)
 	}
 	return
 }

cmd/dockerd/grpclog.go

@@ -4,7 +4,6 @@ import (
 	"context"
 
 	"github.com/containerd/containerd/log"
-	"github.com/sirupsen/logrus"
 	"google.golang.org/grpc/grpclog"
 )
 
@@ -16,5 +15,5 @@ import (
 // error => warn
 func configureGRPCLog() {
 	l := log.G(context.TODO()).WithField("library", "grpc")
-	grpclog.SetLoggerV2(grpclog.NewLoggerV2(l.WriterLevel(logrus.TraceLevel), l.WriterLevel(logrus.DebugLevel), l.WriterLevel(logrus.WarnLevel)))
+	grpclog.SetLoggerV2(grpclog.NewLoggerV2(l.WriterLevel(log.TraceLevel), l.WriterLevel(log.DebugLevel), l.WriterLevel(log.WarnLevel)))
 }

cmd/dockerd/service_windows.go

@@ -10,8 +10,6 @@ import (
 	"path/filepath"
 	"time"
 
-	"github.com/sirupsen/logrus"
-
 	"github.com/containerd/containerd/log"
 	"github.com/spf13/pflag"
 	"golang.org/x/sys/windows"
@@ -63,40 +61,40 @@ type etwHook struct {
 	log *eventlog.Log
 }
 
-func (h *etwHook) Levels() []logrus.Level {
-	return []logrus.Level{
-		logrus.PanicLevel,
-		logrus.FatalLevel,
-		logrus.ErrorLevel,
-		logrus.WarnLevel,
-		logrus.InfoLevel,
-		logrus.DebugLevel,
+func (h *etwHook) Levels() []log.Level {
+	return []log.Level{
+		log.PanicLevel,
+		log.FatalLevel,
+		log.ErrorLevel,
+		log.WarnLevel,
+		log.InfoLevel,
+		log.DebugLevel,
 	}
 }
 
-func (h *etwHook) Fire(e *logrus.Entry) error {
+func (h *etwHook) Fire(e *log.Entry) error {
 	var (
 		etype uint16
 		eid   uint32
 	)
 
 	switch e.Level {
-	case logrus.PanicLevel:
+	case log.PanicLevel:
 		etype = windows.EVENTLOG_ERROR_TYPE
 		eid = eventPanic
-	case logrus.FatalLevel:
+	case log.FatalLevel:
 		etype = windows.EVENTLOG_ERROR_TYPE
 		eid = eventFatal
-	case logrus.ErrorLevel:
+	case log.ErrorLevel:
 		etype = windows.EVENTLOG_ERROR_TYPE
 		eid = eventError
-	case logrus.WarnLevel:
+	case log.WarnLevel:
 		etype = windows.EVENTLOG_WARNING_TYPE
 		eid = eventWarn
-	case logrus.InfoLevel:
+	case log.InfoLevel:
 		etype = windows.EVENTLOG_INFORMATION_TYPE
 		eid = eventInfo
-	case logrus.DebugLevel:
+	case log.DebugLevel:
 		etype = windows.EVENTLOG_INFORMATION_TYPE
 		eid = eventDebug
 	default:
@@ -246,16 +244,16 @@ func initService(daemonCli *DaemonCli) (bool, bool, error) {
 		daemonCli: daemonCli,
 	}
 
-	var log *eventlog.Log
+	var eventLog *eventlog.Log
 	if isService {
-		log, err = eventlog.Open(*flServiceName)
+		eventLog, err = eventlog.Open(*flServiceName)
 		if err != nil {
 			return false, false, err
 		}
 	}
 
-	logrus.AddHook(&etwHook{log})
-	logrus.SetOutput(io.Discard)
+	log.L.Logger.AddHook(&etwHook{eventLog})
+	log.L.Logger.SetOutput(io.Discard)
 
 	service = h
 	go func() {
@@ -373,7 +371,7 @@ func initPanicFile(path string) error {
 	os.Stderr = os.NewFile(panicFile.Fd(), "/dev/stderr")
 
 	// Force threads that panic to write to stderr (the panicFile handle now), otherwise it will go into the ether
-	logrus.SetOutput(os.Stderr)
+	log.L.Logger.SetOutput(os.Stderr)
 
 	return nil
 }

daemon/cluster/controllers/plugin/controller.go

@@ -16,7 +16,6 @@ import (
 	"github.com/gogo/protobuf/proto"
 	"github.com/moby/swarmkit/v2/api"
 	"github.com/pkg/errors"
-	"github.com/sirupsen/logrus"
 )
 
 // Controller is the controller for the plugin backend.
@@ -31,7 +30,7 @@ import (
 type Controller struct {
 	backend Backend
 	spec    runtime.PluginSpec
-	logger  *logrus.Entry
+	logger  *log.Entry
 
 	pluginID  string
 	serviceID string

daemon/cluster/controllers/plugin/controller_test.go

@@ -9,6 +9,7 @@ import (
 	"testing"
 	"time"
 
+	"github.com/containerd/containerd/log"
 	"github.com/distribution/reference"
 	"github.com/docker/docker/api/types"
 	"github.com/docker/docker/api/types/registry"
@@ -321,7 +322,7 @@ func TestRemove(t *testing.T) {
 
 func newTestController(b Backend, disabled bool) *Controller {
 	return &Controller{
-		logger:  &logrus.Entry{Logger: &logrus.Logger{Out: io.Discard}},
+		logger:  &log.Entry{Logger: &logrus.Logger{Out: io.Discard}},
 		backend: b,
 		spec: runtime.PluginSpec{
 			Name:     pluginTestName,

daemon/config/config.go

@@ -19,7 +19,6 @@ import (
 	"github.com/docker/docker/registry"
 	"github.com/imdario/mergo"
 	"github.com/pkg/errors"
-	"github.com/sirupsen/logrus"
 	"github.com/spf13/pflag"
 )
 
@@ -586,7 +585,12 @@ func findConfigurationConflicts(config map[string]interface{}, flags *pflag.Flag
 func Validate(config *Config) error {
 	// validate log-level
 	if config.LogLevel != "" {
-		if _, err := logrus.ParseLevel(config.LogLevel); err != nil {
+		// FIXME(thaJeztah): find a better way for this; this depends on knowledge of containerd's log package internals.
+		// Alternatively: try  log.SetLevel(config.LogLevel), and restore the original level, but this also requires internal knowledge.
+		switch strings.ToLower(config.LogLevel) {
+		case "panic", "fatal", "error", "warn", "info", "debug", "trace":
+			// These are valid. See [log.SetLevel] for a list of accepted levels.
+		default:
 			return errors.Errorf("invalid logging level: %s", config.LogLevel)
 		}
 	}

daemon/container_unix_test.go

@@ -23,7 +23,7 @@ func TestContainerWarningHostAndPublishPorts(t *testing.T) {
 			"8080": []nat.PortBinding{{HostPort: "8989"}},
 		}, warnings: []string{"Published ports are discarded when using host network mode"}},
 	}
-	muteLogs()
+	muteLogs(t)
 
 	for _, tc := range testCases {
 		hostConfig := &containertypes.HostConfig{

daemon/daemon.go

@@ -17,8 +17,6 @@ import (
 	"sync/atomic"
 	"time"
 
-	"github.com/sirupsen/logrus"
-
 	"github.com/containerd/containerd"
 	"github.com/containerd/containerd/defaults"
 	"github.com/containerd/containerd/log"
@@ -358,7 +356,7 @@ func (daemon *Daemon) restore(cfg *configStore) error {
 
 			daemon.setStateCounter(c)
 
-			logger := func(c *container.Container) *logrus.Entry {
+			logger := func(c *container.Container) *log.Entry {
 				return baseLogger.WithFields(log.Fields{
 					"running":    c.IsRunning(),
 					"paused":     c.IsPaused(),

daemon/daemon_unix_test.go

@@ -69,7 +69,7 @@ func TestAdjustCPUShares(t *testing.T) {
 		root:       tmp,
 	}
 	cfg := &config.Config{}
-	muteLogs()
+	muteLogs(t)
 
 	hostConfig := &containertypes.HostConfig{
 		Resources: containertypes.Resources{CPUShares: linuxMinCPUShares - 1},

daemon/graphdriver/overlayutils/randomid.go

@@ -11,12 +11,12 @@ import (
 	"syscall"
 	"time"
 
-	"github.com/sirupsen/logrus"
+	"github.com/containerd/containerd/log"
 	"golang.org/x/sys/unix"
 )
 
 // GenerateID creates a new random string identifier with the given length
-func GenerateID(l int, logger *logrus.Entry) string {
+func GenerateID(l int, logger *log.Entry) string {
 	const (
 		// ensures we backoff for less than 450ms total. Use the following to
 		// select new value, in units of 10ms:

daemon/health_test.go

@@ -78,7 +78,7 @@ func TestHealthStates(t *testing.T) {
 		EventsService:     e,
 		containersReplica: store,
 	}
-	muteLogs()
+	muteLogs(t)
 
 	c.Config.Healthcheck = &containertypes.HealthConfig{
 		Retries: 1,

daemon/logger/loggerutils/follow.go

@@ -9,7 +9,6 @@ import (
 	"github.com/containerd/containerd/log"
 	"github.com/docker/docker/daemon/logger"
 	"github.com/pkg/errors"
-	"github.com/sirupsen/logrus"
 )
 
 type follow struct {
@@ -18,7 +17,7 @@ type follow struct {
 	Decoder   Decoder
 	Forwarder *forwarder
 
-	log *logrus.Entry
+	log *log.Entry
 	c   chan logPos
 }
 

daemon/reload_test.go

@@ -5,18 +5,22 @@ import (
 	"sort"
 	"testing"
 
+	"github.com/containerd/containerd/log"
 	"github.com/docker/docker/daemon/config"
 	"github.com/docker/docker/daemon/images"
 	"github.com/docker/docker/libnetwork"
 	"github.com/docker/docker/registry"
-	"github.com/sirupsen/logrus"
 	"gotest.tools/v3/assert"
 	is "gotest.tools/v3/assert/cmp"
 )
 
 // muteLogs suppresses logs that are generated during the test
-func muteLogs() {
-	logrus.SetLevel(logrus.ErrorLevel)
+func muteLogs(t *testing.T) {
+	t.Helper()
+	err := log.SetLevel("error")
+	if err != nil {
+		t.Error(err)
+	}
 }
 
 func newDaemonForReloadT(t *testing.T, cfg *config.Config) *Daemon {
@@ -37,7 +41,7 @@ func TestDaemonReloadLabels(t *testing.T) {
 			Labels: []string{"foo:bar"},
 		},
 	})
-	muteLogs()
+	muteLogs(t)
 
 	valuesSets := make(map[string]interface{})
 	valuesSets["labels"] = "foo:baz"
@@ -60,7 +64,7 @@ func TestDaemonReloadLabels(t *testing.T) {
 
 func TestDaemonReloadAllowNondistributableArtifacts(t *testing.T) {
 	daemon := newDaemonForReloadT(t, &config.Config{})
-	muteLogs()
+	muteLogs(t)
 
 	var err error
 	// Initialize daemon with some registries.
@@ -116,7 +120,7 @@ func TestDaemonReloadMirrors(t *testing.T) {
 	daemon := &Daemon{
 		imageService: images.NewImageService(images.ImageServiceConfig{}),
 	}
-	muteLogs()
+	muteLogs(t)
 
 	var err error
 	daemon.registryService, err = registry.NewService(registry.ServiceOptions{
@@ -215,7 +219,7 @@ func TestDaemonReloadInsecureRegistries(t *testing.T) {
 	daemon := &Daemon{
 		imageService: images.NewImageService(images.ImageServiceConfig{}),
 	}
-	muteLogs()
+	muteLogs(t)
 
 	var err error
 	// initialize daemon with existing insecure registries: "127.0.0.0/8", "10.10.1.11:5000", "10.10.1.22:5000"
@@ -316,7 +320,7 @@ func TestDaemonReloadNotAffectOthers(t *testing.T) {
 			Debug:  true,
 		},
 	})
-	muteLogs()
+	muteLogs(t)
 
 	valuesSets := make(map[string]interface{})
 	valuesSets["labels"] = "foo:baz"

libcontainerd/local/local_windows.go

@@ -15,8 +15,6 @@ import (
 	"syscall"
 	"time"
 
-	"github.com/sirupsen/logrus"
-
 	"github.com/Microsoft/hcsshim"
 	"github.com/containerd/containerd"
 	"github.com/containerd/containerd/cio"
@@ -81,7 +79,7 @@ const defaultOwner = "docker"
 type client struct {
 	stateDir string
 	backend  libcontainerdtypes.Backend
-	logger   *logrus.Entry
+	logger   *log.Entry
 	eventQ   queue.Queue
 }
 

libcontainerd/remote/client.go

@@ -33,7 +33,6 @@ import (
 	ocispec "github.com/opencontainers/image-spec/specs-go/v1"
 	specs "github.com/opencontainers/runtime-spec/specs-go"
 	"github.com/pkg/errors"
-	"github.com/sirupsen/logrus"
 	"google.golang.org/grpc/codes"
 	"google.golang.org/grpc/status"
 )
@@ -44,7 +43,7 @@ const DockerContainerBundlePath = "com.docker/engine.bundle.path"
 type client struct {
 	client   *containerd.Client
 	stateDir string
-	logger   *logrus.Entry
+	logger   *log.Entry
 	ns       string
 
 	backend libcontainerdtypes.Backend

libcontainerd/remote/client_linux.go

@@ -14,7 +14,6 @@ import (
 	libcontainerdtypes "github.com/docker/docker/libcontainerd/types"
 	"github.com/docker/docker/pkg/idtools"
 	specs "github.com/opencontainers/runtime-spec/specs-go"
-	"github.com/sirupsen/logrus"
 )
 
 func summaryFromInterface(i interface{}) (*libcontainerdtypes.Summary, error) {
@@ -86,7 +85,7 @@ func WithBundle(bundleDir string, ociSpec *specs.Spec) containerd.NewContainerOp
 	}
 }
 
-func withLogLevel(_ logrus.Level) containerd.NewTaskOpts {
+func withLogLevel(_ log.Level) containerd.NewTaskOpts {
 	panic("Not implemented")
 }
 

libcontainerd/remote/client_windows.go

@@ -10,10 +10,10 @@ import (
 	"github.com/containerd/containerd"
 	"github.com/containerd/containerd/cio"
 	"github.com/containerd/containerd/containers"
+	"github.com/containerd/containerd/log"
 	libcontainerdtypes "github.com/docker/docker/libcontainerd/types"
 	specs "github.com/opencontainers/runtime-spec/specs-go"
 	"github.com/pkg/errors"
-	"github.com/sirupsen/logrus"
 )
 
 func summaryFromInterface(i interface{}) (*libcontainerdtypes.Summary, error) {
@@ -47,10 +47,10 @@ func WithBundle(bundleDir string, ociSpec *specs.Spec) containerd.NewContainerOp
 	}
 }
 
-func withLogLevel(level logrus.Level) containerd.NewTaskOpts {
+func withLogLevel(level log.Level) containerd.NewTaskOpts {
 	// Make sure we set the runhcs options to debug if we are at debug level.
 	return func(_ context.Context, _ *containerd.Client, info *containerd.TaskInfo) error {
-		if level == logrus.DebugLevel {
+		if level == log.DebugLevel {
 			info.Options = &options.Options{Debug: true}
 		}
 		return nil

libcontainerd/supervisor/remote_daemon.go

@@ -19,7 +19,6 @@ import (
 	"github.com/docker/docker/pkg/system"
 	"github.com/pelletier/go-toml"
 	"github.com/pkg/errors"
-	"github.com/sirupsen/logrus"
 	"go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc"
 	"google.golang.org/grpc"
 	"google.golang.org/grpc/credentials/insecure"
@@ -44,7 +43,7 @@ type remote struct {
 
 	daemonPid int
 	pidFile   string
-	logger    *logrus.Entry
+	logger    *log.Entry
 
 	daemonWaitCh  chan struct{}
 	daemonStartCh chan error

plugin/executor/containerd/containerd.go

@@ -15,7 +15,6 @@ import (
 	libcontainerdtypes "github.com/docker/docker/libcontainerd/types"
 	specs "github.com/opencontainers/runtime-spec/specs-go"
 	"github.com/pkg/errors"
-	"github.com/sirupsen/logrus"
 )
 
 // ExitHandler represents an object that is called when the exit event is received from containerd
@@ -54,7 +53,7 @@ type Executor struct {
 }
 
 type c8dPlugin struct {
-	log *logrus.Entry
+	log *log.Entry
 	ctr libcontainerdtypes.Container
 	tsk libcontainerdtypes.Task
 }

plugin/manager.go

@@ -307,11 +307,19 @@ func (pm *Manager) GC() {
 
 type logHook struct{ id string }
 
-func (logHook) Levels() []logrus.Level {
-	return logrus.AllLevels
+func (logHook) Levels() []log.Level {
+	return []log.Level{
+		log.PanicLevel,
+		log.FatalLevel,
+		log.ErrorLevel,
+		log.WarnLevel,
+		log.InfoLevel,
+		log.DebugLevel,
+		log.TraceLevel,
+	}
 }
 
-func (l logHook) Fire(entry *logrus.Entry) error {
+func (l logHook) Fire(entry *log.Entry) error {
 	entry.Data = log.Fields{"plugin": l.id}
 	return nil
 }
@@ -319,7 +327,7 @@ func (l logHook) Fire(entry *logrus.Entry) error {
 func makeLoggerStreams(id string) (stdout, stderr io.WriteCloser) {
 	logger := logrus.New()
 	logger.Hooks.Add(logHook{id})
-	return logger.WriterLevel(logrus.InfoLevel), logger.WriterLevel(logrus.ErrorLevel)
+	return logger.WriterLevel(log.InfoLevel), logger.WriterLevel(log.ErrorLevel)
 }
 
 func validatePrivileges(requiredPrivileges, privileges types.PluginPrivileges) error {