Fixed span attributes, updated mcp model, lint, semconv changes

Author: Johnnyl202

aws-opentelemetry-distro/pyproject.toml

@@ -89,7 +89,7 @@ dependencies = [
 # If a new patch is added into the list, it must also be added into tox.ini, dev-requirements.txt and _instrumentation_patch
 patch = [
   "botocore ~= 1.0",
-  "mcp >= 1.1.0"
+  "mcp >= 1.6.0"
 ]
 test = []
 

aws-opentelemetry-distro/src/amazon/opentelemetry/distro/instrumentation/mcp/constants.py

@@ -0,0 +1,38 @@
+# Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
+# SPDX-License-Identifier: Apache-2.0
+
+"""
+MCP (Model Context Protocol) Constants for OpenTelemetry instrumentation.
+
+This module defines constants and configuration variables used by the MCP instrumentor.
+"""
+
+
+class MCPTraceContext:
+    """Constants for MCP distributed tracing context propagation."""
+
+    TRACEPARENT_HEADER = "traceparent"
+    """
+    W3C Trace Context traceparent header name.
+    Used for propagating trace context in MCP request metadata.
+    """
+
+    TRACE_FLAGS_SAMPLED = "01"
+    """
+    W3C Trace Context flags indicating the trace is sampled.
+    """
+
+    TRACEPARENT_VERSION = "00"
+    """
+    W3C Trace Context version identifier.
+    """
+
+
+class MCPEnvironmentVariables:
+    """Environment variable names for MCP instrumentation configuration."""
+
+    SERVER_NAME = "MCP_INSTRUMENTATION_SERVER_NAME"
+    """
+    Environment variable to override the default MCP server name.
+    Default value: "mcp server"
+    """

aws-opentelemetry-distro/src/amazon/opentelemetry/distro/instrumentation/mcp/mcp_instrumentor.py

@@ -2,14 +2,16 @@
 # SPDX-License-Identifier: Apache-2.0
 from typing import Any, Callable, Collection, Dict, Tuple
 
-from mcp import ClientRequest
 from wrapt import register_post_import_hook, wrap_function_wrapper
 
 from opentelemetry import trace
 from opentelemetry.instrumentation.instrumentor import BaseInstrumentor
 from opentelemetry.instrumentation.utils import unwrap
-from .semconv import MCPAttributes, MCPSpanNames, MCPOperations, MCPTraceContext, MCPEnvironmentVariables
-_instruments = ("mcp >= 1.6.0",)
+from opentelemetry.semconv.trace import SpanAttributes
+
+from .constants import MCPEnvironmentVariables, MCPTraceContext
+from .semconv import MCPAttributes, MCPOperations, MCPSpanNames
+
 
 class MCPInstrumentor(BaseInstrumentor):
     """
@@ -22,7 +24,7 @@ def __init__(self):
 
     @staticmethod
     def instrumentation_dependencies() -> Collection[str]:
-        return _instruments
+        return ("mcp >= 1.6.0",)
 
     def _instrument(self, **kwargs: Any) -> None:
         tracer_provider = kwargs.get("tracer_provider")
@@ -65,7 +67,9 @@ def _wrap_send_request(
         """
 
         async def async_wrapper():
-            with self.tracer.start_as_current_span(MCPSpanNames.CLIENT_SEND_REQUEST, kind=trace.SpanKind.CLIENT) as span:
+            with self.tracer.start_as_current_span(
+                MCPSpanNames.CLIENT_SEND_REQUEST, kind=trace.SpanKind.CLIENT
+            ) as span:
                 span_ctx = span.get_span_context()
                 request = args[0] if len(args) > 0 else kwargs.get("request")
                 if request:
@@ -122,11 +126,12 @@ async def _wrap_handle_request(
         else:
             return await wrapped(*args, **kwargs)
 
-    def _generate_mcp_attributes(self, span: trace.Span, request: ClientRequest, is_client: bool) -> None:
+    @staticmethod
+    def _generate_mcp_attributes(span: trace.Span, request: Any, is_client: bool) -> None:
         import mcp.types as types  # pylint: disable=import-outside-toplevel,consider-using-from-import
 
         operation = MCPOperations.UNKNOWN_OPERATION
-        
+
         if isinstance(request, types.ListToolsRequest):
             operation = MCPOperations.LIST_TOOL
             span.set_attribute(MCPAttributes.MCP_LIST_TOOLS, True)
@@ -142,11 +147,11 @@ def _generate_mcp_attributes(self, span: trace.Span, request: ClientRequest, is_
             span.set_attribute(MCPAttributes.MCP_INITIALIZE, True)
             if is_client:
                 span.update_name(MCPSpanNames.CLIENT_INITIALIZE)
-        
+
         if is_client:
-            self._add_client_attributes(span, operation, request)
+            MCPInstrumentor._add_client_attributes(span, operation, request)
         else:
-            self._add_server_attributes(span, operation, request)
+            MCPInstrumentor._add_server_attributes(span, operation, request)
 
     @staticmethod
     def _inject_trace_context(request_data: Dict[str, Any], span_ctx) -> None:
@@ -179,7 +184,8 @@ def _extract_span_context_from_traceparent(traceparent: str):
         return None
 
     @staticmethod
-    def _get_mcp_operation(req: ClientRequest) -> str:
+    def _get_mcp_operation(req: Any) -> str:
+
         import mcp.types as types  # pylint: disable=import-outside-toplevel,consider-using-from-import
 
         span_name = "unknown"
@@ -188,21 +194,19 @@ def _get_mcp_operation(req: ClientRequest) -> str:
             span_name = MCPSpanNames.TOOLS_LIST
         elif isinstance(req, types.CallToolRequest):
             span_name = MCPSpanNames.tools_call(req.params.name)
-        elif isinstance(req, types.InitializeRequest):
-            span_name = MCPSpanNames.TOOLS_INITIALIZE
         return span_name
 
     @staticmethod
-    def _add_client_attributes(span: trace.Span, operation: str, request: ClientRequest) -> None:
+    def _add_client_attributes(span: trace.Span, operation: str, request: Any) -> None:
         import os  # pylint: disable=import-outside-toplevel
 
         service_name = os.environ.get(MCPEnvironmentVariables.SERVER_NAME, "mcp server")
-        span.set_attribute(MCPAttributes.AWS_REMOTE_SERVICE, service_name)
-        span.set_attribute(MCPAttributes.AWS_REMOTE_OPERATION, operation)
+        span.set_attribute(SpanAttributes.RPC_SERVICE, service_name)
+        span.set_attribute(SpanAttributes.RPC_METHOD, operation)
         if hasattr(request, "params") and request.params and hasattr(request.params, "name"):
             span.set_attribute(MCPAttributes.MCP_TOOL_NAME, request.params.name)
 
     @staticmethod
-    def _add_server_attributes(span: trace.Span, operation: str, request: ClientRequest) -> None:
+    def _add_server_attributes(span: trace.Span, operation: str, request: Any) -> None:
         if hasattr(request, "params") and request.params and hasattr(request.params, "name"):
-            span.set_attribute(MCPAttributes.MCP_TOOL_NAME, request.params.name)
+            span.set_attribute(MCPAttributes.MCP_TOOL_NAME, request.params.name)

aws-opentelemetry-distro/src/amazon/opentelemetry/distro/instrumentation/mcp/semconv.py

@@ -13,7 +13,7 @@ class MCPAttributes:
     """MCP-specific span attributes for OpenTelemetry instrumentation."""
 
     # MCP Operation Type Attributes
-    MCP_INITIALIZE = "mcp.initialize"
+    MCP_INITIALIZE = "notifications/initialize"
     """
     Boolean attribute indicating this span represents an MCP initialize operation.
     Set to True when the span tracks session initialization between client and server.
@@ -38,19 +38,6 @@ class MCPAttributes:
     Example: "echo", "search", "calculator"
     """
 
-    # AWS-specific Remote Service Attributes
-    AWS_REMOTE_SERVICE = "aws.remote.service"
-    """
-    The name of the remote MCP service being called.
-    Default: "mcp server" (can be overridden via MCP_INSTRUMENTATION_SERVER_NAME env var)
-    """
-
-    AWS_REMOTE_OPERATION = "aws.remote.operation"
-    """
-    The specific MCP operation being performed.
-    Values: "Initialize", "ListTool", or the specific tool name for call operations
-    """
-
 
 class MCPSpanNames:
     """Standard span names for MCP operations."""
@@ -62,7 +49,7 @@ class MCPSpanNames:
     Used for all outgoing MCP requests (initialize, list tools, call tool).
     """
 
-    CLIENT_INITIALIZE = "mcp.initialize"
+    CLIENT_INITIALIZE = "notifications/initialize"
     """
     Span name for client-side MCP initialization requests.
     """
@@ -85,13 +72,6 @@ def client_call_tool(tool_name: str) -> str:
         """
         return f"mcp.call_tool.{tool_name}"
 
-    # Server-side span names
-    TOOLS_INITIALIZE = "tools/initialize"
-    """
-    Span name for server-side MCP initialization handling.
-    Tracks server processing of client initialization requests.
-    """
-
     TOOLS_LIST = "tools/list"
     """
     Span name for server-side MCP list tools handling.
@@ -115,41 +95,11 @@ def tools_call(tool_name: str) -> str:
 class MCPOperations:
     """Standard operation names for MCP semantic conventions."""
 
-    INITIALIZE = "Initialize"
+    INITIALIZE = "Notifications/Initialize"
     """Operation name for MCP session initialization."""
 
     LIST_TOOL = "ListTool"
     """Operation name for MCP tool discovery."""
 
     UNKNOWN_OPERATION = "UnknownOperation"
     """Fallback operation name for unrecognized MCP operations."""
-
-
-class MCPTraceContext:
-    """Constants for MCP distributed tracing context propagation."""
-
-    TRACEPARENT_HEADER = "traceparent"
-    """
-    W3C Trace Context traceparent header name.
-    Used for propagating trace context in MCP request metadata.
-    """
-
-    TRACE_FLAGS_SAMPLED = "01"
-    """
-    W3C Trace Context flags indicating the trace is sampled.
-    """
-
-    TRACEPARENT_VERSION = "00"
-    """
-    W3C Trace Context version identifier.
-    """
-
-
-class MCPEnvironmentVariables:
-    """Environment variable names for MCP instrumentation configuration."""
-
-    SERVER_NAME = "MCP_INSTRUMENTATION_SERVER_NAME"
-    """
-    Environment variable to override the default MCP server name.
-    Default value: "mcp server"
-    """

aws-opentelemetry-distro/tests/amazon/opentelemetry/distro/test_mcpinstrumentor.py

@@ -105,7 +105,7 @@ def test_instrument_without_tracer_provider_kwargs(self) -> None:
         # Verify - tracer should be set from trace.get_tracer
         self.assertTrue(hasattr(self.instrumentor, "tracer"))
         self.assertEqual(self.instrumentor.tracer, "default_tracer")
-        mock_get_tracer.assert_called_with("mcp")
+        mock_get_tracer.assert_called_with("instrumentation.mcp")
 
     def test_instrument_with_tracer_provider_kwargs(self) -> None:
         """Test _instrument method when tracer_provider is in kwargs - should use provider's tracer"""
@@ -122,7 +122,7 @@ def test_instrument_with_tracer_provider_kwargs(self) -> None:
         self.assertTrue(hasattr(self.instrumentor, "tracer"))
         self.assertEqual(self.instrumentor.tracer, "mock_tracer_from_provider")
         self.assertTrue(provider.get_tracer_called)
-        self.assertEqual(provider.tracer_name, "mcp")
+        self.assertEqual(provider.tracer_name, "instrumentation.mcp")
 
 
 class TestInstrumentationDependencies(unittest.TestCase):
@@ -249,7 +249,7 @@ def __init__(self, name: str) -> None:
 
         # Test server handling without trace context (fallback scenario)
         with unittest.mock.patch("opentelemetry.trace.get_tracer", return_value=mock_tracer), unittest.mock.patch.dict(
-            "sys.modules", {"mcp.types": MagicMock()}
+            "sys.modules", {"mcp.types": MagicMock(), "mcp": MagicMock()}
         ), unittest.mock.patch.object(self.instrumentor, "_generate_mcp_attributes"), unittest.mock.patch.object(
             self.instrumentor, "_get_mcp_operation", return_value="tools/create_metric"
         ):
@@ -384,7 +384,7 @@ async def server_handle_request(self, session: Any, server_request: Any) -> Dict
 
         # STEP 1: Client sends request through instrumentation
         with unittest.mock.patch("opentelemetry.trace.get_tracer", return_value=mock_tracer), unittest.mock.patch.dict(
-            "sys.modules", {"mcp.types": MagicMock()}
+            "sys.modules", {"mcp.types": MagicMock(), "mcp": MagicMock()}
         ), unittest.mock.patch.object(self.instrumentor, "_generate_mcp_attributes"):
             # Override the setup tracer with the properly mocked one
             self.instrumentor.tracer = mock_tracer
@@ -420,7 +420,7 @@ async def server_handle_request(self, session: Any, server_request: Any) -> Dict
 
         # Server processes the request it received
         with unittest.mock.patch("opentelemetry.trace.get_tracer", return_value=mock_tracer), unittest.mock.patch.dict(
-            "sys.modules", {"mcp.types": MagicMock()}
+            "sys.modules", {"mcp.types": MagicMock(), "mcp": MagicMock()}
         ), unittest.mock.patch.object(self.instrumentor, "_generate_mcp_attributes"), unittest.mock.patch.object(
             self.instrumentor, "_get_mcp_operation", return_value="tools/create_metric"
         ):
@@ -465,7 +465,3 @@ async def server_handle_request(self, session: Any, server_request: Any) -> Dict
                 any(expected_entry in log_entry for log_entry in e2e_system.communication_log),
                 f"Expected log entry '{expected_entry}' not found in: {e2e_system.communication_log}",
             )
-
-
-if __name__ == "__main__":
-    unittest.main()