Skip to content

log/slog: fix HandlersOptions.ReplaceAttr documentation when attribute is discarded #60870

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.

Sign up for GitHub

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
Closed
powerman opened this issue Jun 19, 2023 · 2 comments
Closed

log/slog: fix HandlersOptions.ReplaceAttr documentation when attribute is discarded #60870

powerman opened this issue Jun 19, 2023 · 2 comments
Labels
Documentation Issues describing a change to documentation. FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Milestone
Go1.21

Comments

@powerman
Copy link

powerman commented Jun 19, 2023 •
edited by dmitshur
Loading

Documentation for latest (v0.0.0-20230522175609-2e198f4a06a1) version of HandlersOptions.ReplaceAttr says:

If ReplaceAttr returns an Attr with Key == "", the attribute is discarded.

This is counter-intuitive (because Handler.Handle docs discard zero attribute instead), prevent returning group attrs with empty key to embed group, and didn't match actual behaviour of current implementation (which is correct: it discards zero Attr and handles Attr with empty Key but non-zero Value).

Looks like docs should be fixed to match current implementation.
(Maybe golang.org/x/exp/slog docs needs similar fix.)
@gopherbot gopherbot added this to the Unreleased milestone Jun 19, 2023
@gopherbot gopherbot added the Documentation Issues describing a change to documentation. label Jun 19, 2023
@ianlancetaylor
Copy link
Member

CC @jba

@gopherbot
Copy link
Contributor

Change https://go.dev/cl/504535 mentions this issue: log/slog: fix HandlerOptions.ReplaceAttr doc

@dmitshur dmitshur added the NeedsFix The path to resolution is known, but the work has not been done. label Jun 21, 2023
@dmitshur dmitshur changed the title x/exp/slog: Fix HandlersOptions.ReplaceAttr documentation when attribute is discarded log/slog: fix HandlersOptions.ReplaceAttr documentation when attribute is discarded Jun 21, 2023
@dmitshur dmitshur modified the milestones: Unreleased, Go1.21 Jun 21, 2023
bradfitz pushed a commit to tailscale/go that referenced this issue Jul 15, 2023
@jba @bradfitz
log/slog: fix HandlerOptions.ReplaceAttr doc
ef6ce18 
It said that Attrs with an empty key are ignored.
In fact, zero Attrs are ignored.

Fixes golang#60870.

Change-Id: I221d3a25b0f0cc9001e06e9cc76bab29292c0741
Reviewed-on: https://go-review.googlesource.com/c/go/+/504535
Run-TryBot: Jonathan Amsterdam <jba@google.com>
Reviewed-by: Alan Donovan <adonovan@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
eric pushed a commit to fancybits/go that referenced this issue Sep 7, 2023
@jba @eric
log/slog: fix HandlerOptions.ReplaceAttr doc
03972aa 
It said that Attrs with an empty key are ignored.
In fact, zero Attrs are ignored.

Fixes golang#60870.

Change-Id: I221d3a25b0f0cc9001e06e9cc76bab29292c0741
Reviewed-on: https://go-review.googlesource.com/c/go/+/504535
Run-TryBot: Jonathan Amsterdam <jba@google.com>
Reviewed-by: Alan Donovan <adonovan@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
eric pushed a commit to fancybits/go that referenced this issue Sep 7, 2023
@jba @eric
log/slog: fix HandlerOptions.ReplaceAttr doc
eb393ef 
It said that Attrs with an empty key are ignored.
In fact, zero Attrs are ignored.

Fixes golang#60870.

Change-Id: I221d3a25b0f0cc9001e06e9cc76bab29292c0741
Reviewed-on: https://go-review.googlesource.com/c/go/+/504535
Run-TryBot: Jonathan Amsterdam <jba@google.com>
Reviewed-by: Alan Donovan <adonovan@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
@golang golang locked and limited conversation to collaborators Jun 20, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
Documentation Issues describing a change to documentation. FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Milestone
Go1.21
Development

No branches or pull requests
4 participants
@powerman @dmitshur @ianlancetaylor @gopherbot