diff --git a/internal/logger/targets.go b/internal/logger/targets.go index 774237e1d..9fcca6b14 100644 --- a/internal/logger/targets.go +++ b/internal/logger/targets.go @@ -59,11 +59,35 @@ func (tl *targetsList) get() []Target { return tl.list } -func (tl *targetsList) add(t Target) { +// contains reports whether t is already registered. +func (tl *targetsList) contains(t Target) bool { + tl.mu.RLock() + defer tl.mu.RUnlock() + + return tl.indexOf(t) >= 0 +} + +// addIfAbsent appends t unless it is already registered. +// Returns true if t was added. +func (tl *targetsList) addIfAbsent(t Target) bool { tl.mu.Lock() defer tl.mu.Unlock() + if tl.indexOf(t) >= 0 { + return false + } tl.list = append(tl.list, t) + return true +} + +// indexOf must be called with tl.mu held. +func (tl *targetsList) indexOf(t Target) int { + for i, existing := range tl.list { + if existing == t { + return i + } + } + return -1 } func (tl *targetsList) set(tgts []Target) { @@ -125,8 +149,14 @@ func CurrentStats() map[string]types.TargetStats { } // AddSystemTarget adds a new logger target to the -// list of enabled loggers +// list of enabled loggers. Adding a target that is already +// registered is a no-op, so callers may safely re-register +// long-lived targets such as the console logger. func AddSystemTarget(ctx context.Context, t Target) error { + if systemTargets.contains(t) { + return nil + } + if err := t.Init(ctx); err != nil { return err } @@ -137,7 +167,7 @@ func AddSystemTarget(ctx context.Context, t Target) error { } } - systemTargets.add(t) + systemTargets.addIfAbsent(t) return nil } diff --git a/internal/logger/targets_test.go b/internal/logger/targets_test.go new file mode 100644 index 000000000..5552eb734 --- /dev/null +++ b/internal/logger/targets_test.go @@ -0,0 +1,105 @@ +// Copyright (c) 2015-2026 MinIO, Inc. +// +// This file is part of MinIO Object Storage stack +// +// This program is free software: you can redistribute it and/or modify +// it under the terms of the GNU Affero General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. +// +// This program is distributed in the hope that it will be useful +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU Affero General Public License for more details. +// +// You should have received a copy of the GNU Affero General Public License +// along with this program. If not, see . + +package logger + +import ( + "context" + "sync" + "sync/atomic" + "testing" + + types "github.com/minio/minio/internal/logger/target/loggertypes" +) + +// fakeTarget is a minimal Target used to exercise the registry. +type fakeTarget struct { + name string + kind types.TargetType + inits atomic.Int32 +} + +func (f *fakeTarget) String() string { return f.name } +func (f *fakeTarget) Endpoint() string { return "" } +func (f *fakeTarget) Stats() types.TargetStats { return types.TargetStats{} } +func (f *fakeTarget) Init(context.Context) error { f.inits.Add(1); return nil } +func (f *fakeTarget) IsOnline(context.Context) bool { return true } +func (f *fakeTarget) Cancel() {} +func (f *fakeTarget) Send(context.Context, any) error { return nil } +func (f *fakeTarget) Type() types.TargetType { return f.kind } + +// swapSystemTargets isolates the package-level registry for a test. +func swapSystemTargets(t *testing.T) { + t.Helper() + prevTargets, prevConsole := systemTargets, consoleTgt + systemTargets, consoleTgt = newTargetsList(), nil + t.Cleanup(func() { + systemTargets, consoleTgt = prevTargets, prevConsole + }) +} + +// AddSystemTarget must not register the same target twice: the console +// logger is re-added on every console-log subscription, and duplicates +// surface as identical series in the /logger/webhook metrics collector. +func TestAddSystemTargetIdempotent(t *testing.T) { + swapSystemTargets(t) + ctx := context.Background() + + console := &fakeTarget{name: "console+http", kind: types.TargetConsole} + for range 3 { + if err := AddSystemTarget(ctx, console); err != nil { + t.Fatalf("AddSystemTarget: %v", err) + } + } + if got := len(SystemTargets()); got != 1 { + t.Fatalf("expected 1 system target after repeated add, got %d", got) + } + if got := console.inits.Load(); got != 1 { + t.Fatalf("expected Init to run once, ran %d times", got) + } + if consoleTgt != console { + t.Fatalf("consoleTgt not set to the console target") + } + + other := &fakeTarget{name: "other", kind: types.TargetHTTP} + if err := AddSystemTarget(ctx, other); err != nil { + t.Fatalf("AddSystemTarget: %v", err) + } + if got := len(SystemTargets()); got != 2 { + t.Fatalf("expected 2 distinct system targets, got %d", got) + } +} + +func TestAddSystemTargetConcurrent(t *testing.T) { + swapSystemTargets(t) + ctx := context.Background() + + console := &fakeTarget{name: "console+http", kind: types.TargetConsole} + var wg sync.WaitGroup + for range 32 { + wg.Add(1) + go func() { + defer wg.Done() + _ = AddSystemTarget(ctx, console) + }() + } + wg.Wait() + + if got := len(SystemTargets()); got != 1 { + t.Fatalf("expected 1 system target after concurrent adds, got %d", got) + } +}