Merge pull request #8894 from thaJeztah/log_improve

log: cleanups and improvements to decouple more from logrus

cmd/containerd/command/main.go

@@ -357,7 +357,7 @@ func setLogLevel(context *cli.Context, config *srvconfig.Config) error {
 }
 
 func setLogFormat(config *srvconfig.Config) error {
-	f := config.Debug.Format
+	f := log.OutputFormat(config.Debug.Format)
 	if f == "" {
 		f = log.TextFormat
 	}

log/context.go

@@ -14,6 +14,27 @@
    limitations under the License.
 */
 
+// Package log provides types and functions related to logging, passing
+// loggers through a context, and attaching context to the logger.
+//
+// # Transitional types
+//
+// This package contains various types that are aliases for types in [logrus].
+// These aliases are intended for transitioning away from hard-coding logrus
+// as logging implementation. Consumers of this package are encouraged to use
+// the type-aliases from this package instead of directly using their logrus
+// equivalent.
+//
+// The intent is to replace these aliases with locally defined types and
+// interfaces once all consumers are no longer directly importing logrus
+// types.
+//
+// IMPORTANT: due to the transitional purpose of this package, it is not
+// guaranteed for the full logrus API to be provided in the future. As
+// outlined, these aliases are provided as a step to transition away from
+// a specific implementation which, as a result, exposes the full logrus API.
+// While no decisions have been made on the ultimate design and interface
+// provided by this package, we do not expect carrying "less common" features.
 package log
 
 import (
@@ -23,98 +44,138 @@ import (
 	"github.com/sirupsen/logrus"
 )
 
-var (
-	// G is an alias for GetLogger.
-	//
-	// We may want to define this locally to a package to get package tagged log
-	// messages.
-	G = GetLogger
-
 // L is an alias for the standard logger.
-	L = logrus.NewEntry(logrus.StandardLogger())
+var L = &Entry{
-)
+	Logger: logrus.StandardLogger(),
+	// Default is three fields plus a little extra room.
+	Data: make(Fields, 6),
+}
 
-type (
+type loggerKey struct{}
-	loggerKey struct{}
 
-	// Fields type to pass to `WithFields`, alias from `logrus`.
+// Fields type to pass to "WithFields".
-	Fields = logrus.Fields
+type Fields = map[string]any
 
-	// Level is a logging level
+// Entry is a logging entry. It contains all the fields passed with
-	Level = logrus.Level
+// [Entry.WithFields]. It's finally logged when Trace, Debug, Info, Warn,
-)
+// Error, Fatal or Panic is called on it. These objects can be reused and
+// passed around as much as you wish to avoid field duplication.
+//
+// Entry is a transitional type, and currently an alias for [logrus.Entry].
+type Entry = logrus.Entry
 
+// RFC3339NanoFixed is [time.RFC3339Nano] with nanoseconds padded using
+// zeros to ensure the formatted time is always the same number of
+// characters.
+const RFC3339NanoFixed = "2006-01-02T15:04:05.000000000Z07:00"
+
+// Level is a logging level.
+type Level = logrus.Level
+
+// Supported log levels.
 const (
-	// RFC3339NanoFixed is time.RFC3339Nano with nanoseconds padded using zeros to
+	// TraceLevel level. Designates finer-grained informational events
-	// ensure the formatted time is always the same number of characters.
+	// than [DebugLevel].
-	RFC3339NanoFixed = "2006-01-02T15:04:05.000000000Z07:00"
+	TraceLevel Level = logrus.TraceLevel
 
-	// TextFormat represents the text logging format
+	// DebugLevel level. Usually only enabled when debugging. Very verbose
-	TextFormat = "text"
+	// logging.
+	DebugLevel Level = logrus.DebugLevel
 
-	// JSONFormat represents the JSON logging format
+	// InfoLevel level. General operational entries about what's going on
-	JSONFormat = "json"
+	// inside the application.
+	InfoLevel Level = logrus.InfoLevel
 
-	// TraceLevel level.
+	// WarnLevel level. Non-critical entries that deserve eyes.
-	TraceLevel = logrus.TraceLevel
+	WarnLevel Level = logrus.WarnLevel
 
-	// DebugLevel level.
+	// ErrorLevel level. Logs errors that should definitely be noted.
-	DebugLevel = logrus.DebugLevel
+	// Commonly used for hooks to send errors to an error tracking service.
+	ErrorLevel Level = logrus.ErrorLevel
 
-	// InfoLevel level.
+	// FatalLevel level. Logs and then calls "logger.Exit(1)". It exits
-	InfoLevel = logrus.InfoLevel
+	// even if the logging level is set to Panic.
+	FatalLevel Level = logrus.FatalLevel
+
+	// PanicLevel level. This is the highest level of severity. Logs and
+	// then calls panic with the message passed to Debug, Info, ...
+	PanicLevel Level = logrus.PanicLevel
 )
 
-// SetLevel sets log level globally.
+// SetLevel sets log level globally. It returns an error if the given
+// level is not supported.
+//
+// level can be one of:
+//
+//   - "trace" ([TraceLevel])
+//   - "debug" ([DebugLevel])
+//   - "info" ([InfoLevel])
+//   - "warn" ([WarnLevel])
+//   - "error" ([ErrorLevel])
+//   - "fatal" ([FatalLevel])
+//   - "panic" ([PanicLevel])
 func SetLevel(level string) error {
 	lvl, err := logrus.ParseLevel(level)
 	if err != nil {
 		return err
 	}
 
-	logrus.SetLevel(lvl)
+	L.Logger.SetLevel(lvl)
 	return nil
 }
 
 // GetLevel returns the current log level.
 func GetLevel() Level {
-	return logrus.GetLevel()
+	return L.Logger.GetLevel()
 }
 
-// SetFormat sets log output format
+// OutputFormat specifies a log output format.
-func SetFormat(format string) error {
+type OutputFormat string
+
+// Supported log output formats.
+const (
+	// TextFormat represents the text logging format.
+	TextFormat OutputFormat = "text"
+
+	// JSONFormat represents the JSON logging format.
+	JSONFormat OutputFormat = "json"
+)
+
+// SetFormat sets the log output format ([TextFormat] or [JSONFormat]).
+func SetFormat(format OutputFormat) error {
 	switch format {
 	case TextFormat:
-		logrus.SetFormatter(&logrus.TextFormatter{
+		L.Logger.SetFormatter(&logrus.TextFormatter{
 			TimestampFormat: RFC3339NanoFixed,
 			FullTimestamp:   true,
 		})
+		return nil
 	case JSONFormat:
-		logrus.SetFormatter(&logrus.JSONFormatter{
+		L.Logger.SetFormatter(&logrus.JSONFormatter{
 			TimestampFormat: RFC3339NanoFixed,
 		})
+		return nil
 	default:
 		return fmt.Errorf("unknown log format: %s", format)
 	}
-
-	return nil
 }
 
 // WithLogger returns a new context with the provided logger. Use in
 // combination with logger.WithField(s) for great effect.
-func WithLogger(ctx context.Context, logger *logrus.Entry) context.Context {
+func WithLogger(ctx context.Context, logger *Entry) context.Context {
-	e := logger.WithContext(ctx)
+	return context.WithValue(ctx, loggerKey{}, logger.WithContext(ctx))
-	return context.WithValue(ctx, loggerKey{}, e)
 }
 
 // GetLogger retrieves the current logger from the context. If no logger is
 // available, the default logger is returned.
-func GetLogger(ctx context.Context) *logrus.Entry {
+func GetLogger(ctx context.Context) *Entry {
-	logger := ctx.Value(loggerKey{})
+	return G(ctx)
+}
 
-	if logger == nil {
+// G is a shorthand for [GetLogger].
+func G(ctx context.Context) *Entry {
+	if logger := ctx.Value(loggerKey{}); logger != nil {
+		return logger.(*Entry)
+	}
 	return L.WithContext(ctx)
 }
-
-	return logger.(*logrus.Entry)
-}

log/context_test.go

@@ -18,15 +18,46 @@ package log
 
 import (
 	"context"
+	"reflect"
 	"testing"
 
-	"github.com/stretchr/testify/assert"
+	"github.com/sirupsen/logrus"
 )
 
 func TestLoggerContext(t *testing.T) {
+	const expected = "one"
 	ctx := context.Background()
-
+	ctx = WithLogger(ctx, G(ctx).WithField("test", expected))
-	ctx = WithLogger(ctx, G(ctx).WithField("test", "one"))
+	if actual := GetLogger(ctx).Data["test"]; actual != expected {
-	assert.Equal(t, GetLogger(ctx).Data["test"], "one")
+		t.Errorf("expected: %v, got: %v", expected, actual)
-	assert.Same(t, G(ctx), GetLogger(ctx)) // these should be the same.
+	}
+	a := G(ctx)
+	b := GetLogger(ctx)
+	if !reflect.DeepEqual(a, b) || a != b {
+		t.Errorf("should be the same: %+v, %+v", a, b)
+	}
+}
+
+func TestCompat(t *testing.T) {
+	expected := Fields{
+		"hello1": "world1",
+		"hello2": "world2",
+		"hello3": "world3",
+	}
+
+	l := G(context.TODO())
+	l = l.WithFields(logrus.Fields{"hello1": "world1"})
+	l = l.WithFields(Fields{"hello2": "world2"})
+	l = l.WithFields(map[string]any{"hello3": "world3"})
+	if !reflect.DeepEqual(Fields(l.Data), expected) {
+		t.Errorf("expected: (%[1]T) %+[1]v, got: (%[2]T) %+[2]v", expected, l.Data)
+	}
+
+	l2 := L
+	l2 = l2.WithFields(logrus.Fields{"hello1": "world1"})
+	l2 = l2.WithFields(Fields{"hello2": "world2"})
+	l2 = l2.WithFields(map[string]any{"hello3": "world3"})
+	if !reflect.DeepEqual(Fields(l2.Data), expected) {
+		t.Errorf("expected: (%[1]T) %+[1]v, got: (%[2]T) %+[2]v", expected, l2.Data)
+	}
 }

remotes/docker/resolver.go

@@ -673,7 +673,7 @@ func requestFields(req *http.Request) log.Fields {
 		}
 	}
 
-	return log.Fields(fields)
+	return fields
 }
 
 func responseFields(resp *http.Response) log.Fields {
@@ -691,7 +691,7 @@ func responseFields(resp *http.Response) log.Fields {
 		}
 	}
 
-	return log.Fields(fields)
+	return fields
 }
 
 // IsLocalhost checks if the registry host is local.