Skip to content

Commit 2ae8746

Browse files
fix: make mcp span names informative (envoyproxy#1288)
**Description** Before everything was vaguely "mcp.request" which is not untrue, but not helpful. There are plenty of low-cardinality choices for span or metric names and the `req.Method` is the most intuitive. This uses the same naming conventions as mcp-go --------- Signed-off-by: Adrian Cole <[email protected]> Signed-off-by: Hrushikesh Patil <[email protected]>
1 parent 89bc99d commit 2ae8746

File tree

5 files changed

+407
-100
lines changed

5 files changed

+407
-100
lines changed

internal/mcpproxy/handlers.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -234,6 +234,7 @@ func (m *MCPProxy) servePOST(w http.ResponseWriter, r *http.Request) {
234234
}
235235
err = m.handleSetLoggingLevel(ctx, s, w, msg, p, span)
236236
case "ping":
237+
// Ping is intentionally not traced as it's a lightweight health check.
237238
err = m.handlePing(ctx, w, msg)
238239
case "prompts/list":
239240
p := &mcp.ListPromptsParams{}

internal/tracing/mcp.go

Lines changed: 37 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -86,7 +86,9 @@ func (m mcpTracer) StartSpanAndInjectMeta(ctx context.Context, req *jsonrpc.Requ
8686
parentCtx := m.propagator.Extract(ctx, mc)
8787

8888
// Start the span with options appropriate for the semantic convention.
89-
newCtx, span := m.tracer.Start(parentCtx, "mcp.request", trace.WithSpanKind(trace.SpanKindClient))
89+
// Convert method name to span name following mcp-go SDK patterns
90+
spanName := getSpanName(req.Method)
91+
newCtx, span := m.tracer.Start(parentCtx, spanName, trace.WithSpanKind(trace.SpanKindClient))
9092

9193
// Always inject trace context into the header mutation if provided.
9294
// This ensures trace propagation works even for unsampled spans.
@@ -174,3 +176,37 @@ func (c metaMapCarrier) Keys() []string {
174176

175177
return keys
176178
}
179+
180+
// getSpanName converts MCP method names to span names following mcp-go SDK patterns.
181+
func getSpanName(method string) string {
182+
switch method {
183+
case "initialize":
184+
return "Initialize"
185+
case "tools/list":
186+
return "ListTools"
187+
case "tools/call":
188+
return "CallTool"
189+
case "prompts/list":
190+
return "ListPrompts"
191+
case "prompts/get":
192+
return "GetPrompt"
193+
case "resources/list":
194+
return "ListResources"
195+
case "resources/read":
196+
return "ReadResource"
197+
case "resources/subscribe":
198+
return "Subscribe"
199+
case "resources/unsubscribe":
200+
return "Unsubscribe"
201+
case "resources/templates/list":
202+
return "ListResourceTemplates"
203+
case "logging/setLevel":
204+
return "SetLoggingLevel"
205+
case "completion/complete":
206+
return "Complete"
207+
case "ping":
208+
return "Ping"
209+
default:
210+
return method
211+
}
212+
}

internal/tracing/mcp_test.go

Lines changed: 105 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
package tracing
77

88
import (
9+
"context"
910
"testing"
1011

1112
"github.com/modelcontextprotocol/go-sdk/jsonrpc"
@@ -15,6 +16,7 @@ import (
1516
"go.opentelemetry.io/otel/attribute"
1617
"go.opentelemetry.io/otel/sdk/trace"
1718
"go.opentelemetry.io/otel/sdk/trace/tracetest"
19+
oteltrace "go.opentelemetry.io/otel/trace"
1820
)
1921

2022
func TestTracer_StartSpanAndInjectMeta(t *testing.T) {
@@ -134,3 +136,106 @@ func Test_getMCPAttributes(t *testing.T) {
134136
})
135137
}
136138
}
139+
140+
func Test_getSpanName(t *testing.T) {
141+
tests := []struct {
142+
method string
143+
expected string
144+
}{
145+
{method: "initialize", expected: "Initialize"},
146+
{method: "tools/list", expected: "ListTools"},
147+
{method: "tools/call", expected: "CallTool"},
148+
{method: "prompts/list", expected: "ListPrompts"},
149+
{method: "prompts/get", expected: "GetPrompt"},
150+
{method: "resources/list", expected: "ListResources"},
151+
{method: "resources/read", expected: "ReadResource"},
152+
{method: "resources/subscribe", expected: "Subscribe"},
153+
{method: "resources/unsubscribe", expected: "Unsubscribe"},
154+
{method: "resources/templates/list", expected: "ListResourceTemplates"},
155+
{method: "logging/setLevel", expected: "SetLoggingLevel"},
156+
{method: "completion/complete", expected: "Complete"},
157+
{method: "ping", expected: "Ping"},
158+
}
159+
160+
for _, tt := range tests {
161+
t.Run(tt.method, func(t *testing.T) {
162+
actual := getSpanName(tt.method)
163+
require.Equal(t, tt.expected, actual)
164+
})
165+
}
166+
}
167+
168+
func TestMCPTracer_SpanName(t *testing.T) {
169+
tests := []struct {
170+
name string
171+
method string
172+
params mcp.Params
173+
expectedSpanName string
174+
}{
175+
{
176+
name: "tools/list",
177+
method: "tools/list",
178+
params: &mcp.ListToolsParams{},
179+
expectedSpanName: "ListTools",
180+
},
181+
{
182+
name: "tools/call",
183+
method: "tools/call",
184+
params: &mcp.CallToolParams{Name: "test-tool"},
185+
expectedSpanName: "CallTool",
186+
},
187+
{
188+
name: "prompts/list",
189+
method: "prompts/list",
190+
params: &mcp.ListPromptsParams{},
191+
expectedSpanName: "ListPrompts",
192+
},
193+
{
194+
name: "prompts/get",
195+
method: "prompts/get",
196+
params: &mcp.GetPromptParams{Name: "test-prompt"},
197+
expectedSpanName: "GetPrompt",
198+
},
199+
{
200+
name: "resources/list",
201+
method: "resources/list",
202+
params: &mcp.ListResourcesParams{},
203+
expectedSpanName: "ListResources",
204+
},
205+
{
206+
name: "resources/read",
207+
method: "resources/read",
208+
params: &mcp.ReadResourceParams{URI: "test://uri"},
209+
expectedSpanName: "ReadResource",
210+
},
211+
{
212+
name: "initialize",
213+
method: "initialize",
214+
params: &mcp.InitializeParams{},
215+
expectedSpanName: "Initialize",
216+
},
217+
}
218+
219+
for _, tt := range tests {
220+
t.Run(tt.name, func(t *testing.T) {
221+
exporter := tracetest.NewInMemoryExporter()
222+
tp := trace.NewTracerProvider(trace.WithSyncer(exporter))
223+
224+
tracer := newMCPTracer(tp.Tracer("test"), autoprop.NewTextMapPropagator())
225+
226+
reqID, _ := jsonrpc.MakeID("test-id")
227+
req := &jsonrpc.Request{ID: reqID, Method: tt.method}
228+
229+
span := tracer.StartSpanAndInjectMeta(context.Background(), req, tt.params)
230+
require.NotNil(t, span)
231+
span.EndSpan()
232+
233+
spans := exporter.GetSpans()
234+
require.Len(t, spans, 1)
235+
actualSpan := spans[0]
236+
237+
require.Equal(t, tt.expectedSpanName, actualSpan.Name)
238+
require.Equal(t, oteltrace.SpanKindClient, actualSpan.SpanKind)
239+
})
240+
}
241+
}

tests/extproc/mcp/env.go

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@ import (
1717

1818
"github.com/modelcontextprotocol/go-sdk/mcp"
1919
"github.com/stretchr/testify/require"
20-
v1 "go.opentelemetry.io/proto/otlp/trace/v1"
2120

2221
"github.com/envoyproxy/ai-gateway/internal/filterapi"
2322
"github.com/envoyproxy/ai-gateway/internal/testing/testotel"
@@ -241,16 +240,18 @@ func (m *mcpEnv) newSession(t *testing.T) *mcpSession {
241240
ret.session, err = m.client.Connect(t.Context(), &mcp.StreamableClientTransport{Endpoint: m.baseURL}, nil)
242241
require.NoError(t, err)
243242
span := m.collector.TakeSpan()
244-
require.Equal(t, "mcp.request", span.Name)
245-
require.Equal(t, v1.Span_SPAN_KIND_CLIENT, span.Kind)
246-
247243
t.Log("created new MCP session with ID ", ret.session.ID(), ", first span: ", span.String())
248-
require.NotNil(t, span)
244+
requireMCPSpan(t, span, "Initialize", map[string]string{
245+
"mcp.method.name": "initialize",
246+
"mcp.client.name": "demo-http-client",
247+
"mcp.client.title": "",
248+
"mcp.client.version": "0.1.0",
249+
})
249250

250251
// **NOTE*** Do not add any direct access to the in-memory server session. Otherwise, the tests will result in
251252
// not being able to run in end-to-end tests. The test code must solely operate through the client side sessions.
252253
t.Cleanup(func() {
253-
require.NoError(t, ret.session.Close())
254+
_ = ret.session.Close()
254255
m.mux.Lock()
255256
defer m.mux.Unlock()
256257
delete(m.sessions, ret.session.ID())

0 commit comments

Comments
 (0)