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

Jump to bottom

log/slog: arg to Record.Attrs should return bool #59060

Closed
jba opened this issue Mar 15, 2023 · 6 comments
Closed

log/slog: arg to Record.Attrs should return bool #59060

jba opened this issue Mar 15, 2023 · 6 comments
Assignees
@jba
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done. Proposal Proposal-Accepted release-blocker
Milestone
Go1.21

Comments

@jba
Copy link
Contributor

jba commented Mar 15, 2023

The Record.Attrs method currently accepts an argument of type func(Attr).
For future consistency with iteration over func values, it should accept func(Attr) bool.
@jba jba self-assigned this Mar 15, 2023
@cherrymui cherrymui added the NeedsFix The path to resolution is known, but the work has not been done. label Mar 15, 2023
@cherrymui cherrymui added this to the Go1.21 milestone Mar 15, 2023
@jba jba added the Proposal label Mar 28, 2023
@jba
Copy link
Contributor Author

jba commented Mar 28, 2023

Returning a bool is also useful if the Handler wants to look for a particular Attr. For example, some handlers might want to extract an error if there is one and treat it specially.

/cc @ianthehat

Original proposal: #56345

@ianthehat
Copy link

Yes, my use cases were things like detecting a specific attribute key to indicate a message type (operating like a mux to underlying handlers), and detecting specific error types to filter out messages that were not interesting. In both cases the attribute in question will be first by convention, but I have to scan the whole list anyway using .Attrs right now.

@rsc rsc changed the title log/slog: arg to Record.Attrs should return bool proposal: log/slog: arg to Record.Attrs should return bool Mar 29, 2023
@rsc
Copy link
Contributor

rsc commented Mar 29, 2023

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

@rsc
Copy link
Contributor

rsc commented Apr 6, 2023

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

@gopherbot
Copy link

Change https://go.dev/cl/484096 mentions this issue: log/slog: function argument to Record.Attrs returns bool

@rsc
Copy link
Contributor

rsc commented Apr 12, 2023

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

@rsc rsc changed the title proposal: log/slog: arg to Record.Attrs should return bool log/slog: arg to Record.Attrs should return bool Apr 12, 2023
@samber samber mentioned this issue Apr 25, 2023
Closed
@mknyszek mknyszek mentioned this issue Jun 1, 2023
Closed
eric pushed a commit to fancybits/go that referenced this issue Sep 7, 2023
@jba @eric
log/slog: function argument to Record.Attrs returns bool
fe1ee4f 
Record.Attrs stops as soon as its argument function returns false.

Fixes golang#59060.

Change-Id: I578d64635e0e52b0fcdbc57f6d5a27a6efac8c70
Reviewed-on: https://go-review.googlesource.com/c/go/+/484096
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Alan Donovan <adonovan@google.com>
Run-TryBot: Jonathan Amsterdam <jba@google.com>
eric pushed a commit to fancybits/go that referenced this issue Sep 7, 2023
@jba @eric
log/slog: function argument to Record.Attrs returns bool
ffd84db 
Record.Attrs stops as soon as its argument function returns false.

Fixes golang#59060.

Change-Id: I578d64635e0e52b0fcdbc57f6d5a27a6efac8c70
Reviewed-on: https://go-review.googlesource.com/c/go/+/484096
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Alan Donovan <adonovan@google.com>
Run-TryBot: Jonathan Amsterdam <jba@google.com>
@golang golang locked and limited conversation to collaborators Apr 11, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@jba jba

Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done. Proposal Proposal-Accepted release-blocker
Projects
None yet
Milestone
Go1.21
Development

No branches or pull requests
5 participants
@rsc @ianthehat @gopherbot @cherrymui @jba