Logger simplification (#883)

- **`Logger` now holds a single `SinkPtr`** instead of a `std::vector<SinkPtr>`. The `SetSinks`/`AddSink` API is replaced with a single `SetSink`. This removes complexity from `Logger` itself and makes `Clone()` cheaper (no vector copy).

- **New `BroadcastSink`** (`zencore/logging/broadcastsink.h`) acts as a thread-safe, shared indirection point that fans out to a dynamic list of child sinks. Adding or removing a child sink via `AddSink`/`RemoveSink` is immediately visible to every `Logger` that holds a reference to it — including cloned loggers — without requiring each logger to be updated individually.

- **`GetDefaultBroadcastSink()`** (exposed from `zenutil/logging.h`) gives server-layer code access to the shared broadcast sink so it can register optional sinks (OTel, TCP log stream) after logging is initialized, without going through `Default()->AddSink()`.

### Motivation

Previously, dynamically adding sinks post-initialization mutated the default logger's internal sink vector directly. This was fragile: cloned loggers (created before `AddSink` was called) would not pick up the new sinks. `BroadcastSink` fixes this by making the sink list a shared, mutable object that all loggers sharing the same broadcast instance observe uniformly.

10 files changed, 156 insertions, 73 deletions

diff --git a/src/zencore/include/zencore/logging/broadcastsink.h b/src/zencore/include/zencore/logging/broadcastsink.h
new file mode 100644
index 000000000..c2709d87c
--- /dev/null
+++ b/src/zencore/include/zencore/logging/broadcastsink.h
@@ -0,0 +1,86 @@
+// Copyright Epic Games, Inc. All Rights Reserved.
+
+#pragma once
+
+#include <zencore/logging/sink.h>
+#include <zencore/thread.h>
+
+#include <algorithm>
+#include <vector>
+
+namespace zen::logging {
+
+/// A sink that broadcasts log messages to a dynamic list of child sinks.
+///
+/// BroadcastSink acts as a shared indirection point: multiple Loggers can
+/// reference the same BroadcastSink instance, and adding or removing a child
+/// sink is immediately visible to all of them.  This is the recommended way
+/// to manage "default" sinks that should be active on most loggers.
+///
+/// Each child sink owns its own Formatter — BroadcastSink::SetFormatter() is
+/// intentionally a no-op so that per-sink formatting is not accidentally
+/// overwritten by registry-wide formatter changes.
+class BroadcastSink : public Sink
+{
+public:
+	BroadcastSink() = default;
+	explicit BroadcastSink(std::vector<SinkPtr> InSinks) : m_Sinks(std::move(InSinks)) {}
+
+	void Log(const LogMessage& Msg) override
+	{
+		RwLock::SharedLockScope Lock(m_Lock);
+		for (auto& Child : m_Sinks)
+		{
+			if (Child->ShouldLog(Msg.GetLevel()))
+			{
+				try
+				{
+					Child->Log(Msg);
+				}
+				catch (const std::exception&)
+				{
+					// logging here would be sketchy since we could cause a recursive loop
+					// if we wanted to surface this error, we would need to have some sort
+					// of internal error handling mechanism that doesn't rely on sinks
+				}
+			}
+		}
+	}
+
+	void Flush() override
+	{
+		RwLock::SharedLockScope Lock(m_Lock);
+		for (auto& Child : m_Sinks)
+		{
+			try
+			{
+				Child->Flush();
+			}
+			catch (const std::exception&)
+			{
+				// must not log here (see above)
+			}
+		}
+	}
+
+	/// No-op — child sinks manage their own formatters.
+	void SetFormatter(std::unique_ptr<Formatter> /*InFormatter*/) override {}
+
+	void AddSink(SinkPtr InSink)
+	{
+		RwLock::ExclusiveLockScope Lock(m_Lock);
+		m_Sinks.push_back(std::move(InSink));
+	}
+
+	void RemoveSink(const SinkPtr& InSink)
+	{
+		RwLock::ExclusiveLockScope Lock(m_Lock);
+		m_Sinks.erase(std::remove(m_Sinks.begin(), m_Sinks.end(), InSink), m_Sinks.end());
+	}
+
+private:
+	RwLock				 m_Lock;
+	std::vector<SinkPtr> m_Sinks;
+};
+
+}  // namespace zen::logging
diff --git a/src/zencore/include/zencore/logging/logger.h b/src/zencore/include/zencore/logging/logger.h
index c94bc58fa..1706a4455 100644
--- a/src/zencore/include/zencore/logging/logger.h
+++ b/src/zencore/include/zencore/logging/logger.h
@@ -21,7 +21,6 @@ class Logger final : public LoggerBase
 {
 public:
 	Logger(std::string_view InName, SinkPtr InSink);
-	Logger(std::string_view InName, std::span<const SinkPtr> InSinks);
 	~Logger();
 
 	Logger(const Logger&) = delete;
@@ -31,8 +30,7 @@ public:
 
 	std::string_view Name() const;
 
-	void SetSinks(std::vector<SinkPtr> InSinks);
-	void AddSink(SinkPtr InSink);
+	void SetSink(SinkPtr InSink);
 
 	void SetFormatter(std::unique_ptr<Formatter> InFormatter);
 
diff --git a/src/zencore/include/zencore/logging/logmsg.h b/src/zencore/include/zencore/logging/logmsg.h
index a1acb503b..4a777c71e 100644
--- a/src/zencore/include/zencore/logging/logmsg.h
+++ b/src/zencore/include/zencore/logging/logmsg.h
@@ -41,22 +41,14 @@ struct LogMessage
 	void SetSource(const SourceLocation& InSource) { m_Source = InSource; }
 
 private:
-	static constexpr LogPoint s_DefaultPoints[LogLevelCount] = {
-		{{}, Trace, {}},
-		{{}, Debug, {}},
-		{{}, Info, {}},
-		{{}, Warn, {}},
-		{{}, Err, {}},
-		{{}, Critical, {}},
-		{{}, Off, {}},
-	};
+	static constexpr LogPoint s_DefaultPoint{{}, Off, {}};
 
 	std::string_view					  m_LoggerName;
 	LogLevel							  m_Level = Off;
 	std::chrono::system_clock::time_point m_Time;
 	SourceLocation						  m_Source;
 	std::string_view					  m_Payload;
-	const LogPoint*						  m_Point	 = &s_DefaultPoints[Off];
+	const LogPoint*						  m_Point	 = &s_DefaultPoint;
 	int									  m_ThreadId = 0;
 };
 
diff --git a/src/zencore/logging.cpp b/src/zencore/logging.cpp
index 3206e380b..828bea6ed 100644
--- a/src/zencore/logging.cpp
+++ b/src/zencore/logging.cpp
@@ -345,7 +345,7 @@ SuppressConsoleLog()
 	}
 
 	SinkPtr NullSinkPtr(new NullSink());
-	ConLogger = Ref<Logger>(new Logger("console", std::vector<SinkPtr>{NullSinkPtr}));
+	ConLogger = Ref<Logger>(new Logger("console", NullSinkPtr));
 	Registry::Instance().Register(ConLogger);
 }
 
@@ -391,7 +391,7 @@ ConsoleLog()
 		{
 			SinkPtr ConsoleSink(new AnsiColorStdoutSink());
 			ConsoleSink->SetFormatter(std::make_unique<ConsoleFormatter>());
-			ConLogger = Ref<Logger>(new Logger("console", std::vector<SinkPtr>{ConsoleSink}));
+			ConLogger = Ref<Logger>(new Logger("console", ConsoleSink));
 			Registry::Instance().Register(ConLogger);
 		}
 	});
diff --git a/src/zencore/logging/logger.cpp b/src/zencore/logging/logger.cpp
index dd1675bb1..ff1db3edc 100644
--- a/src/zencore/logging/logger.cpp
+++ b/src/zencore/logging/logger.cpp
@@ -4,27 +4,20 @@
 #include <zencore/thread.h>
 
 #include <string>
-#include <vector>
 
 namespace zen::logging {
 
 struct Logger::Impl
 {
-	std::string			 m_Name;
-	std::vector<SinkPtr> m_Sinks;
-	ErrorHandler*		 m_ErrorHandler = nullptr;
+	std::string	  m_Name;
+	SinkPtr		  m_Sink;
+	ErrorHandler* m_ErrorHandler = nullptr;
 };
 
 Logger::Logger(std::string_view InName, SinkPtr InSink) : m_Impl(std::make_unique<Impl>())
 {
 	m_Impl->m_Name = InName;
-	m_Impl->m_Sinks.push_back(std::move(InSink));
-}
-
-Logger::Logger(std::string_view InName, std::span<const SinkPtr> InSinks) : m_Impl(std::make_unique<Impl>())
-{
-	m_Impl->m_Name = InName;
-	m_Impl->m_Sinks.assign(InSinks.begin(), InSinks.end());
+	m_Impl->m_Sink = std::move(InSink);
 }
 
 Logger::~Logger() = default;
@@ -49,20 +42,17 @@ Logger::Log(const LogPoint& Point, fmt::format_args Args)
 void
 Logger::SinkIt(const LogMessage& Msg)
 {
-	for (auto& CurrentSink : m_Impl->m_Sinks)
+	if (m_Impl->m_Sink && m_Impl->m_Sink->ShouldLog(Msg.GetLevel()))
 	{
-		if (CurrentSink->ShouldLog(Msg.GetLevel()))
+		try
 		{
-			try
-			{
-				CurrentSink->Log(Msg);
-			}
-			catch (const std::exception& Ex)
+			m_Impl->m_Sink->Log(Msg);
+		}
+		catch (const std::exception& Ex)
+		{
+			if (m_Impl->m_ErrorHandler)
 			{
-				if (m_Impl->m_ErrorHandler)
-				{
-					m_Impl->m_ErrorHandler->HandleError(Ex.what());
-				}
+				m_Impl->m_ErrorHandler->HandleError(Ex.what());
 			}
 		}
 	}
@@ -80,11 +70,11 @@ Logger::FlushIfNeeded(LogLevel InLevel)
 void
 Logger::Flush()
 {
-	for (auto& CurrentSink : m_Impl->m_Sinks)
+	if (m_Impl->m_Sink)
 	{
 		try
 		{
-			CurrentSink->Flush();
+			m_Impl->m_Sink->Flush();
 		}
 		catch (const std::exception& Ex)
 		{
@@ -97,15 +87,9 @@ Logger::Flush()
 }
 
 void
-Logger::SetSinks(std::vector<SinkPtr> InSinks)
-{
-	m_Impl->m_Sinks = std::move(InSinks);
-}
-
-void
-Logger::AddSink(SinkPtr InSink)
+Logger::SetSink(SinkPtr InSink)
 {
-	m_Impl->m_Sinks.push_back(std::move(InSink));
+	m_Impl->m_Sink = std::move(InSink);
 }
 
 void
@@ -117,9 +101,9 @@ Logger::SetErrorHandler(ErrorHandler* Handler)
 void
 Logger::SetFormatter(std::unique_ptr<Formatter> InFormatter)
 {
-	for (auto& CurrentSink : m_Impl->m_Sinks)
+	if (m_Impl->m_Sink)
 	{
-		CurrentSink->SetFormatter(InFormatter->Clone());
+		m_Impl->m_Sink->SetFormatter(std::move(InFormatter));
 	}
 }
 
@@ -132,7 +116,7 @@ Logger::Name() const
 Ref<Logger>
 Logger::Clone(std::string_view NewName) const
 {
-	Ref<Logger> Cloned(new Logger(NewName, m_Impl->m_Sinks));
+	Ref<Logger> Cloned(new Logger(NewName, m_Impl->m_Sink));
 	Cloned->SetLevel(m_Level.load(std::memory_order_relaxed));
 	Cloned->SetFlushLevel(m_FlushLevel.load(std::memory_order_relaxed));
 	Cloned->SetErrorHandler(m_Impl->m_ErrorHandler);
diff --git a/src/zencore/sentryintegration.cpp b/src/zencore/sentryintegration.cpp
index c9c843dd6..8491bef64 100644
--- a/src/zencore/sentryintegration.cpp
+++ b/src/zencore/sentryintegration.cpp
@@ -341,7 +341,7 @@ SentryIntegration::Initialize(const Config& Conf, const std::string& CommandLine
 		sentry_set_user(SentryUserObject);
 
 		logging::SinkPtr SentrySink(new sentry::SentrySink());
-		m_SentryLogger = Ref<logging::Logger>(new logging::Logger("sentry", std::vector<logging::SinkPtr>{SentrySink}));
+		m_SentryLogger = Ref<logging::Logger>(new logging::Logger("sentry", SentrySink));
 		logging::Registry::Instance().Register(m_SentryLogger);
 		logging::SetErrorLog("sentry");
 
diff --git a/src/zenserver/diag/logging.cpp b/src/zenserver/diag/logging.cpp
index 38b15480a..7513e56f7 100644
--- a/src/zenserver/diag/logging.cpp
+++ b/src/zenserver/diag/logging.cpp
@@ -6,6 +6,7 @@
 
 #include <zencore/filesystem.h>
 #include <zencore/fmtutils.h>
+#include <zencore/logging/broadcastsink.h>
 #include <zencore/logging/logger.h>
 #include <zencore/logging/registry.h>
 #include <zencore/memory/llm.h>
@@ -47,7 +48,7 @@ InitializeServerLogging(const ZenServerConfig& InOptions, bool WithCacheService)
 																 /* max size */ 128 * 1024 * 1024,
 																 /* max files */ 16,
 																 /* rotate on open */ true));
-	Ref<logging::Logger> HttpLogger(new logging::Logger("http_requests", std::vector<logging::SinkPtr>{HttpSink}));
+	Ref<logging::Logger> HttpLogger(new logging::Logger("http_requests", HttpSink));
 	logging::Registry::Instance().Register(HttpLogger);
 
 	if (WithCacheService)
@@ -60,30 +61,32 @@ InitializeServerLogging(const ZenServerConfig& InOptions, bool WithCacheService)
 																	  /* max size */ 128 * 1024 * 1024,
 																	  /* max files */ 16,
 																	  /* rotate on open */ false));
-		Ref<logging::Logger> CacheLogger(new logging::Logger("z$", std::vector<logging::SinkPtr>{CacheSink}));
+		Ref<logging::Logger> CacheLogger(new logging::Logger("z$", CacheSink));
 		logging::Registry::Instance().Register(CacheLogger);
 
 		// Jupiter - only log upstream HTTP traffic to file
 
-		Ref<logging::Logger> JupiterLogger(new logging::Logger("jupiter", std::vector<logging::SinkPtr>{FileSink}));
+		Ref<logging::Logger> JupiterLogger(new logging::Logger("jupiter", FileSink));
 		logging::Registry::Instance().Register(JupiterLogger);
 
 		// Zen - only log upstream HTTP traffic to file
 
-		Ref<logging::Logger> ZenClientLogger(new logging::Logger("zenclient", std::vector<logging::SinkPtr>{FileSink}));
+		Ref<logging::Logger> ZenClientLogger(new logging::Logger("zenclient", FileSink));
 		logging::Registry::Instance().Register(ZenClientLogger);
 	}
 
+	Ref<logging::BroadcastSink> BroadcastSink = GetDefaultBroadcastSink();
+
 #if ZEN_WITH_OTEL
-	if (!InOptions.LoggingConfig.OtelEndpointUri.empty())
+	if (BroadcastSink && !InOptions.LoggingConfig.OtelEndpointUri.empty())
 	{
 		// TODO: Should sanity check that endpoint is reachable? Also, a valid URI?
 		logging::SinkPtr OtelSink(new zen::logging::OtelHttpProtobufSink(InOptions.LoggingConfig.OtelEndpointUri));
-		zen::logging::Default()->AddSink(std::move(OtelSink));
+		BroadcastSink->AddSink(std::move(OtelSink));
 	}
 #endif
 
-	if (!InOptions.LoggingConfig.LogStreamEndpoint.empty())
+	if (BroadcastSink && !InOptions.LoggingConfig.LogStreamEndpoint.empty())
 	{
 		std::string Endpoint = InOptions.LoggingConfig.LogStreamEndpoint;
 		std::string Host	 = "localhost";
@@ -100,7 +103,7 @@ InitializeServerLogging(const ZenServerConfig& InOptions, bool WithCacheService)
 		if (Port > 0)
 		{
 			logging::SinkPtr StreamSink(new TcpLogStreamSink(Host, Port, "zenserver"));
-			zen::logging::Default()->AddSink(std::move(StreamSink));
+			BroadcastSink->AddSink(std::move(StreamSink));
 		}
 	}
 
diff --git a/src/zenserver/storage/zenstorageserver.cpp b/src/zenserver/storage/zenstorageserver.cpp
index 531853f5d..68d722f60 100644
--- a/src/zenserver/storage/zenstorageserver.cpp
+++ b/src/zenserver/storage/zenstorageserver.cpp
@@ -13,6 +13,8 @@
 #include <zencore/iobuffer.h>
 #include <zencore/jobqueue.h>
 #include <zencore/logging.h>
+#include <zencore/logging/broadcastsink.h>
+#include <zencore/logging/registry.h>
 #include <zencore/scopeguard.h>
 #include <zencore/sentryintegration.h>
 #include <zencore/session.h>
@@ -30,6 +32,7 @@
 #include <zenstore/vfsimpl.h>
 #include <zenstore/workspaces.h>
 #include <zentelemetry/otlptrace.h>
+#include <zenutil/logging.h>
 #include <zenutil/service.h>
 #include <zenutil/workerpools.h>
 #include <zenutil/zenserverprocess.h>
diff --git a/src/zenutil/include/zenutil/logging.h b/src/zenutil/include/zenutil/logging.h
index 282ae1b9a..6abf6a96f 100644
--- a/src/zenutil/include/zenutil/logging.h
+++ b/src/zenutil/include/zenutil/logging.h
@@ -18,6 +18,10 @@
 // for sharing across different executables
 //
 
+namespace zen::logging {
+class BroadcastSink;
+}
+
 namespace zen {
 
 struct LoggingOptions
@@ -39,6 +43,7 @@ void FinishInitializeLogging(const LoggingOptions& LoggingOptions);
 void InitializeLogging(const LoggingOptions& LoggingOptions);
 void ShutdownLogging();
 
-logging::SinkPtr GetFileSink();
+logging::SinkPtr			GetFileSink();
+Ref<logging::BroadcastSink> GetDefaultBroadcastSink();
 
 }  // namespace zen
diff --git a/src/zenutil/logging/logging.cpp b/src/zenutil/logging/logging.cpp
index ea2448a42..e1755414b 100644
--- a/src/zenutil/logging/logging.cpp
+++ b/src/zenutil/logging/logging.cpp
@@ -8,6 +8,7 @@
 #include <zencore/logging.h>
 #include <zencore/logging/ansicolorsink.h>
 #include <zencore/logging/asyncsink.h>
+#include <zencore/logging/broadcastsink.h>
 #include <zencore/logging/logger.h>
 #include <zencore/logging/msvcsink.h>
 #include <zencore/logging/registry.h>
@@ -23,8 +24,9 @@
 
 namespace zen {
 
-static bool		 g_IsLoggingInitialized;
-logging::SinkPtr g_FileSink;
+static bool					g_IsLoggingInitialized;
+logging::SinkPtr			g_FileSink;
+Ref<logging::BroadcastSink> g_BroadcastSink;
 
 logging::SinkPtr
 GetFileSink()
@@ -32,6 +34,12 @@ GetFileSink()
 	return g_FileSink;
 }
 
+Ref<logging::BroadcastSink>
+GetDefaultBroadcastSink()
+{
+	return g_BroadcastSink;
+}
+
 void
 InitializeLogging(const LoggingOptions& LogOptions)
 {
@@ -117,8 +125,10 @@ BeginInitializeLogging(const LoggingOptions& LogOptions)
 
 	LoggerRef DefaultLogger = zen::logging::Default();
 
-	// Collect sinks into a local vector first so we can optionally wrap them
-	std::vector<logging::SinkPtr> Sinks;
+	// Build the broadcast sink — a shared indirection point that all
+	// loggers cloned from the default will share.  Adding or removing
+	// a child sink later is immediately visible to every logger.
+	std::vector<logging::SinkPtr> BroadcastChildren;
 
 	if (LogOptions.NoConsoleOutput)
 	{
@@ -126,17 +136,18 @@ BeginInitializeLogging(const LoggingOptions& LogOptions)
 	}
 	else
 	{
-		logging::SinkPtr ConsoleSink(new logging::AnsiColorStdoutSink());
+		logging::SinkPtr ConsoleSink(
+			new logging::AnsiColorStdoutSink(LogOptions.ForceColor ? logging::ColorMode::On : logging::ColorMode::Auto));
 		if (LogOptions.QuietConsole)
 		{
 			ConsoleSink->SetLevel(logging::Warn);
 		}
-		Sinks.push_back(ConsoleSink);
+		BroadcastChildren.push_back(ConsoleSink);
 	}
 
 	if (FileSink)
 	{
-		Sinks.push_back(FileSink);
+		BroadcastChildren.push_back(FileSink);
 	}
 
 #if ZEN_PLATFORM_WINDOWS
@@ -144,21 +155,21 @@ BeginInitializeLogging(const LoggingOptions& LogOptions)
 	{
 		logging::SinkPtr DebugSink(new logging::MsvcSink());
 		DebugSink->SetLevel(logging::Debug);
-		Sinks.push_back(DebugSink);
+		BroadcastChildren.push_back(DebugSink);
 	}
 #endif
 
+	g_BroadcastSink = Ref<logging::BroadcastSink>(new logging::BroadcastSink(std::move(BroadcastChildren)));
+
 	bool IsAsync = LogOptions.AllowAsync && !LogOptions.IsDebug && !LogOptions.IsTest;
 
 	if (IsAsync)
 	{
-		std::vector<logging::SinkPtr> AsyncSinks;
-		AsyncSinks.emplace_back(new logging::AsyncSink(std::move(Sinks)));
-		DefaultLogger->SetSinks(std::move(AsyncSinks));
+		DefaultLogger->SetSink(logging::SinkPtr(new logging::AsyncSink({logging::SinkPtr(g_BroadcastSink.Get())})));
 	}
 	else
 	{
-		DefaultLogger->SetSinks(std::move(Sinks));
+		DefaultLogger->SetSink(logging::SinkPtr(g_BroadcastSink.Get()));
 	}
 
 	static struct : logging::ErrorHandler
@@ -258,7 +269,8 @@ ShutdownLogging()
 
 	zen::logging::ShutdownLogging();
 
-	g_FileSink = nullptr;
+	g_FileSink		= nullptr;
+	g_BroadcastSink = nullptr;
 }
 
 }  // namespace zen