Skip to content
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

ReplaceAttrs incompatible with slog #41

Closed
pschultz opened this issue Sep 19, 2023 · 1 comment · Fixed by #47
Closed

ReplaceAttrs incompatible with slog #41

pschultz opened this issue Sep 19, 2023 · 1 comment · Fixed by #47
Assignees
Labels
bug Something isn't working

Comments

@pschultz
Copy link

I found two things that tint does different to slog, which makes Options not quite a drop-in replacement for slog.HandlerOptions:

First, slog calls Resolve before ReplaceAttr, as documented:

ReplaceAttr is called to rewrite each non-group attribute before it is logged. The attribute's value has been resolved (see [Value.Resolve]).

tint calls Resolve after ReplaceAttr.

Second, tint does not call ReplaceAttr recursively for groups. With the tint handler, slog.Info("foo", slog.Group("bar", "x", "y")) causes a call to ReplaceAttr with an empty groups argument and slog.GroupValue("bar", "x", "y"). slog's handlers instead call ReplaceAttr with []string{"bar"}, slog.Attr{Key:"x", Value: slog.StringValue("y")}.

The documentation on slog.HandlerOptions.ReplaceAttr reads

ReplaceAttr is never called for Group attributes, only their contents.

The two things together interact surprisingly in slog in Go 1.21.1. With a value that Resolves to a GroupValue, slog does call ReplaceAttr with the Group attribute (and after that once for each member of the group). This is either a bug in the implementation or the docs. I opened golang/go#62731 about this. Maybe keep an eye on that.

@lmittmann
Copy link
Owner

Thanks for raising this.

@lmittmann lmittmann self-assigned this Oct 29, 2023
@lmittmann lmittmann added the bug Something isn't working label Oct 29, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants