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

log/slog: JSONHandler can produce empty groups #62512

Closed
jba opened this issue Sep 7, 2023 · 4 comments
Closed

log/slog: JSONHandler can produce empty groups #62512

jba opened this issue Sep 7, 2023 · 4 comments
Assignees
Labels
NeedsFix The path to resolution is known, but the work has not been done.

Comments

@jba
Copy link
Contributor

jba commented Sep 7, 2023

In 1.21, a JSONHandler configured with a ReplaceAttr function that deletes all attributes can output empty groups. Handlers are supposed to drop empty groups.

Case 1: https://go.dev/play/p/323LJrdFALG
slog.Info("hello", slog.Group("g", "a", 1) outputs {"g":{}}.

Case 2: https://go.dev/play/p/RnacoygwUE9
logger.WithGroup("g").Info("hello", "a", 1) produces the same output.

This is hard to fix because we can't know that the group is going to be empty until after we call ReplaceAttr on all its members. That suggests that any fix is going to be expensive: we either have to compute the replaced attrs and store them somewhere to count them, or we have to write the group elements somewhere other than the main buffer, then copy that into the main buffer if non-empty. Another idea would be to write the group name and brace into the buffer as usual, but then if it turns out the group is empty, back up and erase what we wrote.

@jba jba added the NeedsFix The path to resolution is known, but the work has not been done. label Sep 7, 2023
@jba jba self-assigned this Sep 7, 2023
@seankhliao
Copy link
Member

seankhliao commented Sep 7, 2023

I opted for a write and rollback approach when I wrote mine https://github.com/seankhliao/svcrunner/blob/main/jsonlog/jsonlog.go

@jba
Copy link
Contributor Author

jba commented Sep 7, 2023

Nice.

@dsnet
Copy link
Member

dsnet commented Sep 7, 2023

In the v2 JSON experiment, we're aiming to change the definition of omitempty to elide a Go struct field if it marshals as a JSON null or empty JSON string, object, or array. That bit of detail doesn't help you since these are dynamically generated.

That said, the implementation of omitempty supports the ability to unwrite an empty object member:
https://github.com/go-json-experiment/json/blob/699550ab4a68a9d525a8d0b09d77125692a4c1d9/jsontext/encode.go#L239-L241

If slog one day switches over that implementation, we could look into exporting the UnwriteEmptyObjectMember method.

@gopherbot
Copy link

Change https://go.dev/cl/529855 mentions this issue: log/slog: JSONHandler elides empty groups even with replacement

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Development

No branches or pull requests

4 participants