Skip to content
Merged
Show file tree
Hide file tree
Changes from 11 commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
a8bf0b0
initial logging stack for http
mattdholloway Feb 12, 2026
4a2f8ed
Merge branch 'main' of https://github.com/github/github-mcp-server in…
mattdholloway Feb 12, 2026
ba9d315
Merge branch 'main' of https://github.com/github/github-mcp-server in…
mattdholloway Feb 17, 2026
cbf99e7
add metrics adapter
mattdholloway Feb 18, 2026
cc5b8e7
Merge branch 'main' into add-logging-stack-v2
mattdholloway Feb 18, 2026
77f6e57
fix linter issues
mattdholloway Feb 18, 2026
3bbfaa0
make log fields generic
mattdholloway Feb 18, 2026
dd57d84
Merge branch 'main' into add-logging-stack-v2
mattdholloway Feb 25, 2026
2a20a2a
Update pkg/github/server_test.go
mattdholloway Feb 25, 2026
1648775
Merge branch 'main' into add-logging-stack-v2
mattdholloway Feb 25, 2026
7c5ea0a
Remove unused SlogMetrics adapter
mattdholloway Feb 25, 2026
121daa3
Update pkg/github/dependencies.go
mattdholloway Feb 26, 2026
39445b0
fmt
mattdholloway Feb 26, 2026
f7d3a75
Merge branch 'main' into add-logging-stack-v2
mattdholloway Feb 27, 2026
30b2952
Merge branch 'main' into add-logging-stack-v2
mattdholloway Mar 17, 2026
c0a68f5
change to use slog
mattdholloway Mar 18, 2026
457f64d
address feedback
mattdholloway Mar 26, 2026
f94b729
Merge branch 'main' into add-logging-stack-v2
mattdholloway Mar 26, 2026
e31ea3b
rename noop adapter to noop sink
mattdholloway Mar 26, 2026
095d9f2
Update pkg/http/server.go
mattdholloway Mar 26, 2026
1ae4a03
[WIP] [WIP] Address feedback on OSS logging adapter for http implemen…
Copilot Mar 26, 2026
2b18639
replace nil with stubs
mattdholloway Mar 31, 2026
a7778ca
Merge branch 'main' into add-logging-stack-v2
mattdholloway Mar 31, 2026
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
1 change: 1 addition & 0 deletions internal/ghmcp/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -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).
Expand Down
52 changes: 52 additions & 0 deletions pkg/github/dependencies.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,17 @@ import (
"context"
"errors"
"fmt"
"log/slog"
"net/http"
"os"

ghcontext "github.com/github/github-mcp-server/pkg/context"
"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"
obsvMetrics "github.com/github/github-mcp-server/pkg/observability/metrics"
"github.com/github/github-mcp-server/pkg/raw"
"github.com/github/github-mcp-server/pkg/scopes"
"github.com/github/github-mcp-server/pkg/translations"
Expand Down Expand Up @@ -94,6 +98,12 @@ 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

// Metrics returns the metrics client
Metrics(ctx context.Context) obsvMetrics.Metrics
}

// BaseDeps is the standard implementation of ToolDependencies for the local server.
Expand All @@ -113,6 +123,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.
Expand All @@ -128,7 +141,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), obsvMetrics.NewNoopMetrics())
} else {
obsv = observability.NewExporters(obsvLog.NewNoopLogger(), obsvMetrics.NewNoopMetrics())
Copy link

Copilot AI Feb 18, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The default observability/exporters initialization logic is duplicated in NewBaseDeps and NewRequestDeps. Consider extracting a small helper (e.g., newDefaultExporters(logger *slog.Logger) observability.Exporters) to keep behavior consistent and avoid future drift.

Copilot uses AI. Check for mistakes.
}
return &BaseDeps{
Client: client,
GQLClient: gqlClient,
Expand All @@ -138,6 +158,7 @@ func NewBaseDeps(
Flags: flags,
ContentWindowSize: contentWindowSize,
featureChecker: featureChecker,
Obsv: obsv,
}
}

Expand Down Expand Up @@ -170,6 +191,16 @@ 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)
}

// Metrics implements ToolDependencies.
func (d BaseDeps) Metrics(ctx context.Context) obsvMetrics.Metrics {
return d.Obsv.Metrics(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.
Expand Down Expand Up @@ -247,6 +278,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.
Expand All @@ -258,7 +292,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), obsvMetrics.NewNoopMetrics())
} else {
obsv = observability.NewExporters(obsvLog.NewNoopLogger(), obsvMetrics.NewNoopMetrics())
}
return &RequestDeps{
apiHosts: apiHosts,
version: version,
Expand All @@ -267,6 +308,7 @@ func NewRequestDeps(
T: t,
ContentWindowSize: contentWindowSize,
featureChecker: featureChecker,
obsv: obsv,
}
}

Expand Down Expand Up @@ -374,6 +416,16 @@ 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)
}

// Metrics implements ToolDependencies.
func (d *RequestDeps) Metrics(ctx context.Context) obsvMetrics.Metrics {
return d.obsv.Metrics(ctx)
}

// IsFeatureEnabled checks if a feature flag is enabled.
func (d *RequestDeps) IsFeatureEnabled(ctx context.Context, flagName string) bool {
if d.featureChecker == nil || flagName == "" {
Expand Down
4 changes: 4 additions & 0 deletions pkg/github/dependencies_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ func TestIsFeatureEnabled_WithEnabledFlag(t *testing.T) {
github.FeatureFlags{},
0, // contentWindowSize
checker, // featureChecker
nil, // logger
)

// Test enabled flag
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion pkg/github/dynamic_tools_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
}

Expand Down
2 changes: 2 additions & 0 deletions pkg/github/feature_flags_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,7 @@ func TestHelloWorld_ConditionalBehavior_Featureflag(t *testing.T) {
FeatureFlags{},
0,
checker,
nil,
)

// Get the tool and its handler
Expand Down Expand Up @@ -166,6 +167,7 @@ func TestHelloWorld_ConditionalBehavior_Config(t *testing.T) {
FeatureFlags{InsidersMode: tt.insidersMode},
0,
nil,
nil,
)

// Get the tool and its handler
Expand Down
16 changes: 16 additions & 0 deletions pkg/github/server_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,9 @@ 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"
mcpMetrics "github.com/github/github-mcp-server/pkg/observability/metrics"
"github.com/github/github-mcp-server/pkg/raw"
"github.com/github/github-mcp-server/pkg/translations"
gogithub "github.com/google/go-github/v82/github"
Expand All @@ -29,6 +32,7 @@ type stubDeps struct {
t translations.TranslationHelperFunc
flags FeatureFlags
contentWindowSize int
obsv observability.Exporters
}

func (s stubDeps) GetClient(ctx context.Context) (*gogithub.Client, error) {
Expand Down Expand Up @@ -59,6 +63,18 @@ 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(ctx context.Context) mcpLog.Logger {
if s.obsv != nil {
return s.obsv.Logger(ctx)
}
return mcpLog.NewNoopLogger()
}
func (s stubDeps) Metrics(ctx context.Context) mcpMetrics.Metrics {
if s.obsv != nil {
return s.obsv.Metrics(ctx)
}
return mcpMetrics.NewNoopMetrics()
}

// Helper functions to create stub client functions for error testing
func stubClientFnFromHTTP(httpClient *http.Client) func(context.Context) (*gogithub.Client, error) {
Expand Down
1 change: 1 addition & 0 deletions pkg/http/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,7 @@ func RunHTTPServer(cfg ServerConfig) error {
t,
cfg.ContentWindowSize,
featureChecker,
logger,
)

// Initialize the global tool scope map
Expand Down
29 changes: 29 additions & 0 deletions pkg/observability/log/field.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
package log

import (
"fmt"
"time"
)

// Field is a key-value pair for structured logging.
// This type is backend-agnostic — adapters convert it to their native field type.
type Field struct {
Key string
Value any
}

// Convenience constructors for common field types.

func String(key, value string) Field { return Field{Key: key, Value: value} }
func Int(key string, value int) Field { return Field{Key: key, Value: value} }
func Int64(key string, value int64) Field { return Field{Key: key, Value: value} }
func Float64(key string, value float64) Field { return Field{Key: key, Value: value} }
func Bool(key string, value bool) Field { return Field{Key: key, Value: value} }
func Err(err error) Field { return Field{Key: "error", Value: err} }
func Duration(key string, value time.Duration) Field { return Field{Key: key, Value: value} }
func Any(key string, value any) Field { return Field{Key: key, Value: value} }

// Stringer returns the string representation of a Field.
func (f Field) String() string {
return fmt.Sprintf("%s=%v", f.Key, f.Value)
}
23 changes: 23 additions & 0 deletions pkg/observability/log/level.go
Original file line number Diff line number Diff line change
@@ -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
}
20 changes: 20 additions & 0 deletions pkg/observability/log/logger.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
package log

import (
"context"
)

type Logger interface {
Log(ctx context.Context, level Level, msg string, fields ...Field)
Debug(msg string, fields ...Field)
Info(msg string, fields ...Field)
Warn(msg string, fields ...Field)
Error(msg string, fields ...Field)
Fatal(msg string, fields ...Field)
WithFields(fields ...Field) Logger
WithError(err error) Logger
Named(name string) Logger
WithLevel(level Level) Logger
Sync() error
Level() Level
}
61 changes: 61 additions & 0 deletions pkg/observability/log/noop_adapter.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
package log

import (
"context"
)

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, _ ...Field) {
// No-op
}

func (l *NoopLogger) Debug(_ string, _ ...Field) {
// No-op
}

func (l *NoopLogger) Info(_ string, _ ...Field) {
// No-op
}

func (l *NoopLogger) Warn(_ string, _ ...Field) {
// No-op
}

func (l *NoopLogger) Error(_ string, _ ...Field) {
// No-op
}

func (l *NoopLogger) Fatal(_ string, _ ...Field) {
// No-op
}

func (l *NoopLogger) WithFields(_ ...Field) 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
}
Loading