From ac128e2f40ef0d895385b49653f954bae59f3991 Mon Sep 17 00:00:00 2001 From: Adam Holt Date: Wed, 7 Jan 2026 13:02:24 +0100 Subject: [PATCH 1/2] Add logging stack interfaces and adapters --- internal/ghmcp/server.go | 1 + pkg/github/dependencies.go | 14 + pkg/github/dynamic_tools_test.go | 6 +- pkg/github/issues.go | 10 + pkg/github/server_test.go | 3 + pkg/observability/log/level.go | 28 ++ 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 | 283 +++++++++++++++++++++ pkg/observability/observability.go | 25 ++ pkg/observability/observability_test.go | 26 ++ 12 files changed, 586 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 165886606..9903c0897 100644 --- a/internal/ghmcp/server.go +++ b/internal/ghmcp/server.go @@ -207,6 +207,7 @@ func NewMCPServer(cfg MCPServerConfig) (*mcp.Server, error) { cfg.Translator, github.FeatureFlags{LockdownMode: cfg.LockdownMode}, cfg.ContentWindowSize, + cfg.Logger, ) // Inject dependencies into context for all tool handlers diff --git a/pkg/github/dependencies.go b/pkg/github/dependencies.go index b41bf0b87..833cd862e 100644 --- a/pkg/github/dependencies.go +++ b/pkg/github/dependencies.go @@ -3,9 +3,12 @@ package github import ( "context" "errors" + "log/slog" "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" @@ -77,6 +80,9 @@ type ToolDependencies interface { // GetContentWindowSize returns the content window size for log truncation GetContentWindowSize() int + + // GetLogger returns the logger + Logger(ctx context.Context) obsvLog.Logger } // BaseDeps is the standard implementation of ToolDependencies for the local server. @@ -93,6 +99,7 @@ type BaseDeps struct { T translations.TranslationHelperFunc Flags FeatureFlags ContentWindowSize int + Obsv observability.Exporters } // NewBaseDeps creates a BaseDeps with the provided clients and configuration. @@ -104,6 +111,7 @@ func NewBaseDeps( t translations.TranslationHelperFunc, flags FeatureFlags, contentWindowSize int, + logger *slog.Logger, ) *BaseDeps { return &BaseDeps{ Client: client, @@ -113,6 +121,7 @@ func NewBaseDeps( T: t, Flags: flags, ContentWindowSize: contentWindowSize, + Obsv: observability.NewExporters(obsvLog.NewSlogLogger(logger, obsvLog.InfoLevel)), } } @@ -134,6 +143,11 @@ func (d BaseDeps) GetRawClient(_ context.Context) (*raw.Client, error) { // GetRepoAccessCache implements ToolDependencies. func (d BaseDeps) GetRepoAccessCache() *lockdown.RepoAccessCache { return d.RepoAccessCache } +// GetLogger implements ToolDependencies. +func (d BaseDeps) Logger(ctx context.Context) obsvLog.Logger { + return d.Obsv.Logger(ctx) +} + // GetT implements ToolDependencies. func (d BaseDeps) GetT() translations.TranslationHelperFunc { return d.T } diff --git a/pkg/github/dynamic_tools_test.go b/pkg/github/dynamic_tools_test.go index 8d12b78c2..b85b69022 100644 --- a/pkg/github/dynamic_tools_test.go +++ b/pkg/github/dynamic_tools_test.go @@ -1,8 +1,10 @@ package github import ( + "bytes" "context" "encoding/json" + "log/slog" "testing" "github.com/github/github-mcp-server/pkg/inventory" @@ -128,12 +130,14 @@ func TestDynamicTools_EnableToolset(t *testing.T) { // Create a mock server server := mcp.NewServer(&mcp.Implementation{Name: "test"}, nil) + var logBuffer bytes.Buffer + logger := slog.New(slog.NewTextHandler(&logBuffer, nil)) // Create dynamic tool dependencies deps := DynamicToolDependencies{ Server: server, Inventory: reg, - ToolDeps: NewBaseDeps(nil, nil, nil, nil, translations.NullTranslationHelper, FeatureFlags{}, 0), + ToolDeps: NewBaseDeps(nil, nil, nil, nil, translations.NullTranslationHelper, FeatureFlags{}, 0, logger), T: translations.NullTranslationHelper, } diff --git a/pkg/github/issues.go b/pkg/github/issues.go index 1e29a0eef..0fd74d65a 100644 --- a/pkg/github/issues.go +++ b/pkg/github/issues.go @@ -5,6 +5,7 @@ import ( "encoding/json" "fmt" "io" + "log/slog" "net/http" "strings" "time" @@ -1459,12 +1460,21 @@ func ListIssues(t translations.TranslationHelperFunc) inventory.ServerTool { return utils.NewToolResultError(err.Error()), nil, nil } + deps.Logger(ctx).Info("list_issues called", + slog.String("owner", owner), + slog.String("repo", repo), + slog.String("orderBy", orderBy), + slog.String("direction", direction), + slog.String("since", since), + ) + // There are two optional parameters: since and labels. var sinceTime time.Time var hasSince bool if since != "" { sinceTime, err = parseISOTimestamp(since) if err != nil { + deps.Logger(ctx).Info("failed to parse 'since' timestamp", slog.String("error", err.Error())) return utils.NewToolResultError(fmt.Sprintf("failed to list issues: %s", err.Error())), nil, nil } hasSince = true diff --git a/pkg/github/server_test.go b/pkg/github/server_test.go index a59cd9a93..5eef9c3cc 100644 --- a/pkg/github/server_test.go +++ b/pkg/github/server_test.go @@ -10,6 +10,7 @@ import ( "time" "github.com/github/github-mcp-server/pkg/lockdown" + "github.com/github/github-mcp-server/pkg/observability" "github.com/github/github-mcp-server/pkg/raw" "github.com/github/github-mcp-server/pkg/translations" "github.com/google/go-github/v79/github" @@ -28,6 +29,7 @@ type stubDeps struct { t translations.TranslationHelperFunc flags FeatureFlags contentWindowSize int + obsv observability.Exporters } func (s stubDeps) GetClient(ctx context.Context) (*github.Client, error) { @@ -52,6 +54,7 @@ func (s stubDeps) GetRawClient(ctx context.Context) (*raw.Client, error) { } func (s stubDeps) GetRepoAccessCache() *lockdown.RepoAccessCache { return s.repoAccessCache } +func (s stubDeps) GetObsv() *observability.Exporters { return &s.obsv } func (s stubDeps) GetT() translations.TranslationHelperFunc { return s.t } func (s stubDeps) GetFlags() FeatureFlags { return s.flags } func (s stubDeps) GetContentWindowSize() int { return s.contentWindowSize } diff --git a/pkg/observability/log/level.go b/pkg/observability/log/level.go new file mode 100644 index 000000000..7e8109df2 --- /dev/null +++ b/pkg/observability/log/level.go @@ -0,0 +1,28 @@ +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 +// +// This is useful when trying to get the string values for Level and mapping level in other external libraries. For example: +// ``` +// trace.SetLogLevel(kvp.String("loglevel", log.DebugLevel.String()) +// ``` +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..34d824e89 --- /dev/null +++ b/pkg/observability/log/slog_adapter_test.go @@ -0,0 +1,283 @@ +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}, // slog has no fatal, use error + } + + 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) { + // Unknown level should default to info + unknownLevel := Level{"unknown"} + result := convertLevel(unknownLevel) + assert.Equal(t, slog.LevelInfo, result) +} + +func TestSlogLogger_ImplementsInterface(_ *testing.T) { + // Compile-time check that SlogLogger implements Logger + 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")) + + // Level should be preserved + assert.Equal(t, WarnLevel, withFields.Level()) + + // Should log with the added field + 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) + + // WithError with nil should return the same logger (no error field added) + require.NotPanics(t, func() { + result := logger.WithError(nil) + assert.NotNil(t, result) + // Should return the same logger instance when error is nil + 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)) +} From 90f11722060f7dc24c22e4251a717dbe7279d0f5 Mon Sep 17 00:00:00 2001 From: Adam Holt Date: Wed, 7 Jan 2026 13:51:11 +0100 Subject: [PATCH 2/2] Update stub implementation --- pkg/github/server_test.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/pkg/github/server_test.go b/pkg/github/server_test.go index 5eef9c3cc..9feb28fbe 100644 --- a/pkg/github/server_test.go +++ b/pkg/github/server_test.go @@ -11,6 +11,7 @@ import ( "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" "github.com/google/go-github/v79/github" @@ -54,10 +55,10 @@ func (s stubDeps) GetRawClient(ctx context.Context) (*raw.Client, error) { } func (s stubDeps) GetRepoAccessCache() *lockdown.RepoAccessCache { return s.repoAccessCache } -func (s stubDeps) GetObsv() *observability.Exporters { return &s.obsv } func (s stubDeps) GetT() translations.TranslationHelperFunc { return s.t } func (s stubDeps) GetFlags() FeatureFlags { return s.flags } func (s stubDeps) GetContentWindowSize() int { return s.contentWindowSize } +func (s stubDeps) Logger(ctx context.Context) mcpLog.Logger { return s.obsv.Logger(ctx) } // Helper functions to create stub client functions for error testing func stubClientFnFromHTTP(httpClient *http.Client) func(context.Context) (*github.Client, error) {