Skip to content

Commit a8bf0b0

Browse files
committed
initial logging stack for http
1 parent 1b829dc commit a8bf0b0

File tree

14 files changed

+598
-1
lines changed

14 files changed

+598
-1
lines changed

internal/ghmcp/server.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -128,6 +128,7 @@ func NewStdioMCPServer(ctx context.Context, cfg github.MCPServerConfig) (*mcp.Se
128128
},
129129
cfg.ContentWindowSize,
130130
featureChecker,
131+
cfg.Logger,
131132
)
132133
// Build and register the tool/resource/prompt inventory
133134
inventoryBuilder := github.NewInventory(cfg.Translator).

pkg/github/dependencies.go

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,13 +4,16 @@ import (
44
"context"
55
"errors"
66
"fmt"
7+
"log/slog"
78
"net/http"
89
"os"
910

1011
ghcontext "github.com/github/github-mcp-server/pkg/context"
1112
"github.com/github/github-mcp-server/pkg/http/transport"
1213
"github.com/github/github-mcp-server/pkg/inventory"
1314
"github.com/github/github-mcp-server/pkg/lockdown"
15+
"github.com/github/github-mcp-server/pkg/observability"
16+
obsvLog "github.com/github/github-mcp-server/pkg/observability/log"
1417
"github.com/github/github-mcp-server/pkg/raw"
1518
"github.com/github/github-mcp-server/pkg/scopes"
1619
"github.com/github/github-mcp-server/pkg/translations"
@@ -94,6 +97,9 @@ type ToolDependencies interface {
9497

9598
// IsFeatureEnabled checks if a feature flag is enabled.
9699
IsFeatureEnabled(ctx context.Context, flagName string) bool
100+
101+
// Logger returns the logger
102+
Logger(ctx context.Context) obsvLog.Logger
97103
}
98104

99105
// BaseDeps is the standard implementation of ToolDependencies for the local server.
@@ -113,6 +119,9 @@ type BaseDeps struct {
113119

114120
// Feature flag checker for runtime checks
115121
featureChecker inventory.FeatureFlagChecker
122+
123+
// Observability exporters (includes logger)
124+
Obsv observability.Exporters
116125
}
117126

118127
// Compile-time assertion to verify that BaseDeps implements the ToolDependencies interface.
@@ -128,7 +137,14 @@ func NewBaseDeps(
128137
flags FeatureFlags,
129138
contentWindowSize int,
130139
featureChecker inventory.FeatureFlagChecker,
140+
logger *slog.Logger,
131141
) *BaseDeps {
142+
var obsv observability.Exporters
143+
if logger != nil {
144+
obsv = observability.NewExporters(obsvLog.NewSlogLogger(logger, obsvLog.InfoLevel))
145+
} else {
146+
obsv = observability.NewExporters(obsvLog.NewNoopLogger())
147+
}
132148
return &BaseDeps{
133149
Client: client,
134150
GQLClient: gqlClient,
@@ -138,6 +154,7 @@ func NewBaseDeps(
138154
Flags: flags,
139155
ContentWindowSize: contentWindowSize,
140156
featureChecker: featureChecker,
157+
Obsv: obsv,
141158
}
142159
}
143160

@@ -170,6 +187,11 @@ func (d BaseDeps) GetFlags(_ context.Context) FeatureFlags { return d.Flags }
170187
// GetContentWindowSize implements ToolDependencies.
171188
func (d BaseDeps) GetContentWindowSize() int { return d.ContentWindowSize }
172189

190+
// Logger implements ToolDependencies.
191+
func (d BaseDeps) Logger(ctx context.Context) obsvLog.Logger {
192+
return d.Obsv.Logger(ctx)
193+
}
194+
173195
// IsFeatureEnabled checks if a feature flag is enabled.
174196
// Returns false if the feature checker is nil, flag name is empty, or an error occurs.
175197
// This allows tools to conditionally change behavior based on feature flags.
@@ -247,6 +269,9 @@ type RequestDeps struct {
247269

248270
// Feature flag checker for runtime checks
249271
featureChecker inventory.FeatureFlagChecker
272+
273+
// Observability exporters (includes logger)
274+
obsv observability.Exporters
250275
}
251276

252277
// NewRequestDeps creates a RequestDeps with the provided clients and configuration.
@@ -258,7 +283,14 @@ func NewRequestDeps(
258283
t translations.TranslationHelperFunc,
259284
contentWindowSize int,
260285
featureChecker inventory.FeatureFlagChecker,
286+
logger *slog.Logger,
261287
) *RequestDeps {
288+
var obsv observability.Exporters
289+
if logger != nil {
290+
obsv = observability.NewExporters(obsvLog.NewSlogLogger(logger, obsvLog.InfoLevel))
291+
} else {
292+
obsv = observability.NewExporters(obsvLog.NewNoopLogger())
293+
}
262294
return &RequestDeps{
263295
apiHosts: apiHosts,
264296
version: version,
@@ -267,6 +299,7 @@ func NewRequestDeps(
267299
T: t,
268300
ContentWindowSize: contentWindowSize,
269301
featureChecker: featureChecker,
302+
obsv: obsv,
270303
}
271304
}
272305

@@ -374,6 +407,11 @@ func (d *RequestDeps) GetFlags(ctx context.Context) FeatureFlags {
374407
// GetContentWindowSize implements ToolDependencies.
375408
func (d *RequestDeps) GetContentWindowSize() int { return d.ContentWindowSize }
376409

410+
// Logger implements ToolDependencies.
411+
func (d *RequestDeps) Logger(ctx context.Context) obsvLog.Logger {
412+
return d.obsv.Logger(ctx)
413+
}
414+
377415
// IsFeatureEnabled checks if a feature flag is enabled.
378416
func (d *RequestDeps) IsFeatureEnabled(ctx context.Context, flagName string) bool {
379417
if d.featureChecker == nil || flagName == "" {

pkg/github/dependencies_test.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ func TestIsFeatureEnabled_WithEnabledFlag(t *testing.T) {
2828
github.FeatureFlags{},
2929
0, // contentWindowSize
3030
checker, // featureChecker
31+
nil, // logger
3132
)
3233

3334
// Test enabled flag
@@ -52,6 +53,7 @@ func TestIsFeatureEnabled_WithoutChecker(t *testing.T) {
5253
github.FeatureFlags{},
5354
0, // contentWindowSize
5455
nil, // featureChecker (nil)
56+
nil, // logger
5557
)
5658

5759
// Should return false when checker is nil
@@ -76,6 +78,7 @@ func TestIsFeatureEnabled_EmptyFlagName(t *testing.T) {
7678
github.FeatureFlags{},
7779
0, // contentWindowSize
7880
checker, // featureChecker
81+
nil, // logger
7982
)
8083

8184
// Should return false for empty flag name
@@ -100,6 +103,7 @@ func TestIsFeatureEnabled_CheckerError(t *testing.T) {
100103
github.FeatureFlags{},
101104
0, // contentWindowSize
102105
checker, // featureChecker
106+
nil, // logger
103107
)
104108

105109
// Should return false and log error (not crash)

pkg/github/dynamic_tools_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -136,7 +136,7 @@ func TestDynamicTools_EnableToolset(t *testing.T) {
136136
deps := DynamicToolDependencies{
137137
Server: server,
138138
Inventory: reg,
139-
ToolDeps: NewBaseDeps(nil, nil, nil, nil, translations.NullTranslationHelper, FeatureFlags{}, 0, nil),
139+
ToolDeps: NewBaseDeps(nil, nil, nil, nil, translations.NullTranslationHelper, FeatureFlags{}, 0, nil, nil),
140140
T: translations.NullTranslationHelper,
141141
}
142142

pkg/github/feature_flags_test.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -104,6 +104,7 @@ func TestHelloWorld_ConditionalBehavior_Featureflag(t *testing.T) {
104104
FeatureFlags{},
105105
0,
106106
checker,
107+
nil,
107108
)
108109

109110
// Get the tool and its handler
@@ -166,6 +167,7 @@ func TestHelloWorld_ConditionalBehavior_Config(t *testing.T) {
166167
FeatureFlags{InsidersMode: tt.insidersMode},
167168
0,
168169
nil,
170+
nil,
169171
)
170172

171173
// Get the tool and its handler

pkg/github/server_test.go

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,8 @@ import (
1010
"time"
1111

1212
"github.com/github/github-mcp-server/pkg/lockdown"
13+
"github.com/github/github-mcp-server/pkg/observability"
14+
mcpLog "github.com/github/github-mcp-server/pkg/observability/log"
1315
"github.com/github/github-mcp-server/pkg/raw"
1416
"github.com/github/github-mcp-server/pkg/translations"
1517
gogithub "github.com/google/go-github/v82/github"
@@ -29,6 +31,7 @@ type stubDeps struct {
2931
t translations.TranslationHelperFunc
3032
flags FeatureFlags
3133
contentWindowSize int
34+
obsv observability.Exporters
3235
}
3336

3437
func (s stubDeps) GetClient(ctx context.Context) (*gogithub.Client, error) {
@@ -59,6 +62,12 @@ func (s stubDeps) GetT() translations.TranslationHelperFunc { return s.
5962
func (s stubDeps) GetFlags(_ context.Context) FeatureFlags { return s.flags }
6063
func (s stubDeps) GetContentWindowSize() int { return s.contentWindowSize }
6164
func (s stubDeps) IsFeatureEnabled(_ context.Context, _ string) bool { return false }
65+
func (s stubDeps) Logger(_ context.Context) mcpLog.Logger {
66+
if s.obsv != nil {
67+
return s.obsv.Logger(context.Background())
68+
}
69+
return mcpLog.NewNoopLogger()
70+
}
6271

6372
// Helper functions to create stub client functions for error testing
6473
func stubClientFnFromHTTP(httpClient *http.Client) func(context.Context) (*gogithub.Client, error) {

pkg/http/server.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -114,6 +114,7 @@ func RunHTTPServer(cfg ServerConfig) error {
114114
t,
115115
cfg.ContentWindowSize,
116116
featureChecker,
117+
logger,
117118
)
118119

119120
// Initialize the global tool scope map

pkg/observability/log/level.go

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
package log
2+
3+
type Level struct {
4+
level string
5+
}
6+
7+
var (
8+
// DebugLevel causes all logs to be logged
9+
DebugLevel = Level{"debug"}
10+
// InfoLevel causes all logs of level info or more severe to be logged
11+
InfoLevel = Level{"info"}
12+
// WarnLevel causes all logs of level warn or more severe to be logged
13+
WarnLevel = Level{"warn"}
14+
// ErrorLevel causes all logs of level error or more severe to be logged
15+
ErrorLevel = Level{"error"}
16+
// FatalLevel causes only logs of level fatal to be logged
17+
FatalLevel = Level{"fatal"}
18+
)
19+
20+
// String returns the string representation for Level
21+
func (l Level) String() string {
22+
return l.level
23+
}

pkg/observability/log/logger.go

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
package log
2+
3+
import (
4+
"context"
5+
"log/slog"
6+
)
7+
8+
type Logger interface {
9+
Log(ctx context.Context, level Level, msg string, fields ...slog.Attr)
10+
Debug(msg string, fields ...slog.Attr)
11+
Info(msg string, fields ...slog.Attr)
12+
Warn(msg string, fields ...slog.Attr)
13+
Error(msg string, fields ...slog.Attr)
14+
Fatal(msg string, fields ...slog.Attr)
15+
WithFields(fields ...slog.Attr) Logger
16+
WithError(err error) Logger
17+
Named(name string) Logger
18+
WithLevel(level Level) Logger
19+
Sync() error
20+
Level() Level
21+
}
Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,62 @@
1+
package log
2+
3+
import (
4+
"context"
5+
"log/slog"
6+
)
7+
8+
type NoopLogger struct{}
9+
10+
var _ Logger = (*NoopLogger)(nil)
11+
12+
func NewNoopLogger() *NoopLogger {
13+
return &NoopLogger{}
14+
}
15+
16+
func (l *NoopLogger) Level() Level {
17+
return DebugLevel
18+
}
19+
20+
func (l *NoopLogger) Log(_ context.Context, _ Level, _ string, _ ...slog.Attr) {
21+
// No-op
22+
}
23+
24+
func (l *NoopLogger) Debug(_ string, _ ...slog.Attr) {
25+
// No-op
26+
}
27+
28+
func (l *NoopLogger) Info(_ string, _ ...slog.Attr) {
29+
// No-op
30+
}
31+
32+
func (l *NoopLogger) Warn(_ string, _ ...slog.Attr) {
33+
// No-op
34+
}
35+
36+
func (l *NoopLogger) Error(_ string, _ ...slog.Attr) {
37+
// No-op
38+
}
39+
40+
func (l *NoopLogger) Fatal(_ string, _ ...slog.Attr) {
41+
// No-op
42+
}
43+
44+
func (l *NoopLogger) WithFields(_ ...slog.Attr) Logger {
45+
return l
46+
}
47+
48+
func (l *NoopLogger) WithError(_ error) Logger {
49+
return l
50+
}
51+
52+
func (l *NoopLogger) Named(_ string) Logger {
53+
return l
54+
}
55+
56+
func (l *NoopLogger) WithLevel(_ Level) Logger {
57+
return l
58+
}
59+
60+
func (l *NoopLogger) Sync() error {
61+
return nil
62+
}

0 commit comments

Comments
 (0)