fix(python-sdk): make OTel tracing strictly best-effort (review)

- A fault in the tracer itself (broken provider/sampler/exporter) now degrades
  to a no-op instead of propagating into and breaking a discover/inspect/call:
  span start is guarded (fall back to untraced), set_span_attributes swallows
  set_attribute errors, and span end is guarded in finally. The traced body's
  own exception is still recorded and re-raised unchanged.
- Record the exception once: pass record_exception=False/set_status_on_exception
  =False to start_as_current_span (the SDK would otherwise double-record on exit).
- Broaden the module-load guard so a broken otel install can't break importing
  qveris. Added a test that a throwing tracer doesn't break the call; clarified
  the README (active when the API is importable; near-zero, not literal-zero).

Author: linfangw

packages/python-sdk/README.md

@@ -153,10 +153,12 @@ agent = Agent(llm_provider=MyProvider())
 
 ## Observability (OpenTelemetry)
 
-`discover` / `inspect` / `call` emit one OpenTelemetry span each, so you can trace QVeris activity and correlate it with the usage/ledger records. Tracing is **opt-in and dependency-free**:
+`discover` / `inspect` / `call` emit one OpenTelemetry span each, so you can trace QVeris activity and correlate it with the usage/ledger records. Tracing is **dependency-free and best-effort**:
 
-- Without the extra, the spans are a no-op — zero overhead, zero behavior change.
-- Install `pip install qveris[otel]` (adds `opentelemetry-api`) and configure any OTLP exporter to see them.
+- If `opentelemetry-api` is not importable (install it with `pip install qveris[otel]`), the helpers are a no-op — no overhead, no behavior change.
+- If it is importable but no tracer provider is configured, spans go to OpenTelemetry's default no-op provider (near-zero cost, nothing exported).
+- Configure a provider + OTLP exporter and the spans flow to Jaeger/Tempo/any OTLP backend.
+- A fault in the tracer itself (broken provider/sampler/exporter) degrades to a no-op — it never breaks a `discover`/`inspect`/`call`.
 
 Span attributes live under a `qveris.` namespace: `operation`, `tool_id` / `tool_id_count`, `search_id`, `execution_id`, `elapsed_time_ms`, `success`, and `credits` (pre-settlement). The natural-language query and tool parameters are intentionally **not** recorded. A failed `call` is marked as an error span.
 

packages/python-sdk/qveris/observability.py

@@ -25,20 +25,22 @@
 try:  # opentelemetry-api is an optional extra (qveris[otel]).
     from opentelemetry import trace as _otel_trace
     from opentelemetry.trace import Status, StatusCode
-
+except ImportError:  # opentelemetry not installed -> fully no-op
+    _otel_trace = None  # type: ignore[assignment]
+    Status = None  # type: ignore[assignment]
+    StatusCode = None  # type: ignore[assignment]
+    _tracer: Any = None
+else:
     try:  # keep the instrumenting-library version on the tracer when available.
         from importlib.metadata import version as _pkg_version
 
         _QVERIS_VERSION: Optional[str] = _pkg_version("qveris")
     except Exception:  # pragma: no cover - metadata missing in odd installs
         _QVERIS_VERSION = None
-
-    _tracer: Any = _otel_trace.get_tracer("qveris", _QVERIS_VERSION)
-except ImportError:  # opentelemetry not installed -> fully no-op
-    _otel_trace = None  # type: ignore[assignment]
-    Status = None  # type: ignore[assignment]
-    StatusCode = None  # type: ignore[assignment]
-    _tracer = None
+    try:
+        _tracer = _otel_trace.get_tracer("qveris", _QVERIS_VERSION)
+    except Exception:  # pragma: no cover - a broken otel install must not break import
+        _tracer = None
 
 
 # Span attribute keys (kept under a `qveris.` namespace).
@@ -61,31 +63,62 @@ def is_tracing_enabled() -> bool:
 
 
 def set_span_attributes(span: Any, attributes: Dict[str, Any]) -> None:
-    """Set non-``None`` attributes on ``span`` (no-op when tracing is off)."""
+    """Set non-``None`` attributes on ``span``, best-effort.
+
+    No-op when tracing is off, and swallows any tracer error — instrumentation
+    must never break the traced operation.
+    """
     if span is None:
         return
     for key, value in attributes.items():
         if value is not None:
-            span.set_attribute(key, value)
+            try:
+                span.set_attribute(key, value)
+            except Exception:  # pragma: no cover - defensive
+                pass
 
 
 @contextmanager
 def start_span(name: str, attributes: Optional[Dict[str, Any]] = None) -> Iterator[Any]:
     """Start a span for a QVeris operation.
 
-    Yields the span (or ``None`` when tracing is disabled). On an exception the
-    span is marked ``ERROR`` and the exception recorded before re-raising, so a
-    failed ``call`` still shows up as a failed span.
+    Yields the span (or ``None`` when tracing is disabled/unavailable). On an
+    exception from the traced body the span is marked ``ERROR`` and the exception
+    recorded before re-raising, so a failed ``call`` shows up as a failed span.
+
+    Tracing is strictly best-effort: any fault in the tracer itself (a broken
+    provider, sampler, or exporter) degrades to a no-op rather than propagating
+    into — and breaking — the traced operation.
     """
     if _tracer is None:
         yield None
         return
 
-    with _tracer.start_as_current_span(name) as span:
-        set_span_attributes(span, attributes or {})
+    try:
+        # Disable the SDK's own exception recording; we record once, manually.
+        span_cm = _tracer.start_as_current_span(
+            name, record_exception=False, set_status_on_exception=False
+        )
+        span = span_cm.__enter__()
+    except Exception:  # pragma: no cover - tracer setup fault -> run untraced
+        yield None
+        return
+
+    set_span_attributes(span, attributes or {})
+    exc_info: Any = (None, None, None)
+    try:
+        yield span
+    except BaseException as exc:  # noqa: BLE001 - record then re-raise the ORIGINAL error
+        exc_info = (type(exc), exc, exc.__traceback__)
         try:
-            yield span
-        except BaseException as exc:  # noqa: BLE001 - record then re-raise
             span.record_exception(exc)
             span.set_status(Status(StatusCode.ERROR, str(exc)))
-            raise
+        except Exception:  # pragma: no cover - defensive
+            pass
+        raise
+    finally:
+        # End the span; guard so an exporter/exit fault can't mask the result.
+        try:
+            span_cm.__exit__(*exc_info)
+        except Exception:  # pragma: no cover - defensive
+            pass

packages/python-sdk/tests/test_observability.py

@@ -114,6 +114,27 @@ def handler(_request: httpx.Request) -> httpx.Response:
     assert span.status.status_code == StatusCode.ERROR
 
 
+@pytest.mark.asyncio
+async def test_broken_tracer_does_not_break_the_call(monkeypatch: pytest.MonkeyPatch) -> None:
+    # A misbehaving tracer provider must never break the traced operation.
+    class BrokenTracer:
+        def start_as_current_span(self, *args, **kwargs):
+            raise RuntimeError("sampler exploded")
+
+    monkeypatch.setattr(observability, "_tracer", BrokenTracer())
+
+    def handler(_request: httpx.Request) -> httpx.Response:
+        return httpx.Response(200, json={"execution_id": "e1", "success": True})
+
+    client = make_client(handler)
+    try:
+        result = await client.call("t1", {"x": 1}, search_id="s1")
+    finally:
+        await client.close()
+
+    assert result.execution_id == "e1"  # call succeeded despite the broken tracer
+
+
 @pytest.mark.asyncio
 async def test_tracing_disabled_is_a_noop(monkeypatch: pytest.MonkeyPatch) -> None:
     # Simulate opentelemetry not installed: the client must still work unchanged.