Skip to content

Commit eacee24

Browse files
committed
log: remove notion of lazy handler
The lazy handler was useful in the old log package, since it could defer the evaluation of costly attributes until later in the log pipeline: thus, if the logging was done at 'Trace', we could skip evaluation if logging only was set to 'Info'. With the move to slog, this way of deferring evaluation is no longer needed, since slog introduced 'Enabled': the caller can thus do the evaluate-or-not decision at the callsite, which is much more straight-forward than dealing with lazy reflect-based evaluation. Also, lazy evaluation would not work with 'native' slog, as in, these two statements would be evaluated differently: ```golang log.Info("foo", "my lazy", lazyObj) slog.Info("foo", "my lazy", lazyObj) ```
1 parent 75b5c8f commit eacee24

File tree

10 files changed

+25
-71
lines changed

10 files changed

+25
-71
lines changed

cmd/geth/logtestcmd_active.go

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -102,9 +102,6 @@ func logTest(ctx *cli.Context) error {
102102
var c customQuotedStringer
103103
log.Info("a custom stringer that emits quoted text", "output", c)
104104
}
105-
{ // Lazy eval
106-
log.Info("Lazy evaluation of value", "key", log.Lazy{Fn: func() interface{} { return "lazy value" }})
107-
}
108105
{ // Multi-line message
109106
log.Info("A message with wonky \U0001F4A9 characters")
110107
log.Info("A multiline message \nINFO [10-18|14:11:31.106] with wonky characters \U0001F4A9")

cmd/geth/testdata/logging/logtest-json.txt

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,6 @@
2121
{"t":"2023-11-22T15:42:00.408197+08:00","lvl":"info","msg":"an error message with quotes","error":"this is an 'error'"}
2222
{"t":"2023-11-22T15:42:00.408202+08:00","lvl":"info","msg":"Custom Stringer value","2562047h47m16.854s":"2562047h47m16.854s"}
2323
{"t":"2023-11-22T15:42:00.408208+08:00","lvl":"info","msg":"a custom stringer that emits quoted text","output":"output with 'quotes'"}
24-
{"t":"2023-11-22T15:42:00.408215+08:00","lvl":"info","msg":"Lazy evaluation of value","key":"lazy value"}
2524
{"t":"2023-11-22T15:42:00.408219+08:00","lvl":"info","msg":"A message with wonky 💩 characters"}
2625
{"t":"2023-11-22T15:42:00.408222+08:00","lvl":"info","msg":"A multiline message \nINFO [10-18|14:11:31.106] with wonky characters 💩"}
2726
{"t":"2023-11-22T15:42:00.408226+08:00","lvl":"info","msg":"A multiline message \nLALA [ZZZZZZZZZZZZZZZZZZ] Actually part of message above"}

cmd/geth/testdata/logging/logtest-logfmt.txt

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,6 @@ t=xxxx-xx-xxTxx:xx:xx+xxxx lvl=info msg="\x1b[35mColored\x1b[0m[" "\x1b[35mColor
2121
t=xxxx-xx-xxTxx:xx:xx+xxxx lvl=info msg="an error message with quotes" error="this is an 'error'"
2222
t=xxxx-xx-xxTxx:xx:xx+xxxx lvl=info msg="Custom Stringer value" 2562047h47m16.854s=2562047h47m16.854s
2323
t=xxxx-xx-xxTxx:xx:xx+xxxx lvl=info msg="a custom stringer that emits quoted text" output="output with 'quotes'"
24-
t=xxxx-xx-xxTxx:xx:xx+xxxx lvl=info msg="Lazy evaluation of value" key="lazy value"
2524
t=xxxx-xx-xxTxx:xx:xx+xxxx lvl=info msg="A message with wonky 💩 characters"
2625
t=xxxx-xx-xxTxx:xx:xx+xxxx lvl=info msg="A multiline message \nINFO [10-18|14:11:31.106] with wonky characters 💩"
2726
t=xxxx-xx-xxTxx:xx:xx+xxxx lvl=info msg="A multiline message \nLALA [ZZZZZZZZZZZZZZZZZZ] Actually part of message above"

cmd/geth/testdata/logging/logtest-terminal.txt

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,8 +21,7 @@ INFO [xx-xx|xx:xx:xx.xxx] "\x1b[35mColored\x1b[0m[" "\x1b[35mColo
2121
INFO [xx-xx|xx:xx:xx.xxx] an error message with quotes error="this is an 'error'"
2222
INFO [xx-xx|xx:xx:xx.xxx] Custom Stringer value 2562047h47m16.854s=2562047h47m16.854s
2323
INFO [xx-xx|xx:xx:xx.xxx] a custom stringer that emits quoted text output="output with 'quotes'"
24-
INFO [xx-xx|xx:xx:xx.xxx] Lazy evaluation of value key="lazy value"
25-
INFO [xx-xx|xx:xx:xx.xxx] "A message with wonky 💩 characters"
24+
INFO [xx-xx|xx:xx:xx.xxx] "A message with wonky 💩 characters"
2625
INFO [xx-xx|xx:xx:xx.xxx] "A multiline message \nINFO [10-18|14:11:31.106] with wonky characters 💩"
2726
INFO [xx-xx|xx:xx:xx.xxx] A multiline message
2827
LALA [ZZZZZZZZZZZZZZZZZZ] Actually part of message above

internal/testlog/testlog.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -100,6 +100,10 @@ func LoggerWithHandler(t *testing.T, handler slog.Handler) log.Logger {
100100

101101
func (l *logger) Write(level slog.Level, msg string, ctx ...interface{}) {}
102102

103+
func (l *logger) Enabled(ctx context.Context, level slog.Level) bool {
104+
return l.l.Enabled(ctx, level)
105+
}
106+
103107
func (l *logger) Trace(msg string, ctx ...interface{}) {
104108
l.t.Helper()
105109
l.mu.Lock()

log/handler.go

Lines changed: 0 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -13,42 +13,6 @@ import (
1313
"golang.org/x/exp/slog"
1414
)
1515

16-
// Lazy allows you to defer calculation of a logged value that is expensive
17-
// to compute until it is certain that it must be evaluated with the given filters.
18-
//
19-
// You may wrap any function which takes no arguments to Lazy. It may return any
20-
// number of values of any type.
21-
type Lazy struct {
22-
Fn interface{}
23-
}
24-
25-
func evaluateLazy(lz Lazy) (interface{}, error) {
26-
t := reflect.TypeOf(lz.Fn)
27-
28-
if t.Kind() != reflect.Func {
29-
return nil, fmt.Errorf("INVALID_LAZY, not func: %+v", lz.Fn)
30-
}
31-
32-
if t.NumIn() > 0 {
33-
return nil, fmt.Errorf("INVALID_LAZY, func takes args: %+v", lz.Fn)
34-
}
35-
36-
if t.NumOut() == 0 {
37-
return nil, fmt.Errorf("INVALID_LAZY, no func return val: %+v", lz.Fn)
38-
}
39-
40-
value := reflect.ValueOf(lz.Fn)
41-
results := value.Call([]reflect.Value{})
42-
if len(results) == 1 {
43-
return results[0].Interface(), nil
44-
}
45-
values := make([]interface{}, len(results))
46-
for i, v := range results {
47-
values[i] = v.Interface()
48-
}
49-
return values, nil
50-
}
51-
5216
type discardHandler struct{}
5317

5418
// DiscardHandler returns a no-op handler

log/logger.go

Lines changed: 8 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -134,6 +134,9 @@ type Logger interface {
134134

135135
// Write logs a message at the specified level
136136
Write(level slog.Level, msg string, attrs ...any)
137+
138+
// Enabled reports whether l emits log records at the given context and level.
139+
Enabled(ctx context.Context, level slog.Level) bool
137140
}
138141

139142
type logger struct {
@@ -159,26 +162,6 @@ func (l *logger) Write(level slog.Level, msg string, attrs ...any) {
159162
if len(attrs)%2 != 0 {
160163
attrs = append(attrs, nil, errorKey, "Normalized odd number of arguments by adding nil")
161164
}
162-
163-
// evaluate lazy values
164-
var hadErr bool
165-
for i := 1; i < len(attrs); i += 2 {
166-
lz, ok := attrs[i].(Lazy)
167-
if ok {
168-
v, err := evaluateLazy(lz)
169-
if err != nil {
170-
hadErr = true
171-
attrs[i] = err
172-
} else {
173-
attrs[i] = v
174-
}
175-
}
176-
}
177-
178-
if hadErr {
179-
attrs = append(attrs, errorKey, "bad lazy")
180-
}
181-
182165
r := slog.NewRecord(time.Now(), level, msg, pcs[0])
183166
r.Add(attrs...)
184167
l.inner.Handler().Handle(context.Background(), r)
@@ -196,6 +179,11 @@ func (l *logger) New(ctx ...interface{}) Logger {
196179
return l.With(ctx...)
197180
}
198181

182+
// Enabled reports whether l emits log records at the given context and level.
183+
func (l *logger) Enabled(ctx context.Context, level slog.Level) bool {
184+
return l.inner.Enabled(ctx, level)
185+
}
186+
199187
func (l *logger) Trace(msg string, ctx ...interface{}) {
200188
l.Write(LevelTrace, msg, ctx...)
201189
}

log/logger_test.go

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -62,12 +62,12 @@ func BenchmarkTerminalHandler(b *testing.B) {
6262
benchmarkLogger(b, l)
6363
}
6464
func BenchmarkLogfmtHandler(b *testing.B) {
65-
l := NewLogger(LogfmtHandler(io.Discard))
65+
l := NewLogger(NewLogfmtHandler(io.Discard))
6666
benchmarkLogger(b, l)
6767
}
6868

6969
func BenchmarkJSONHandler(b *testing.B) {
70-
l := NewLogger(JSONHandler(io.Discard))
70+
l := NewLogger(NewJSONHandler(io.Discard))
7171
benchmarkLogger(b, l)
7272
}
7373

@@ -107,7 +107,6 @@ func TestLoggerOutput(t *testing.T) {
107107
bigint = big.NewInt(100)
108108
nilbig *big.Int
109109
err = fmt.Errorf("Oh nooes it's crap")
110-
lazy = Lazy{Fn: func() interface{} { return "lazy value" }}
111110
smallUint = uint256.NewInt(500_000)
112111
bigUint = &uint256.Int{0xff, 0xff, 0xff, 0xff}
113112
)
@@ -126,13 +125,12 @@ func TestLoggerOutput(t *testing.T) {
126125
"struct", customA,
127126
"struct", customB,
128127
"ptrstruct", &customA,
129-
"lazy", lazy,
130128
"smalluint", smallUint,
131129
"bigUint", bigUint)
132130

133131
have := out.String()
134132
t.Logf("output %v", out.String())
135-
want := `INFO [11-07|19:14:33.821] This is a message foo=123 bytes="[0 0 0 0 0 0 0 0 0 0]" bonk="a string with text" time=0001-01-01T00:00:00+0000 bigint=100 nilbig=<nil> err="Oh nooes it's crap" struct="{A:Foo B:12}" struct="{A:Foo\nLinebreak B:122}" ptrstruct="&{A:Foo B:12}" lazy="lazy value" smalluint=500,000 bigUint=1,600,660,942,523,603,594,864,898,306,482,794,244,293,965,082,972,225,630,372,095
133+
want := `INFO [11-07|19:14:33.821] This is a message foo=123 bytes="[0 0 0 0 0 0 0 0 0 0]" bonk="a string with text" time=0001-01-01T00:00:00+0000 bigint=100 nilbig=<nil> err="Oh nooes it's crap" struct="{A:Foo B:12}" struct="{A:Foo\nLinebreak B:122}" ptrstruct="&{A:Foo B:12}" smalluint=500,000 bigUint=1,600,660,942,523,603,594,864,898,306,482,794,244,293,965,082,972,225,630,372,095
136134
`
137135
if !bytes.Equal([]byte(have)[25:], []byte(want)[25:]) {
138136
t.Errorf("Error\nhave: %q\nwant: %q", have, want)

p2p/discover/table.go

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
package discover
2424

2525
import (
26+
"context"
2627
crand "crypto/rand"
2728
"encoding/binary"
2829
"fmt"
@@ -330,8 +331,10 @@ func (tab *Table) loadSeedNodes() {
330331
seeds = append(seeds, tab.nursery...)
331332
for i := range seeds {
332333
seed := seeds[i]
333-
age := log.Lazy{Fn: func() interface{} { return time.Since(tab.db.LastPongReceived(seed.ID(), seed.IP())) }}
334-
tab.log.Trace("Found seed node in database", "id", seed.ID(), "addr", seed.addr(), "age", age)
334+
if tab.log.Enabled(context.Background(), log.LevelTrace) {
335+
age := time.Since(tab.db.LastPongReceived(seed.ID(), seed.IP()))
336+
tab.log.Trace("Found seed node in database", "id", seed.ID(), "addr", seed.addr(), "age", age)
337+
}
335338
tab.addSeenNode(seed)
336339
}
337340
}

p2p/msgrate/msgrate.go

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
package msgrate
1919

2020
import (
21+
"context"
2122
"errors"
2223
"math"
2324
"sort"
@@ -410,7 +411,9 @@ func (t *Trackers) tune() {
410411

411412
t.tuned = time.Now()
412413
t.log.Debug("Recalculated msgrate QoS values", "rtt", t.roundtrip, "confidence", t.confidence, "ttl", t.targetTimeout(), "next", t.tuned.Add(t.roundtrip))
413-
t.log.Trace("Debug dump of mean capacities", "caps", log.Lazy{Fn: t.meanCapacities})
414+
if t.log.Enabled(context.Background(), log.LevelTrace) {
415+
t.log.Trace("Debug dump of mean capacities", "caps", t.meanCapacities())
416+
}
414417
}
415418

416419
// detune reduces the tracker's confidence in order to make fresh measurements

0 commit comments

Comments
 (0)