Skip to content

Commit 1f51e0d

Browse files
tommaso-moroCopilot
andcommitted
Drop mcp.fields.bytes_saved metric
Remove the mcp.fields.bytes_saved counter. It is derivable on the dashboard from the two remaining byte counters, since sum(bytes_full) - sum(bytes_sent) equals the total saved at any rollup, so emitting it separately is redundant. Keeping only bytes_full and bytes_sent shrinks the emitted telemetry surface. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
1 parent a877d9c commit 1f51e0d

4 files changed

Lines changed: 11 additions & 32 deletions

File tree

pkg/github/fields_telemetry.go

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -13,11 +13,14 @@ import (
1313
// (a small fixed set of tool names) and `filtered` (a boolean). Unbounded values
1414
// such as repository, owner, user, the query, or the requested field list are
1515
// never used as tags.
16+
//
17+
// The realized savings (bytes_full - bytes_sent) is intentionally not emitted as
18+
// its own metric: it is derivable on the dashboard from the two byte counters,
19+
// since sum(bytes_full) - sum(bytes_sent) equals the total saved at any rollup.
1620
const (
17-
metricFieldsToolCall = "mcp.fields.tool_call"
18-
metricFieldsBytesFull = "mcp.fields.bytes_full"
19-
metricFieldsBytesSent = "mcp.fields.bytes_sent"
20-
metricFieldsBytesSaved = "mcp.fields.bytes_saved"
21+
metricFieldsToolCall = "mcp.fields.tool_call"
22+
metricFieldsBytesFull = "mcp.fields.bytes_full"
23+
metricFieldsBytesSent = "mcp.fields.bytes_sent"
2124
)
2225

2326
// recordFieldsUsage emits telemetry for a single call to a tool that supports
@@ -27,10 +30,9 @@ const (
2730
// Every call increments mcp.fields.tool_call tagged by tool and whether the
2831
// response was filtered, which yields the adoption rate (filtered / total). When
2932
// the response was filtered, it also records the unfiltered (fullBytes) and
30-
// returned (sentBytes) payload sizes plus their difference, which yields the
31-
// realized savings. Byte counters are only emitted for filtered calls so that
32-
// "percent saved" (bytes_saved / bytes_full) is computed over the population
33-
// where filtering actually applied.
33+
// returned (sentBytes) payload sizes. Byte counters are only emitted for
34+
// filtered calls so that "percent saved" (1 - bytes_sent / bytes_full) is
35+
// computed over the population where filtering actually applied.
3436
func recordFieldsUsage(ctx context.Context, deps ToolDependencies, tool string, filtered bool, fullBytes, sentBytes int) {
3537
m := deps.Metrics(ctx)
3638
if m == nil {
@@ -47,8 +49,6 @@ func recordFieldsUsage(ctx context.Context, deps ToolDependencies, tool string,
4749
}
4850

4951
toolTag := map[string]string{"tool": tool}
50-
saved := max(fullBytes-sentBytes, 0)
5152
m.Counter(metricFieldsBytesFull, toolTag, int64(fullBytes))
5253
m.Counter(metricFieldsBytesSent, toolTag, int64(sentBytes))
53-
m.Counter(metricFieldsBytesSaved, toolTag, int64(saved))
5454
}

pkg/github/fields_telemetry_test.go

Lines changed: 1 addition & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -97,10 +97,6 @@ func Test_recordFieldsUsage_Filtered(t *testing.T) {
9797
sent, ok := rec.counter(metricFieldsBytesSent)
9898
require.True(t, ok)
9999
assert.Equal(t, int64(30), sent.value)
100-
101-
saved, ok := rec.counter(metricFieldsBytesSaved)
102-
require.True(t, ok)
103-
assert.Equal(t, int64(70), saved.value)
104100
}
105101

106102
func Test_recordFieldsUsage_NotFiltered(t *testing.T) {
@@ -115,21 +111,10 @@ func Test_recordFieldsUsage_NotFiltered(t *testing.T) {
115111
// No byte counters are emitted when the response was not filtered.
116112
_, ok = rec.counter(metricFieldsBytesFull)
117113
assert.False(t, ok)
118-
_, ok = rec.counter(metricFieldsBytesSaved)
114+
_, ok = rec.counter(metricFieldsBytesSent)
119115
assert.False(t, ok)
120116
}
121117

122-
func Test_recordFieldsUsage_ClampsNegativeSavings(t *testing.T) {
123-
deps, rec := depsWithRecordingMetrics(t, BaseDeps{})
124-
125-
// sent larger than full should never yield negative savings.
126-
recordFieldsUsage(context.Background(), deps, "get_file_contents", true, 10, 25)
127-
128-
saved, ok := rec.counter(metricFieldsBytesSaved)
129-
require.True(t, ok)
130-
assert.Equal(t, int64(0), saved.value)
131-
}
132-
133118
func Test_recordFieldsUsage_NilExporterDoesNotPanic(t *testing.T) {
134119
// BaseDeps with no Obsv falls back to a noop sink rather than panicking.
135120
assert.NotPanics(t, func() {

pkg/github/repositories_test.go

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -610,10 +610,7 @@ func Test_GetFileContents_DirectoryFieldsTelemetry(t *testing.T) {
610610
require.True(t, ok)
611611
sent, ok := rec.counter(metricFieldsBytesSent)
612612
require.True(t, ok)
613-
saved, ok := rec.counter(metricFieldsBytesSaved)
614-
require.True(t, ok)
615613
assert.Greater(t, full.value, sent.value, "filtering should remove bytes")
616-
assert.Equal(t, full.value-sent.value, saved.value)
617614
}
618615

619616
func Test_GetFileContents_IFC_InsidersMode(t *testing.T) {

pkg/github/search_test.go

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -631,10 +631,7 @@ func Test_SearchCode_FieldsTelemetry(t *testing.T) {
631631
require.True(t, ok)
632632
sent, ok := rec.counter(metricFieldsBytesSent)
633633
require.True(t, ok)
634-
saved, ok := rec.counter(metricFieldsBytesSaved)
635-
require.True(t, ok)
636634
assert.Greater(t, full.value, sent.value, "filtering should remove bytes")
637-
assert.Equal(t, full.value-sent.value, saved.value)
638635
})
639636

640637
t.Run("unfiltered call records adoption only", func(t *testing.T) {

0 commit comments

Comments
 (0)