Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 16 additions & 4 deletions internal/logger/common.go
Original file line number Diff line number Diff line change
Expand Up @@ -194,21 +194,33 @@ import (
// The three sets and their internal helpers are:
//
// file_logger.go LogInfo / LogWarn / LogError / LogDebug → logWithLevel
// markdown_logger.go LogInfoMd / LogWarnMd / LogErrorMd / LogDebugMd → logWithMarkdownLevel
// markdown_logger.go LogInfoMd / LogWarnMd / LogErrorMd / LogDebugMd → logWithMarkdown
// server_file_logger.go LogInfoWithServer / ... / LogDebugWithServer → logWithLevelAndServer
//
// This pattern is intentionally kept across the three files because:
// - Each set is a distinct public API with a different signature and set of callers.
// - The one-liner wrappers are trivial and unlikely to diverge.
// - Go lacks the metaprogramming to eliminate them without sacrificing readability.
//
// The shared logFuncs map (file_logger.go) centralises the LogLevel → log-function
// mapping so that the internal helpers (logWithMarkdownLevel, logWithLevelAndServer)
// 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.
//
// When adding a new LogLevel constant (e.g., LogLevelTrace):
// 1. Add a new entry to the logFuncs map in file_logger.go.
// 1. Add a new entry to the logFuncs map below.
// 2. Add a new LogTrace wrapper to each of the three files above.
//
// 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.
var logFuncs = map[LogLevel]func(string, string, ...interface{}){
LogLevelInfo: LogInfo,
LogLevelWarn: LogWarn,
LogLevelError: LogError,
LogLevelDebug: LogDebug,
}

// Global Logger RWMutex Access Pattern
//
Expand Down
12 changes: 0 additions & 12 deletions internal/logger/file_logger.go
Original file line number Diff line number Diff line change
Expand Up @@ -133,18 +133,6 @@ func LogDebug(category, format string, args ...interface{}) {
logWithLevel(LogLevelDebug, category, format, args...)
}

// logFuncs maps each LogLevel to its corresponding global log function.
// This eliminates repeated switch-on-level blocks in logWithMarkdownLevel
// (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.
var logFuncs = map[LogLevel]func(string, string, ...interface{}){
LogLevelInfo: LogInfo,
LogLevelWarn: LogWarn,
LogLevelError: LogError,
LogLevelDebug: LogDebug,
}

// CloseGlobalLogger closes the global file logger
func CloseGlobalLogger() error {
return closeGlobalLogger(&globalLoggerMu, &globalFileLogger)
Expand Down
4 changes: 2 additions & 2 deletions internal/logger/helper_functions_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -99,9 +99,9 @@ func TestLogWithLevelAndServer(t *testing.T) {
assert.Contains(t, unifiedLog, "Debug message via server helper")
}

// TestLogWithMarkdownLevel verifies the logWithMarkdownLevel helper function
// TestLogWithMarkdown verifies the logWithMarkdown helper function
// correctly handles both regular and markdown logging
func TestLogWithMarkdownLevel(t *testing.T) {
func TestLogWithMarkdown(t *testing.T) {
tmpDir := t.TempDir()
logDir := filepath.Join(tmpDir, "logs")

Expand Down
19 changes: 6 additions & 13 deletions internal/logger/markdown_logger.go
Original file line number Diff line number Diff line change
Expand Up @@ -170,41 +170,34 @@ func (ml *MarkdownLogger) Log(level LogLevel, category, format string, args ...i
// 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.
func logWithMarkdown(level LogLevel, regularLogFunc func(string, string, ...interface{}), category, format string, args ...interface{}) {
func logWithMarkdown(level LogLevel, category, format string, args ...interface{}) {
// Log to regular logger
regularLogFunc(category, format, args...)
logFuncs[level](category, format, args...)

// Log to markdown logger using withGlobalLogger helper
withGlobalLogger(&globalMarkdownMu, &globalMarkdownLogger, func(logger *MarkdownLogger) {
logger.Log(level, category, format, args...)
})
}

// logWithMarkdownLevel is a helper that reduces code duplication for markdown logging at different levels.
// It uses the logFuncs map (file_logger.go) to look up the regular log function for the given level,
// eliminating repeated switch-on-level patterns across LogInfoMd, LogWarnMd, LogErrorMd, and LogDebugMd.
func logWithMarkdownLevel(level LogLevel, category, format string, args ...interface{}) {
logWithMarkdown(level, logFuncs[level], category, format, args...)
}

// LogInfoMd logs to both regular and markdown loggers
func LogInfoMd(category, format string, args ...interface{}) {
logWithMarkdownLevel(LogLevelInfo, category, format, args...)
logWithMarkdown(LogLevelInfo, category, format, args...)
}

// LogWarnMd logs to both regular and markdown loggers
func LogWarnMd(category, format string, args ...interface{}) {
logWithMarkdownLevel(LogLevelWarn, category, format, args...)
logWithMarkdown(LogLevelWarn, category, format, args...)
}

// LogErrorMd logs to both regular and markdown loggers
func LogErrorMd(category, format string, args ...interface{}) {
logWithMarkdownLevel(LogLevelError, category, format, args...)
logWithMarkdown(LogLevelError, category, format, args...)
}

// LogDebugMd logs to both regular and markdown loggers
func LogDebugMd(category, format string, args ...interface{}) {
logWithMarkdownLevel(LogLevelDebug, category, format, args...)
logWithMarkdown(LogLevelDebug, category, format, args...)
}

// CloseMarkdownLogger closes the global markdown logger
Expand Down
2 changes: 1 addition & 1 deletion internal/logger/server_file_logger.go
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,7 @@ func (sfl *ServerFileLogger) Close() error {
// logWithLevelAndServer is a helper that reduces code duplication for per-server logging at different levels.
// It uses the withGlobalLogger helper from global_helpers.go to handle mutex locking and nil-checking,
// eliminating repeated patterns across LogInfoWithServer, LogWarnWithServer, LogErrorWithServer, and LogDebugWithServer.
// It uses the logFuncs map (file_logger.go) for the unified log dispatch, avoiding a repeated switch-on-level block.
// It uses the logFuncs map (common.go) for the unified log dispatch, avoiding a repeated switch-on-level block.
func logWithLevelAndServer(serverID string, level LogLevel, category, format string, args ...interface{}) {
withGlobalLogger(&globalServerLoggerMu, &globalServerFileLogger, func(logger *ServerFileLogger) {
logger.Log(serverID, level, category, format, args...)
Expand Down
5 changes: 1 addition & 4 deletions internal/proxy/handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,10 +40,7 @@ type proxyHandler struct {

// getTracer returns the cached tracer if set, otherwise falls back to the global tracer.
func (h *proxyHandler) getTracer() oteltrace.Tracer {
if h.tracer != nil {
return h.tracer
}
return tracing.Tracer()
return tracing.GetCachedOrGlobal(h.tracer)
}

func (h *proxyHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
Expand Down
2 changes: 2 additions & 0 deletions internal/server/tool_registry.go
Original file line number Diff line number Diff line change
Expand Up @@ -350,6 +350,8 @@ func (us *UnifiedServer) callSysServer(toolName string) (interface{}, error) {
}

func marshalAndSanitizeForLog(value interface{}) string {
// Best-effort logging helper: if marshaling fails, we intentionally keep logging
// a sanitized empty string rather than surfacing an additional logging-only error.
resultJSON, _ := json.Marshal(value)
return sanitize.SanitizeString(string(resultJSON))
}
Expand Down
5 changes: 1 addition & 4 deletions internal/server/unified.go
Original file line number Diff line number Diff line change
Expand Up @@ -124,10 +124,7 @@ type UnifiedServer struct {

// getTracer returns the cached tracer if set, otherwise falls back to the global tracer.
func (us *UnifiedServer) getTracer() oteltrace.Tracer {
if us.tracer != nil {
return us.tracer
}
return tracing.Tracer()
return tracing.GetCachedOrGlobal(us.tracer)
}

// NewUnified creates a new unified MCP server
Expand Down
8 changes: 8 additions & 0 deletions internal/tracing/provider.go
Original file line number Diff line number Diff line change
Expand Up @@ -306,6 +306,14 @@ func Tracer() trace.Tracer {
return otel.Tracer(instrumentationName)
}

// GetCachedOrGlobal returns cached if non-nil, otherwise falls back to the global tracer.
func GetCachedOrGlobal(cached trace.Tracer) trace.Tracer {
if cached != nil {
return cached
}
return Tracer()
}

// ParentContext returns a context carrying the W3C remote parent span context
// from the configured traceId and spanId (spec §4.1.3.6).
// Exported for use at startup to build the root span's parent context.
Expand Down
14 changes: 14 additions & 0 deletions internal/tracing/provider_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,20 @@ func TestTracer_ReturnsNonNil(t *testing.T) {
assert.NotNil(t, tr)
}

func TestGetCachedOrGlobal_WithCachedTracer_ReturnsCached(t *testing.T) {
cached := noop.NewTracerProvider().Tracer("cached")
assert.Equal(t, cached, tracing.GetCachedOrGlobal(cached))
}

func TestGetCachedOrGlobal_WithNilTracer_ReturnsGlobal(t *testing.T) {
ctx := context.Background()
provider, err := tracing.InitProvider(ctx, nil)
require.NoError(t, err)
defer provider.Shutdown(ctx)

assert.NotNil(t, tracing.GetCachedOrGlobal(nil))
}

func TestInitProvider_SampleRateZero_UsesNeverSampler(t *testing.T) {
ctx := context.Background()

Expand Down
Loading