fix: serialize logger initialization and publish the console target safely

Signed-off-by: Feng Ruohang <rh@vonng.com>

internal/logger/targets_test.go

@@ -19,6 +19,7 @@ package logger
 
 import (
 	"context"
+	"errors"
 	"sync"
 	"sync/atomic"
 	"testing"
@@ -31,12 +32,19 @@ type fakeTarget struct {
 	name  string
 	kind  types.TargetType
 	inits atomic.Int32
+	init  func() error
 }
 
-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) 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)
+	if f.init != nil {
+		return f.init()
+	}
+	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 }
@@ -45,10 +53,12 @@ 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
+	prevTargets, prevConsole := systemTargets, consoleTgt.Load()
+	systemTargets = newTargetsList()
+	consoleTgt.Store(nil)
 	t.Cleanup(func() {
-		systemTargets, consoleTgt = prevTargets, prevConsole
+		systemTargets = prevTargets
+		consoleTgt.Store(prevConsole)
 	})
 }
 
@@ -71,7 +81,7 @@ func TestAddSystemTargetIdempotent(t *testing.T) {
 	if got := console.inits.Load(); got != 1 {
 		t.Fatalf("expected Init to run once, ran %d times", got)
 	}
-	if consoleTgt != console {
+	if got := consoleTgt.Load(); got == nil || *got != console {
 		t.Fatalf("consoleTgt not set to the console target")
 	}
 
@@ -89,17 +99,53 @@ func TestAddSystemTargetConcurrent(t *testing.T) {
 	ctx := context.Background()
 
 	console := &fakeTarget{name: "console+http", kind: types.TargetConsole}
+	console.init = func() error {
+		// Initialization is allowed to inspect or write to the existing logger.
+		_ = SystemTargets()
+		consoleLogIf(ctx, "test", errors.New("initializing logger"))
+		return nil
+	}
 	var wg sync.WaitGroup
+	start := make(chan struct{})
 	for range 32 {
 		wg.Add(1)
 		go func() {
 			defer wg.Done()
-			_ = AddSystemTarget(ctx, console)
+			<-start
+			if err := AddSystemTarget(ctx, console); err != nil {
+				t.Errorf("AddSystemTarget: %v", err)
+			}
+			consoleLogIf(ctx, "test", errors.New("concurrent logger"))
 		}()
 	}
+	close(start)
 	wg.Wait()
 
 	if got := len(SystemTargets()); got != 1 {
 		t.Fatalf("expected 1 system target after concurrent adds, got %d", got)
 	}
+	if got := console.inits.Load(); got != 1 {
+		t.Fatalf("expected one concurrent initialization, got %d", got)
+	}
+}
+
+func TestAddSystemTargetRetriesFailedInit(t *testing.T) {
+	swapSystemTargets(t)
+	ctx := context.Background()
+	initErr := errors.New("target unavailable")
+	target := &fakeTarget{name: "console", kind: types.TargetConsole}
+	target.init = func() error { return initErr }
+	if err := AddSystemTarget(ctx, target); !errors.Is(err, initErr) {
+		t.Fatalf("expected initialization error, got %v", err)
+	}
+	if len(SystemTargets()) != 0 || consoleTgt.Load() != nil {
+		t.Fatal("failed initialization published a target")
+	}
+	target.init = nil
+	if err := AddSystemTarget(ctx, target); err != nil {
+		t.Fatal(err)
+	}
+	if got := len(SystemTargets()); got != 1 || target.inits.Load() != 2 {
+		t.Fatalf("retry: targets=%d, initializations=%d", got, target.inits.Load())
+	}
 }