Skip to content

Commit f04393c

Browse files
authored
Only call handle method on multihandler inner handler if enabled (#43613)
### What does this PR do? Update `MultiHandler` to only call a inner handler's `Handle` method if this `Handler` is enabled. ### Motivation The [documentation](https://pkg.go.dev/log/slog#Handler) of the `Handle` method explicitly says that the method should only be called when enabled. ``` // Handle handles the Record. // It will only be called when Enabled returns true. ``` ### Describe how you validated your changes CI ### Additional Notes Co-authored-by: pierre.gimalac <[email protected]>
1 parent 69a1798 commit f04393c

File tree

2 files changed

+26
-2
lines changed

2 files changed

+26
-2
lines changed

pkg/util/log/slog/handlers/multi.go

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -32,8 +32,10 @@ func NewMulti(handlers ...slog.Handler) slog.Handler {
3232
func (h *multi) Handle(ctx context.Context, r slog.Record) error {
3333
var errs []error
3434
for _, handler := range h.handlers {
35-
if err := handler.Handle(ctx, r); err != nil {
36-
errs = append(errs, err)
35+
if handler.Enabled(ctx, r.Level) {
36+
if err := handler.Handle(ctx, r); err != nil {
37+
errs = append(errs, err)
38+
}
3739
}
3840
}
3941
return errors.Join(errs...)

pkg/util/log/slog/handlers/multi_test.go

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -223,3 +223,25 @@ func TestMultiHandlerChaining(t *testing.T) {
223223
err := handler.Handle(context.Background(), record)
224224
assert.NoError(t, err)
225225
}
226+
227+
func TestMultiHandlerOnlyCallsEnabledHandlers(t *testing.T) {
228+
inner1 := newMockInnerHandler()
229+
inner1.enabled = true
230+
inner2 := newMockInnerHandler()
231+
inner2.enabled = false
232+
inner3 := newMockInnerHandler()
233+
inner3.enabled = true
234+
235+
handler := NewMulti(inner1, inner2, inner3)
236+
237+
record := slog.NewRecord(time.Now(), slog.LevelInfo, "test message", 0)
238+
err := handler.Handle(context.Background(), record)
239+
240+
// Should not return error
241+
assert.NoError(t, err)
242+
243+
// Only enabled handlers should have Handle called
244+
assert.Equal(t, 1, inner1.recordCount())
245+
assert.Equal(t, 0, inner2.recordCount())
246+
assert.Equal(t, 1, inner3.recordCount())
247+
}

0 commit comments

Comments
 (0)