log/slog: change slog.Group signature to ...any

[ianthehat]

Currently there is now way to access the standard conversion of ...any to ...Attr without building a record.
It would be nice to be able to build a group Attr this way, for patterns where you want to be able to pass around a set of attrs (like queries that return a standard set for logging, or building a set of labels before an if that adds more).
All existing calls to slog.Group would keep working as the ...any forms happily accept Attr directly anyway, they would just be a fraction slower, in cases where the performance is needed you could use GroupValue directly.
This also exposes the standard conversion to helpers that are not performance critical so they can have a signature that looks like the normal logging functions.
Performance critical helpers need to build a Record directly to avoid allocations, and could not use this approach, but exposing an API for that is a much larger surface area.

[AndrewHarrisSPU]

I don't have a strong opinion on this but I agree there is some value in having the standard ("suggested", "conventional"?) conversion defined and easily available. Another situation where a common conversion might be useful is when implementing alternatives to Logger. Using something other than Logger to wrap a Handler seems entirely anticipated by slog, and many of these might want to accept variadic any - my impression from experiments along these lines is that the utility of different conversions is not very high, but the confusion resulting from different conversions is surprisingly high.

Rather than changing slog.Group, what about a func Attrs(...any) []slog.Attr, with logic invoking slog's argsToAttr?

For groups, something like this would result in more tokens than changing the signature to Group(string, ...any), but far fewer tokens than requiring per-attribute attention.

slog.Group("label", slog.Attrs("a", 1, "b", 2)...)

[ianlancetaylor]

CC @jba

[jba]

I'm generally in favor of this; I agree there should be some way to expose the logic that converts k-v pairs to Attrs. The Attrs function is a bit more general, but I think Group(string, ...any) slog.Attr is a slightly better choice for efficiency reasons, though I'd have to implement both and see. The other nice thing about Group is that we wouldn't be adding something.

Linking to the main proposal: #56345.

[ianthehat]

It's worth noting that you can easily do Attrs in terms of Group anyway, slog.Group("", args...).Group(), so I would lean in the direction of no new API, especially as I think the group case will be the most common use of attr building outside of a record.
I think most new Logger "replacements" will be building records and using Record.Add rather than a custom Attr builder

[rsc]

It sounds like people are in favor of slog.Group taking ...any over adding slog.Attrs. Do I have that right?

[rsc]

This proposal has been added to the active column of the proposals project
and will now be reviewed at the weekly proposal review meetings.
— rsc for the proposal review group

[ChrisHines]

Suppose someone wants to write an alternative to slog.Logger that always requires ...slog.Attr signatures and does not allow ...any in its API. Would accepting this proposal create too many holes in the damn to plug?

[ianthehat]

Not sure if this is what you are asking, but you can still build a group using Attr{"label", GroupValue(attrs...)} if you want to build one directly from a []Attr.

[ChrisHines]

I don't think that's what I'm asking.

At the moment none of the Attr constructors accept ...any. So it is possible to write a package that provides an alternate Logger API restricted to ...Attr parameters but reusing the slog.Attr constructors and Handler unmodified. The only "hole" left is the Record.Add method, but client code doesn't normally use that so it's OK. If the slog.Group signature takes ...any it backdoors this hypothetical package's attempts to hide ...any signatures.

I understand the desire to provide a tool for people who want access to the the ...any -> []Attr logic, so I prefer adding the func Attrs(...any) []slog.Attr function as both a more orthogonal and opt-in approach.

[jba]

Suppose someone wants to write an alternative to slog.Logger that always requires ...slog.Attr signatures and does not allow ...any in its API. Would accepting this proposal create too many holes in the damn to plug?

That is an interesting point. But wouldn't they just go the whole way?

package kvless_slog

type Logger ...
type Handler = slog.Handler
type Attr = slog.Attr
type Value = slog.Value
func Int(key string, val int) Attr { return slog.Int(key, val) }
...
func Group(key string, attrs ...Attr) { return slog.Group(attrs...) }

Yes, there are a lot of those shims, but it would probably take no more than an hour and since slog is in the standard library, your maintenance burden would be small—a few tweaks every six months. And then you'd be completely locked down: no chance of calling Record.Add, or slog.Logger.Info for that matter (since client packages would no longer import log/slog at all).

[ChrisHines]

But wouldn't they just go the whole way?

Now that you've show us how, I suppose they would. That approach had not occurred to me yet, though, so it might not occur to others. ;). But it's good to know it's possible and enterprising individuals can find the path. Thanks.

[jba]

Is there a use case for func Attrs(...any) []slog.Attr, other than using it to create a Group?

[ChrisHines]

Is there a use case for func Attrs(...any) []slog.Attr, other than using it to create a Group?

I am not sure. If there is a use case for code to create variables of type []slog.Attr and we want to populate them conveniently with the same rules that slog uses to handle ...any parameters then that seems like a use case.

But when I review code that has variables like that with our internal logging package it usually seems like a code smell and should be replaced by a use of logger.With call. But there is debate about that, for example: #59126 (comment).

[AndrewHarrisSPU]

Is there a use case for func Attrs(...any) []slog.Attr, other than using it to create a Group?

I would characterize the utility as syntactic, in a language that on principle doesn't embrace syntax metaprogramming ... there's some tension in this and it seems like a sensitive question.

A dumb playground example of what I mean: it's possible to prepare any error with file:line annotation and arbitrary additional logging attributes while still passing it around as a valid error. There would be ways to write this without a func Attrs, but I feel like the utility is tangible here.

I think Group(string, ...any) slog.Attr is a slightly better choice for efficiency reasons

I did some quick benchmarking of a slog.Group(string, ...Attr), groupAny, and AttrsFunc, using test lists based on the ones from slog/benchmark, and didn't spot a difference between the latter two. Both had worse numbers than the current Group(string, ...Attr) version (slower, and sometimes allocating more).

type Attr = slog.Attr

func groupAny(name string, args ...any) Attr {
	as := make([]Attr, 0, len(args))
	var a Attr
	for len(args) > 0 {
		a, args = argsToAttr(args)
		as = append(as, a)
	}
	return Attr{name, slog.GroupValue(as...)}
}

func AttrsFunc(args ...any) []Attr {
	as := make([]Attr, 0, len(args))
	var a Attr
	for len(args) > 0 {
		a, args = argsToAttr(args)
		as = append(as, a)
	}
	return as
}

[jba]

Here is another consideration, briefly mentioned in the top post:

This also exposes the standard conversion to helpers that are not performance critical so they can have a signature that looks like the normal logging functions.

To elaborate Ian's argument: we've decided that ...any is the main way to use the log output functions, with ...Attr relegated to a performance-sensitive corner. Given that decision, and the ability write groups inline, it seems natural to allow

logger.Info("msg",
    "a", 1,
    "b", 2,
    slog.Group("g",
          "c", 3,
          "d", 4,
    ),
    "e", 5)

and jarring to require that arguments to Group be Attrs.

Indeed, a developer who only needed to produce log output, and didn't care about how Handlers or LogValuers worked, wouldn't even have to learn about Attrs and Values except for Group. (There is also the fact that the package doc begins with Attrs and Values; I now think that material should be moved later.)

To me, this is a strong reason to prefer changing the signature of Group.

[rsc]

Have all remaining concerns about this proposal been addressed?

[rsc]

Based on the discussion above, this proposal seems like a likely accept.
— rsc for the proposal review group

[gopherbot]

Change https://go.dev/cl/487855 mentions this issue: log/slog: Group takes ...any

[rsc]

No change in consensus, so accepted. 🎉
This issue now tracks the work of implementing the proposal.
— rsc for the proposal review group