From a8bf0b099f0a81bdb4a23e2236878d1d01cccc81 Mon Sep 17 00:00:00 2001 From: Matt Holloway Date: Thu, 12 Feb 2026 12:23:48 +0000 Subject: [PATCH] initial logging stack for http --- internal/ghmcp/server.go | 1 + pkg/github/dependencies.go | 38 +++ pkg/github/dependencies_test.go | 4 + pkg/github/dynamic_tools_test.go | 2 +- pkg/github/feature_flags_test.go | 2 + pkg/github/server_test.go | 9 + pkg/http/server.go | 1 + pkg/observability/log/level.go | 23 ++ pkg/observability/log/logger.go | 21 ++ pkg/observability/log/noop_adapter.go | 62 +++++ pkg/observability/log/slog_adapter.go | 108 ++++++++ pkg/observability/log/slog_adapter_test.go | 277 +++++++++++++++++++++ pkg/observability/observability.go | 25 ++ pkg/observability/observability_test.go | 26 ++ 14 files changed, 598 insertions(+), 1 deletion(-) create mode 100644 pkg/observability/log/level.go create mode 100644 pkg/observability/log/logger.go create mode 100644 pkg/observability/log/noop_adapter.go create mode 100644 pkg/observability/log/slog_adapter.go create mode 100644 pkg/observability/log/slog_adapter_test.go create mode 100644 pkg/observability/observability.go create mode 100644 pkg/observability/observability_test.go diff --git a/internal/ghmcp/server.go b/internal/ghmcp/server.go index f9c37db62..e94e0945f 100644 --- a/internal/ghmcp/server.go +++ b/internal/ghmcp/server.go @@ -128,6 +128,7 @@ func NewStdioMCPServer(ctx context.Context, cfg github.MCPServerConfig) (*mcp.Se }, cfg.ContentWindowSize, featureChecker, + cfg.Logger, ) // Build and register the tool/resource/prompt inventory inventoryBuilder := github.NewInventory(cfg.Translator). diff --git a/pkg/github/dependencies.go b/pkg/github/dependencies.go index f966c531e..2c2ec1f23 100644 --- a/pkg/github/dependencies.go +++ b/pkg/github/dependencies.go @@ -4,6 +4,7 @@ import ( "context" "errors" "fmt" + "log/slog" "net/http" "os" @@ -11,6 +12,8 @@ import ( "github.com/github/github-mcp-server/pkg/http/transport" "github.com/github/github-mcp-server/pkg/inventory" "github.com/github/github-mcp-server/pkg/lockdown" + "github.com/github/github-mcp-server/pkg/observability" + obsvLog "github.com/github/github-mcp-server/pkg/observability/log" "github.com/github/github-mcp-server/pkg/raw" "github.com/github/github-mcp-server/pkg/scopes" "github.com/github/github-mcp-server/pkg/translations" @@ -94,6 +97,9 @@ type ToolDependencies interface { // IsFeatureEnabled checks if a feature flag is enabled. IsFeatureEnabled(ctx context.Context, flagName string) bool + + // Logger returns the logger + Logger(ctx context.Context) obsvLog.Logger } // BaseDeps is the standard implementation of ToolDependencies for the local server. @@ -113,6 +119,9 @@ type BaseDeps struct { // Feature flag checker for runtime checks featureChecker inventory.FeatureFlagChecker + + // Observability exporters (includes logger) + Obsv observability.Exporters } // Compile-time assertion to verify that BaseDeps implements the ToolDependencies interface. @@ -128,7 +137,14 @@ func NewBaseDeps( flags FeatureFlags, contentWindowSize int, featureChecker inventory.FeatureFlagChecker, + logger *slog.Logger, ) *BaseDeps { + var obsv observability.Exporters + if logger != nil { + obsv = observability.NewExporters(obsvLog.NewSlogLogger(logger, obsvLog.InfoLevel)) + } else { + obsv = observability.NewExporters(obsvLog.NewNoopLogger()) + } return &BaseDeps{ Client: client, GQLClient: gqlClient, @@ -138,6 +154,7 @@ func NewBaseDeps( Flags: flags, ContentWindowSize: contentWindowSize, featureChecker: featureChecker, + Obsv: obsv, } } @@ -170,6 +187,11 @@ func (d BaseDeps) GetFlags(_ context.Context) FeatureFlags { return d.Flags } // GetContentWindowSize implements ToolDependencies. func (d BaseDeps) GetContentWindowSize() int { return d.ContentWindowSize } +// Logger implements ToolDependencies. +func (d BaseDeps) Logger(ctx context.Context) obsvLog.Logger { + return d.Obsv.Logger(ctx) +} + // IsFeatureEnabled checks if a feature flag is enabled. // Returns false if the feature checker is nil, flag name is empty, or an error occurs. // This allows tools to conditionally change behavior based on feature flags. @@ -247,6 +269,9 @@ type RequestDeps struct { // Feature flag checker for runtime checks featureChecker inventory.FeatureFlagChecker + + // Observability exporters (includes logger) + obsv observability.Exporters } // NewRequestDeps creates a RequestDeps with the provided clients and configuration. @@ -258,7 +283,14 @@ func NewRequestDeps( t translations.TranslationHelperFunc, contentWindowSize int, featureChecker inventory.FeatureFlagChecker, + logger *slog.Logger, ) *RequestDeps { + var obsv observability.Exporters + if logger != nil { + obsv = observability.NewExporters(obsvLog.NewSlogLogger(logger, obsvLog.InfoLevel)) + } else { + obsv = observability.NewExporters(obsvLog.NewNoopLogger()) + } return &RequestDeps{ apiHosts: apiHosts, version: version, @@ -267,6 +299,7 @@ func NewRequestDeps( T: t, ContentWindowSize: contentWindowSize, featureChecker: featureChecker, + obsv: obsv, } } @@ -374,6 +407,11 @@ func (d *RequestDeps) GetFlags(ctx context.Context) FeatureFlags { // GetContentWindowSize implements ToolDependencies. func (d *RequestDeps) GetContentWindowSize() int { return d.ContentWindowSize } +// Logger implements ToolDependencies. +func (d *RequestDeps) Logger(ctx context.Context) obsvLog.Logger { + return d.obsv.Logger(ctx) +} + // IsFeatureEnabled checks if a feature flag is enabled. func (d *RequestDeps) IsFeatureEnabled(ctx context.Context, flagName string) bool { if d.featureChecker == nil || flagName == "" { diff --git a/pkg/github/dependencies_test.go b/pkg/github/dependencies_test.go index d13160d4c..a26e20c83 100644 --- a/pkg/github/dependencies_test.go +++ b/pkg/github/dependencies_test.go @@ -28,6 +28,7 @@ func TestIsFeatureEnabled_WithEnabledFlag(t *testing.T) { github.FeatureFlags{}, 0, // contentWindowSize checker, // featureChecker + nil, // logger ) // Test enabled flag @@ -52,6 +53,7 @@ func TestIsFeatureEnabled_WithoutChecker(t *testing.T) { github.FeatureFlags{}, 0, // contentWindowSize nil, // featureChecker (nil) + nil, // logger ) // Should return false when checker is nil @@ -76,6 +78,7 @@ func TestIsFeatureEnabled_EmptyFlagName(t *testing.T) { github.FeatureFlags{}, 0, // contentWindowSize checker, // featureChecker + nil, // logger ) // Should return false for empty flag name @@ -100,6 +103,7 @@ func TestIsFeatureEnabled_CheckerError(t *testing.T) { github.FeatureFlags{}, 0, // contentWindowSize checker, // featureChecker + nil, // logger ) // Should return false and log error (not crash) diff --git a/pkg/github/dynamic_tools_test.go b/pkg/github/dynamic_tools_test.go index 3e63c5d7b..6e3d221dc 100644 --- a/pkg/github/dynamic_tools_test.go +++ b/pkg/github/dynamic_tools_test.go @@ -136,7 +136,7 @@ func TestDynamicTools_EnableToolset(t *testing.T) { deps := DynamicToolDependencies{ Server: server, Inventory: reg, - ToolDeps: NewBaseDeps(nil, nil, nil, nil, translations.NullTranslationHelper, FeatureFlags{}, 0, nil), + ToolDeps: NewBaseDeps(nil, nil, nil, nil, translations.NullTranslationHelper, FeatureFlags{}, 0, nil, nil), T: translations.NullTranslationHelper, } diff --git a/pkg/github/feature_flags_test.go b/pkg/github/feature_flags_test.go index 2f0a435c9..2149c9f49 100644 --- a/pkg/github/feature_flags_test.go +++ b/pkg/github/feature_flags_test.go @@ -104,6 +104,7 @@ func TestHelloWorld_ConditionalBehavior_Featureflag(t *testing.T) { FeatureFlags{}, 0, checker, + nil, ) // Get the tool and its handler @@ -166,6 +167,7 @@ func TestHelloWorld_ConditionalBehavior_Config(t *testing.T) { FeatureFlags{InsidersMode: tt.insidersMode}, 0, nil, + nil, ) // Get the tool and its handler diff --git a/pkg/github/server_test.go b/pkg/github/server_test.go index 2b99cab12..9a0b96b90 100644 --- a/pkg/github/server_test.go +++ b/pkg/github/server_test.go @@ -10,6 +10,8 @@ import ( "time" "github.com/github/github-mcp-server/pkg/lockdown" + "github.com/github/github-mcp-server/pkg/observability" + mcpLog "github.com/github/github-mcp-server/pkg/observability/log" "github.com/github/github-mcp-server/pkg/raw" "github.com/github/github-mcp-server/pkg/translations" gogithub "github.com/google/go-github/v82/github" @@ -29,6 +31,7 @@ type stubDeps struct { t translations.TranslationHelperFunc flags FeatureFlags contentWindowSize int + obsv observability.Exporters } func (s stubDeps) GetClient(ctx context.Context) (*gogithub.Client, error) { @@ -59,6 +62,12 @@ func (s stubDeps) GetT() translations.TranslationHelperFunc { return s. func (s stubDeps) GetFlags(_ context.Context) FeatureFlags { return s.flags } func (s stubDeps) GetContentWindowSize() int { return s.contentWindowSize } func (s stubDeps) IsFeatureEnabled(_ context.Context, _ string) bool { return false } +func (s stubDeps) Logger(_ context.Context) mcpLog.Logger { + if s.obsv != nil { + return s.obsv.Logger(context.Background()) + } + return mcpLog.NewNoopLogger() +} // Helper functions to create stub client functions for error testing func stubClientFnFromHTTP(httpClient *http.Client) func(context.Context) (*gogithub.Client, error) { diff --git a/pkg/http/server.go b/pkg/http/server.go index 7397e54a8..efdde12aa 100644 --- a/pkg/http/server.go +++ b/pkg/http/server.go @@ -114,6 +114,7 @@ func RunHTTPServer(cfg ServerConfig) error { t, cfg.ContentWindowSize, featureChecker, + logger, ) // Initialize the global tool scope map diff --git a/pkg/observability/log/level.go b/pkg/observability/log/level.go new file mode 100644 index 000000000..39fdc6286 --- /dev/null +++ b/pkg/observability/log/level.go @@ -0,0 +1,23 @@ +package log + +type Level struct { + level string +} + +var ( + // DebugLevel causes all logs to be logged + DebugLevel = Level{"debug"} + // InfoLevel causes all logs of level info or more severe to be logged + InfoLevel = Level{"info"} + // WarnLevel causes all logs of level warn or more severe to be logged + WarnLevel = Level{"warn"} + // ErrorLevel causes all logs of level error or more severe to be logged + ErrorLevel = Level{"error"} + // FatalLevel causes only logs of level fatal to be logged + FatalLevel = Level{"fatal"} +) + +// String returns the string representation for Level +func (l Level) String() string { + return l.level +} diff --git a/pkg/observability/log/logger.go b/pkg/observability/log/logger.go new file mode 100644 index 000000000..6b7fb6ab6 --- /dev/null +++ b/pkg/observability/log/logger.go @@ -0,0 +1,21 @@ +package log + +import ( + "context" + "log/slog" +) + +type Logger interface { + Log(ctx context.Context, level Level, msg string, fields ...slog.Attr) + Debug(msg string, fields ...slog.Attr) + Info(msg string, fields ...slog.Attr) + Warn(msg string, fields ...slog.Attr) + Error(msg string, fields ...slog.Attr) + Fatal(msg string, fields ...slog.Attr) + WithFields(fields ...slog.Attr) Logger + WithError(err error) Logger + Named(name string) Logger + WithLevel(level Level) Logger + Sync() error + Level() Level +} diff --git a/pkg/observability/log/noop_adapter.go b/pkg/observability/log/noop_adapter.go new file mode 100644 index 000000000..eab44eda8 --- /dev/null +++ b/pkg/observability/log/noop_adapter.go @@ -0,0 +1,62 @@ +package log + +import ( + "context" + "log/slog" +) + +type NoopLogger struct{} + +var _ Logger = (*NoopLogger)(nil) + +func NewNoopLogger() *NoopLogger { + return &NoopLogger{} +} + +func (l *NoopLogger) Level() Level { + return DebugLevel +} + +func (l *NoopLogger) Log(_ context.Context, _ Level, _ string, _ ...slog.Attr) { + // No-op +} + +func (l *NoopLogger) Debug(_ string, _ ...slog.Attr) { + // No-op +} + +func (l *NoopLogger) Info(_ string, _ ...slog.Attr) { + // No-op +} + +func (l *NoopLogger) Warn(_ string, _ ...slog.Attr) { + // No-op +} + +func (l *NoopLogger) Error(_ string, _ ...slog.Attr) { + // No-op +} + +func (l *NoopLogger) Fatal(_ string, _ ...slog.Attr) { + // No-op +} + +func (l *NoopLogger) WithFields(_ ...slog.Attr) Logger { + return l +} + +func (l *NoopLogger) WithError(_ error) Logger { + return l +} + +func (l *NoopLogger) Named(_ string) Logger { + return l +} + +func (l *NoopLogger) WithLevel(_ Level) Logger { + return l +} + +func (l *NoopLogger) Sync() error { + return nil +} diff --git a/pkg/observability/log/slog_adapter.go b/pkg/observability/log/slog_adapter.go new file mode 100644 index 000000000..a4c6d6c1f --- /dev/null +++ b/pkg/observability/log/slog_adapter.go @@ -0,0 +1,108 @@ +package log + +import ( + "context" + "log/slog" +) + +type SlogLogger struct { + logger *slog.Logger + level Level +} + +func NewSlogLogger(logger *slog.Logger, level Level) *SlogLogger { + return &SlogLogger{ + logger: logger, + level: level, + } +} + +func (l *SlogLogger) Level() Level { + return l.level +} + +func (l *SlogLogger) Log(ctx context.Context, level Level, msg string, fields ...slog.Attr) { + slogLevel := convertLevel(level) + l.logger.LogAttrs(ctx, slogLevel, msg, fields...) +} + +func (l *SlogLogger) Debug(msg string, fields ...slog.Attr) { + l.Log(context.Background(), DebugLevel, msg, fields...) +} + +func (l *SlogLogger) Info(msg string, fields ...slog.Attr) { + l.Log(context.Background(), InfoLevel, msg, fields...) +} + +func (l *SlogLogger) Warn(msg string, fields ...slog.Attr) { + l.Log(context.Background(), WarnLevel, msg, fields...) +} + +func (l *SlogLogger) Error(msg string, fields ...slog.Attr) { + l.Log(context.Background(), ErrorLevel, msg, fields...) +} + +func (l *SlogLogger) Fatal(msg string, fields ...slog.Attr) { + l.Log(context.Background(), FatalLevel, msg, fields...) + // Sync to ensure the log message is flushed before panic + _ = l.Sync() + panic("fatal: " + msg) +} + +func (l *SlogLogger) WithFields(fields ...slog.Attr) Logger { + fieldKvPairs := make([]any, 0, len(fields)*2) + for _, attr := range fields { + k, v := attr.Key, attr.Value + fieldKvPairs = append(fieldKvPairs, k, v.Any()) + } + return &SlogLogger{ + logger: l.logger.With(fieldKvPairs...), + level: l.level, + } +} + +func (l *SlogLogger) WithError(err error) Logger { + if err == nil { + return l + } + return &SlogLogger{ + logger: l.logger.With("error", err.Error()), + level: l.level, + } +} + +func (l *SlogLogger) Named(name string) Logger { + return &SlogLogger{ + logger: l.logger.With("logger", name), + level: l.level, + } +} + +func (l *SlogLogger) WithLevel(level Level) Logger { + return &SlogLogger{ + logger: l.logger, + level: level, + } +} + +func (l *SlogLogger) Sync() error { + // Slog does not require syncing + return nil +} + +func convertLevel(level Level) slog.Level { + switch level { + case DebugLevel: + return slog.LevelDebug + case InfoLevel: + return slog.LevelInfo + case WarnLevel: + return slog.LevelWarn + case ErrorLevel: + return slog.LevelError + case FatalLevel: + return slog.LevelError + default: + return slog.LevelInfo + } +} diff --git a/pkg/observability/log/slog_adapter_test.go b/pkg/observability/log/slog_adapter_test.go new file mode 100644 index 000000000..95a898864 --- /dev/null +++ b/pkg/observability/log/slog_adapter_test.go @@ -0,0 +1,277 @@ +package log + +import ( + "bytes" + "context" + "errors" + "log/slog" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestNewSlogLogger(t *testing.T) { + slogger := slog.New(slog.NewTextHandler(&bytes.Buffer{}, nil)) + logger := NewSlogLogger(slogger, InfoLevel) + + assert.NotNil(t, logger) + assert.Equal(t, InfoLevel, logger.Level()) +} + +func TestSlogLogger_Level(t *testing.T) { + tests := []struct { + name string + level Level + }{ + {"debug", DebugLevel}, + {"info", InfoLevel}, + {"warn", WarnLevel}, + {"error", ErrorLevel}, + {"fatal", FatalLevel}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + buf := &bytes.Buffer{} + slogger := slog.New(slog.NewTextHandler(buf, nil)) + logger := NewSlogLogger(slogger, tt.level) + + assert.Equal(t, tt.level, logger.Level()) + }) + } +} + +func TestSlogLogger_ConvenienceMethods(t *testing.T) { + buf := &bytes.Buffer{} + slogger := slog.New(slog.NewTextHandler(buf, &slog.HandlerOptions{Level: slog.LevelDebug})) + logger := NewSlogLogger(slogger, DebugLevel) + + tests := []struct { + name string + logFunc func(string, ...slog.Attr) + level string + }{ + {"Debug", logger.Debug, "DEBUG"}, + {"Info", logger.Info, "INFO"}, + {"Warn", logger.Warn, "WARN"}, + {"Error", logger.Error, "ERROR"}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + buf.Reset() + tt.logFunc("test message", slog.String("key", "value")) + + output := buf.String() + assert.Contains(t, output, "test message") + assert.Contains(t, output, tt.level) + assert.Contains(t, output, "key=value") + }) + } +} + +func TestSlogLogger_Fatal(t *testing.T) { + buf := &bytes.Buffer{} + slogger := slog.New(slog.NewTextHandler(buf, &slog.HandlerOptions{Level: slog.LevelDebug})) + logger := NewSlogLogger(slogger, DebugLevel) + + // Fatal should panic after logging + assert.Panics(t, func() { + logger.Fatal("fatal message") + }) + + // Verify the message was logged before panic + assert.Contains(t, buf.String(), "fatal message") +} + +func TestSlogLogger_WithFields(t *testing.T) { + buf := &bytes.Buffer{} + slogger := slog.New(slog.NewTextHandler(buf, &slog.HandlerOptions{Level: slog.LevelDebug})) + logger := NewSlogLogger(slogger, InfoLevel) + + // Add fields and log + loggerWithFields := logger.WithFields( + slog.String("service", "test-service"), + slog.Int("port", 8080), + ) + + loggerWithFields.Info("message with fields") + + output := buf.String() + assert.Contains(t, output, "message with fields") + assert.Contains(t, output, "service") + assert.Contains(t, output, "test-service") + assert.Contains(t, output, "port") + assert.Contains(t, output, "8080") + + // Original logger should not have the fields + buf.Reset() + logger.Info("message without fields") + output = buf.String() + assert.NotContains(t, output, "service") +} + +func TestSlogLogger_WithError(t *testing.T) { + buf := &bytes.Buffer{} + slogger := slog.New(slog.NewTextHandler(buf, &slog.HandlerOptions{Level: slog.LevelDebug})) + logger := NewSlogLogger(slogger, InfoLevel) + + testErr := errors.New("test error message") + loggerWithError := logger.WithError(testErr) + + loggerWithError.Error("operation failed") + + output := buf.String() + assert.Contains(t, output, "operation failed") + assert.Contains(t, output, "error") + assert.Contains(t, output, "test error message") +} + +func TestSlogLogger_Named(t *testing.T) { + buf := &bytes.Buffer{} + slogger := slog.New(slog.NewTextHandler(buf, &slog.HandlerOptions{Level: slog.LevelDebug})) + logger := NewSlogLogger(slogger, InfoLevel) + + namedLogger := logger.Named("my-component") + namedLogger.Info("component message") + + output := buf.String() + assert.Contains(t, output, "component message") + assert.Contains(t, output, "logger") + assert.Contains(t, output, "my-component") +} + +func TestSlogLogger_WithLevel(t *testing.T) { + buf := &bytes.Buffer{} + slogger := slog.New(slog.NewTextHandler(buf, &slog.HandlerOptions{Level: slog.LevelDebug})) + logger := NewSlogLogger(slogger, InfoLevel) + + // New logger with debug level + debugLogger := logger.WithLevel(DebugLevel) + + // Verify levels are correct + assert.Equal(t, InfoLevel, logger.Level()) + assert.Equal(t, DebugLevel, debugLogger.Level()) +} + +func TestSlogLogger_Sync(t *testing.T) { + buf := &bytes.Buffer{} + slogger := slog.New(slog.NewTextHandler(buf, nil)) + logger := NewSlogLogger(slogger, InfoLevel) + + // Sync should not error for slog (no-op) + err := logger.Sync() + assert.NoError(t, err) +} + +func TestSlogLogger_Chaining(t *testing.T) { + buf := &bytes.Buffer{} + slogger := slog.New(slog.NewTextHandler(buf, &slog.HandlerOptions{Level: slog.LevelDebug})) + logger := NewSlogLogger(slogger, DebugLevel) + + // Chain multiple operations + chainedLogger := logger. + Named("service"). + WithFields(slog.String("version", "1.0")). + WithLevel(InfoLevel) + + chainedLogger.Info("chained message") + + output := buf.String() + assert.Contains(t, output, "chained message") + assert.Contains(t, output, "service") + assert.Contains(t, output, "version") + assert.Contains(t, output, "1.0") +} + +func TestConvertLevel(t *testing.T) { + tests := []struct { + name string + input Level + expected slog.Level + }{ + {"debug to slog debug", DebugLevel, slog.LevelDebug}, + {"info to slog info", InfoLevel, slog.LevelInfo}, + {"warn to slog warn", WarnLevel, slog.LevelWarn}, + {"error to slog error", ErrorLevel, slog.LevelError}, + {"fatal to slog error", FatalLevel, slog.LevelError}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := convertLevel(tt.input) + assert.Equal(t, tt.expected, result) + }) + } +} + +func TestConvertLevel_Unknown(t *testing.T) { + unknownLevel := Level{"unknown"} + result := convertLevel(unknownLevel) + assert.Equal(t, slog.LevelInfo, result) +} + +func TestSlogLogger_ImplementsInterface(_ *testing.T) { + var _ Logger = (*SlogLogger)(nil) +} + +func TestSlogLogger_LogWithContext(t *testing.T) { + buf := &bytes.Buffer{} + slogger := slog.New(slog.NewTextHandler(buf, &slog.HandlerOptions{Level: slog.LevelDebug})) + logger := NewSlogLogger(slogger, DebugLevel) + + ctx := context.Background() + logger.Log(ctx, InfoLevel, "context message", slog.String("trace_id", "abc123")) + + output := buf.String() + assert.Contains(t, output, "context message") + assert.Contains(t, output, "trace_id") + assert.Contains(t, output, "abc123") +} + +func TestSlogLogger_WithFields_PreservesLevel(t *testing.T) { + buf := &bytes.Buffer{} + slogger := slog.New(slog.NewTextHandler(buf, &slog.HandlerOptions{Level: slog.LevelDebug})) + logger := NewSlogLogger(slogger, WarnLevel) + + withFields := logger.WithFields(slog.String("key", "value")) + + assert.Equal(t, WarnLevel, withFields.Level()) + + withFields.Warn("should appear") + assert.Contains(t, buf.String(), "should appear") + assert.Contains(t, buf.String(), "key=value") +} + +func TestSlogLogger_WithError_NilSafe(t *testing.T) { + buf := &bytes.Buffer{} + slogger := slog.New(slog.NewTextHandler(buf, &slog.HandlerOptions{Level: slog.LevelDebug})) + logger := NewSlogLogger(slogger, InfoLevel) + + require.NotPanics(t, func() { + result := logger.WithError(nil) + assert.NotNil(t, result) + assert.Equal(t, logger, result) + }) +} + +func TestSlogLogger_MultipleFields(t *testing.T) { + buf := &bytes.Buffer{} + slogger := slog.New(slog.NewTextHandler(buf, &slog.HandlerOptions{Level: slog.LevelDebug})) + logger := NewSlogLogger(slogger, DebugLevel) + + logger.Info("multi-field message", + slog.String("string_field", "value"), + slog.Int("int_field", 42), + slog.Bool("bool_field", true), + slog.Float64("float_field", 3.14), + ) + + output := buf.String() + assert.Contains(t, output, "multi-field message") + assert.Contains(t, output, "string_field") + assert.Contains(t, output, "int_field") + assert.Contains(t, output, "bool_field") + assert.Contains(t, output, "float_field") +} diff --git a/pkg/observability/observability.go b/pkg/observability/observability.go new file mode 100644 index 000000000..7c0a74c1e --- /dev/null +++ b/pkg/observability/observability.go @@ -0,0 +1,25 @@ +package observability + +import ( + "context" + + "github.com/github/github-mcp-server/pkg/observability/log" +) + +type Exporters interface { + Logger(context.Context) log.Logger +} + +type ObservabilityExporters struct { + logger log.Logger +} + +func NewExporters(logger log.Logger) Exporters { + return &ObservabilityExporters{ + logger: logger, + } +} + +func (e *ObservabilityExporters) Logger(_ context.Context) log.Logger { + return e.logger +} diff --git a/pkg/observability/observability_test.go b/pkg/observability/observability_test.go new file mode 100644 index 000000000..4c3882633 --- /dev/null +++ b/pkg/observability/observability_test.go @@ -0,0 +1,26 @@ +package observability + +import ( + "context" + "testing" + + "github.com/github/github-mcp-server/pkg/observability/log" + "github.com/stretchr/testify/assert" +) + +func TestNewExporters(t *testing.T) { + logger := log.NewNoopLogger() + exp := NewExporters(logger) + ctx := context.Background() + + assert.NotNil(t, exp) + assert.Equal(t, logger, exp.Logger(ctx)) +} + +func TestExporters_WithNilLogger(t *testing.T) { + exp := NewExporters(nil) + ctx := context.Background() + + assert.NotNil(t, exp) + assert.Nil(t, exp.Logger(ctx)) +}