RFC: slogr: add context support

[pohly]

The normal logr API for storing and retrieving a logger is used. This is necessary to ensure interoperability.

The downside is that storing a slog.Handler and retrieving it again is more costly than storing and retrieving a logr.Logger, because additional allocations are needed.

The new API is purely syntactic sugar. The same can also be done outside of logr.

[pohly]

It "feels" wrong that context handling favors storing and retrieving a logr.Logger, but I think that's okay: I can't think of a strong reason why developers who want to support contextual logging (i.e. those who would call slogr.HandlerFromContext) shouldn't use the logr.FromContext call and the returned Logger instance as their API.

But do we then need the new functions at all?

The alternative is what I had in #196:

• support slog directly in the main package
• store either a slog.Handler, slog.Logger or logr.Logger under the same key
• retrieve what's stored and convert only if needed

[thockin]

I read #196 as "store whatever you want, and tell us what you want to read". It's attractive in the sense that it doesn't waste allocations but I am really unsure of how it will be used.

Do you have a stronger sense of the real need here?

[thockin]

Should this be NewContext for consistency?

[pohly]

I wanted Handler in the name because a variant with *slog.Logger as parameter might also be useful at some point.

If we used slogr.NewContext, it would probably be better defined as taking *slog.Logger as parameter for full consistency with logr.NewContext.

As it stands now, the name is more consistent with context.WithDeadline or context.WithCancel. We cannot have consistency with both the logr and context package, though, and consistency with logr is more important ("closer" package).

NewContextWithHandler (and future NewContextWithLogger)?

[pohly]

I read #196 as "store whatever you want, and tell us what you want to read".

Correct.

It's attractive in the sense that it doesn't waste allocations but I am really unsure of how it will be used.

Do you have a stronger sense of the real need here?

Not really. My initial goal was to support both APIs equally well, including context support, and then simply let developers pick what they prefer. Perhaps some developer has heard about slog, starting using it, and then wants to use it with contextual logging in APIs which already take a ctx. Then an efficient logr.HandlerFromContext would be useful.

I've no idea how realistic that scenario is.

One possible path forward might be to merge this API to show that this works (developers can always do something like this in their own code, using existing APIs - we just add some helpers) and if it gets used, improve the implementation while keeping the API the same: such an improvement would have to move most of the implementation into internal and then make the main package depend on it by having FromContext accept a value which isn't a slog.Logger, i.e. do the conversion from slog.Handler on demand.

[pohly]

Closing in favor of more basic discussion in #234.