8000 proposal: log/slog: export Source method in Record to support custom handler implementations · Issue #70280 · golang/go · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

proposal: log/slog: export Source method in Record to support custom handler implementations #70280

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
nesty92 opened this issue Nov 11, 2024 · 17 comments
Closed

proposal: log/slog: export Source method in Record to support custom handler implementations #70280

nesty92 opened this issue Nov 11, 2024 · 17 comments
Labels
Proposal Proposal-FinalCommentPeriod
Milestone
Proposal

Comments

@nesty92
Copy link
Contributor
nesty92 commented Nov 11, 2024

Go version

go version go1.23.1 linux/amd64

Output of go env in your module/workspace:

Not relevant

What did you do?

Implement a custom handler for the log/slog package. My custom handler requires access to the source location (file and line) of log entries for detailed logging.

What did you see happen?

The source method in slog.Record is not exported, meaning it’s inaccessible to custom handlers. As a result, I had to re-implement logic to retrieve source location information, which led to redundant code and reduced consistency with Go's built-in logging behavior.

What did you expect to see?

Expected an accessible Source method in slog.Record that allows custom handlers to retrieve source location information without re-implementing the existing internal logic. This would simplify the creation of custom handlers and ensure consistent behavior when accessing log entry sources.
@gabyhelp
Copy link

Related Issues

(Emoji vote if this was helpful or unhelpful; more detailed feedback welcome in this discussion.)

nesty92 added a commit to nesty92/go that referenced this issue Nov 11, 2024
@nesty92
log/slog: export Source method in Record for custom handler support
5c37e88 
Currently, the `source` method in `slog.Record` is not accessible to custom handlers, requiring developers to re-implement logic for retrieving source location information. This commit exports the `source` method as `Source`, enabling consistent access for custom logging handlers and reducing code redundancy.

Fixes golang#70280
@nesty92 nesty92 mentioned this issue Nov 11, 2024
Closed
@gopherbot
Copy link
Contributor

Change https://go.dev/cl/626976 mentions this issue: log/slog: export Source method in Record for custom handler support

@seankhliao seankhliao changed the title log/slog: export Source method in Record to support custom handler implementations proposal: log/slog: export Source method in Record to support custom handler implementations Nov 11, 2024
@gopherbot gopherbot added this to the Proposal milestone Nov 11, 2024
@seankhliao
Copy link
Member

Do we need to export it when it's just a single call to runtime.CallersFrames?

@nesty92
Copy link
Contributor Author
nesty92 commented Nov 11, 2024

@seankhliao Yes, it's just a single call, but having it exported prevents each developer creating a custom handler from having to re-implement the same logic.

@ianlancetaylor ianlancetaylor moved this to Incoming in Proposals Nov 11, 2024
@ianlancetaylor
Copy link
Contributor

CC @jba

@jba
Copy link
Contributor
jba commented Nov 11, 2024

I can't think of a good reason not to export it. It's not much code, but it's also not code that we would expect most people to be comfortable writing. The fact that it's a method and not a field is enough of a clue that it could be expensive.

I'm in favor of this.

/cc @aclements @rsc

nesty92 added a commit to nesty92/go that referenced this issue Nov 12, 2024
@nesty92
log/slog: export Source method in Record for custom handler support
9121c71 
Currently, the `source` method in `slog.Record` is not accessible to
custom handlers, requiring developers to re-implement logic for
retrieving source location information. This commit exports the `source`
method as `Source`, enabling consistent access for custom logging
handlers and reducing code redundancy.

Fixes golang#70280

Change-Id: I3eb3bc60658abc5de95697a10bddd11ab54c6e13
GitHub-Last-Rev: 5c37e88
GitHub-Pull-Request: golang#70281
@aclements
Copy link
Member

Can someone write up the proposed API? Thanks.

@jba
Copy link
Contributor
jba commented Apr 9, 2025

The proposal is to export the existing Record.source method. It would then look like this:

// Source returns a Source for the log event.
// If the Record was created without the necessary information,
// or if the location is unavailable, it returns a non-nil *Source
// with zero fields.
func (r Record) Source() *Source

@aclements aclements moved this from Incoming to Active in Proposals Apr 9, 2025
@aclements
Copy link
Member

This proposal has been added to the active column of the proposals project
and will now be reviewed at the weekly proposal review meetings.
— aclements for the proposal review group

@aclements
Copy link
Member

Just for reference, the unexported implementation of Record.source is here.

@aclements
Copy link
Member

I think it's unclear from the documentation whether Source would scan the Record's attributes, since we do have special handling for *Source-typed attributes in the built-in handlers. Presumably the answer is that it would not, and exclusively symbolizes Record.PC, but it would be good to clarify that for public documentation.

@aclements
Copy link
Member 
// If the Record was created without the necessary information,
// or if the location is unavailable, it returns a non-nil *Source
// with zero fields.

If we're going to always return a non-nil *Source, we should say something about whether the result aliases with other calls to Source. Or maybe this should just return a nil source in that case?

The documentation could also be a bit clearer that the result is always non-nil. You can get that from the proposed documentation, but it takes a little thought.

@jba
Copy link
Contributor
jba commented Apr 16, 2025

How about this:

// Source returns a new, non-nil Source for the log event using r's PC.
// If the PC field is zero, meaning the Record was created without the necessary information
// or the location is unavailable, then a Source with zero fields is returned.

Here, "new" is supposed to imply that it doesn't alias.
Explicitly saying "Source doesn't access the other fields of Record" seems like a lot more than we usually would say, but
I can certainly add it if you like.

@seankhliao
Copy link
Member

isn't it nicer to write:

if s := r.Source(); s != nil {
  // encode s
}

vs

if s := r.Source(), *s != (slog.Source{}) {
  // encode s
}
if r.PC != 0 {
  s := r.Source()
  // encode s
}

@jba
Copy link
Contributor
jba commented Apr 16, 2025

That's a good point. I don't see a good reason to return a non-nil Source. I think that's left over from when we were using a value rather than a pointer.
Let's change "then a Source with zero fields is returned" to "then nil is returned" (and of course remove the "non-nil" from the first sentence).
So:

// Source returns a new Source for the log event using r's PC.
// If the PC field is zero, meaning the Record was created without the necessary information
// or the location is unavailable, then nil is returned.

@aclements aclements moved this from Active to Likely Accept in Proposals Apr 23, 2025
@aclements
Copy link
Member

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

The proposal is to add the following to package log/slog:

// Source returns a new Source for the log event using r's PC.
// If the PC field is zero, meaning the Record was created without the necessary information
// or the location is unavailable, then nil is returned.
func (r Record) Source() *Source

(This is almost though not quite the existing unexported Record.source method.)

nesty92 added a commit to nesty92/go that referenced this issue Apr 24, 2025
@nesty92
log/slog: export Source method in Record for custom handler support
ec6e88c 
Currently, the `source` method in `slog.Record` is not accessible to
custom handlers, requiring developers to re-implement logic for
retrieving source location information. This commit exports the `source`
method as `Source`, enabling consistent access for custom logging
handlers and reducing code redundancy.

Fixes golang#70280

Change-Id: I3eb3bc60658abc5de95697a10bddd11ab54c6e13
GitHub-Last-Rev: 5c37e88
GitHub-Pull-Request: golang#70281
Signed-off-by: Ernesto Alejandro Santana Hidalgo <ernesto.alejandrosantana@gmail.com>
nesty92 added a commit to nesty92/go that referenced this issue Apr 24, 2025
@nesty92
log/slog: update Source method to return nil for unavailable locations
5ea3796 
Following proposal golang#70280

Signed-off-by: Ernesto Alejandro Santana Hidalgo <ernesto.alejandrosantana@gmail.com>
@nesty92
Copy link
Contributor Author
nesty92 commented Apr 24, 2025

@aclements @jba I updated https://go-review.googlesource.com/c/go/+/626976 following the proposal decision

nesty92 added a commit to nesty92/go that referenced this issue Apr 24, 2025
@nesty92
log/slog: update Source method to return nil for unavailable locations
f81428e 
Following proposal golang#70280

Signed-off-by: Ernesto Alejandro Santana Hidalgo <ernesto.alejandrosantana@gmail.com>
nesty92 added a commit to nesty92/go that referenced this issue Apr 24, 2025
@nesty92
log/slog: update Source method to return nil for unavailable locations
214dc8b 
Following proposal golang#70280

Signed-off-by: Ernesto Alejandro Santana Hidalgo <ernesto.alejandrosantana@gmail.com>
nesty92 added a commit to nesty92/go that referenced this issue Apr 24, 2025
@nesty92
log/slog: update Source method to return nil for unavailable locations
468119e 
Following proposal golang#70280

Signed-off-by: Ernesto Alejandro Santana Hidalgo <ernesto.alejandrosantana@gmail.com>
nesty92 added a commit to nesty92/go that referenced this issue Apr 25, 2025
@nesty92
log/slog: update Source method to return nil for unavailable locations
0dc2b06 
Following proposal golang#70280

Signed-off-by: Ernesto Alejandro Santana Hidalgo <ernesto.alejandrosantana@gmail.com>
nesty92 added a commit to nesty92/go that referenced this issue May 2, 2025
@nesty92
log/slog: update Source method to return nil for unavailable locations
6fcd5b9 
Following proposal golang#70280

Signed-off-by: Ernesto Alejandro Santana Hidalgo <ernesto.alejandrosantana@gmail.com>
nesty92 added a commit to nesty92/go that referenced this issue May 2, 2025
@nesty92
log/slog: update Source method to return nil for unavailable locations
6756b96 
Following proposal golang#70280

Signed-off-by: Ernesto Alejandro Santana Hidalgo <ernesto.alejandrosantana@gmail.com>
nesty92 added a commit to nesty92/go that referenced this issue May 4, 2025
@nesty92
log/slog: export Source method in Record for custom handler support
5dbbd9d 
Currently, the `source` method in `slog.Record` is not accessible to
custom handlers, requiring developers to re-implement logic for
retrieving source location information. This commit exports the `source`
method as `Source`, enabling consistent access for custom logging
handlers and reducing code redundancy.

Fixes golang#70280

Change-Id: I3eb3bc60658abc5de95697a10bddd11ab54c6e13
GitHub-Last-Rev: 5c37e88
GitHub-Pull-Request: golang#70281
nesty92 added a commit to nesty92/go that referenced this issue May 4, 2025
@nesty92
log/slog: update Source method to return nil for unavailable locations
bd81afe 
Following proposal golang#70280

Signed-off-by: Ernesto Alejandro Santana Hidalgo <ernesto.alejandrosantana@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Proposal Proposal-FinalCommentPeriod
Projects
Proposals
Status: Likely Accept
Milestone
Proposal
Development

Successfully merging a pull request may close this issue.

log/slog: export Source method in Record for custom handler support nesty92/go
7 participants
@aclements @ianlancetaylor @gopherbot @seankhliao @jba @nesty92 @gabyhelp
0