diff --git a/pkg/telemetry/integration_test.go b/pkg/telemetry/integration_test.go index 36a327fb02..addfe823b1 100644 --- a/pkg/telemetry/integration_test.go +++ b/pkg/telemetry/integration_test.go @@ -233,7 +233,7 @@ func TestTelemetryIntegration_WithRealProviders(t *testing.T) { require.Len(t, spans, 1) span := spans[0] - assert.Equal(t, "mcp.tools/call", span.Name) + assert.Equal(t, "tools/call", span.Name) // Verify span attributes attrs := span.Attributes @@ -242,14 +242,22 @@ func TestTelemetryIntegration_WithRealProviders(t *testing.T) { attrMap[string(attr.Key)] = attr.Value.AsInterface() } + // New OTEL semantic convention attributes (always present) + assert.Equal(t, "tools/call", attrMap["mcp.method.name"]) + assert.Equal(t, testToolName, attrMap["gen_ai.tool.name"]) + assert.Equal(t, "test-123", attrMap["jsonrpc.request.id"]) + assert.Equal(t, "POST", attrMap["http.request.method"]) + assert.Equal(t, int64(200), attrMap["http.response.status_code"]) + + // Legacy attributes (present because UseLegacyAttributes=true) assert.Equal(t, "tools/call", attrMap["mcp.method"]) assert.Equal(t, testToolName, attrMap["mcp.tool.name"]) assert.Equal(t, "test-123", attrMap["mcp.request.id"]) assert.Equal(t, "POST", attrMap["http.method"]) assert.Equal(t, int64(200), attrMap["http.status_code"]) - // Verify sensitive data is redacted - if toolArgs, exists := attrMap["mcp.tool.arguments"]; exists { + // Verify sensitive data is redacted in new attribute name + if toolArgs, exists := attrMap["gen_ai.tool.call.arguments"]; exists { argsStr := toolArgs.(string) assert.Contains(t, argsStr, "api_key=[REDACTED]") assert.Contains(t, argsStr, "query=test query") diff --git a/pkg/telemetry/middleware.go b/pkg/telemetry/middleware.go index 833c3802f1..2edb1454a5 100644 --- a/pkg/telemetry/middleware.go +++ b/pkg/telemetry/middleware.go @@ -29,6 +29,12 @@ import ( const ( // instrumentationName is the name of this instrumentation package instrumentationName = "github.com/stacklok/toolhive/pkg/telemetry" + // methodPromptsGet is the MCP method name for prompts/get + methodPromptsGet = "prompts/get" + // networkTransportTCP is the OTEL value for TCP transport + networkTransportTCP = "tcp" + // networkProtocolHTTP is the OTEL value for HTTP protocol + networkProtocolHTTP = "http" ) // HTTPMiddleware provides OpenTelemetry instrumentation for HTTP requests. @@ -125,7 +131,10 @@ func (m *HTTPMiddleware) Handler(next http.Handler) http.Handler { )) // Create span name based on MCP method if available, otherwise use HTTP method + path - spanName := m.createSpanName(ctx, r) + spanName := m.createSpanName(ctx) + if spanName == "" { + spanName = fmt.Sprintf("%s %s", r.Method, r.URL.Path) + } ctx, span := m.tracer.Start(ctx, spanName, trace.WithSpanKind(trace.SpanKindServer)) defer span.End() @@ -158,19 +167,35 @@ func (m *HTTPMiddleware) Handler(next http.Handler) http.Handler { }) } -// createSpanName creates an appropriate span name based on available context. -func (*HTTPMiddleware) createSpanName(ctx context.Context, r *http.Request) string { - // Try to get MCP method from parsed data - if mcpMethod := mcpparser.GetMCPMethod(ctx); mcpMethod != "" { - return fmt.Sprintf("mcp.%s", mcpMethod) +func (*HTTPMiddleware) createSpanName(ctx context.Context) string { + parsedMCP := mcpparser.GetParsedMCPRequest(ctx) + if parsedMCP == nil || parsedMCP.Method == "" { + return "" } - - // Fall back to HTTP method + path - return fmt.Sprintf("%s %s", r.Method, r.URL.Path) + return parsedMCP.Method } // addHTTPAttributes adds standard HTTP attributes to the span. func (m *HTTPMiddleware) addHTTPAttributes(span trace.Span, r *http.Request) { + // New OTEL HTTP semantic convention attributes (always emitted) + span.SetAttributes( + attribute.String("http.request.method", r.Method), + attribute.String("url.full", r.URL.String()), + attribute.String("url.scheme", r.URL.Scheme), + attribute.String("server.address", r.Host), + attribute.String("url.path", r.URL.Path), + attribute.String("user_agent.original", r.UserAgent()), + ) + + if r.ContentLength > 0 { + span.SetAttributes(attribute.Int64("http.request.body.size", r.ContentLength)) + } + + if r.URL.RawQuery != "" { + span.SetAttributes(attribute.String("url.query", r.URL.RawQuery)) + } + + // Legacy attribute names (emitted only when UseLegacyAttributes is true) if m.config.UseLegacyAttributes { span.SetAttributes( attribute.String("http.method", r.Method), @@ -181,12 +206,10 @@ func (m *HTTPMiddleware) addHTTPAttributes(span trace.Span, r *http.Request) { attribute.String("http.user_agent", r.UserAgent()), ) - // Add content length if available if contentLength := r.Header.Get("Content-Length"); contentLength != "" { span.SetAttributes(attribute.String("http.request_content_length", contentLength)) } - // Add query parameters if present if r.URL.RawQuery != "" { span.SetAttributes(attribute.String("http.query", r.URL.RawQuery)) } @@ -219,20 +242,37 @@ func (m *HTTPMiddleware) addMCPAttributes(ctx context.Context, span trace.Span, return } + // New OTEL MCP semantic convention attributes (always emitted) + span.SetAttributes( + attribute.String("mcp.method.name", parsedMCP.Method), + attribute.String("rpc.system.name", "jsonrpc"), + attribute.String("jsonrpc.protocol.version", "2.0"), + ) + + if parsedMCP.ID != nil { + span.SetAttributes(attribute.String("jsonrpc.request.id", formatRequestID(parsedMCP.ID))) + } + + // Resource URI: only set for resource-related methods + if parsedMCP.ResourceID != "" { + switch parsedMCP.Method { + case "resources/read", "resources/subscribe", "resources/unsubscribe", "notifications/resources/updated": + span.SetAttributes(attribute.String("mcp.resource.uri", parsedMCP.ResourceID)) + } + } + + // Legacy attribute names (emitted only when UseLegacyAttributes is true) if m.config.UseLegacyAttributes { - // Add basic MCP attributes span.SetAttributes( attribute.String("mcp.method", parsedMCP.Method), attribute.String("rpc.system", "jsonrpc"), attribute.String("rpc.service", "mcp"), ) - // Add request ID if available if parsedMCP.ID != nil { span.SetAttributes(attribute.String("mcp.request.id", formatRequestID(parsedMCP.ID))) } - // Add resource ID if available if parsedMCP.ResourceID != "" { span.SetAttributes(attribute.String("mcp.resource.id", parsedMCP.ResourceID)) } @@ -241,15 +281,21 @@ func (m *HTTPMiddleware) addMCPAttributes(ctx context.Context, span trace.Span, // Add method-specific attributes m.addMethodSpecificAttributes(span, parsedMCP) - // Extract server name from the request, defaulting to the middleware's configured server name + // Extract server name from the request serverName := m.extractServerName(r) span.SetAttributes(attribute.String("mcp.server.name", serverName)) - // Determine backend transport type - // Note: ToolHive supports multiple transport types including stdio, sse, streamable-http - // The transport should never be empty as both CLI and API have fallbacks to "streamable-http" - // If transport is still empty, it indicates a configuration issue in middleware construction + // Determine backend transport type and map to OTEL network.transport backendTransport := m.extractBackendTransport(r) + networkTransport, protocolName, protocolVersion := mapTransport(backendTransport) + span.SetAttributes(attribute.String("network.transport", networkTransport)) + if protocolName != "" { + span.SetAttributes(attribute.String("network.protocol.name", protocolName)) + } + if protocolVersion != "" { + span.SetAttributes(attribute.String("network.protocol.version", protocolVersion)) + } + if m.config.UseLegacyAttributes { span.SetAttributes(attribute.String("mcp.transport", backendTransport)) } @@ -262,35 +308,43 @@ func (m *HTTPMiddleware) addMCPAttributes(ctx context.Context, span trace.Span, // addMethodSpecificAttributes adds attributes specific to certain MCP methods. func (m *HTTPMiddleware) addMethodSpecificAttributes(span trace.Span, parsedMCP *mcpparser.ParsedMCPRequest) { - if !m.config.UseLegacyAttributes { - return - } - switch parsedMCP.Method { case string(mcp.MethodToolsCall): - // For tool calls, the ResourceID is the tool name + // New gen_ai namespace attributes (always emitted) if parsedMCP.ResourceID != "" { - span.SetAttributes(attribute.String("mcp.tool.name", parsedMCP.ResourceID)) + span.SetAttributes(attribute.String("gen_ai.tool.name", parsedMCP.ResourceID)) } - // Add sanitized arguments - if args := m.sanitizeArguments(parsedMCP.Arguments); args != "" { - span.SetAttributes(attribute.String("mcp.tool.arguments", args)) + span.SetAttributes(attribute.String("gen_ai.operation.name", "execute_tool")) + + sanitizedArgs := m.sanitizeArguments(parsedMCP.Arguments) + if sanitizedArgs != "" { + span.SetAttributes(attribute.String("gen_ai.tool.call.arguments", sanitizedArgs)) } - case "resources/read": - // For resource reads, the ResourceID is the URI - if parsedMCP.ResourceID != "" { - span.SetAttributes(attribute.String("mcp.resource.uri", parsedMCP.ResourceID)) + // Legacy names + if m.config.UseLegacyAttributes { + if parsedMCP.ResourceID != "" { + span.SetAttributes(attribute.String("mcp.tool.name", parsedMCP.ResourceID)) + } + if sanitizedArgs != "" { + span.SetAttributes(attribute.String("mcp.tool.arguments", sanitizedArgs)) + } } - case "prompts/get": - // For prompt gets, the ResourceID is the prompt name + case methodPromptsGet: + // New gen_ai namespace attribute (always emitted) if parsedMCP.ResourceID != "" { - span.SetAttributes(attribute.String("mcp.prompt.name", parsedMCP.ResourceID)) + span.SetAttributes(attribute.String("gen_ai.prompt.name", parsedMCP.ResourceID)) + } + + // Legacy name + if m.config.UseLegacyAttributes { + if parsedMCP.ResourceID != "" { + span.SetAttributes(attribute.String("mcp.prompt.name", parsedMCP.ResourceID)) + } } case "initialize": - // For initialize, the ResourceID is the client name if parsedMCP.ResourceID != "" { span.SetAttributes(attribute.String("mcp.client.name", parsedMCP.ResourceID)) } @@ -331,6 +385,19 @@ func (m *HTTPMiddleware) extractBackendTransport(r *http.Request) string { return m.transport } +func mapTransport(mcpTransport string) (networkTransport, protocolName, protocolVersion string) { + switch mcpTransport { + case "stdio": + return "pipe", "", "" + case "sse": + return networkTransportTCP, networkProtocolHTTP, "1.1" + case "streamable-http": + return networkTransportTCP, networkProtocolHTTP, "2" + default: + return networkTransportTCP, networkProtocolHTTP, "" + } +} + // sanitizeArguments converts arguments to a safe string representation. func (m *HTTPMiddleware) sanitizeArguments(arguments map[string]interface{}) string { if len(arguments) == 0 { @@ -397,7 +464,13 @@ func formatRequestID(id interface{}) string { // finalizeSpan adds response attributes and sets the span status. func (m *HTTPMiddleware) finalizeSpan(span trace.Span, rw *responseWriter, duration time.Duration) { - // Add response attributes + // New OTEL HTTP semantic convention response attributes (always emitted) + span.SetAttributes( + attribute.Int("http.response.status_code", rw.statusCode), + attribute.Int64("http.response.body.size", rw.bytesWritten), + ) + + // Legacy response attributes if m.config.UseLegacyAttributes { span.SetAttributes( attribute.Int("http.status_code", rw.statusCode), @@ -534,10 +607,18 @@ func (m *HTTPMiddleware) recordSSEConnection(ctx context.Context, r *http.Reques m.addHTTPAttributes(span, r) // Add SSE-specific attributes + networkTransport, protocolName, protocolVersion := mapTransport(m.transport) span.SetAttributes( attribute.String("sse.event_type", "connection_established"), attribute.String("mcp.server.name", m.serverName), + attribute.String("network.transport", networkTransport), ) + if protocolName != "" { + span.SetAttributes(attribute.String("network.protocol.name", protocolName)) + } + if protocolVersion != "" { + span.SetAttributes(attribute.String("network.protocol.version", protocolVersion)) + } if m.config.UseLegacyAttributes { span.SetAttributes(attribute.String("mcp.transport", m.transport)) } diff --git a/pkg/telemetry/middleware_test.go b/pkg/telemetry/middleware_test.go index ab167aedfe..ae99babcff 100644 --- a/pkg/telemetry/middleware_test.go +++ b/pkg/telemetry/middleware_test.go @@ -135,30 +135,37 @@ func TestHTTPMiddleware_CreateSpanName(t *testing.T) { tests := []struct { name string mcpMethod string - httpMethod string - path string + resourceID string expectedSpan string }{ { - name: "with MCP method", + name: "tools/call with resource ID", mcpMethod: "tools/call", - httpMethod: "POST", - path: "/messages", - expectedSpan: "mcp.tools/call", + resourceID: "github_search", + expectedSpan: "tools/call", }, { - name: "without MCP method", - mcpMethod: "", - httpMethod: "GET", - path: "/health", - expectedSpan: "GET /health", + name: "prompts/get with resource ID", + mcpMethod: "prompts/get", + resourceID: "code_review", + expectedSpan: "prompts/get", + }, + { + name: "tools/call without resource ID", + mcpMethod: "tools/call", + resourceID: "", + expectedSpan: "tools/call", }, { - name: "with different MCP method", + name: "resources/read (no resource appended)", mcpMethod: "resources/read", - httpMethod: "POST", - path: "/api/v1/messages", - expectedSpan: "mcp.resources/read", + resourceID: "file://test.txt", + expectedSpan: "resources/read", + }, + { + name: "no MCP method returns empty", + mcpMethod: "", + expectedSpan: "", }, } @@ -166,22 +173,49 @@ func TestHTTPMiddleware_CreateSpanName(t *testing.T) { t.Run(tt.name, func(t *testing.T) { t.Parallel() - req := httptest.NewRequest(tt.httpMethod, tt.path, nil) - ctx := req.Context() + ctx := context.Background() if tt.mcpMethod != "" { mcpRequest := &mcpparser.ParsedMCPRequest{ - Method: tt.mcpMethod, + Method: tt.mcpMethod, + ResourceID: tt.resourceID, } ctx = context.WithValue(ctx, mcpparser.MCPRequestContextKey, mcpRequest) } - spanName := middleware.createSpanName(ctx, req) + spanName := middleware.createSpanName(ctx) assert.Equal(t, tt.expectedSpan, spanName) }) } } +func TestMapTransport(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + transport string + expectedNetwork string + expectedProtocol string + expectedVersion string + }{ + {"stdio", "stdio", "pipe", "", ""}, + {"sse", "sse", "tcp", "http", "1.1"}, + {"streamable-http", "streamable-http", "tcp", "http", "2"}, + {"unknown defaults to tcp", "unknown", "tcp", "http", ""}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + network, protocol, version := mapTransport(tt.transport) + assert.Equal(t, tt.expectedNetwork, network) + assert.Equal(t, tt.expectedProtocol, protocol) + assert.Equal(t, tt.expectedVersion, version) + }) + } +} + func TestHTTPMiddleware_AddHTTPAttributes_Logic(t *testing.T) { t.Parallel() @@ -535,72 +569,6 @@ func TestHTTPMiddleware_ExtractBackendTransport(t *testing.T) { } } -func TestHTTPMiddleware_FinalizeSpan_Logic(t *testing.T) { - t.Parallel() - - middleware := &HTTPMiddleware{} - - tests := []struct { - name string - statusCode int - bytesWritten int64 - duration time.Duration - expectedStatus codes.Code - }{ - { - name: "success response", - statusCode: 200, - bytesWritten: 1024, - duration: 100 * time.Millisecond, - expectedStatus: codes.Ok, - }, - { - name: "client error", - statusCode: 400, - bytesWritten: 256, - duration: 50 * time.Millisecond, - expectedStatus: codes.Error, - }, - { - name: "server error", - statusCode: 500, - bytesWritten: 128, - duration: 200 * time.Millisecond, - expectedStatus: codes.Error, - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - t.Parallel() - - rw := &responseWriter{ - statusCode: tt.statusCode, - bytesWritten: tt.bytesWritten, - } - - // Test the logic for determining status codes - var expectedStatus codes.Code - if tt.statusCode >= 400 { - expectedStatus = codes.Error - } else { - expectedStatus = codes.Ok - } - - assert.Equal(t, tt.expectedStatus, expectedStatus) - assert.Equal(t, tt.statusCode, rw.statusCode) - assert.Equal(t, tt.bytesWritten, rw.bytesWritten) - - // Test duration calculation - durationMs := float64(tt.duration.Nanoseconds()) / 1e6 - assert.Greater(t, durationMs, 0.0) - - // Test middleware exists - assert.NotNil(t, middleware) - }) - } -} - func TestResponseWriter(t *testing.T) { t.Parallel() @@ -880,7 +848,9 @@ func TestHTTPMiddleware_addEnvironmentAttributes(t *testing.T) { // mockSpan implements trace.Span for testing type mockSpan struct { trace.Span - attributes map[string]interface{} + attributes map[string]interface{} + statusCode codes.Code + statusDescription string } func (m *mockSpan) SetAttributes(kv ...attribute.KeyValue) { @@ -894,9 +864,12 @@ func (*mockSpan) AddEvent(string, ...trace.EventOption) {} func (*mockSpan) IsRecording() bool { return true } func (*mockSpan) RecordError(error, ...trace.EventOption) {} func (*mockSpan) SpanContext() trace.SpanContext { return trace.SpanContext{} } -func (*mockSpan) SetStatus(codes.Code, string) {} -func (*mockSpan) SetName(string) {} -func (*mockSpan) TracerProvider() trace.TracerProvider { return tracenoop.NewTracerProvider() } +func (s *mockSpan) SetStatus(code codes.Code, description string) { + s.statusCode = code + s.statusDescription = description +} +func (*mockSpan) SetName(string) {} +func (*mockSpan) TracerProvider() trace.TracerProvider { return tracenoop.NewTracerProvider() } // contains checks if a slice contains a string func contains(slice []string, item string) bool { @@ -1596,7 +1569,7 @@ func TestHTTPMiddleware_LegacyAttributes_Disabled(t *testing.T) { testFunc func(t *testing.T, middleware *HTTPMiddleware, mockSpan *mockSpan) }{ { - name: "addHTTPAttributes - no attributes set", + name: "addHTTPAttributes - only new OTEL names, no legacy", testFunc: func(t *testing.T, middleware *HTTPMiddleware, span *mockSpan) { t.Helper() req := httptest.NewRequest("POST", "http://localhost:8080/messages", nil) @@ -1604,12 +1577,25 @@ func TestHTTPMiddleware_LegacyAttributes_Disabled(t *testing.T) { middleware.addHTTPAttributes(span, req) - // No HTTP attributes should be set - assert.Empty(t, span.attributes, "Expected no attributes with UseLegacyAttributes disabled") + // New OTEL semconv names should be present + assert.Contains(t, span.attributes, "http.request.method") + assert.Contains(t, span.attributes, "url.full") + assert.Contains(t, span.attributes, "url.scheme") + assert.Contains(t, span.attributes, "server.address") + assert.Contains(t, span.attributes, "url.path") + assert.Contains(t, span.attributes, "user_agent.original") + + // Legacy names should NOT be present + assert.NotContains(t, span.attributes, "http.method") + assert.NotContains(t, span.attributes, "http.url") + assert.NotContains(t, span.attributes, "http.scheme") + assert.NotContains(t, span.attributes, "http.host") + assert.NotContains(t, span.attributes, "http.target") + assert.NotContains(t, span.attributes, "http.user_agent") }, }, { - name: "addMCPAttributes - only unconditional attributes set (non-batch)", + name: "addMCPAttributes - new names present, legacy absent", testFunc: func(t *testing.T, middleware *HTTPMiddleware, span *mockSpan) { t.Helper() req := httptest.NewRequest("POST", "/messages", nil) @@ -1618,23 +1604,22 @@ func TestHTTPMiddleware_LegacyAttributes_Disabled(t *testing.T) { ID: "test-123", ResourceID: "github_search", IsRequest: true, - IsBatch: false, } ctx := context.WithValue(req.Context(), mcpparser.MCPRequestContextKey, mcpRequest) middleware.addMCPAttributes(ctx, span, req) - // Only unconditional attributes should be set - // mcp.server.name is always set + // New OTEL semconv names should be present + assert.Contains(t, span.attributes, "mcp.method.name") + assert.Contains(t, span.attributes, "rpc.system.name") + assert.Contains(t, span.attributes, "jsonrpc.request.id") + assert.Contains(t, span.attributes, "jsonrpc.protocol.version") + assert.Contains(t, span.attributes, "network.transport") assert.Contains(t, span.attributes, "mcp.server.name") - assert.Equal(t, "github", span.attributes["mcp.server.name"]) + assert.NotContains(t, span.attributes, "network.protocol.version") - // mcp.is_batch is only set when IsBatch is true (not for false) - assert.NotContains(t, span.attributes, "mcp.is_batch") - - // Legacy attributes should NOT be set + // Legacy names should NOT be present assert.NotContains(t, span.attributes, "mcp.method") - assert.NotContains(t, span.attributes, "rpc.system") assert.NotContains(t, span.attributes, "rpc.service") assert.NotContains(t, span.attributes, "mcp.request.id") assert.NotContains(t, span.attributes, "mcp.resource.id") @@ -1642,67 +1627,123 @@ func TestHTTPMiddleware_LegacyAttributes_Disabled(t *testing.T) { }, }, { - name: "addMCPAttributes - batch request includes mcp.is_batch", + name: "addMethodSpecificAttributes - new gen_ai names, no legacy", + testFunc: func(t *testing.T, middleware *HTTPMiddleware, span *mockSpan) { + t.Helper() + parsedMCP := &mcpparser.ParsedMCPRequest{ + Method: "tools/call", + ResourceID: "github_search", + Arguments: map[string]interface{}{"query": "test"}, + } + + middleware.addMethodSpecificAttributes(span, parsedMCP) + + // New gen_ai names should be present + assert.Contains(t, span.attributes, "gen_ai.tool.name") + assert.Contains(t, span.attributes, "gen_ai.operation.name") + assert.Contains(t, span.attributes, "gen_ai.tool.call.arguments") + + // Legacy names should NOT be present + assert.NotContains(t, span.attributes, "mcp.tool.name") + assert.NotContains(t, span.attributes, "mcp.tool.arguments") + }, + }, + { + name: "finalizeSpan - new response names, no legacy", + testFunc: func(t *testing.T, middleware *HTTPMiddleware, span *mockSpan) { + t.Helper() + rw := &responseWriter{statusCode: 200, bytesWritten: 1024} + + middleware.finalizeSpan(span, rw, 100*time.Millisecond) + + // New names should be present + assert.Contains(t, span.attributes, "http.response.status_code") + assert.Contains(t, span.attributes, "http.response.body.size") + + // Status should be set to Ok for 200 + assert.Equal(t, codes.Ok, span.statusCode) + + // Legacy names should NOT be present + assert.NotContains(t, span.attributes, "http.status_code") + assert.NotContains(t, span.attributes, "http.response_content_length") + assert.NotContains(t, span.attributes, "http.duration_ms") + }, + }, + { + name: "finalizeSpan - error status code", + testFunc: func(t *testing.T, middleware *HTTPMiddleware, span *mockSpan) { + t.Helper() + rw := &responseWriter{statusCode: 500, bytesWritten: 128} + + middleware.finalizeSpan(span, rw, 50*time.Millisecond) + + // Status should be set to Error for 500 + assert.Equal(t, codes.Error, span.statusCode) + assert.Equal(t, "HTTP 500", span.statusDescription) + }, + }, + { + name: "addMCPAttributes - resource URI for resources/read", testFunc: func(t *testing.T, middleware *HTTPMiddleware, span *mockSpan) { t.Helper() req := httptest.NewRequest("POST", "/messages", nil) mcpRequest := &mcpparser.ParsedMCPRequest{ - Method: "tools/list", - ID: "batch-123", - IsRequest: true, - IsBatch: true, + Method: "resources/read", + ID: "test-789", + ResourceID: "file://test.txt", + IsRequest: true, } ctx := context.WithValue(req.Context(), mcpparser.MCPRequestContextKey, mcpRequest) middleware.addMCPAttributes(ctx, span, req) - // Unconditional attributes - assert.Contains(t, span.attributes, "mcp.server.name") - assert.Contains(t, span.attributes, "mcp.is_batch") - assert.Equal(t, true, span.attributes["mcp.is_batch"]) - - // Legacy attributes should NOT be set - assert.NotContains(t, span.attributes, "mcp.method") - assert.NotContains(t, span.attributes, "rpc.system") + // mcp.resource.uri should be present for resources/read + assert.Contains(t, span.attributes, "mcp.resource.uri") + assert.Equal(t, "file://test.txt", span.attributes["mcp.resource.uri"]) }, }, { - name: "addMethodSpecificAttributes - early return, no attributes", + name: "addMCPAttributes - no resource URI for tools/call", testFunc: func(t *testing.T, middleware *HTTPMiddleware, span *mockSpan) { t.Helper() - parsedMCP := &mcpparser.ParsedMCPRequest{ + req := httptest.NewRequest("POST", "/messages", nil) + mcpRequest := &mcpparser.ParsedMCPRequest{ Method: "tools/call", + ID: "test-999", ResourceID: "github_search", - Arguments: map[string]interface{}{ - "query": "test", - }, + IsRequest: true, } + ctx := context.WithValue(req.Context(), mcpparser.MCPRequestContextKey, mcpRequest) - middleware.addMethodSpecificAttributes(span, parsedMCP) + middleware.addMCPAttributes(ctx, span, req) - // No attributes should be set due to early return - assert.Empty(t, span.attributes, "Expected no attributes with UseLegacyAttributes disabled") + // mcp.resource.uri should NOT be present for tools/call + assert.NotContains(t, span.attributes, "mcp.resource.uri") }, }, { - name: "finalizeSpan - no http attributes, span status still set", - testFunc: func(t *testing.T, middleware *HTTPMiddleware, span *mockSpan) { + name: "addMCPAttributes - network.protocol.version for SSE", + testFunc: func(t *testing.T, _ *HTTPMiddleware, span *mockSpan) { t.Helper() - rw := &responseWriter{ - statusCode: 200, - bytesWritten: 1024, + middlewareSSE := &HTTPMiddleware{ + config: Config{UseLegacyAttributes: false}, + serverName: "github", + transport: "sse", } - duration := 100 * time.Millisecond - - middleware.finalizeSpan(span, rw, duration) + req := httptest.NewRequest("POST", "/messages", nil) + mcpRequest := &mcpparser.ParsedMCPRequest{ + Method: "tools/call", + ID: "test-sse", + IsRequest: true, + } + ctx := context.WithValue(req.Context(), mcpparser.MCPRequestContextKey, mcpRequest) - // HTTP attributes should NOT be set - assert.NotContains(t, span.attributes, "http.status_code") - assert.NotContains(t, span.attributes, "http.response_content_length") - assert.NotContains(t, span.attributes, "http.duration_ms") + middlewareSSE.addMCPAttributes(ctx, span, req) - // Span status is still set (mockSpan has no-op SetStatus) - // We can't verify SetStatus was called, but we verify no attributes were set + // network.protocol.version should be present for SSE + assert.Contains(t, span.attributes, "network.protocol.version") + assert.Equal(t, "1.1", span.attributes["network.protocol.version"]) + assert.Equal(t, "http", span.attributes["network.protocol.name"]) }, }, } @@ -1711,19 +1752,12 @@ func TestHTTPMiddleware_LegacyAttributes_Disabled(t *testing.T) { t.Run(tt.name, func(t *testing.T) { t.Parallel() - // Create middleware with UseLegacyAttributes disabled middleware := &HTTPMiddleware{ - config: Config{ - UseLegacyAttributes: false, - }, + config: Config{UseLegacyAttributes: false}, serverName: "github", transport: "stdio", } - - // Create mock span span := &mockSpan{attributes: make(map[string]interface{})} - - // Run test tt.testFunc(t, middleware, span) }) } @@ -1737,7 +1771,7 @@ func TestHTTPMiddleware_LegacyAttributes_Enabled(t *testing.T) { testFunc func(t *testing.T, middleware *HTTPMiddleware, mockSpan *mockSpan) }{ { - name: "addHTTPAttributes - all attributes set", + name: "addHTTPAttributes - both new and legacy names present", testFunc: func(t *testing.T, middleware *HTTPMiddleware, span *mockSpan) { t.Helper() req := httptest.NewRequest("POST", "http://localhost:8080/api/v1/messages?session=123", nil) @@ -1746,21 +1780,21 @@ func TestHTTPMiddleware_LegacyAttributes_Enabled(t *testing.T) { middleware.addHTTPAttributes(span, req) - // All HTTP attributes should be set - assert.Contains(t, span.attributes, "http.method") + // New OTEL semconv names + assert.Equal(t, "POST", span.attributes["http.request.method"]) + assert.Equal(t, "http", span.attributes["url.scheme"]) + assert.Equal(t, "localhost:8080", span.attributes["server.address"]) + assert.Equal(t, "test-client/1.0", span.attributes["user_agent.original"]) + + // Legacy names also present assert.Equal(t, "POST", span.attributes["http.method"]) - assert.Contains(t, span.attributes, "http.url") - assert.Contains(t, span.attributes, "http.scheme") assert.Equal(t, "http", span.attributes["http.scheme"]) - assert.Contains(t, span.attributes, "http.host") assert.Equal(t, "localhost:8080", span.attributes["http.host"]) - assert.Contains(t, span.attributes, "http.target") - assert.Contains(t, span.attributes, "http.user_agent") assert.Equal(t, "test-client/1.0", span.attributes["http.user_agent"]) }, }, { - name: "addMCPAttributes - all attributes including legacy", + name: "addMCPAttributes - both new and legacy names present", testFunc: func(t *testing.T, middleware *HTTPMiddleware, span *mockSpan) { t.Helper() req := httptest.NewRequest("POST", "/messages", nil) @@ -1769,74 +1803,66 @@ func TestHTTPMiddleware_LegacyAttributes_Enabled(t *testing.T) { ID: "test-456", ResourceID: "github_search", IsRequest: true, - IsBatch: false, } ctx := context.WithValue(req.Context(), mcpparser.MCPRequestContextKey, mcpRequest) middleware.addMCPAttributes(ctx, span, req) - // Legacy attributes should be set - assert.Contains(t, span.attributes, "mcp.method") + // New names + assert.Equal(t, "tools/call", span.attributes["mcp.method.name"]) + assert.Equal(t, "test-456", span.attributes["jsonrpc.request.id"]) + assert.Equal(t, "jsonrpc", span.attributes["rpc.system.name"]) + assert.Contains(t, span.attributes, "network.transport") + + // Legacy names also present assert.Equal(t, "tools/call", span.attributes["mcp.method"]) - assert.Contains(t, span.attributes, "rpc.system") assert.Equal(t, "jsonrpc", span.attributes["rpc.system"]) - assert.Contains(t, span.attributes, "rpc.service") assert.Equal(t, "mcp", span.attributes["rpc.service"]) - assert.Contains(t, span.attributes, "mcp.request.id") assert.Equal(t, "test-456", span.attributes["mcp.request.id"]) - assert.Contains(t, span.attributes, "mcp.resource.id") assert.Equal(t, "github_search", span.attributes["mcp.resource.id"]) - assert.Contains(t, span.attributes, "mcp.transport") assert.Equal(t, "stdio", span.attributes["mcp.transport"]) - - // Unconditional attributes - assert.Contains(t, span.attributes, "mcp.server.name") - assert.Equal(t, "github", span.attributes["mcp.server.name"]) - - // mcp.is_batch is only set when IsBatch is true - assert.NotContains(t, span.attributes, "mcp.is_batch") }, }, { - name: "addMethodSpecificAttributes - tool name set for tools/call", + name: "addMethodSpecificAttributes - both gen_ai and legacy names", testFunc: func(t *testing.T, middleware *HTTPMiddleware, span *mockSpan) { t.Helper() parsedMCP := &mcpparser.ParsedMCPRequest{ Method: "tools/call", ResourceID: "github_search", - Arguments: map[string]interface{}{ - "query": "test", - }, + Arguments: map[string]interface{}{"query": "test"}, } middleware.addMethodSpecificAttributes(span, parsedMCP) - // Tool-specific attributes should be set - // Tool name comes from ResourceID - assert.Contains(t, span.attributes, "mcp.tool.name") + // New gen_ai names + assert.Equal(t, "github_search", span.attributes["gen_ai.tool.name"]) + assert.Equal(t, "execute_tool", span.attributes["gen_ai.operation.name"]) + + // Legacy names also present assert.Equal(t, "github_search", span.attributes["mcp.tool.name"]) }, }, { - name: "finalizeSpan - http attributes set", + name: "finalizeSpan - both new and legacy response names", testFunc: func(t *testing.T, middleware *HTTPMiddleware, span *mockSpan) { t.Helper() - rw := &responseWriter{ - statusCode: 201, - bytesWritten: 2048, - } + rw := &responseWriter{statusCode: 201, bytesWritten: 2048} duration := 250 * time.Millisecond middleware.finalizeSpan(span, rw, duration) - // HTTP attributes should be set - assert.Contains(t, span.attributes, "http.status_code") + // New names + assert.Equal(t, int64(201), span.attributes["http.response.status_code"]) + assert.Equal(t, int64(2048), span.attributes["http.response.body.size"]) + + // Status should be set to Ok for 201 + assert.Equal(t, codes.Ok, span.statusCode) + + // Legacy names also present assert.Equal(t, int64(201), span.attributes["http.status_code"]) - assert.Contains(t, span.attributes, "http.response_content_length") assert.Equal(t, int64(2048), span.attributes["http.response_content_length"]) assert.Contains(t, span.attributes, "http.duration_ms") - durationMs := float64(duration.Nanoseconds()) / 1e6 - assert.Equal(t, durationMs, span.attributes["http.duration_ms"]) }, }, } @@ -1845,19 +1871,12 @@ func TestHTTPMiddleware_LegacyAttributes_Enabled(t *testing.T) { t.Run(tt.name, func(t *testing.T) { t.Parallel() - // Create middleware with UseLegacyAttributes enabled middleware := &HTTPMiddleware{ - config: Config{ - UseLegacyAttributes: true, - }, + config: Config{UseLegacyAttributes: true}, serverName: "github", transport: "stdio", } - - // Create mock span span := &mockSpan{attributes: make(map[string]interface{})} - - // Run test tt.testFunc(t, middleware, span) }) }