contextual logging for slog

[pohly]

The Golang community decided against introducing an API for passing a slog.Logger or slog.Handler via a context.Context. logr supports this with NewContext and FromContext[orDiscard] for logr.Logger. https://github.com/veqryn/slog-context supports this for slog.Logger. Both are not interoperable.

This issue is about exploring whether further work is needed in this area and if so, what that should be.

Here are some possibilities:

1. Add slogr.NewContext(ctx context.Context, logger *slog.Logger) and slogr.FromContext(ctx context.Context) *slog.Logger on top of the corresponding logr functions, with conversion to and from logr.Logger. Currently there's only documentation for how to write such functions. Drawback: additional allocations, in particular on each retrieval.

2. Move the conversion code from slogr and the context key definition into logr/internal. The value for that key can be either a logr.Logger or a * slog.Logger. Then slogr.NewContext can store its parameter under that context key and slogr.FromContext can retrieved it without allocations. Both slogr.FromContext and logr.FromContext would have to do type checks and convert if needed. Drawback: the main logr package depends on slog when built with Go > 1.21 (currently it doesn't).

If we do option 2, then https://github.com/veqryn/slog-context could use slogr under the hood to set and retrieve a logger instance and code using one or the other package would become interoperable.

Note that option 2 suggests to store and retrieve a slog.Logger because that avoids the memory allocation. Storing a slog.Handler implies that code must construct a slog.Logger when it wants to use one. We could also have functions for storing and retrieving a slog.Handler as a third alternative.

cc @veqryn @antichris

[pohly]

Option 3: None of the above. Developers who want to support contextual logging can do so with the logr.Logger API, which is more or less functionally equivalent to the slog.Logger API.

If this is the consensus, then we can do a few things in logr (all optional):

• document this recommendation more explicitly in slogr
• same in slog-context
• introduce log level constants in logr that match the ones in slog (e.g. const DEBUG int = 4 // -slog.DEBUG -> logger.V(logr.DEBUG).Info(...)) and/or aliases for V (logger.Debug().Info(...) - looks a bit weird?)

[thockin]

Given that we already have context support for logr users, can we more clearly state the problem we are trying to solve? Is it:

a) to implement context support for slog users in general
b) to implement context support for users who store slog in Context via some external lib
c) to define a common key under which many slog+context libs store *slog.Logger?
d) something else?

Given that Go itself does not define a standard way to get a slog.Logger into and out of a Context, code which knows slog will have to either use the slog default Logger, pass *slog.Logger manually, or choose some non-standard lib (including DIY) for slog+context. It's not really logr's responsibility. That said, if we offer such a lib or throw what little weight we have behind one, it might help establish a norm.

Do we think slog-context has or might have critical mass for this?

Can we describe the desired UX for whomever is going to consume whatever it is we are conceptualizing? I feel like there are 3 categories of users:

1. Code that knows how to use slog and not logr. I think they want the equivalent of func NewContext(ctx context.Context, logger *slog.Logger) context.Context and func FromContext(ctx context.Context) (*slog.Logger, error) - but maybe they want to speak in terms of slog.Handler ?

2. Code that knows how to use logr and not slog. We already have logr.NewContext() and logr.FromContext().

3. Code that bridges slog and logr.

Do we feel like there's going to be a lot of back-and-forth between domains (1) and (2) or is it more like "this lib uses logr, so let me convert my slog to logr and stick it in context" ?

[pohly]

Let's start with a problem statement: different packages defining their own keys for passing a logger (whether it's slog or logr) is a problem because it fragments the ecosystem. We have that problem with slog-context and logr.

My option 2 matches your point a - solve this for all slog users by trying to make logr the manager of "logger in context", regardless of the high-level API. The advantage of doing it in logr is that we can achieve interoperability with code which wants to use logr.Logger (like Kubernetes).

I'm not sure about your point b - is that "implement logr.Logger support for users who store slog in Context via some external lib"? My option 2 also would achieve that.

Point c is what slog-context already does. We don't need to replicate that in logr or slogr.

Point d might match my option 3: advocate to use logr.Logger when extracting a logger from a context.

[thockin]

different packages defining their own keys for passing a logger (whether it's slog or logr) is a problem because it fragments the ecosystem.

I agree with a caveat. I'm not sure I buy that "passing a logger" is a generic statement. Surely we don't intend this to support any arbitrary logging API with the same key, with a matrix of transformations, do we? As fun as that sounds, it doesn't seem appropriate to store a log.Logger and fetch back a logr.Logger or slog.Logger or slog.Handler or zap.SugaredLogger or ...

So what are the reasonable bounds?

We have that problem with slog-context and logr.

From a strict POV, I don't think we have a "problem", per se. Both slog-context and logr work just fine with their own logger types. But, because logr and slog are so closely aligned, we're trying to do better, right? It's not exactly "arbitrary" but "a select list of implementations which choose to coordinate".

My option 2 matches your point a - solve this for all slog users ... The advantage of doing it in logr is that we can achieve interoperability with code which wants to use logr.Logger (like Kubernetes).

It feels like overreaching, to me. If I am a slog user, why is logr involved? If I were building slog-context, why should I take a dep on logr?

I'm not sure about your point b

I mistyped - I meant "to implement /context/ support for users who store slog in Context via some external lib"

Point c is what slog-context already does.

slog-context includes a lot more than that, though most of it is just wrapping and re-presenting a similar interface.

Point d might match my option 3: advocate to use logr.Logger when extracting a logger from a context.

I think, realistically, this is backwards. IMO slog obviates logr. We have no grounds on which to advocate for new users of logr, even if we prefer our own technical decisions :)

So if we think slog-context is the "winner" of the "slog-plus-context" debate (right up until Go decides to subsume it), we should put energy there.

Simple mode:

• Make logr.FromContext() read our own key for logr or else fall back on slog-context and use slogr (circular dep needs to be fixed) to produce a logger.Logger.
• Make logr.NewContext() store both our logr key and (if applicable) the slog-context key.

Fancy mode (hand-waving):

• Define a single unilog key and library which allows plug-in converter functions, which implementations offer to convert to/from other loggers they know.
• Example: logr registers conversion from logr.Logger to *slog.Logger and vice-versa (and maybe slog.Handler, too).
• unilog.NewContext[T] stores whatever it was given, on the assumption that the next caller will want that back
• unilog.FromContext[T] sees what was stored and figures how to convert from that to T via plugins

Extra points for adding a "cost" per conversion function and doing a shortest-path conversion. But realisitically, how many of these will we have in a single binary? I see https://pkg.go.dev/github.com/juju/zaputil/zapctx and logrus and zerolog all do things a little differently.

[pohly]

Surely we don't intend this to support any arbitrary logging API with the same key, with a matrix of transformations, do we?

True. But slog and logr are conceptually similar enough that it is feasible and (my expectation) will win enough "market share" that covering those two would go a long way towards solving the problem.

It feels like overreaching, to me. If I am a slog user, why is logr involved? If I were building slog-context, why should I take a dep on logr?

The main advantage would be that a package using slog-context would work seamlessly in a Kubernetes binary that uses logr and klog's text logger or zapr. Without the integration, that binary can set the default slog logger (PR pending) but log entries then lack the context values.

So if we think slog-context is the "winner" of the "slog-plus-context" debate (right up until Go decides to subsume it), we should put energy there. Simple mode: ...

If the call sequence is logr.NewContext, slog-context.NewContext, logr.FromContext then looking first for the logr key will return the wrong logger. Same problem when reversing the order. It has to be a single key.

Fancy mode

That would work, but is more complex than doing it for logr and slog in logr. I'm just trying to be pragmatic here: IMHO accepting a dependency on slog is a small price to pay when (without changing program code) that enables using packages that use slog for contextual logging - if there are any.

I'm still not sure whether it's worth it. Option 3 seems perfectly fine to me, so it all depends on what other developers will pick for their packages.

[pohly]

Ping @veqryn - we need your input.

[thockin]

If the call sequence is logr.NewContext, slog-context.NewContext, logr.FromContext then looking first for the logr key will return the wrong logger. Same problem when reversing the order. It has to be a single key.

I feel like there's got to be some way, but I don't have time to prove it right now. If we get a logr, check to see if the logr.sloghandler == context's slog handler, and if not reconstruct the logr.

Writing it out, it starts to feel "too clever to be useful"

[pohly]

#213 is a prior PR for option 1. I am going to close it while we figure out what to do in this issue.

[pohly]

The main advantage would be that a package using slog-context would work seamlessly in a Kubernetes binary that uses logr and klog's text logger or zapr.

Perhaps I should have led with the inverse situation: Kubernetes packages like client-go will work seamlessly in a binary which uses slog + slog-context + some slog.Handler. That's probably more common than adding new dependencies to Kubernetes.

[veqryn]

Thank you for including me @pohly

So, just to recap:

• logr library has methods for storing and retrieving a logr.Logger from a context.Context
• slog-context library has methods for storing and retrieving a slog.Logger from a context.Context
• There exists some applications that presumably are using both a logr.Logger (perhaps in the main application) and also slog.Logger (perhaps in a library)
• We are exploring if we can make that easier or less error prone

And let me explore what integrating the two libraries looks like right now, without any changes to either library:

Primarily using logr workflow:

package main

import (
	"context"
	"log"
	"log/slog"
	"os"

	"github.com/go-logr/logr"
	"github.com/go-logr/logr/slogr"
	"github.com/go-logr/stdr"
	slogcontext "github.com/veqryn/slog-context"
)

func main() {
	logger := stdr.New(log.Default())                    // Create a logr.Logger
	ctx := logr.NewContext(context.Background(), logger) // Set it in the context

	slogger := slog.New(slogr.NewSlogHandler(logger)) // Convert logr.Logger to slog.Logger
	slog.SetDefault(slogger)                          // Set as default so that libraries that just use the default package level slog will still output the way we like
	ctx = slogcontext.ToCtx(ctx, slogger)             // Store the slog.Logger in the context too, so that libraries using slog.Logger from context output the way we like

	doSomething(ctx)
}

func doSomething(ctx context.Context) {
	logger := logr.FromContextOrDiscard(ctx).WithValues("myKey", "myVal") // Retrieve from context and store some attributes
	ctx = logr.NewContext(context.Background(), logger)                   // Set the new logr in the context, again
	ctx = slogcontext.ToCtx(ctx, slog.New(slogr.NewSlogHandler(logger)))  // Set the new slog in the context, again

	logger.Info("calling into library...")
	someLibraryWeHaveNoControlOver(ctx)
}

func someLibraryWeHaveNoControlOver(ctx context.Context) {
	slogcontext.Info(ctx, "in the library...")
}

And the primarily using slog-context workflow:

func main2() {
	logger := slog.New(slogcontext.NewHandler(slog.NewJSONHandler(os.Stdout, nil), nil)) // Create a slog.Logger
	slog.SetDefault(logger)                                                              // Set as default
	ctx := slogcontext.ToCtx(context.Background(), logger)                               // Store in the context

	// Convert slog.Logger to logr.Logger, and store the logr.Logger in the context too, so that libraries using logr.Logger from context output the way we like
	ctx = logr.NewContext(ctx, slogr.NewLogr(logger.Handler()))

	doSomething2(ctx)
}

func doSomething2(ctx context.Context) {
	ctx = slogcontext.With(ctx, "myKey", "myVal")                                // Retrieve from context and store some attributes, and store back in the context
	ctx = logr.NewContext(ctx, slogr.NewLogr(slogcontext.Logger(ctx).Handler())) // Convert the new slog to logr and set in the context

	slogcontext.Info(ctx, "calling into library...")
	someLibraryWeHaveNoControlOver2(ctx)
}

func someLibraryWeHaveNoControlOver2(ctx context.Context) {
	logr.FromContextOrDiscard(ctx).Info("in the library...")
}

We could cut down on some boilerplate with a few helper methods.

For the primarily logr workflow:

func doSomething(ctx context.Context) {
	logger := logr.FromContextOrDiscard(ctx).WithValues("myKey", "myVal")
	ctx = ContextWithLogrAndSlogr(ctx, logger)

	logger.Info("calling into library...")
	someLibraryWeHaveNoControlOver(ctx)
}

func ContextWithLogrAndSlogr(ctx context.Context, logger logr.Logger) context.Context {
	ctx = logr.NewContext(context.Background(), logger)
	return slogcontext.ToCtx(ctx, slog.New(slogr.NewSlogHandler(logger)))
}

For the primarily slog-context workflow:

func doSomething2(ctx context.Context) {
	ctx = ContextWithLogr(slogcontext.With(ctx, "myKey", "myVal"))

	slogcontext.Info(ctx, "calling into library...")
	someLibraryWeHaveNoControlOver2(ctx)
}

func ContextWithLogr(ctx context.Context) context.Context {
	return logr.NewContext(ctx, slogr.NewLogr(slogcontext.Logger(ctx).Handler()))
}

It could make sense that those helper methods live in logr, with better names and inlining some of the function calls.

I can also see a use for both of @thockin's "simple mode" ideas.

The "fancy" versions might be overkill in a world where devs are hopefully moving towards slog for most everything.