Merge branch 'main' into patch-1

Author: Git-Hub-Chris

internal/ghmcp/server.go

@@ -18,6 +18,8 @@ import (
 	"github.com/github/github-mcp-server/pkg/inventory"
 	"github.com/github/github-mcp-server/pkg/lockdown"
 	mcplog "github.com/github/github-mcp-server/pkg/log"
+	"github.com/github/github-mcp-server/pkg/observability"
+	"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"
@@ -116,6 +118,10 @@ func NewStdioMCPServer(ctx context.Context, cfg github.MCPServerConfig) (*mcp.Se
 	featureChecker := createFeatureChecker(cfg.EnabledFeatures)
 
 	// Create dependencies for tool handlers
+	obs, err := observability.NewExporters(cfg.Logger, metrics.NewNoopMetrics())
+	if err != nil {
+		return nil, fmt.Errorf("failed to create observability exporters: %w", err)
+	}
 	deps := github.NewBaseDeps(
 		clients.rest,
 		clients.gql,
@@ -128,6 +134,7 @@ func NewStdioMCPServer(ctx context.Context, cfg github.MCPServerConfig) (*mcp.Se
 		},
 		cfg.ContentWindowSize,
 		featureChecker,
+		obs,
 	)
 	// Build and register the tool/resource/prompt inventory
 	inventoryBuilder := github.NewInventory(cfg.Translator).

pkg/github/context_tools_test.go

@@ -96,9 +96,10 @@ func Test_GetMe(t *testing.T) {
 		t.Run(tc.name, func(t *testing.T) {
 			var deps ToolDependencies
 			if tc.clientErr != "" {
-				deps = stubDeps{clientFn: stubClientFnErr(tc.clientErr)}
+				deps = stubDeps{clientFn: stubClientFnErr(tc.clientErr), obsv: stubExporters()}
 			} else {
-				deps = BaseDeps{Client: github.NewClient(tc.mockedClient)}
+				obs := stubExporters()
+				deps = BaseDeps{Client: github.NewClient(tc.mockedClient), Obsv: obs}
 			}
 			handler := serverTool.Handler(deps)
 
@@ -304,7 +305,7 @@ func Test_GetTeams(t *testing.T) {
 		{
 			name: "getting client fails",
 			makeDeps: func() ToolDependencies {
-				return stubDeps{clientFn: stubClientFnErr("expected test error")}
+				return stubDeps{clientFn: stubClientFnErr("expected test error"), obsv: stubExporters()}
 			},
 			requestArgs:        map[string]any{},
 			expectToolError:    true,
@@ -315,6 +316,7 @@ func Test_GetTeams(t *testing.T) {
 			makeDeps: func() ToolDependencies {
 				return BaseDeps{
 					Client: github.NewClient(httpClientUserFails()),
+					Obsv:   stubExporters(),
 				}
 			},
 			requestArgs:        map[string]any{},
@@ -327,6 +329,7 @@ func Test_GetTeams(t *testing.T) {
 				return stubDeps{
 					clientFn:    stubClientFnFromHTTP(httpClientWithUser()),
 					gqlClientFn: stubGQLClientFnErr("GraphQL client error"),
+					obsv:        stubExporters(),
 				}
 			},
 			requestArgs:        map[string]any{},
@@ -469,7 +472,7 @@ func Test_GetTeamMembers(t *testing.T) {
 		},
 		{
 			name: "getting GraphQL client fails",
-			deps: stubDeps{gqlClientFn: stubGQLClientFnErr("GraphQL client error")},
+			deps: stubDeps{gqlClientFn: stubGQLClientFnErr("GraphQL client error"), obsv: stubExporters()},
 			requestArgs: map[string]any{
 				"org":       "testorg",
 				"team_slug": "testteam",

pkg/github/dependencies.go

@@ -4,13 +4,16 @@ 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"
+	"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"
@@ -94,6 +97,14 @@ type ToolDependencies interface {
 
 	// IsFeatureEnabled checks if a feature flag is enabled.
 	IsFeatureEnabled(ctx context.Context, flagName string) bool
+
+	// Logger returns the structured logger, optionally enriched with
+	// request-scoped data from ctx. Integrators provide their own slog.Handler
+	// to control where logs are sent.
+	Logger(ctx context.Context) *slog.Logger
+
+	// Metrics returns the metrics client
+	Metrics(ctx context.Context) metrics.Metrics
 }
 
 // BaseDeps is the standard implementation of ToolDependencies for the local server.
@@ -113,6 +124,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.
@@ -128,6 +142,7 @@ func NewBaseDeps(
 	flags FeatureFlags,
 	contentWindowSize int,
 	featureChecker inventory.FeatureFlagChecker,
+	obsv observability.Exporters,
 ) *BaseDeps {
 	return &BaseDeps{
 		Client:            client,
@@ -138,6 +153,7 @@ func NewBaseDeps(
 		Flags:             flags,
 		ContentWindowSize: contentWindowSize,
 		featureChecker:    featureChecker,
+		Obsv:              obsv,
 	}
 }
 
@@ -170,6 +186,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(_ context.Context) *slog.Logger {
+	return d.Obsv.Logger()
+}
+
+// Metrics implements ToolDependencies.
+func (d BaseDeps) Metrics(ctx context.Context) metrics.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.
@@ -247,6 +273,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.
@@ -258,6 +287,7 @@ func NewRequestDeps(
 	t translations.TranslationHelperFunc,
 	contentWindowSize int,
 	featureChecker inventory.FeatureFlagChecker,
+	obsv observability.Exporters,
 ) *RequestDeps {
 	return &RequestDeps{
 		apiHosts:          apiHosts,
@@ -267,6 +297,7 @@ func NewRequestDeps(
 		T:                 t,
 		ContentWindowSize: contentWindowSize,
 		featureChecker:    featureChecker,
+		obsv:              obsv,
 	}
 }
 
@@ -374,6 +405,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(_ context.Context) *slog.Logger {
+	return d.obsv.Logger()
+}
+
+// Metrics implements ToolDependencies.
+func (d *RequestDeps) Metrics(ctx context.Context) metrics.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 == "" {

pkg/github/dependencies_test.go

@@ -3,13 +3,21 @@ package github_test
 import (
 	"context"
 	"errors"
+	"log/slog"
 	"testing"
 
 	"github.com/github/github-mcp-server/pkg/github"
+	"github.com/github/github-mcp-server/pkg/observability"
+	"github.com/github/github-mcp-server/pkg/observability/metrics"
 	"github.com/github/github-mcp-server/pkg/translations"
 	"github.com/stretchr/testify/assert"
 )
 
+func testExporters() observability.Exporters {
+	obs, _ := observability.NewExporters(slog.New(slog.DiscardHandler), metrics.NewNoopMetrics())
+	return obs
+}
+
 func TestIsFeatureEnabled_WithEnabledFlag(t *testing.T) {
 	t.Parallel()
 
@@ -28,6 +36,7 @@ func TestIsFeatureEnabled_WithEnabledFlag(t *testing.T) {
 		github.FeatureFlags{},
 		0,       // contentWindowSize
 		checker, // featureChecker
+		testExporters(),
 	)
 
 	// Test enabled flag
@@ -52,6 +61,7 @@ func TestIsFeatureEnabled_WithoutChecker(t *testing.T) {
 		github.FeatureFlags{},
 		0,   // contentWindowSize
 		nil, // featureChecker (nil)
+		testExporters(),
 	)
 
 	// Should return false when checker is nil
@@ -76,6 +86,7 @@ func TestIsFeatureEnabled_EmptyFlagName(t *testing.T) {
 		github.FeatureFlags{},
 		0,       // contentWindowSize
 		checker, // featureChecker
+		testExporters(),
 	)
 
 	// Should return false for empty flag name
@@ -100,6 +111,7 @@ func TestIsFeatureEnabled_CheckerError(t *testing.T) {
 		github.FeatureFlags{},
 		0,       // contentWindowSize
 		checker, // featureChecker
+		testExporters(),
 	)
 
 	// Should return false and log error (not crash)

pkg/github/dynamic_tools_test.go

@@ -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, stubExporters()),
 		T:         translations.NullTranslationHelper,
 	}
 

pkg/github/feature_flags_test.go

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

pkg/github/server_test.go

@@ -5,11 +5,14 @@ import (
 	"encoding/json"
 	"errors"
 	"fmt"
+	"log/slog"
 	"net/http"
 	"testing"
 	"time"
 
 	"github.com/github/github-mcp-server/pkg/lockdown"
+	"github.com/github/github-mcp-server/pkg/observability"
+	"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"
@@ -30,6 +33,7 @@ type stubDeps struct {
 	t                 translations.TranslationHelperFunc
 	flags             FeatureFlags
 	contentWindowSize int
+	obsv              observability.Exporters
 }
 
 func (s stubDeps) GetClient(ctx context.Context) (*gogithub.Client, error) {
@@ -60,8 +64,21 @@ 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(_ context.Context) *slog.Logger {
+	return s.obsv.Logger()
+}
+func (s stubDeps) Metrics(ctx context.Context) metrics.Metrics {
+	return s.obsv.Metrics(ctx)
+}
 
 // Helper functions to create stub client functions for error testing
+
+// stubExporters returns a discard-logger + noop-metrics Exporters for tests.
+func stubExporters() observability.Exporters {
+	obs, _ := observability.NewExporters(slog.New(slog.DiscardHandler), metrics.NewNoopMetrics())
+	return obs
+}
+
 func stubClientFnFromHTTP(httpClient *http.Client) func(context.Context) (*gogithub.Client, error) {
 	return func(_ context.Context) (*gogithub.Client, error) {
 		return gogithub.NewClient(httpClient), nil
@@ -125,7 +142,7 @@ func TestNewMCPServer_CreatesSuccessfully(t *testing.T) {
 		InsidersMode:      false,
 	}
 
-	deps := stubDeps{}
+	deps := stubDeps{obsv: stubExporters()}
 
 	// Build inventory
 	inv, err := NewInventory(cfg.Translator).

pkg/http/server.go

@@ -17,6 +17,8 @@ import (
 	"github.com/github/github-mcp-server/pkg/http/oauth"
 	"github.com/github/github-mcp-server/pkg/inventory"
 	"github.com/github/github-mcp-server/pkg/lockdown"
+	"github.com/github/github-mcp-server/pkg/observability"
+	"github.com/github/github-mcp-server/pkg/observability/metrics"
 	"github.com/github/github-mcp-server/pkg/scopes"
 	"github.com/github/github-mcp-server/pkg/translations"
 	"github.com/github/github-mcp-server/pkg/utils"
@@ -106,6 +108,11 @@ func RunHTTPServer(cfg ServerConfig) error {
 
 	featureChecker := createHTTPFeatureChecker()
 
+	obs, err := observability.NewExporters(logger, metrics.NewNoopMetrics())
+	if err != nil {
+		return fmt.Errorf("failed to create observability exporters: %w", err)
+	}
+
 	deps := github.NewRequestDeps(
 		apiHost,
 		cfg.Version,
@@ -114,6 +121,7 @@ func RunHTTPServer(cfg ServerConfig) error {
 		t,
 		cfg.ContentWindowSize,
 		featureChecker,
+		obs,
 	)
 
 	// Initialize the global tool scope map

pkg/observability/metrics/metrics.go

@@ -0,0 +1,13 @@
+package metrics
+
+import "time"
+
+// Metrics is a backend-agnostic interface for emitting metrics.
+// Implementations can route to DataDog, log to slog, or discard (noop).
+type Metrics interface {
+	Increment(key string, tags map[string]string)
+	Counter(key string, tags map[string]string, value int64)
+	Distribution(key string, tags map[string]string, value float64)
+	DistributionMs(key string, tags map[string]string, value time.Duration)
+	WithTags(tags map[string]string) Metrics
+}

pkg/observability/metrics/noop_sink.go

@@ -0,0 +1,19 @@
+package metrics
+
+import "time"
+
+// NoopMetrics is a no-op implementation of the Metrics interface.
+type NoopMetrics struct{}
+
+var _ Metrics = (*NoopMetrics)(nil)
+
+// NewNoopMetrics returns a new NoopMetrics.
+func NewNoopMetrics() *NoopMetrics {
+	return &NoopMetrics{}
+}
+
+func (n *NoopMetrics) Increment(_ string, _ map[string]string)                       {}
+func (n *NoopMetrics) Counter(_ string, _ map[string]string, _ int64)                {}
+func (n *NoopMetrics) Distribution(_ string, _ map[string]string, _ float64)         {}
+func (n *NoopMetrics) DistributionMs(_ string, _ map[string]string, _ time.Duration) {}
+func (n *NoopMetrics) WithTags(_ map[string]string) Metrics                          { return n }