slog: export Record.PC
NewRecord now takes a pc instead of a call depth: callers must obtain
the pc themselves.
This makes it possible to copy a Record without copying its
attributes.
Remove Record.SourceLine; users can obtain source information by passing
the PC to runtime.CallersFrames.
Change-Id: I3e63ffefdbdc317cb3b6e559dd8ba88d576a47b8
Reviewed-on: https://go-review.googlesource.com/c/exp/+/463257
Run-TryBot: Jonathan Amsterdam <jba@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>
diff --git a/slog/handler.go b/slog/handler.go
index b7b2a1d..42edaf5 100644
--- a/slog/handler.go
+++ b/slog/handler.go
@@ -263,17 +263,17 @@
}
// source
if h.opts.AddSource {
- file, line := r.SourceLine()
- if file != "" {
+ frame := r.frame()
+ if frame.File != "" {
key := SourceKey
if rep == nil {
state.appendKey(key)
- state.appendSource(file, line)
+ state.appendSource(frame.File, frame.Line)
} else {
buf := buffer.New()
- buf.WriteString(file) // TODO: escape?
+ buf.WriteString(frame.File) // TODO: escape?
buf.WriteByte(':')
- buf.WritePosInt(line)
+ buf.WritePosInt(frame.Line)
s := buf.String()
buf.Free()
state.appendAttr(String(key, s))
diff --git a/slog/logger.go b/slog/logger.go
index cde76c6..0f0c844 100644
--- a/slog/logger.go
+++ b/slog/logger.go
@@ -48,16 +48,17 @@
if !w.h.Enabled(LevelInfo) {
return 0, nil
}
- var depth int
+ var pc uintptr
if w.flags&(log.Lshortfile|log.Llongfile) != 0 {
- depth = 2
+ pc = callerPC(5)
}
+
// Remove final newline.
origLen := len(buf) // Report that the entire buf was written.
if len(buf) > 0 && buf[len(buf)-1] == '\n' {
buf = buf[:len(buf)-1]
}
- r := NewRecord(time.Now(), LevelInfo, string(buf), depth, nil)
+ r := NewRecord(time.Now(), LevelInfo, string(buf), pc, nil)
return origLen, w.h.Handle(r)
}
@@ -172,7 +173,7 @@
Message: msg,
Level: level,
Context: l.ctx,
- pc: pc,
+ PC: pc,
}
}
diff --git a/slog/logger_test.go b/slog/logger_test.go
index 3c94a43..50ec6dc 100644
--- a/slog/logger_test.go
+++ b/slog/logger_test.go
@@ -104,9 +104,9 @@
// Once slog.SetDefault is called, the direction is reversed: the default
// log.Logger's output goes through the handler.
- SetDefault(New(NewTextHandler(&slogbuf)))
+ SetDefault(New(HandlerOptions{AddSource: true}.NewTextHandler(&slogbuf)))
log.Print("msg2")
- checkLogOutput(t, slogbuf.String(), "time="+timeRE+` level=INFO msg=msg2`)
+ checkLogOutput(t, slogbuf.String(), "time="+timeRE+` level=INFO source=.*logger_test.go:\d{3} msg=msg2`)
// The default log.Logger always outputs at Info level.
slogbuf.Reset()
@@ -155,6 +155,11 @@
check(attrsSlice(h.r), Int("c", 3))
}
+func sourceLine(r Record) (string, int) {
+ f := r.frame()
+ return f.File, f.Line
+}
+
func TestCallDepth(t *testing.T) {
h := &captureHandler{}
var startLine int
@@ -163,7 +168,7 @@
t.Helper()
const wantFile = "logger_test.go"
wantLine := startLine + count*2
- gotFile, gotLine := h.r.SourceLine()
+ gotFile, gotLine := sourceLine(h.r)
gotFile = filepath.Base(gotFile)
if gotFile != wantFile || gotLine != wantLine {
t.Errorf("got (%s, %d), want (%s, %d)", gotFile, gotLine, wantFile, wantLine)
@@ -175,7 +180,7 @@
// Calls to check must be one line apart.
// Determine line where calls start.
- f, _ := runtime.CallersFrames([]uintptr{pc(2)}).Next()
+ f, _ := runtime.CallersFrames([]uintptr{callerPC(2)}).Next()
startLine = f.Line + 4
// Do not change the number of lines between here and the call to check(0).
diff --git a/slog/nopc.go b/slog/nopc.go
index 0461cab..d6ecf44 100644
--- a/slog/nopc.go
+++ b/slog/nopc.go
@@ -43,5 +43,5 @@
l.logPC(err, 0, level, msg, args...)
}
-// pc returns 0 to avoid incurring the cost of runtime.Callers.
-func pc(depth int) uintptr { return 0 }
+// callerPC returns 0 to avoid incurring the cost of runtime.Callers.
+func callerPC(depth int) uintptr { return 0 }
diff --git a/slog/pc.go b/slog/pc.go
index 52e04b3..414eeab 100644
--- a/slog/pc.go
+++ b/slog/pc.go
@@ -53,8 +53,8 @@
l.logPC(err, pcs[0], level, msg, args...)
}
-// pc returns the program counter at the given stack depth.
-func pc(depth int) uintptr {
+// callerPC returns the program counter at the given stack depth.
+func callerPC(depth int) uintptr {
var pcs [1]uintptr
runtime.Callers(depth, pcs[:])
return pcs[0]
diff --git a/slog/record.go b/slog/record.go
index 081db1b..43422a4 100644
--- a/slog/record.go
+++ b/slog/record.go
@@ -33,9 +33,13 @@
// Canceling the context should not affect record processing.
Context context.Context
- // The pc at the time the record was constructed, as determined
- // by runtime.Callers using the calldepth argument to NewRecord.
- pc uintptr
+ // The program counter at the time the record was constructed, as determined
+ // by runtime.Callers. If zero, no program counter is available.
+ //
+ // The only valid use for this value is as an argument to
+ // [runtime.CallersFrames]. In particular, it must not be passed to
+ // [runtime.FuncForPC].
+ PC uintptr
// Allocation optimization: an inline array sized to hold
// the majority of log calls (based on examination of open-source
@@ -54,38 +58,26 @@
// NewRecord creates a Record from the given arguments.
// Use [Record.AddAttrs] to add attributes to the Record.
-// If calldepth is greater than zero, [Record.SourceLine] will
-// return the file and line number at that depth,
-// where 1 means the caller of NewRecord.
//
// NewRecord is intended for logging APIs that want to support a [Handler] as
// a backend.
-func NewRecord(t time.Time, level Level, msg string, calldepth int, ctx context.Context) Record {
- var p uintptr
- if calldepth > 0 {
- p = pc(calldepth + 2)
- }
+func NewRecord(t time.Time, level Level, msg string, pc uintptr, ctx context.Context) Record {
return Record{
Time: t,
Message: msg,
Level: level,
+ PC: pc,
Context: ctx,
- pc: p,
}
}
-// Context returns the context in the Record.
-// If the Record was created from a Logger,
-// this will be the Logger's context.
-
-// SourceLine returns the file and line of the log event.
+// frame returns the runtime.Frame of the log event.
// If the Record was created without the necessary information,
-// or if the location is unavailable, it returns ("", 0).
-func (r Record) SourceLine() (file string, line int) {
- fs := runtime.CallersFrames([]uintptr{r.pc})
- // TODO: error-checking?
+// or if the location is unavailable, it returns a zero Frame.
+func (r Record) frame() runtime.Frame {
+ fs := runtime.CallersFrames([]uintptr{r.PC})
f, _ := fs.Next()
- return f.File, f.Line
+ return f
}
// Clone returns a copy of the record with no shared state.
diff --git a/slog/record_test.go b/slog/record_test.go
index 0a608ce..e178f2c 100644
--- a/slog/record_test.go
+++ b/slog/record_test.go
@@ -38,8 +38,12 @@
{1, "record_test.go", true}, // 1: caller of NewRecord
{2, "testing.go", true},
} {
- r := NewRecord(time.Time{}, 0, "", test.depth, nil)
- gotFile, gotLine := r.SourceLine()
+ var pc uintptr
+ if test.depth > 0 {
+ pc = callerPC(test.depth + 1)
+ }
+ r := NewRecord(time.Time{}, 0, "", pc, nil)
+ gotFile, gotLine := sourceLine(r)
if i := strings.LastIndexByte(gotFile, '/'); i >= 0 {
gotFile = gotFile[i+1:]
}
@@ -115,7 +119,7 @@
b.ReportAllocs()
var x uintptr
for i := 0; i < b.N; i++ {
- x = pc(depth)
+ x = callerPC(depth)
}
_ = x
})
@@ -126,14 +130,14 @@
r := NewRecord(time.Now(), LevelInfo, "", 5, nil)
b.Run("alone", func(b *testing.B) {
for i := 0; i < b.N; i++ {
- file, line := r.SourceLine()
+ file, line := sourceLine(r)
_ = file
_ = line
}
})
b.Run("stringifying", func(b *testing.B) {
for i := 0; i < b.N; i++ {
- file, line := r.SourceLine()
+ file, line := sourceLine(r)
buf := buffer.New()
buf.WriteString(file)
buf.WriteByte(':')
@@ -158,15 +162,3 @@
}
_ = a
}
-
-func BenchmarkNewRecordCallDepth(b *testing.B) {
- for d := 0; d < 5; d++ {
- b.Run(strconv.Itoa(d), func(b *testing.B) {
- var x Record
- for i := 0; i < b.N; i++ {
- x = NewRecord(time.Time{}, LevelInfo, "", d, nil)
- }
- _ = x
- })
- }
-}
diff --git a/slog/text_handler_test.go b/slog/text_handler_test.go
index faece65..87cd962 100644
--- a/slog/text_handler_test.go
+++ b/slog/text_handler_test.go
@@ -114,7 +114,7 @@
func TestTextHandlerSource(t *testing.T) {
var buf bytes.Buffer
h := HandlerOptions{AddSource: true}.NewTextHandler(&buf)
- r := NewRecord(testTime, LevelInfo, "m", 1, nil)
+ r := NewRecord(testTime, LevelInfo, "m", callerPC(2), nil)
if err := h.Handle(r); err != nil {
t.Fatal(err)
}
