Skip to content

Commit cbf99e7

Browse files
mattdhollowaymattdholloway
committed
add metrics adapter
1 parent ba9d315 commit cbf99e7

File tree

9 files changed

+283
-9
lines changed

9 files changed

+283
-9
lines changed

‎pkg/github/dependencies.go‎

Lines changed: 18 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import (
1414
"github.com/github/github-mcp-server/pkg/lockdown"
1515
"github.com/github/github-mcp-server/pkg/observability"
1616
obsvLog "github.com/github/github-mcp-server/pkg/observability/log"
17+
obsvMetrics "github.com/github/github-mcp-server/pkg/observability/metrics"
1718
"github.com/github/github-mcp-server/pkg/raw"
1819
"github.com/github/github-mcp-server/pkg/scopes"
1920
"github.com/github/github-mcp-server/pkg/translations"
@@ -100,6 +101,9 @@ type ToolDependencies interface {
100101

101102
// Logger returns the logger
102103
Logger(ctx context.Context) obsvLog.Logger
104+
105+
// Metrics returns the metrics client
106+
Metrics(ctx context.Context) obsvMetrics.Metrics
103107
}
104108

105109
// BaseDeps is the standard implementation of ToolDependencies for the local server.
@@ -141,9 +145,9 @@ func NewBaseDeps(
141145
) *BaseDeps {
142146
var obsv observability.Exporters
143147
if logger != nil {
144-
obsv = observability.NewExporters(obsvLog.NewSlogLogger(logger, obsvLog.InfoLevel))
148+
obsv = observability.NewExporters(obsvLog.NewSlogLogger(logger, obsvLog.InfoLevel), obsvMetrics.NewNoopMetrics())
145149
} else {
146-
obsv = observability.NewExporters(obsvLog.NewNoopLogger())
150+
obsv = observability.NewExporters(obsvLog.NewNoopLogger(), obsvMetrics.NewNoopMetrics())
147151
}
148152
return &BaseDeps{
149153
Client: client,
@@ -192,6 +196,11 @@ func (d BaseDeps) Logger(ctx context.Context) obsvLog.Logger {
192196
return d.Obsv.Logger(ctx)
193197
}
194198

199+
// Metrics implements ToolDependencies.
200+
func (d BaseDeps) Metrics(ctx context.Context) obsvMetrics.Metrics {
201+
return d.Obsv.Metrics(ctx)
202+
}
203+
195204
// IsFeatureEnabled checks if a feature flag is enabled.
196205
// Returns false if the feature checker is nil, flag name is empty, or an error occurs.
197206
// This allows tools to conditionally change behavior based on feature flags.
@@ -287,9 +296,9 @@ func NewRequestDeps(
287296
) *RequestDeps {
288297
var obsv observability.Exporters
289298
if logger != nil {
290-
obsv = observability.NewExporters(obsvLog.NewSlogLogger(logger, obsvLog.InfoLevel))
299+
obsv = observability.NewExporters(obsvLog.NewSlogLogger(logger, obsvLog.InfoLevel), obsvMetrics.NewNoopMetrics())
291300
} else {
292-
obsv = observability.NewExporters(obsvLog.NewNoopLogger())
301+
obsv = observability.NewExporters(obsvLog.NewNoopLogger(), obsvMetrics.NewNoopMetrics())
293302
}
294303
return &RequestDeps{
295304
apiHosts: apiHosts,
@@ -412,6 +421,11 @@ func (d *RequestDeps) Logger(ctx context.Context) obsvLog.Logger {
412421
return d.obsv.Logger(ctx)
413422
}
414423

424+
// Metrics implements ToolDependencies.
425+
func (d *RequestDeps) Metrics(ctx context.Context) obsvMetrics.Metrics {
426+
return d.obsv.Metrics(ctx)
427+
}
428+
415429
// IsFeatureEnabled checks if a feature flag is enabled.
416430
func (d *RequestDeps) IsFeatureEnabled(ctx context.Context, flagName string) bool {
417431
if d.featureChecker == nil || flagName == "" {

‎pkg/github/server_test.go‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import (
1212
"github.com/github/github-mcp-server/pkg/lockdown"
1313
"github.com/github/github-mcp-server/pkg/observability"
1414
mcpLog "github.com/github/github-mcp-server/pkg/observability/log"
15+
mcpMetrics "github.com/github/github-mcp-server/pkg/observability/metrics"
1516
"github.com/github/github-mcp-server/pkg/raw"
1617
"github.com/github/github-mcp-server/pkg/translations"
1718
gogithub "github.com/google/go-github/v82/github"
@@ -68,6 +69,12 @@ func (s stubDeps) Logger(_ context.Context) mcpLog.Logger {
6869
}
6970
return mcpLog.NewNoopLogger()
7071
}
72+
func (s stubDeps) Metrics(_ context.Context) mcpMetrics.Metrics {
73+
if s.obsv != nil {
74+
return s.obsv.Metrics(context.Background())
75+
}
76+
return mcpMetrics.NewNoopMetrics()
77+
}
7178

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

‎pkg/observability/metrics/metrics.go‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
package metrics
2+
3+
import "time"
4+
5+
// Metrics is a backend-agnostic interface for emitting metrics.
6+
// Implementations can route to DataDog, log to slog, or discard (noop).
7+
type Metrics interface {
8+
Increment(key string, tags map[string]string)
9+
Counter(key string, tags map[string]string, value int64)
10+
Distribution(key string, tags map[string]string, value float64)
11+
DistributionMs(key string, tags map[string]string, value time.Duration)
12+
WithTags(tags map[string]string) Metrics
13+
}

‎pkg/observability/metrics/noop_adapter.go‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
package metrics
2+
3+
import "time"
4+
5+
// NoopMetrics is a no-op implementation of the Metrics interface.
6+
type NoopMetrics struct{}
7+
8+
var _ Metrics = (*NoopMetrics)(nil)
9+
10+
// NewNoopMetrics returns a new NoopMetrics.
11+
func NewNoopMetrics() *NoopMetrics {
12+
return &NoopMetrics{}
13+
}
14+
15+
func (n *NoopMetrics) Increment(_ string, _ map[string]string) {}
16+
func (n *NoopMetrics) Counter(_ string, _ map[string]string, _ int64) {}
17+
func (n *NoopMetrics) Distribution(_ string, _ map[string]string, _ float64) {}
18+
func (n *NoopMetrics) DistributionMs(_ string, _ map[string]string, _ time.Duration) {}
19+
func (n *NoopMetrics) WithTags(_ map[string]string) Metrics { return n }

‎pkg/observability/metrics/noop_adapter_test.go‎

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
package metrics
2+
3+
import (
4+
"testing"
5+
"time"
6+
7+
"github.com/stretchr/testify/assert"
8+
)
9+
10+
func TestNoopMetrics_ImplementsInterface(t *testing.T) {
11+
var _ Metrics = (*NoopMetrics)(nil)
12+
}
13+
14+
func TestNoopMetrics_NoPanics(t *testing.T) {
15+
m := NewNoopMetrics()
16+
17+
assert.NotPanics(t, func() {
18+
m.Increment("key", map[string]string{"a": "b"})
19+
m.Counter("key", map[string]string{"a": "b"}, 1)
20+
m.Distribution("key", map[string]string{"a": "b"}, 1.5)
21+
m.DistributionMs("key", map[string]string{"a": "b"}, time.Second)
22+
})
23+
}
24+
25+
func TestNoopMetrics_NilTags(t *testing.T) {
26+
m := NewNoopMetrics()
27+
28+
assert.NotPanics(t, func() {
29+
m.Increment("key", nil)
30+
m.Counter("key", nil, 1)
31+
m.Distribution("key", nil, 1.5)
32+
m.DistributionMs("key", nil, time.Second)
33+
})
34+
}
35+
36+
func TestNoopMetrics_WithTags(t *testing.T) {
37+
m := NewNoopMetrics()
38+
tagged := m.WithTags(map[string]string{"env": "prod"})
39+
40+
assert.NotNil(t, tagged)
41+
assert.Equal(t, m, tagged)
42+
}

‎pkg/observability/metrics/slog_adapter.go‎

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,58 @@
1+
package metrics
2+
3+
import (
4+
"fmt"
5+
"log/slog"
6+
"maps"
7+
"time"
8+
)
9+
10+
// SlogMetrics implements Metrics by logging metric emissions via slog.
11+
// Useful for debugging metrics in local development.
12+
type SlogMetrics struct {
13+
logger *slog.Logger
14+
tags map[string]string
15+
}
16+
17+
var _ Metrics = (*SlogMetrics)(nil)
18+
19+
// NewSlogMetrics returns a new SlogMetrics that logs to the given slog.Logger.
20+
func NewSlogMetrics(logger *slog.Logger) *SlogMetrics {
21+
return &SlogMetrics{logger: logger}
22+
}
23+
24+
func (s *SlogMetrics) mergedTags(tags map[string]string) map[string]string {
25+
if len(s.tags) == 0 {
26+
return tags
27+
}
28+
if len(tags) == 0 {
29+
return s.tags
30+
}
31+
merged := make(map[string]string, len(s.tags)+len(tags))
32+
maps.Copy(merged, s.tags)
33+
maps.Copy(merged, tags)
34+
return merged
35+
}
36+
37+
func (s *SlogMetrics) Increment(key string, tags map[string]string) {
38+
s.logger.Debug("metric.increment", slog.String("key", key), slog.Any("tags", s.mergedTags(tags)))
39+
}
40+
41+
func (s *SlogMetrics) Counter(key string, tags map[string]string, value int64) {
42+
s.logger.Debug("metric.counter", slog.String("key", key), slog.Int64("value", value), slog.Any("tags", s.mergedTags(tags)))
43+
}
44+
45+
func (s *SlogMetrics) Distribution(key string, tags map[string]string, value float64) {
46+
s.logger.Debug("metric.distribution", slog.String("key", key), slog.Float64("value", value), slog.Any("tags", s.mergedTags(tags)))
47+
}
48+
49+
func (s *SlogMetrics) DistributionMs(key string, tags map[string]string, value time.Duration) {
50+
s.logger.Debug("metric.distribution_ms", slog.String("key", key), slog.String("value", fmt.Sprintf("%dms", value.Milliseconds())), slog.Any("tags", s.mergedTags(tags)))
51+
}
52+
53+
func (s *SlogMetrics) WithTags(tags map[string]string) Metrics {
54+
merged := make(map[string]string, len(s.tags)+len(tags))
55+
maps.Copy(merged, s.tags)
56+
maps.Copy(merged, tags)
57+
return &SlogMetrics{logger: s.logger, tags: merged}
58+
}

‎pkg/observability/metrics/slog_adapter_test.go‎

Lines changed: 109 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,109 @@
1+
package metrics
2+
3+
import (
4+
"bytes"
5+
"log/slog"
6+
"testing"
7+
"time"
8+
9+
"github.com/stretchr/testify/assert"
10+
)
11+
12+
func TestSlogMetrics_ImplementsInterface(t *testing.T) {
13+
var _ Metrics = (*SlogMetrics)(nil)
14+
}
15+
16+
func newTestSlogMetrics() (*SlogMetrics, *bytes.Buffer) {
17+
buf := &bytes.Buffer{}
18+
logger := slog.New(slog.NewTextHandler(buf, &slog.HandlerOptions{Level: slog.LevelDebug}))
19+
return NewSlogMetrics(logger), buf
20+
}
21+
22+
func TestSlogMetrics_Increment(t *testing.T) {
23+
m, buf := newTestSlogMetrics()
24+
m.Increment("req.count", map[string]string{"tool": "search"})
25+
26+
output := buf.String()
27+
assert.Contains(t, output, "metric.increment")
28+
assert.Contains(t, output, "req.count")
29+
assert.Contains(t, output, "search")
30+
}
31+
32+
func TestSlogMetrics_Counter(t *testing.T) {
33+
m, buf := newTestSlogMetrics()
34+
m.Counter("api.calls", map[string]string{"status": "200"}, 5)
35+
36+
output := buf.String()
37+
assert.Contains(t, output, "metric.counter")
38+
assert.Contains(t, output, "api.calls")
39+
assert.Contains(t, output, "5")
40+
}
41+
42+
func TestSlogMetrics_Distribution(t *testing.T) {
43+
m, buf := newTestSlogMetrics()
44+
m.Distribution("latency", map[string]string{"endpoint": "/api"}, 42.5)
45+
46+
output := buf.String()
47+
assert.Contains(t, output, "metric.distribution")
48+
assert.Contains(t, output, "latency")
49+
assert.Contains(t, output, "42.5")
50+
}
51+
52+
func TestSlogMetrics_DistributionMs(t *testing.T) {
53+
m, buf := newTestSlogMetrics()
54+
m.DistributionMs("duration", map[string]string{"op": "fetch"}, 150*time.Millisecond)
55+
56+
output := buf.String()
57+
assert.Contains(t, output, "metric.distribution_ms")
58+
assert.Contains(t, output, "duration")
59+
assert.Contains(t, output, "150ms")
60+
}
61+
62+
func TestSlogMetrics_WithTags(t *testing.T) {
63+
m, buf := newTestSlogMetrics()
64+
tagged := m.WithTags(map[string]string{"env": "prod"})
65+
66+
tagged.Increment("req.count", map[string]string{"tool": "search"})
67+
68+
output := buf.String()
69+
assert.Contains(t, output, "env")
70+
assert.Contains(t, output, "prod")
71+
assert.Contains(t, output, "search")
72+
}
73+
74+
func TestSlogMetrics_WithTags_Chaining(t *testing.T) {
75+
m, buf := newTestSlogMetrics()
76+
tagged := m.WithTags(map[string]string{"env": "prod"}).WithTags(map[string]string{"region": "us"})
77+
78+
tagged.Increment("req.count", nil)
79+
80+
output := buf.String()
81+
assert.Contains(t, output, "env")
82+
assert.Contains(t, output, "prod")
83+
assert.Contains(t, output, "region")
84+
assert.Contains(t, output, "us")
85+
}
86+
87+
func TestSlogMetrics_WithTags_DoesNotMutateOriginal(t *testing.T) {
88+
m, buf := newTestSlogMetrics()
89+
_ = m.WithTags(map[string]string{"env": "prod"})
90+
91+
m.Increment("req.count", nil)
92+
93+
output := buf.String()
94+
assert.NotContains(t, output, "prod")
95+
}
96+
97+
func TestSlogMetrics_NilTags(t *testing.T) {
98+
m, buf := newTestSlogMetrics()
99+
100+
assert.NotPanics(t, func() {
101+
m.Increment("key", nil)
102+
m.Counter("key", nil, 1)
103+
m.Distribution("key", nil, 1.5)
104+
m.DistributionMs("key", nil, time.Second)
105+
})
106+
107+
output := buf.String()
108+
assert.Contains(t, output, "metric.increment")
109+
}

‎pkg/observability/observability.go‎

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,22 +4,30 @@ import (
44
"context"
55

66
"github.com/github/github-mcp-server/pkg/observability/log"
7+
"github.com/github/github-mcp-server/pkg/observability/metrics"
78
)
89

910
type Exporters interface {
1011
Logger(context.Context) log.Logger
12+
Metrics(context.Context) metrics.Metrics
1113
}
1214

1315
type ObservabilityExporters struct {
14-
logger log.Logger
16+
logger log.Logger
17+
metrics metrics.Metrics
1518
}
1619

17-
func NewExporters(logger log.Logger) Exporters {
20+
func NewExporters(logger log.Logger, metrics metrics.Metrics) Exporters {
1821
return &ObservabilityExporters{
19-
logger: logger,
22+
logger: logger,
23+
metrics: metrics,
2024
}
2125
}
2226

2327
func (e *ObservabilityExporters) Logger(_ context.Context) log.Logger {
2428
return e.logger
2529
}
30+
31+
func (e *ObservabilityExporters) Metrics(_ context.Context) metrics.Metrics {
32+
return e.metrics
33+
}

‎pkg/observability/observability_test.go‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,22 +5,26 @@ import (
55
"testing"
66

77
"github.com/github/github-mcp-server/pkg/observability/log"
8+
"github.com/github/github-mcp-server/pkg/observability/metrics"
89
"github.com/stretchr/testify/assert"
910
)
1011

1112
func TestNewExporters(t *testing.T) {
1213
logger := log.NewNoopLogger()
13-
exp := NewExporters(logger)
14+
m := metrics.NewNoopMetrics()
15+
exp := NewExporters(logger, m)
1416
ctx := context.Background()
1517

1618
assert.NotNil(t, exp)
1719
assert.Equal(t, logger, exp.Logger(ctx))
20+
assert.Equal(t, m, exp.Metrics(ctx))
1821
}
1922

2023
func TestExporters_WithNilLogger(t *testing.T) {
21-
exp := NewExporters(nil)
24+
exp := NewExporters(nil, nil)
2225
ctx := context.Background()
2326

2427
assert.NotNil(t, exp)
2528
assert.Nil(t, exp.Logger(ctx))
29+
assert.Nil(t, exp.Metrics(ctx))
2630
}

0 commit comments

Comments
 (0)