Emit only bytes_full from the fields filter point

Narrow the OSS-side `fields` telemetry to the single metric that can only
be measured inside the tool handler: mcp.fields.bytes_full, the size of
the unfiltered payload, emitted when a call actually applied filtering
and tagged only by tool.

Adoption (whether the model requested `fields`) and the filtered response
size (bytes_sent) are observable from outside the handler, so they are
better emitted by the hosted deployment's request middleware rather than
here. This keeps the open-source telemetry surface to one metric while
still capturing the counterfactual needed to compute realized savings on
the dashboard.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Author: tommaso-moro

pkg/github/fields_telemetry.go

@@ -2,53 +2,30 @@ package github
 
 import (
 	"context"
-	"strconv"
 )
 
-// Metric names for the optional `fields` response-filtering feature. They let a
-// dashboard answer two questions on real traffic: how often the model actually
-// filters (adoption) and how many bytes that filtering removes (effectiveness).
+// metricFieldsBytesFull is the size, in bytes, of the unfiltered response
+// payload for a tool call that applied `fields` filtering. It is emitted only
+// when filtering actually happened and is tagged solely by `tool` to keep
+// cardinality low.
 //
-// Cardinality is kept deliberately low: the only tags ever attached are `tool`
-// (a small fixed set of tool names) and `filtered` (a boolean). Unbounded values
-// such as repository, owner, user, the query, or the requested field list are
-// never used as tags.
-//
-// The realized savings (bytes_full - bytes_sent) is intentionally not emitted as
-// its own metric: it is derivable on the dashboard from the two byte counters,
-// since sum(bytes_full) - sum(bytes_sent) equals the total saved at any rollup.
-const (
-	metricFieldsToolCall  = "mcp.fields.tool_call"
-	metricFieldsBytesFull = "mcp.fields.bytes_full"
-	metricFieldsBytesSent = "mcp.fields.bytes_sent"
-)
+// This is the one `fields` signal that can only be measured inside the tool
+// handler: the unfiltered payload exists briefly at the filter point and is gone
+// by the time the response leaves the process. The companion signals — adoption
+// (whether the model requested `fields`) and the filtered response size
+// (bytes_sent) — are observable from outside the handler and are emitted by the
+// hosted deployment's request middleware, so they are intentionally not emitted
+// here. On the dashboard, bytes_full and bytes_sent combine into the realized
+// savings (1 - sum(bytes_sent) / sum(bytes_full)) over filtered calls.
+const metricFieldsBytesFull = "mcp.fields.bytes_full"
 
-// recordFieldsUsage emits telemetry for a single call to a tool that supports
-// the `fields` parameter. It is best-effort: the local server wires a no-op
-// metrics sink, while hosted deployments inject a real sink.
-//
-// Every call increments mcp.fields.tool_call tagged by tool and whether the
-// response was filtered, which yields the adoption rate (filtered / total). When
-// the response was filtered, it also records the unfiltered (fullBytes) and
-// returned (sentBytes) payload sizes. Byte counters are only emitted for
-// filtered calls so that "percent saved" (1 - bytes_sent / bytes_full) is
-// computed over the population where filtering actually applied.
-func recordFieldsUsage(ctx context.Context, deps ToolDependencies, tool string, filtered bool, fullBytes, sentBytes int) {
+// recordFieldsFullBytes emits mcp.fields.bytes_full for a tool call that applied
+// `fields` filtering. It is best-effort: the local server wires a no-op metrics
+// sink, while hosted deployments inject a real sink.
+func recordFieldsFullBytes(ctx context.Context, deps ToolDependencies, tool string, fullBytes int) {
 	m := deps.Metrics(ctx)
 	if m == nil {
 		return
 	}
-
-	m.Increment(metricFieldsToolCall, map[string]string{
-		"tool":     tool,
-		"filtered": strconv.FormatBool(filtered),
-	})
-
-	if !filtered {
-		return
-	}
-
-	toolTag := map[string]string{"tool": tool}
-	m.Counter(metricFieldsBytesFull, toolTag, int64(fullBytes))
-	m.Counter(metricFieldsBytesSent, toolTag, int64(sentBytes))
+	m.Counter(metricFieldsBytesFull, map[string]string{"tool": tool}, int64(fullBytes))
 }

pkg/github/fields_telemetry_test.go

@@ -14,11 +14,10 @@ import (
 )
 
 // recordingMetrics is a metrics.Metrics implementation that captures emitted
-// metrics so tests can assert on telemetry. It is safe for concurrent use.
+// counters so tests can assert on telemetry. It is safe for concurrent use.
 type recordingMetrics struct {
-	mu         sync.Mutex
-	increments []recordedMetric
-	counters   []recordedMetric
+	mu       sync.Mutex
+	counters []recordedMetric
 }
 
 type recordedMetric struct {
@@ -27,11 +26,7 @@ type recordedMetric struct {
 	value int64
 }
 
-func (m *recordingMetrics) Increment(key string, tags map[string]string) {
-	m.mu.Lock()
-	defer m.mu.Unlock()
-	m.increments = append(m.increments, recordedMetric{key: key, tags: tags, value: 1})
-}
+func (m *recordingMetrics) Increment(_ string, _ map[string]string) {}
 
 func (m *recordingMetrics) Counter(key string, tags map[string]string, value int64) {
 	m.mu.Lock()
@@ -55,18 +50,6 @@ func (m *recordingMetrics) counter(key string) (recordedMetric, bool) {
 	return recordedMetric{}, false
 }
 
-// increment returns the recorded increment for the given key, or false if absent.
-func (m *recordingMetrics) increment(key string) (recordedMetric, bool) {
-	m.mu.Lock()
-	defer m.mu.Unlock()
-	for _, c := range m.increments {
-		if c.key == key {
-			return c, true
-		}
-	}
-	return recordedMetric{}, false
-}
-
 // depsWithRecordingMetrics returns BaseDeps wired with a recording metrics sink
 // plus the sink for assertions.
 func depsWithRecordingMetrics(t *testing.T, base BaseDeps) (BaseDeps, *recordingMetrics) {
@@ -78,46 +61,23 @@ func depsWithRecordingMetrics(t *testing.T, base BaseDeps) (BaseDeps, *recording
 	return base, rec
 }
 
-func Test_recordFieldsUsage_Filtered(t *testing.T) {
+func Test_recordFieldsFullBytes(t *testing.T) {
 	deps, rec := depsWithRecordingMetrics(t, BaseDeps{})
 
-	recordFieldsUsage(context.Background(), deps, "search_code", true, 100, 30)
-
-	call, ok := rec.increment(metricFieldsToolCall)
-	require.True(t, ok)
-	assert.Equal(t, "search_code", call.tags["tool"])
-	assert.Equal(t, "true", call.tags["filtered"])
+	recordFieldsFullBytes(context.Background(), deps, "search_code", 100)
 
 	full, ok := rec.counter(metricFieldsBytesFull)
 	require.True(t, ok)
 	assert.Equal(t, int64(100), full.value)
 	assert.Equal(t, "search_code", full.tags["tool"])
+	// The only tag is `tool`; adoption and filtered/unfiltered distinctions are
+	// emitted by the hosted request middleware, not here.
 	assert.NotContains(t, full.tags, "filtered")
-
-	sent, ok := rec.counter(metricFieldsBytesSent)
-	require.True(t, ok)
-	assert.Equal(t, int64(30), sent.value)
-}
-
-func Test_recordFieldsUsage_NotFiltered(t *testing.T) {
-	deps, rec := depsWithRecordingMetrics(t, BaseDeps{})
-
-	recordFieldsUsage(context.Background(), deps, "search_code", false, 100, 100)
-
-	call, ok := rec.increment(metricFieldsToolCall)
-	require.True(t, ok)
-	assert.Equal(t, "false", call.tags["filtered"])
-
-	// No byte counters are emitted when the response was not filtered.
-	_, ok = rec.counter(metricFieldsBytesFull)
-	assert.False(t, ok)
-	_, ok = rec.counter(metricFieldsBytesSent)
-	assert.False(t, ok)
 }
 
-func Test_recordFieldsUsage_NilExporterDoesNotPanic(t *testing.T) {
+func Test_recordFieldsFullBytes_NilExporterDoesNotPanic(t *testing.T) {
 	// BaseDeps with no Obsv falls back to a noop sink rather than panicking.
 	assert.NotPanics(t, func() {
-		recordFieldsUsage(context.Background(), BaseDeps{}, "search_code", true, 100, 30)
+		recordFieldsFullBytes(context.Background(), BaseDeps{}, "search_code", 100)
 	})
 }

pkg/github/repositories.go

@@ -944,8 +944,8 @@ func getFileContentsTool(t translations.TranslationHelperFunc, includeFields boo
 				if err != nil {
 					return utils.NewToolResultError("failed to marshal response"), nil, nil
 				}
-				if includeFields {
-					recordDirContentsFieldsUsage(ctx, deps, dirContent, filtered, len(r))
+				if filtered {
+					recordDirContentsFieldsFull(ctx, deps, dirContent)
 				}
 				return attachIFC(utils.NewToolResultText(string(r))), nil, nil
 			}
@@ -955,18 +955,17 @@ func getFileContentsTool(t translations.TranslationHelperFunc, includeFields boo
 	)
 }
 
-// recordDirContentsFieldsUsage emits fields telemetry for a get_file_contents
-// directory listing. sentBytes is the size of the payload actually returned.
-// When the listing was filtered, the unfiltered size is computed from the full
-// directory content so the realized savings can be measured.
-func recordDirContentsFieldsUsage(ctx context.Context, deps ToolDependencies, full []*github.RepositoryContent, filtered bool, sentBytes int) {
-	fullBytes := sentBytes
-	if filtered {
-		if data, err := json.Marshal(full); err == nil {
-			fullBytes = len(data)
-		}
+// recordDirContentsFieldsFull emits mcp.fields.bytes_full for a filtered
+// get_file_contents directory listing. The unfiltered size is computed from the
+// full directory content so the hosted dashboard can measure realized savings
+// against the filtered response size (bytes_sent), which the hosted request
+// middleware emits.
+func recordDirContentsFieldsFull(ctx context.Context, deps ToolDependencies, full []*github.RepositoryContent) {
+	data, err := json.Marshal(full)
+	if err != nil {
+		return
 	}
-	recordFieldsUsage(ctx, deps, "get_file_contents", filtered, fullBytes, sentBytes)
+	recordFieldsFullBytes(ctx, deps, "get_file_contents", len(data))
 }
 
 // ForkRepository creates a tool to fork a repository.

pkg/github/repositories_test.go

@@ -601,16 +601,13 @@ func Test_GetFileContents_DirectoryFieldsTelemetry(t *testing.T) {
 	require.NoError(t, err)
 	require.False(t, result.IsError)
 
-	call, ok := rec.increment(metricFieldsToolCall)
-	require.True(t, ok)
-	assert.Equal(t, "get_file_contents", call.tags["tool"])
-	assert.Equal(t, "true", call.tags["filtered"])
-
+	// The unfiltered payload size is emitted from the handler. Adoption and the
+	// filtered response size (bytes_sent) are emitted by the hosted request
+	// middleware, so they are not asserted here.
 	full, ok := rec.counter(metricFieldsBytesFull)
 	require.True(t, ok)
-	sent, ok := rec.counter(metricFieldsBytesSent)
-	require.True(t, ok)
-	assert.Greater(t, full.value, sent.value, "filtering should remove bytes")
+	assert.Equal(t, "get_file_contents", full.tags["tool"])
+	assert.Greater(t, full.value, int64(0))
 }
 
 func Test_GetFileContents_IFC_InsidersMode(t *testing.T) {

pkg/github/search.go

@@ -360,8 +360,8 @@ func searchCodeTool(t translations.TranslationHelperFunc, includeFields bool) in
 				return utils.NewToolResultErrorFromErr("failed to marshal response", err), nil, nil
 			}
 
-			if includeFields {
-				recordSearchCodeFieldsUsage(ctx, deps, minimalResult, filtered, len(r))
+			if filtered {
+				recordSearchCodeFieldsFull(ctx, deps, minimalResult)
 			}
 
 			callResult := utils.NewToolResultText(string(r))
@@ -380,18 +380,16 @@ func searchCodeTool(t translations.TranslationHelperFunc, includeFields bool) in
 	)
 }
 
-// recordSearchCodeFieldsUsage emits fields telemetry for a search_code call.
-// sentBytes is the size of the payload actually returned. When the response was
-// filtered, the unfiltered size is computed from the full minimal result so the
-// realized savings can be measured.
-func recordSearchCodeFieldsUsage(ctx context.Context, deps ToolDependencies, full *MinimalCodeSearchResult, filtered bool, sentBytes int) {
-	fullBytes := sentBytes
-	if filtered {
-		if data, err := json.Marshal(full); err == nil {
-			fullBytes = len(data)
-		}
+// recordSearchCodeFieldsFull emits mcp.fields.bytes_full for a filtered
+// search_code call. The unfiltered size is computed from the full minimal result
+// so the hosted dashboard can measure realized savings against the filtered
+// response size (bytes_sent), which the hosted request middleware emits.
+func recordSearchCodeFieldsFull(ctx context.Context, deps ToolDependencies, full *MinimalCodeSearchResult) {
+	data, err := json.Marshal(full)
+	if err != nil {
+		return
 	}
-	recordFieldsUsage(ctx, deps, "search_code", filtered, fullBytes, sentBytes)
+	recordFieldsFullBytes(ctx, deps, "search_code", len(data))
 }
 
 func userOrOrgHandler(ctx context.Context, accountType string, deps ToolDependencies, args map[string]any) (*mcp.CallToolResult, any, error) {

pkg/github/search_test.go

@@ -610,7 +610,7 @@ func Test_SearchCode_FieldsTelemetry(t *testing.T) {
 		GetSearchCode: mockResponse(t, http.StatusOK, mockSearchResult),
 	}))
 
-	t.Run("filtered call records savings", func(t *testing.T) {
+	t.Run("filtered call records full bytes", func(t *testing.T) {
 		deps, rec := depsWithRecordingMetrics(t, BaseDeps{Client: client})
 		handler := serverTool.Handler(deps)
 
@@ -622,19 +622,16 @@ func Test_SearchCode_FieldsTelemetry(t *testing.T) {
 		require.NoError(t, err)
 		require.False(t, result.IsError)
 
-		call, ok := rec.increment(metricFieldsToolCall)
-		require.True(t, ok)
-		assert.Equal(t, "search_code", call.tags["tool"])
-		assert.Equal(t, "true", call.tags["filtered"])
-
+		// The unfiltered payload size is emitted from the handler. Adoption and
+		// the filtered response size (bytes_sent) are emitted by the hosted
+		// request middleware, so they are not asserted here.
 		full, ok := rec.counter(metricFieldsBytesFull)
 		require.True(t, ok)
-		sent, ok := rec.counter(metricFieldsBytesSent)
-		require.True(t, ok)
-		assert.Greater(t, full.value, sent.value, "filtering should remove bytes")
+		assert.Equal(t, "search_code", full.tags["tool"])
+		assert.Greater(t, full.value, int64(0))
 	})
 
-	t.Run("unfiltered call records adoption only", func(t *testing.T) {
+	t.Run("unfiltered call records nothing", func(t *testing.T) {
 		deps, rec := depsWithRecordingMetrics(t, BaseDeps{Client: client})
 		handler := serverTool.Handler(deps)
 
@@ -645,12 +642,8 @@ func Test_SearchCode_FieldsTelemetry(t *testing.T) {
 		require.NoError(t, err)
 		require.False(t, result.IsError)
 
-		call, ok := rec.increment(metricFieldsToolCall)
-		require.True(t, ok)
-		assert.Equal(t, "false", call.tags["filtered"])
-
-		_, ok = rec.counter(metricFieldsBytesFull)
-		assert.False(t, ok, "no byte counters when not filtered")
+		_, ok := rec.counter(metricFieldsBytesFull)
+		assert.False(t, ok, "no bytes_full when not filtered")
 	})
 }