Skip to content

Commit ac128e2

Browse files
committed
Add logging stack interfaces and adapters
1 parent 80b0306 commit ac128e2

File tree

12 files changed

+586
-1
lines changed

12 files changed

+586
-1
lines changed

internal/ghmcp/server.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -207,6 +207,7 @@ func NewMCPServer(cfg MCPServerConfig) (*mcp.Server, error) {
207207
cfg.Translator,
208208
github.FeatureFlags{LockdownMode: cfg.LockdownMode},
209209
cfg.ContentWindowSize,
210+
cfg.Logger,
210211
)
211212

212213
// Inject dependencies into context for all tool handlers

pkg/github/dependencies.go

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,9 +3,12 @@ package github
33
import (
44
"context"
55
"errors"
6+
"log/slog"
67

78
"github.com/github/github-mcp-server/pkg/inventory"
89
"github.com/github/github-mcp-server/pkg/lockdown"
10+
"github.com/github/github-mcp-server/pkg/observability"
11+
obsvLog "github.com/github/github-mcp-server/pkg/observability/log"
912
"github.com/github/github-mcp-server/pkg/raw"
1013
"github.com/github/github-mcp-server/pkg/scopes"
1114
"github.com/github/github-mcp-server/pkg/translations"
@@ -77,6 +80,9 @@ type ToolDependencies interface {
7780

7881
// GetContentWindowSize returns the content window size for log truncation
7982
GetContentWindowSize() int
83+
84+
// GetLogger returns the logger
85+
Logger(ctx context.Context) obsvLog.Logger
8086
}
8187

8288
// BaseDeps is the standard implementation of ToolDependencies for the local server.
@@ -93,6 +99,7 @@ type BaseDeps struct {
9399
T translations.TranslationHelperFunc
94100
Flags FeatureFlags
95101
ContentWindowSize int
102+
Obsv observability.Exporters
96103
}
97104

98105
// NewBaseDeps creates a BaseDeps with the provided clients and configuration.
@@ -104,6 +111,7 @@ func NewBaseDeps(
104111
t translations.TranslationHelperFunc,
105112
flags FeatureFlags,
106113
contentWindowSize int,
114+
logger *slog.Logger,
107115
) *BaseDeps {
108116
return &BaseDeps{
109117
Client: client,
@@ -113,6 +121,7 @@ func NewBaseDeps(
113121
T: t,
114122
Flags: flags,
115123
ContentWindowSize: contentWindowSize,
124+
Obsv: observability.NewExporters(obsvLog.NewSlogLogger(logger, obsvLog.InfoLevel)),
116125
}
117126
}
118127

@@ -134,6 +143,11 @@ func (d BaseDeps) GetRawClient(_ context.Context) (*raw.Client, error) {
134143
// GetRepoAccessCache implements ToolDependencies.
135144
func (d BaseDeps) GetRepoAccessCache() *lockdown.RepoAccessCache { return d.RepoAccessCache }
136145

146+
// GetLogger implements ToolDependencies.
147+
func (d BaseDeps) Logger(ctx context.Context) obsvLog.Logger {
148+
return d.Obsv.Logger(ctx)
149+
}
150+
137151
// GetT implements ToolDependencies.
138152
func (d BaseDeps) GetT() translations.TranslationHelperFunc { return d.T }
139153

pkg/github/dynamic_tools_test.go

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,10 @@
11
package github
22

33
import (
4+
"bytes"
45
"context"
56
"encoding/json"
7+
"log/slog"
68
"testing"
79

810
"github.com/github/github-mcp-server/pkg/inventory"
@@ -128,12 +130,14 @@ func TestDynamicTools_EnableToolset(t *testing.T) {
128130

129131
// Create a mock server
130132
server := mcp.NewServer(&mcp.Implementation{Name: "test"}, nil)
133+
var logBuffer bytes.Buffer
134+
logger := slog.New(slog.NewTextHandler(&logBuffer, nil))
131135

132136
// Create dynamic tool dependencies
133137
deps := DynamicToolDependencies{
134138
Server: server,
135139
Inventory: reg,
136-
ToolDeps: NewBaseDeps(nil, nil, nil, nil, translations.NullTranslationHelper, FeatureFlags{}, 0),
140+
ToolDeps: NewBaseDeps(nil, nil, nil, nil, translations.NullTranslationHelper, FeatureFlags{}, 0, logger),
137141
T: translations.NullTranslationHelper,
138142
}
139143

pkg/github/issues.go

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import (
55
"encoding/json"
66
"fmt"
77
"io"
8+
"log/slog"
89
"net/http"
910
"strings"
1011
"time"
@@ -1459,12 +1460,21 @@ func ListIssues(t translations.TranslationHelperFunc) inventory.ServerTool {
14591460
return utils.NewToolResultError(err.Error()), nil, nil
14601461
}
14611462

1463+
deps.Logger(ctx).Info("list_issues called",
1464+
slog.String("owner", owner),
1465+
slog.String("repo", repo),
1466+
slog.String("orderBy", orderBy),
1467+
slog.String("direction", direction),
1468+
slog.String("since", since),
1469+
)
1470+
14621471
// There are two optional parameters: since and labels.
14631472
var sinceTime time.Time
14641473
var hasSince bool
14651474
if since != "" {
14661475
sinceTime, err = parseISOTimestamp(since)
14671476
if err != nil {
1477+
deps.Logger(ctx).Info("failed to parse 'since' timestamp", slog.String("error", err.Error()))
14681478
return utils.NewToolResultError(fmt.Sprintf("failed to list issues: %s", err.Error())), nil, nil
14691479
}
14701480
hasSince = true

pkg/github/server_test.go

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

1212
"github.com/github/github-mcp-server/pkg/lockdown"
13+
"github.com/github/github-mcp-server/pkg/observability"
1314
"github.com/github/github-mcp-server/pkg/raw"
1415
"github.com/github/github-mcp-server/pkg/translations"
1516
"github.com/google/go-github/v79/github"
@@ -28,6 +29,7 @@ type stubDeps struct {
2829
t translations.TranslationHelperFunc
2930
flags FeatureFlags
3031
contentWindowSize int
32+
obsv observability.Exporters
3133
}
3234

3335
func (s stubDeps) GetClient(ctx context.Context) (*github.Client, error) {
@@ -52,6 +54,7 @@ func (s stubDeps) GetRawClient(ctx context.Context) (*raw.Client, error) {
5254
}
5355

5456
func (s stubDeps) GetRepoAccessCache() *lockdown.RepoAccessCache { return s.repoAccessCache }
57+
func (s stubDeps) GetObsv() *observability.Exporters { return &s.obsv }
5558
func (s stubDeps) GetT() translations.TranslationHelperFunc { return s.t }
5659
func (s stubDeps) GetFlags() FeatureFlags { return s.flags }
5760
func (s stubDeps) GetContentWindowSize() int { return s.contentWindowSize }

pkg/observability/log/level.go

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
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+
//
22+
// This is useful when trying to get the string values for Level and mapping level in other external libraries. For example:
23+
// ```
24+
// trace.SetLogLevel(kvp.String("loglevel", log.DebugLevel.String())
25+
// ```
26+
func (l Level) String() string {
27+
return l.level
28+
}

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+
}
Lines changed: 108 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,108 @@
1+
package log
2+
3+
import (
4+
"context"
5+
"log/slog"
6+
)
7+
8+
type SlogLogger struct {
9+
logger *slog.Logger
10+
level Level
11+
}
12+
13+
func NewSlogLogger(logger *slog.Logger, level Level) *SlogLogger {
14+
return &SlogLogger{
15+
logger: logger,
16+
level: level,
17+
}
18+
}
19+
20+
func (l *SlogLogger) Level() Level {
21+
return l.level
22+
}
23+
24+
func (l *SlogLogger) Log(ctx context.Context, level Level, msg string, fields ...slog.Attr) {
25+
slogLevel := convertLevel(level)
26+
l.logger.LogAttrs(ctx, slogLevel, msg, fields...)
27+
}
28+
29+
func (l *SlogLogger) Debug(msg string, fields ...slog.Attr) {
30+
l.Log(context.Background(), DebugLevel, msg, fields...)
31+
}
32+
33+
func (l *SlogLogger) Info(msg string, fields ...slog.Attr) {
34+
l.Log(context.Background(), InfoLevel, msg, fields...)
35+
}
36+
37+
func (l *SlogLogger) Warn(msg string, fields ...slog.Attr) {
38+
l.Log(context.Background(), WarnLevel, msg, fields...)
39+
}
40+
41+
func (l *SlogLogger) Error(msg string, fields ...slog.Attr) {
42+
l.Log(context.Background(), ErrorLevel, msg, fields...)
43+
}
44+
45+
func (l *SlogLogger) Fatal(msg string, fields ...slog.Attr) {
46+
l.Log(context.Background(), FatalLevel, msg, fields...)
47+
// Sync to ensure the log message is flushed before panic
48+
_ = l.Sync()
49+
panic("fatal: " + msg)
50+
}
51+
52+
func (l *SlogLogger) WithFields(fields ...slog.Attr) Logger {
53+
fieldKvPairs := make([]any, 0, len(fields)*2)
54+
for _, attr := range fields {
55+
k, v := attr.Key, attr.Value
56+
fieldKvPairs = append(fieldKvPairs, k, v.Any())
57+
}
58+
return &SlogLogger{
59+
logger: l.logger.With(fieldKvPairs...),
60+
level: l.level,
61+
}
62+
}
63+
64+
func (l *SlogLogger) WithError(err error) Logger {
65+
if err == nil {
66+
return l
67+
}
68+
return &SlogLogger{
69+
logger: l.logger.With("error", err.Error()),
70+
level: l.level,
71+
}
72+
}
73+
74+
func (l *SlogLogger) Named(name string) Logger {
75+
return &SlogLogger{
76+
logger: l.logger.With("logger", name),
77+
level: l.level,
78+
}
79+
}
80+
81+
func (l *SlogLogger) WithLevel(level Level) Logger {
82+
return &SlogLogger{
83+
logger: l.logger,
84+
level: level,
85+
}
86+
}
87+
88+
func (l *SlogLogger) Sync() error {
89+
// Slog does not require syncing
90+
return nil
91+
}
92+
93+
func convertLevel(level Level) slog.Level {
94+
switch level {
95+
case DebugLevel:
96+
return slog.LevelDebug
97+
case InfoLevel:
98+
return slog.LevelInfo
99+
case WarnLevel:
100+
return slog.LevelWarn
101+
case ErrorLevel:
102+
return slog.LevelError
103+
case FatalLevel:
104+
return slog.LevelError
105+
default:
106+
return slog.LevelInfo
107+
}
108+
}

0 commit comments

Comments
 (0)