diff --git a/pkg/github/fields_telemetry.go b/pkg/github/fields_telemetry.go index 4e06d9f42b..41e79179c0 100644 --- a/pkg/github/fields_telemetry.go +++ b/pkg/github/fields_telemetry.go @@ -4,6 +4,8 @@ import ( "context" "encoding/json" "strconv" + + "github.com/github/github-mcp-server/v2/pkg/observability/metrics" ) // Metric names for the optional `fields` response-filtering feature. They let a @@ -40,6 +42,10 @@ func recordFieldsUsage(ctx context.Context, deps ToolDependencies, tool string, return } + recordFieldsUsageWithMetrics(m, tool, filtered, fullBytes, sentBytes) +} + +func recordFieldsUsageWithMetrics(m metrics.Metrics, tool string, filtered bool, fullBytes, sentBytes int) { m.Increment(metricFieldsToolCall, map[string]string{ "tool": tool, "filtered": strconv.FormatBool(filtered), @@ -61,11 +67,19 @@ func recordFieldsUsage(ctx context.Context, deps ToolDependencies, tool string, // measured; full should be the complete, unfiltered payload. It centralizes the // full-size computation shared by every fields-enabled tool. func recordFieldsUsageFor(ctx context.Context, deps ToolDependencies, tool string, full any, filtered bool, sentBytes int) { + m := deps.Metrics(ctx) + if m == nil { + return + } + if _, ok := m.(*metrics.NoopMetrics); ok { + return + } + fullBytes := sentBytes if filtered { if data, err := json.Marshal(full); err == nil { fullBytes = len(data) } } - recordFieldsUsage(ctx, deps, tool, filtered, fullBytes, sentBytes) + recordFieldsUsageWithMetrics(m, tool, filtered, fullBytes, sentBytes) } diff --git a/pkg/github/fields_telemetry_test.go b/pkg/github/fields_telemetry_test.go index 63e4da9326..5209f7700b 100644 --- a/pkg/github/fields_telemetry_test.go +++ b/pkg/github/fields_telemetry_test.go @@ -2,6 +2,7 @@ package github import ( "context" + "encoding/json" "log/slog" "sync" "testing" @@ -121,3 +122,35 @@ func Test_recordFieldsUsage_NilExporterDoesNotPanic(t *testing.T) { recordFieldsUsage(context.Background(), BaseDeps{}, "search_code", true, 100, 30) }) } + +type countingMarshaler struct { + calls int +} + +func (m *countingMarshaler) MarshalJSON() ([]byte, error) { + m.calls++ + return json.Marshal(map[string]string{"full": "payload"}) +} + +func Test_recordFieldsUsageFor_NoopMetricsSkipsFullPayloadSerialization(t *testing.T) { + full := &countingMarshaler{} + + recordFieldsUsageFor(context.Background(), BaseDeps{}, "search_code", full, true, 10) + + assert.Zero(t, full.calls) +} + +func Test_recordFieldsUsageFor_RecordingMetricsMeasuresFullPayload(t *testing.T) { + deps, rec := depsWithRecordingMetrics(t, BaseDeps{}) + full := &countingMarshaler{} + + recordFieldsUsageFor(context.Background(), deps, "search_code", full, true, 10) + + require.Equal(t, 1, full.calls) + fullBytes, ok := rec.counter(metricFieldsBytesFull) + require.True(t, ok) + assert.Equal(t, int64(len(`{"full":"payload"}`)), fullBytes.value) + sentBytes, ok := rec.counter(metricFieldsBytesSent) + require.True(t, ok) + assert.Equal(t, int64(10), sentBytes.value) +}