refactor(mcp): drop _RequestSpan wrapper, use logfire span directly

Feedback from PR #754 review: _RequestSpan used speculative getattr
to probe for set_attributes / set_attribute on its stored span, which
CLAUDE.md explicitly forbids. The check existed because the span was
typed as typing.Any | None, hiding the fact that logfire.span always
yields a LogfireSpan with a stable .set_attributes method.

This strip:
- Deletes the _RequestSpan adapter class entirely.
- _request_scope now yields the logfire span itself; callers hold a
  logfire.LogfireSpan | None and call .set_attributes directly.
- Extracts two one-liner helpers (_response_span_attrs,
  _transport_error_span_attrs) for the attribute dicts, preserving
  the dedup across the 5 call_{get,put,post,patch,delete} sites.
- Test fakes yield a _NoopSpan stub so patched `logfire.span` stays
  compatible with the new direct `.set_attributes` calls.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Signed-off-by: phernandez <paul@basicmachines.co>

Author: phernandez

src/basic_memory/mcp/tools/utils.py

@@ -41,43 +41,22 @@ def _classify_http_outcome(status_code: int) -> str:
     return "unknown"  # pragma: no cover
 
 
-class _RequestSpan:
-    """Small adapter for attaching outcome metadata to a live request span."""
-
-    def __init__(self, active_span: typing.Any | None):
-        self._active_span = active_span
-
-    def record_response(self, response: Response) -> None:
-        self._set_attributes(
-            {
-                "status_code": response.status_code,
-                "is_success": response.is_success,
-                "outcome": _classify_http_outcome(response.status_code),
-            }
-        )
-
-    def record_transport_error(self, exc: Exception) -> None:
-        self._set_attributes(
-            {
-                "is_success": False,
-                "outcome": "transport_error",
-                "error_type": type(exc).__name__,
-            }
-        )
-
-    def _set_attributes(self, attrs: dict[str, typing.Any]) -> None:
-        if self._active_span is None:
-            return
+def _response_span_attrs(response: Response) -> dict[str, Any]:
+    """Attributes to attach to a request span after a response lands."""
+    return {
+        "status_code": response.status_code,
+        "is_success": response.is_success,
+        "outcome": _classify_http_outcome(response.status_code),
+    }
 
-        set_attributes = getattr(self._active_span, "set_attributes", None)
-        if callable(set_attributes):
-            set_attributes(attrs)
-            return
 
-        set_attribute = getattr(self._active_span, "set_attribute", None)
-        if callable(set_attribute):
-            for key, value in attrs.items():
-                set_attribute(key, value)
+def _transport_error_span_attrs(exc: Exception) -> dict[str, Any]:
+    """Attributes to attach when the transport layer fails before any response."""
+    return {
+        "is_success": False,
+        "outcome": "transport_error",
+        "error_type": type(exc).__name__,
+    }
 
 
 def get_error_message(
@@ -200,17 +179,17 @@ def _request_scope(
     has_body: bool = False,
 ):
     """Create the shared MCP transport span used by all HTTP helpers."""
-    attrs: dict[str, Any] = {
-        "method": method,
-        "client_name": client_name,
-        "operation": operation,
-        "path_template": path_template,
-        "phase": "request",
-        "has_query": bool(params),
-        "has_body": has_body,
-    }
-    with logfire.span("mcp.http.request", **attrs) as active_span:
-        yield _RequestSpan(active_span)
+    with logfire.span(
+        "mcp.http.request",
+        method=method,
+        client_name=client_name,
+        operation=operation,
+        path_template=path_template,
+        phase="request",
+        has_query=bool(params),
+        has_body=has_body,
+    ) as active_span:
+        yield active_span
 
 
 async def call_get(
@@ -249,7 +228,7 @@ async def call_get(
     """
     logger.debug(f"Calling GET '{url}' params: '{params}'")
     error_message = None
-    request_span: _RequestSpan | None = None
+    request_span: logfire.LogfireSpan | None = None
 
     try:
         with _request_scope(
@@ -269,7 +248,7 @@ async def call_get(
                 timeout=timeout,
                 extensions=extensions,
             )
-            request_span.record_response(response)
+            request_span.set_attributes(_response_span_attrs(response))
 
         if response.is_success:
             return response
@@ -298,7 +277,7 @@ async def call_get(
         raise ToolError(error_message) from e
     except Exception as e:
         if request_span is not None:
-            request_span.record_transport_error(e)
+            request_span.set_attributes(_transport_error_span_attrs(e))
         raise
 
 
@@ -346,7 +325,7 @@ async def call_put(
     """
     logger.debug(f"Calling PUT '{url}'")
     error_message = None
-    request_span: _RequestSpan | None = None
+    request_span: logfire.LogfireSpan | None = None
 
     try:
         with _request_scope(
@@ -371,7 +350,7 @@ async def call_put(
                 timeout=timeout,
                 extensions=extensions,
             )
-            request_span.record_response(response)
+            request_span.set_attributes(_response_span_attrs(response))
 
         if response.is_success:
             return response
@@ -401,7 +380,7 @@ async def call_put(
         raise ToolError(error_message) from e
     except Exception as e:
         if request_span is not None:
-            request_span.record_transport_error(e)
+            request_span.set_attributes(_transport_error_span_attrs(e))
         raise
 
 
@@ -448,7 +427,7 @@ async def call_patch(
         ToolError: If the request fails with an appropriate error message
     """
     logger.debug(f"Calling PATCH '{url}'")
-    request_span: _RequestSpan | None = None
+    request_span: logfire.LogfireSpan | None = None
 
     try:
         with _request_scope(
@@ -473,7 +452,7 @@ async def call_patch(
                 timeout=timeout,
                 extensions=extensions,
             )
-            request_span.record_response(response)
+            request_span.set_attributes(_response_span_attrs(response))
 
         if response.is_success:
             return response
@@ -508,7 +487,7 @@ async def call_patch(
         raise ToolError(error_message) from e
     except Exception as e:
         if request_span is not None:
-            request_span.record_transport_error(e)
+            request_span.set_attributes(_transport_error_span_attrs(e))
         raise
 
 
@@ -556,7 +535,7 @@ async def call_post(
     """
     logger.debug(f"Calling POST '{url}'")
     error_message = None
-    request_span: _RequestSpan | None = None
+    request_span: logfire.LogfireSpan | None = None
 
     try:
         with _request_scope(
@@ -581,7 +560,7 @@ async def call_post(
                 timeout=timeout,
                 extensions=extensions,
             )
-            request_span.record_response(response)
+            request_span.set_attributes(_response_span_attrs(response))
         logger.debug(f"response: {_extract_response_data(response)}")
 
         if response.is_success:
@@ -611,7 +590,7 @@ async def call_post(
         raise ToolError(error_message) from e
     except Exception as e:
         if request_span is not None:
-            request_span.record_transport_error(e)
+            request_span.set_attributes(_transport_error_span_attrs(e))
         raise
 
 
@@ -683,7 +662,7 @@ async def call_delete(
     """
     logger.debug(f"Calling DELETE '{url}'")
     error_message = None
-    request_span: _RequestSpan | None = None
+    request_span: logfire.LogfireSpan | None = None
 
     try:
         with _request_scope(
@@ -703,7 +682,7 @@ async def call_delete(
                 timeout=timeout,
                 extensions=extensions,
             )
-            request_span.record_response(response)
+            request_span.set_attributes(_response_span_attrs(response))
 
         if response.is_success:
             return response
@@ -732,5 +711,5 @@ async def call_delete(
         raise ToolError(error_message) from e
     except Exception as e:
         if request_span is not None:
-            request_span.record_transport_error(e)
+            request_span.set_attributes(_transport_error_span_attrs(e))
         raise

tests/mcp/test_tool_telemetry.py

@@ -15,13 +15,23 @@
 write_note_module = importlib.import_module("basic_memory.mcp.tools.write_note")
 
 
+class _NoopSpan:
+    """Minimal stand-in for a live logfire span during tests."""
+
+    def set_attributes(self, attrs: dict) -> None:
+        pass
+
+    def set_attribute(self, key: str, value) -> None:
+        pass
+
+
 def _recording_spans():
     spans: list[tuple[str, dict]] = []
 
     @contextmanager
     def fake_span(name: str, **attrs):
         spans.append((name, attrs))
-        yield
+        yield _NoopSpan()
 
     return spans, fake_span