@cryptotaxi247 / netdata-1 / commits / 95d790219

chore(go.d/logger): improve caller attribution and unify terminal checks (#21817)

Ilya Mashchenko committed Feb 25, 2026 at 15:58 UTC 95d79021969c2e204ac934b45b6f4ac7df8d3084
10 files changed +222 -20
src/go/logger/default.go
+3 -4
@@ -4,16 +4,15 @@ package logger
4
5 import (
6 "log/slog"
7 - "os"
7 "time"
8
10 - "github.com/mattn/go-isatty"
9 + "github.com/netdata/netdata/go/plugins/pkg/terminal"
10 )
11
12 func newDefaultLogger() *Logger {
14 - if isatty.IsTerminal(os.Stderr.Fd()) {
13 + if terminal.IsTerminal() {
14 // skip 2 slog pkg calls, 3 this pkg calls
16 - return &Logger{sl: slog.New(withCallDepth(5, newTerminalHandler())), rl: newRateLimiter()}
15 + return &Logger{sl: slog.New(withTerminalCallDepth(5, newTerminalHandler())), rl: newRateLimiter()}
16 }
17 return &Logger{sl: slog.New(newTextHandler()).With(pluginAttr), rl: newRateLimiter()}
18 }
src/go/logger/handler.go
+62 -2
@@ -59,13 +59,22 @@ func newTerminalHandler() slog.Handler {
59 func withCallDepth(depth int, sh slog.Handler) slog.Handler {
60 if v, ok := sh.(*callDepthHandler); ok {
61 sh = v.sh
62 + return &callDepthHandler{depth: depth, isTerminal: v.isTerminal, sh: sh}
63 }
64 return &callDepthHandler{depth: depth, sh: sh}
65 }
66
67 +func withTerminalCallDepth(depth int, sh slog.Handler) slog.Handler {
68 + if v, ok := sh.(*callDepthHandler); ok {
69 + sh = v.sh
70 + }
71 + return &callDepthHandler{depth: depth, isTerminal: true, sh: sh}
72 +}
73 +
74 type callDepthHandler struct {
67 - depth int
68 - sh slog.Handler
75 + depth int
76 + isTerminal bool
77 + sh slog.Handler
78 }
79
80 func (h *callDepthHandler) Enabled(ctx context.Context, level slog.Level) bool {
@@ -73,14 +82,27 @@ func (h *callDepthHandler) Enabled(ctx context.Context, level slog.Level) bool {
82 }
83
84 func (h *callDepthHandler) WithAttrs(attrs []slog.Attr) slog.Handler {
85 + if h.isTerminal {
86 + return withTerminalCallDepth(h.depth, h.sh.WithAttrs(attrs))
87 + }
88 return withCallDepth(h.depth, h.sh.WithAttrs(attrs))
89 }
90
91 func (h *callDepthHandler) WithGroup(name string) slog.Handler {
92 + if h.isTerminal {
93 + return withTerminalCallDepth(h.depth, h.sh.WithGroup(name))
94 + }
95 return withCallDepth(h.depth, h.sh.WithGroup(name))
96 }
97
98 func (h *callDepthHandler) Handle(ctx context.Context, r slog.Record) error {
99 + if h.isTerminal && Level.Enabled(slog.LevelDebug) {
100 + // Keep fixed-skip math identical to the non-dynamic path.
101 + // resolveCallerPC will adjust for its own extra stack frame on fallback.
102 + r.PC = resolveCallerPC(h.depth + 2)
103 + return h.sh.Handle(ctx, r)
104 + }
105 +
106 // https://pkg.go.dev/log/slog#example-package-Wrapping
107 var pcs [1]uintptr
108 // skip Callers and this function
@@ -89,3 +111,41 @@ func (h *callDepthHandler) Handle(ctx context.Context, r slog.Record) error {
111
112 return h.sh.Handle(ctx, r)
113 }
114 +
115 +var callerSkipPrefixes = []string{
116 + "github.com/netdata/netdata/go/plugins/logger.",
117 + "log/slog.",
118 + "runtime.",
119 +}
120 +
121 +func resolveCallerPC(fixedSkip int) uintptr {
122 + var pcs [15]uintptr
123 + n := runtime.Callers(2, pcs[:]) // skip runtime.Callers + resolveCallerPC
124 + if n > 0 {
125 + frames := runtime.CallersFrames(pcs[:n])
126 + for {
127 + frame, more := frames.Next()
128 + if !isSkippedCaller(frame.Function) {
129 + return frame.PC
130 + }
131 + if !more {
132 + break
133 + }
134 + }
135 + }
136 +
137 + var fallback [1]uintptr
138 + // fixedSkip is the skip value used by Handle's fixed path.
139 + // Add one extra frame to account for resolveCallerPC itself.
140 + runtime.Callers(fixedSkip+1, fallback[:])
141 + return fallback[0]
142 +}
143 +
144 +func isSkippedCaller(function string) bool {
145 + for _, prefix := range callerSkipPrefixes {
146 + if strings.HasPrefix(function, prefix) {
147 + return true
148 + }
149 + }
150 + return false
151 +}
src/go/logger/handler_test.go new
+144
@@ -0,0 +1,144 @@
1 +package logger
2 +
3 +import (
4 + "context"
5 + "log/slog"
6 + "reflect"
7 + "runtime"
8 + "strings"
9 + "sync"
10 + "testing"
11 +
12 + "github.com/stretchr/testify/assert"
13 +)
14 +
15 +type pcCaptureHandler struct {
16 + mu sync.Mutex
17 + pcs []uintptr
18 +}
19 +
20 +const expectedLoggerPrefix = "github.com/netdata/netdata/go/plugins/logger."
21 +
22 +func (h *pcCaptureHandler) Enabled(context.Context, slog.Level) bool {
23 + return true
24 +}
25 +
26 +func (h *pcCaptureHandler) Handle(_ context.Context, r slog.Record) error {
27 + h.mu.Lock()
28 + defer h.mu.Unlock()
29 + h.pcs = append(h.pcs, r.PC)
30 + return nil
31 +}
32 +
33 +func (h *pcCaptureHandler) WithAttrs(_ []slog.Attr) slog.Handler {
34 + return h
35 +}
36 +
37 +func (h *pcCaptureHandler) WithGroup(_ string) slog.Handler {
38 + return h
39 +}
40 +
41 +func (h *pcCaptureHandler) lastFunction() string {
42 + h.mu.Lock()
43 + defer h.mu.Unlock()
44 + if len(h.pcs) == 0 {
45 + return ""
46 + }
47 + fn := runtime.FuncForPC(h.pcs[len(h.pcs)-1])
48 + if fn == nil {
49 + return ""
50 + }
51 + return fn.Name()
52 +}
53 +
54 +func newDepthTestLogger(isTerminal bool) (*Logger, *pcCaptureHandler) {
55 + h := &pcCaptureHandler{}
56 + var sh slog.Handler = h
57 + if isTerminal {
58 + sh = withTerminalCallDepth(4, sh)
59 + } else {
60 + sh = withCallDepth(4, sh)
61 + }
62 + return &Logger{sl: slog.New(sh), rl: newRateLimiter()}, h
63 +}
64 +
65 +//go:noinline
66 +func emitWhenInfo(l *Logger) {
67 + l.When(true).Info("x")
68 +}
69 +
70 +func TestCallDepthTerminalDebugUsesDynamicResolverForWhen(t *testing.T) {
71 + setTestLevel(t, slog.LevelDebug)
72 +
73 + l, h := newDepthTestLogger(true)
74 + emitWhenInfo(l)
75 +
76 + fn := h.lastFunction()
77 + assert.NotEmpty(t, fn)
78 + assert.False(t, strings.HasPrefix(fn, expectedLoggerPrefix), "expected non-logger caller, got %q", fn)
79 +}
80 +
81 +func TestCallDepthTerminalDebugUsesDynamicResolverForOnce(t *testing.T) {
82 + setTestLevel(t, slog.LevelDebug)
83 +
84 + l, h := newDepthTestLogger(true)
85 + l.Once("k").Info("x")
86 +
87 + fn := h.lastFunction()
88 + assert.NotEmpty(t, fn)
89 + assert.False(t, strings.HasPrefix(fn, expectedLoggerPrefix), "expected non-logger caller, got %q", fn)
90 +}
91 +
92 +func TestCallDepthGatingUsesFixedPathOutsideTerminalDebug(t *testing.T) {
93 + t.Run("terminal non-debug", func(t *testing.T) {
94 + setTestLevel(t, slog.LevelInfo)
95 +
96 + l, h := newDepthTestLogger(true)
97 + emitWhenInfo(l)
98 +
99 + fn := h.lastFunction()
100 + assert.NotEmpty(t, fn)
101 + assert.True(t, strings.HasPrefix(fn, expectedLoggerPrefix), "expected logger frame with fixed path, got %q", fn)
102 + })
103 +
104 + t.Run("non-terminal debug", func(t *testing.T) {
105 + setTestLevel(t, slog.LevelDebug)
106 +
107 + l, h := newDepthTestLogger(false)
108 + emitWhenInfo(l)
109 +
110 + fn := h.lastFunction()
111 + assert.NotEmpty(t, fn)
112 + assert.True(t, strings.HasPrefix(fn, expectedLoggerPrefix), "expected logger frame with fixed path, got %q", fn)
113 + })
114 +}
115 +
116 +func TestCallerSkipPrefixMatchesRuntimeLoggerPath(t *testing.T) {
117 + fn := runtime.FuncForPC(reflect.ValueOf((*Logger).Info).Pointer())
118 + if assert.NotNil(t, fn) {
119 + assert.True(t, strings.HasPrefix(fn.Name(), expectedLoggerPrefix))
120 + }
121 + assert.Equal(t, expectedLoggerPrefix, callerSkipPrefixes[0])
122 +}
123 +
124 +func TestResolveCallerPCFallbackMatchesFixedPath(t *testing.T) {
125 + setTestLevel(t, slog.LevelDebug)
126 +
127 + orig := callerSkipPrefixes
128 + callerSkipPrefixes = []string{""} // force fallback branch
129 + t.Cleanup(func() {
130 + callerSkipPrefixes = orig
131 + })
132 +
133 + fixedLogger, fixedHandler := newDepthTestLogger(false)
134 + emitWhenInfo(fixedLogger)
135 + expected := fixedHandler.lastFunction()
136 + assert.NotEmpty(t, expected)
137 +
138 + dynamicLogger, dynamicHandler := newDepthTestLogger(true)
139 + emitWhenInfo(dynamicLogger)
140 + actual := dynamicHandler.lastFunction()
141 + assert.NotEmpty(t, actual)
142 +
143 + assert.Equal(t, expected, actual)
144 +}
src/go/logger/logger.go
+3 -5
@@ -6,15 +6,13 @@ import (
6 "context"
7 "fmt"
8 "log/slog"
9 - "os"
9 "sync/atomic"
10
11 "github.com/netdata/netdata/go/plugins/pkg/executable"
13 -
14 - "github.com/mattn/go-isatty"
12 + "github.com/netdata/netdata/go/plugins/pkg/terminal"
13 )
14
17 -var isTerm = isatty.IsTerminal(os.Stderr.Fd())
15 +var isTerm = terminal.IsTerminal()
16
17 var isJournal = isStderrConnectedToJournal()
18
@@ -23,7 +21,7 @@ var pluginAttr = slog.String("plugin", executable.Name)
21 func New() *Logger {
22 if isTerm {
23 // skip 2 slog pkg calls, 2 this pkg calls
26 - return &Logger{sl: slog.New(withCallDepth(4, newTerminalHandler())), rl: newRateLimiter()}
24 + return &Logger{sl: slog.New(withTerminalCallDepth(4, newTerminalHandler())), rl: newRateLimiter()}
25 }
26 return &Logger{sl: slog.New(newTextHandler()).With(pluginAttr), rl: newRateLimiter()}
27 }
src/go/pkg/pluginconfig/pluginconfig.go
+2 -3
@@ -19,8 +19,7 @@ import (
19 "github.com/netdata/netdata/go/plugins/pkg/cli"
20 "github.com/netdata/netdata/go/plugins/pkg/executable"
21 "github.com/netdata/netdata/go/plugins/pkg/multipath"
22 -
23 - "github.com/mattn/go-isatty"
22 + "github.com/netdata/netdata/go/plugins/pkg/terminal"
23 )
24
25 var (
@@ -259,7 +258,7 @@ func (d *directories) validate() error {
258 return nil
259 }
260
262 -var isTerm = isatty.IsTerminal(os.Stderr.Fd()) || isatty.IsTerminal(os.Stdout.Fd())
261 +var isTerm = terminal.IsTerminal()
262
263 func readEnvFromOS(execDir string) envData {
264 e := envData{
src/go/pkg/terminal/terminal.go renamed
+4 -2
@@ -9,7 +9,9 @@ import (
9 )
10
11 // IsTerminal reports whether plugin IO is attached to a terminal.
12 -// It checks stdout and stdin for consistent behavior across components.
12 +// It checks stderr, stdout, and stdin.
13 func IsTerminal() bool {
14 - return isatty.IsTerminal(os.Stdout.Fd()) || isatty.IsTerminal(os.Stdin.Fd())
14 + return isatty.IsTerminal(os.Stderr.Fd()) ||
15 + isatty.IsTerminal(os.Stdout.Fd()) ||
16 + isatty.IsTerminal(os.Stdin.Fd())
17 }
src/go/plugin/agent/agent.go
+1 -1
@@ -16,8 +16,8 @@ import (
16 "github.com/netdata/netdata/go/plugins/pkg/multipath"
17 "github.com/netdata/netdata/go/plugins/pkg/netdataapi"
18 "github.com/netdata/netdata/go/plugins/pkg/safewriter"
19 + "github.com/netdata/netdata/go/plugins/pkg/terminal"
20 "github.com/netdata/netdata/go/plugins/plugin/agent/discovery"
20 - "github.com/netdata/netdata/go/plugins/plugin/agent/internal/terminal"
21 "github.com/netdata/netdata/go/plugins/plugin/agent/jobmgr"
22 "github.com/netdata/netdata/go/plugins/plugin/agent/runtimemgr"
23 "github.com/netdata/netdata/go/plugins/plugin/framework/collectorapi"
src/go/plugin/agent/discovery/sd/sd.go
+1 -1
@@ -8,8 +8,8 @@ import (
8 "log/slog"
9 "sync"
10
11 + "github.com/netdata/netdata/go/plugins/pkg/terminal"
12 "github.com/netdata/netdata/go/plugins/plugin/agent/discovery/sd/pipeline"
12 - "github.com/netdata/netdata/go/plugins/plugin/agent/internal/terminal"
13 "github.com/netdata/netdata/go/plugins/plugin/framework/confgroup"
14 "github.com/netdata/netdata/go/plugins/plugin/framework/dyncfg"
15 "github.com/netdata/netdata/go/plugins/plugin/framework/functions"
src/go/plugin/agent/jobmgr/filestatus.go
+1 -1
@@ -12,7 +12,7 @@ import (
12 "sync"
13
14 "github.com/netdata/netdata/go/plugins/pkg/executable"
15 - "github.com/netdata/netdata/go/plugins/plugin/agent/internal/terminal"
15 + "github.com/netdata/netdata/go/plugins/pkg/terminal"
16 "github.com/netdata/netdata/go/plugins/plugin/framework/confgroup"
17 "github.com/netdata/netdata/go/plugins/plugin/framework/filepersister"
18 )
src/go/plugin/agent/jobmgr/manager.go
+1 -1
@@ -18,9 +18,9 @@ import (
18 "github.com/netdata/netdata/go/plugins/pkg/funcapi"
19 "github.com/netdata/netdata/go/plugins/pkg/netdataapi"
20 "github.com/netdata/netdata/go/plugins/pkg/safewriter"
21 + "github.com/netdata/netdata/go/plugins/pkg/terminal"
22 "github.com/netdata/netdata/go/plugins/pkg/ticker"
23 "github.com/netdata/netdata/go/plugins/plugin/agent/internal/naming"
23 - "github.com/netdata/netdata/go/plugins/plugin/agent/internal/terminal"
24 "github.com/netdata/netdata/go/plugins/plugin/framework/collectorapi"
25 "github.com/netdata/netdata/go/plugins/plugin/framework/confgroup"
26 "github.com/netdata/netdata/go/plugins/plugin/framework/dyncfg"