Skip to content

Commit ec6e88c

Browse files
committed
log/slog: export Source method in Record for custom handler support
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 #70280 Change-Id: I3eb3bc60658abc5de95697a10bddd11ab54c6e13 GitHub-Last-Rev: 5c37e88 GitHub-Pull-Request: #70281 Signed-off-by: Ernesto Alejandro Santana Hidalgo <[email protected]>
1 parent 583d750 commit ec6e88c

File tree

5 files changed

+64
-14
lines changed

5 files changed

+64
-14
lines changed

src/log/slog/handler.go

+3-1
Original file line numberDiff line numberDiff line change
@@ -294,7 +294,9 @@ func (h *commonHandler) handle(r Record) error {
294294
}
295295
// source
296296
if h.opts.AddSource {
297-
state.appendAttr(Any(SourceKey, r.source()))
297+
if src := r.Source(); src != nil {
298+
state.appendAttr(Any(SourceKey, r.Source()))
299+
}
298300
}
299301
key = MessageKey
300302
msg := r.Message

src/log/slog/handler_test.go

+35-1
Original file line numberDiff line numberDiff line change
@@ -530,7 +530,7 @@ func TestJSONAndTextHandlers(t *testing.T) {
530530
},
531531
} {
532532
r := NewRecord(testTime, LevelInfo, "message", callerPC(2))
533-
line := strconv.Itoa(r.source().Line)
533+
line := strconv.Itoa(r.Source().Line)
534534
r.AddAttrs(test.attrs...)
535535
var buf bytes.Buffer
536536
opts := HandlerOptions{ReplaceAttr: test.replace, AddSource: test.addSource}
@@ -617,6 +617,40 @@ func TestHandlerEnabled(t *testing.T) {
617617
}
618618
}
619619

620+
func TestJSONAndTextHandlersWithUnavailableSource(t *testing.T) {
621+
// Verify that a nil source does not cause a panic.
622+
// and that the source is not printed.
623+
var buf bytes.Buffer
624+
opts := &HandlerOptions{
625+
ReplaceAttr: removeKeys(LevelKey),
626+
AddSource: true,
627+
}
628+
629+
for _, test := range []struct {
630+
name string
631+
h Handler
632+
want string
633+
}{
634+
{"text", NewTextHandler(&buf, opts), "msg=message"},
635+
{"json", NewJSONHandler(&buf, opts), `{"msg":"message"}`},
636+
} {
637+
t.Run(test.name, func(t *testing.T) {
638+
buf.Reset()
639+
r := NewRecord(time.Time{}, LevelInfo, "message", 0)
640+
err := test.h.Handle(t.Context(), r)
641+
if err != nil {
642+
t.Fatal(err)
643+
}
644+
645+
want := strings.TrimSpace(test.want)
646+
got := strings.TrimSpace(buf.String())
647+
if got != want {
648+
t.Errorf("\ngot %s\nwant %s", got, want)
649+
}
650+
})
651+
}
652+
}
653+
620654
func TestSecondWith(t *testing.T) {
621655
// Verify that a second call to Logger.With does not corrupt
622656
// the original.

src/log/slog/logger_test.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -185,7 +185,7 @@ func TestCallDepth(t *testing.T) {
185185
const wantFunc = "log/slog.TestCallDepth"
186186
const wantFile = "logger_test.go"
187187
wantLine := startLine + count*2
188-
got := h.r.source()
188+
got := h.r.Source()
189189
gotFile := filepath.Base(got.File)
190190
if got.Function != wantFunc || gotFile != wantFile || got.Line != wantLine {
191191
t.Errorf("got (%s, %s, %d), want (%s, %s, %d)",

src/log/slog/record.go

+8-5
Original file line numberDiff line numberDiff line change
@@ -211,11 +211,14 @@ func (s *Source) group() Value {
211211
return GroupValue(as...)
212212
}
213213

214-
// source returns a Source for the log event.
215-
// If the Record was created without the necessary information,
216-
// or if the location is unavailable, it returns a non-nil *Source
217-
// with zero fields.
218-
func (r Record) source() *Source {
214+
// Source returns a new Source for the log event using r's PC.
215+
// If the PC field is zero, meaning the Record was created without the necessary information
216+
// or the location is unavailable, then nil is returned.
217+
func (r Record) Source() *Source {
218+
if r.PC == 0 {
219+
return nil
220+
}
221+
219222
fs := runtime.CallersFrames([]uintptr{r.PC})
220223
f, _ := fs.Next()
221224
return &Source{

src/log/slog/record_test.go

+17-6
Original file line numberDiff line numberDiff line change
@@ -39,24 +39,35 @@ func TestRecordAttrs(t *testing.T) {
3939
}
4040

4141
func TestRecordSource(t *testing.T) {
42-
// Zero call depth => empty *Source.
42+
// Zero call depth => nil *Source.
4343
for _, test := range []struct {
4444
depth int
4545
wantFunction string
4646
wantFile string
4747
wantLinePositive bool
48+
wantNil bool
4849
}{
49-
{0, "", "", false},
50-
{-16, "", "", false},
51-
{1, "log/slog.TestRecordSource", "record_test.go", true}, // 1: caller of NewRecord
52-
{2, "testing.tRunner", "testing.go", true},
50+
{0, "", "", false, true},
51+
{-16, "", "", false, true},
52+
{1, "log/slog.TestRecordSource", "record_test.go", true, false}, // 1: caller of NewRecord
53+
{2, "testing.tRunner", "testing.go", true, false},
5354
} {
5455
var pc uintptr
5556
if test.depth > 0 {
5657
pc = callerPC(test.depth + 1)
5758
}
5859
r := NewRecord(time.Time{}, 0, "", pc)
59-
got := r.source()
60+
got := r.Source()
61+
if test.wantNil {
62+
if got != nil {
63+
t.Errorf("depth %d: got non-nil Source, want nil", test.depth)
64+
}
65+
continue
66+
}
67+
if got == nil {
68+
t.Errorf("depth %d: got nil Source, want non-nil", test.depth)
69+
continue
70+
}
6071
if i := strings.LastIndexByte(got.File, '/'); i >= 0 {
6172
got.File = got.File[i+1:]
6273
}

0 commit comments

Comments
 (0)