Skip to content

Proposal: Go 2: Create logger interface in the standard log package, with additional levels. Optionally add it to context package, too #28412

Closed
@EVODelavega

Description

@EVODelavega

Logging is important, I think we can all agree that it is. The standard "log" package simply is too limited as it stands, so it's extremely common to use a third party package (logrus, zerolog, etc...). These third party packages don't necessarily share the same interface. It would be great to have a usable logger in the standard lib, and an interface so the community can adopt a standard approach to logging.

So what is the current logger missing?

Currently, only focusing on the output functions, we have 3 to choose from:

  • Print & co (Println, Printf)
  • Fatal & co (Fatalln, Fatalf) - equivalent to the Print equivalent + os.Exit(1)
  • Panic & co (Panicln, Panicf) - equivalent to Print equivalent + panic()

Naturally, the latter is not commonly used if you want to honour the go proverbs.

The most obvious reason for people to use a third party logger is to be able to differentiate the log output, using the standard levels. To this end, Print & co are effectively replaced with:

  • Debug & co
  • Info & co
  • Warn & co
  • Error & co

There are 2 more (fairly) common log levels that will be discussed separately further down: Trace and Custom.

Contextual fields/values (prefix &co)

Something that is commonly done, too, is adding trace id's or correlation id's in logger output. With the standard package, it's possible to write something like this:

    l := log.NewLogger(
       os.Stdout,
       fmt.Sprintf("trace ID: %v", ctx.Value(traceIDKey)),
       log.Ldate | log.Ltime | log.Lshortfile,
    )

Third party packages allow you to add any number of prefixes/fields to individual messages, by doing things like:

    zerologger.Debug().Str("trace ID", traceIDValue).Msg("this is the message")
    logrus.WithField("trace ID", traceIDValue).Debug("this is the message")

Should I want to do the same thing with the standard logger, I need to make sure the particular logger isn't used concurrently. Sure, the logger guarantees the writer is accessed sequentially, but I'm forced to write this:

    oldPrefix := stdLogger.Prefix()
    stdLogger.SetPrefix(fmt.Sprintf("%s - trace ID: %s", oldPrefix traceID)) // sets the prefix
    stdLogger.Println("this is the message")
    stdLogger.SetPrefix(oldPrefix)

Clearly, though the writer is accessed sequentially, it's not hard to imagine that there could be a second routine either calling Print after you've set the prefix, or conversely: it's equally possible that the prefix you've just set is overwritten by another routine between your setting a prefix, and actually logging the output. This is a big problem that needs to be address should we want the standard log package to be usable in real life.

What about context.Context?

That brings me to something I'd personally consider to be a nice addition to the log package in general. Those trace ID's are very commonly used, and very useful. Quite often people end up writing their own packages, or handlers to automatically get the trace ID's from request headers (or generate them, or wherever), and add them to the context. It would be nice to be able to add this to the logger API:

type Logger Interface {
    Debug(string) // all the log levels etc...
    SetCtxCallback(func(context.Context, Logger) context.Context) /// add this
    Ctx(context.Context) (Logger, context.Context) // add this
}

With these additional functions, I'd be able to create a logger and do something like this:

    logger.SetCtxCallback(func (ctx context.Context, logger log.Logger) context.Context {
        tID := ctx.Value(traceIDKey)
        if traceID, ok := tID.(string); ok {
            logger.SetPrefix(fmt.Sprintf("trace ID: ", traceID))
            return ctx// done, added the prefix
        }
        // generate new traceID
        logger.SetPrefix(fmt.Sprintf("trace ID: ", traceID))
        return context.WithValue(ctx, traceIDKey, traceID) // return context with value
    })

// use like this:

logger, ctx = logger.Ctx(ctx)
logger.Infof("There is a prefix with the trace ID %v", ctx.Value(traceIDKey))

Something that might be worth discussing is that, given the logger can be made context aware now, whether or not the log levels could be ramped up should the context be cancelled by the time the writer is reached. Maybe that could be configured. This probably doesn't add much value, and is a bit silly, but just thought I'd mention it.

Trace, custom levels, and the Print & co calls

The trace log level is debatable. There already is a Log function in the runtime/trace package, and I'd argue it's better to keep it there. The Trace(f|ln) functions can simply be omitted from the logger interface.

Personally, I'm not sure if I've ever felt the need to have a custom log level. It's possible that there might be a genuine need to support this, perhaps not. Should this be added, I think the best approach to use is to implement a Custom & co function that takes 1 additional argument allowing to specify the desired level:

logger.Custom("my-level", "this is the message")

My idea for the Printcalls is to have them dynamically log at the minimum log level that is enabled. During development (or whenever the log level is set to debug, Print will be equivalent to Debug, if the level is set to warn, then Print is equivalent to Warn etc... This is not to encourage abuse, but more to make the transition to the standard package easier. Of course, the Print calls should not ramp up to Fatal if the configured log level is Fatal. Arguably, it shouldn't go higher than Warn.

Specific changes to the log package

TL;DR, here's what this proposal boils down to:

Change the log.Logger type to an interface

Having a common denominator logger interface in the standard package would make life a lot easier, and could go a long way WRT encouraging drop-in replacement packages for logging, or at the very least: uniformity. The interface I have in mind is this:

type Logger interface {
    // all levels + Prin
    Print(v ...interface{})
    Printf(format string, v ...interface{})
    Println(v ...interface{})
    Debug(v ...interface{})
    Debugf(format string, v ...interface{})
    Debugln(v ...interface{})
    Info(v ...interface{})
    Infof(format string, v ...interface{})
    Infoln(v ...interface{})
    Warn(v ...interface{})
    Warnf(format string, v ...interface{})
    Warnln(v ...interface{})
    Error(v ...interface{})
    Errorf(format string, v ...interface{})
    Errorln(v ...interface{})
    Fatal(v ...interface{})
    Fatalf(format string, v ...interface{})
    Fatalln(v ...interface{})
    Panic(v ...interface{})
    Panicf(format string, v ...interface{})
    Panicln(v ...interface{})

    // Prefix - chainable so it can create a logger instance, safe for concurrent use
    Prefix(prefix string) Logger
    // Prefixf to avoid having to use fmt.Sprintf whenever using this
    Prefixf(fromat string, v ...interface{}) Logger
    // fields for other formatters, acts like prefix be default
    WithField(k, v interface{}) Logger

    // Context stuff as explained earlier
    Ctx(ctx context.Context) (Logger, context.Context)
    SetCtxCallback(func(ctx context.Context, logger Logger) context.Context)
}

Closing thoughts

I'm aware that there are a couple of things about this proposal that are perhaps too broad. I'm looking forward to your feedback, comments, ideas, etc... I honestly believe the log package needs to be a bit more flexible, and I think this is a good way of going about it. Then again, that's one persons idea/opinion

Metadata

Metadata

Assignees

No one assigned

    Labels

    Type

    No type

    Projects

    No projects

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions