diff --git a/internal/logger/doc.go b/internal/logger/doc.go index 1136abf96..6b47be0da 100644 --- a/internal/logger/doc.go +++ b/internal/logger/doc.go @@ -7,7 +7,9 @@ // for per-server logs // // These APIs target different sinks and can be used together when a message should -// appear in multiple outputs. +// appear in multiple outputs. The unified file and markdown helper families +// share a common sink-dispatch path so new sink fan-out behavior stays +// centralized. // // # Per-type setup and error-handler functions // diff --git a/internal/logger/file_logger.go b/internal/logger/file_logger.go index a4cf72ecc..bba59f5d2 100644 --- a/internal/logger/file_logger.go +++ b/internal/logger/file_logger.go @@ -95,14 +95,12 @@ func (fl *FileLogger) GetWriter() io.Writer { // Global logging functions that use the global file logger -// logWithLevel is a helper that reduces code duplication for logging at different levels. -// It uses the withGlobalLogger helper from global_helpers.go to handle mutex locking and -// nil-checking, eliminating the need for repeated RWMutex lock/unlock patterns across -// LogInfo, LogWarn, LogError, and LogDebug. +var fileLevelSink = newGlobalLevelSink(&globalLoggerMu, &globalFileLogger) + +// logWithLevel is a helper that reduces code duplication for logging at +// different levels by delegating to the shared sink dispatcher. func logWithLevel(level LogLevel, category, format string, args ...interface{}) { - withGlobalLogger(&globalLoggerMu, &globalFileLogger, func(logger *FileLogger) { - logger.Log(level, category, format, args...) - }) + dispatchLevelToSinks(level, category, format, args, fileLevelSink) } // The exported wrappers below follow the Log-Level Quad-Function Pattern diff --git a/internal/logger/global_state.go b/internal/logger/global_state.go index ea0df5b96..06cb6aa52 100644 --- a/internal/logger/global_state.go +++ b/internal/logger/global_state.go @@ -201,8 +201,8 @@ import ( // Info/Warn/Error/Debug closures is centralized here. // // The shared logFuncs map below centralises the LogLevel → log-function -// mapping so that the internal helpers (logWithMarkdown, logWithLevelAndServer) -// do not need their own switch-on-level blocks. +// mapping so that internal helpers like logWithLevelAndServer do not need +// their own switch-on-level blocks. // // If a new LogLevel constant is ever added (e.g., LogLevelTrace), update all // required locations to keep the public API consistent: @@ -214,10 +214,10 @@ import ( // 6. Update TestLogLevelWrappers_CoverAllRegisteredLevels in log_level_wrappers_test.go. // // logFuncs maps each LogLevel to its corresponding global log function. -// This eliminates repeated switch-on-level blocks in logWithMarkdown -// (markdown_logger.go) and logWithLevelAndServer (server_file_logger.go). -// When adding a new LogLevel constant, add a corresponding entry here so -// that all dispatch sites automatically support the new level. +// This eliminates repeated switch-on-level blocks in helpers like +// logWithLevelAndServer (server_file_logger.go). When adding a new LogLevel +// constant, add a corresponding entry here so that all dispatch sites +// automatically support the new level. // logFuncSet is a generic bundle of per-level logging closures all sharing the // same function signature F. It is the single source of truth for the @@ -274,6 +274,40 @@ var logFuncs = map[LogLevel]func(string, string, ...interface{}){ LogLevelDebug: LogDebug, } +// globalLevelLogger captures logger types whose Log method accepts the shared +// (level, category, format, args...) signature used by the unified file and +// markdown sinks. +type globalLevelLogger interface { + closableLogger + Log(LogLevel, string, string, ...interface{}) +} + +// levelSinkFunc writes a single log entry to one destination. +type levelSinkFunc func(LogLevel, string, string, ...interface{}) + +// newGlobalLevelSink adapts a global logger pointer plus its mutex into a +// reusable levelSinkFunc. This lets the file and markdown helper families share +// the same sink-dispatch helper instead of each open-coding their own +// withGlobalLogger wrapper. +func newGlobalLevelSink[T globalLevelLogger](mu *sync.RWMutex, logger *T) levelSinkFunc { + return func(level LogLevel, category, format string, args ...interface{}) { + withGlobalLogger(mu, logger, func(l T) { + l.Log(level, category, format, args...) + }) + } +} + +// dispatchLevelToSinks writes the same log entry to each provided sink in +// order. Nil sinks are ignored so callers can build destination lists without +// extra conditionals. +func dispatchLevelToSinks(level LogLevel, category, format string, args []interface{}, sinks ...levelSinkFunc) { + for _, sink := range sinks { + if sink != nil { + sink(level, category, format, args...) + } + } +} + // Global Logger RWMutex Access Pattern // // All access to global logger instances uses the withGlobalLogger helper function @@ -295,8 +329,7 @@ var logFuncs = map[LogLevel]func(string, string, ...interface{}){ // }) // // The withGlobalLogger helper is used in: -// - file_logger.go: logWithLevel (for FileLogger) -// - markdown_logger.go: logWithMarkdown (for MarkdownLogger) +// - global_state.go: newGlobalLevelSink (used by FileLogger and MarkdownLogger sinks) // - jsonl_logger.go: LogRPCMessageJSONLWithTags and logRPCMessageJSONLWithTagsAndSanitized (for JSONLLogger) // - server_file_logger.go: logWithLevelAndServer (for ServerFileLogger) // - tools_logger.go: LogToolsForServer (for ToolsLogger) diff --git a/internal/logger/helper_functions_test.go b/internal/logger/helper_functions_test.go index b89edd974..02402b1c7 100644 --- a/internal/logger/helper_functions_test.go +++ b/internal/logger/helper_functions_test.go @@ -10,6 +10,29 @@ import ( "github.com/stretchr/testify/require" ) +func TestDispatchLevelToSinks(t *testing.T) { + t.Run("writes to each sink in order", func(t *testing.T) { + var calls []string + + dispatchLevelToSinks(LogLevelWarn, "startup", "message %d", []interface{}{1}, + func(level LogLevel, category, format string, args ...interface{}) { + calls = append(calls, string(level)+":"+category+":"+format) + assert.Equal(t, []interface{}{1}, args) + }, + nil, + func(level LogLevel, category, format string, args ...interface{}) { + calls = append(calls, string(level)+":"+category+":"+format) + assert.Equal(t, []interface{}{1}, args) + }, + ) + + assert.Equal(t, []string{ + "WARN:startup:message %d", + "WARN:startup:message %d", + }, calls) + }) +} + // TestLogWithLevel verifies the logWithLevel helper function works correctly // and eliminates duplicate code in file_logger.go func TestLogWithLevel(t *testing.T) { diff --git a/internal/logger/markdown_logger.go b/internal/logger/markdown_logger.go index 28cc9ecae..c81dfac3e 100644 --- a/internal/logger/markdown_logger.go +++ b/internal/logger/markdown_logger.go @@ -156,17 +156,13 @@ func (ml *MarkdownLogger) Log(level LogLevel, category, format string, args ...i // Global logging functions that also write to markdown logger +var markdownLevelSink = newGlobalLevelSink(&globalMarkdownMu, &globalMarkdownLogger) + // logWithMarkdown is a helper that logs to both regular and markdown loggers. -// It uses the withGlobalLogger helper from global_helpers.go to handle mutex locking -// and nil-checking for the markdown logger access. +// It delegates sink fan-out to dispatchLevelToSinks so the unified file sink and +// markdown sink share the same dispatch path. func logWithMarkdown(level LogLevel, category, format string, args ...interface{}) { - // Log to regular logger - logFuncs[level](category, format, args...) - - // Log to markdown logger using withGlobalLogger helper - withGlobalLogger(&globalMarkdownMu, &globalMarkdownLogger, func(logger *MarkdownLogger) { - logger.Log(level, category, format, args...) - }) + dispatchLevelToSinks(level, category, format, args, fileLevelSink, markdownLevelSink) } // The exported wrappers below follow the Log-Level Quad-Function Pattern