New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
proposal: log/slog: rename Logger.WithGroup #59151
Comments
It is my understanding that these groups are actually related, and as such I believe the naming is appropriate. |
I think you're right that in practice the uses of "group" are fairly distinct / distinctly motivated. IMHO ... they are significantly related in terms of the data model. It's almost just that the following log lines should encode data equivalently into log.WithGroup("g").With("a", 1).Info("")
log.Info("", slog.Group("g", slog.Int("a", 1))) Where it's about the structure of the data model more exclusively, and not about encoding or formatting, is for functionality like log := slog.New(slog.HandlerOptions{
ReplaceAttr: func(groups []string, a slog.Attr) slog.Attr {
if len(groups) == 1 && groups[0] == "g" {
a.Value = slog.IntValue(2)
}
return a
},
}.NewJSONHandler(os.Stdout)) It's maybe a little more straightforward that we expect |
According to the documentation, That makes this proposal incorrect. |
@pohly, thanks for closing. I used the phrase "qualified by the group name" in the docs deliberately, to avoid committing to what that might actually look like. I tried to clarify that in https://go-review.googlesource.com/c/go/+/478056. |
There are are currently two different usages of "group" in the API which are completely unrelated:
Logger.WithGroup(name string)
- group different attributes togetherGroupValue(as ...Attr) Value
- influence the keys that get recordedI remember that "namespace" was also a candidate. I personally would prefer
Logger.WithNamespace
to avoid the ambiguity. But if you feel that there's no need to revisit this topic, then I'm also fine with simply closing this issue without changing anything.cc @jba
The text was updated successfully, but these errors were encountered: