observability: phase 6.1 concurrency-safe observer (PR-A)

[chris-colinsky]

Summary

PR-A of Phase 6.1, the architectural refactor that closes the three
limitations Phase 6.0 explicitly deferred:

1. Concurrency-safe state scoping. OTelObserver internal
   span maps are outer-keyed by invocation_id (per spec §5.1) so
   a single observer instance is safe to share across concurrent
   invocations. The original plan-of-record said correlation_id;
   that broke test_phase5_fixture_031_span_assertions during
   impl because resume preserves the cid across runs (per §5.6) and
   a cid-keyed map would have resumed runs inherit the prior
   invocation's trace_id, violating §5.1's "each invocation owns
   its own trace_id" contract. Switching to invocation_id closes
   the gap; the cid stays as the cross-run join attribute on every
   span.
2. Single-handler attach/detach. Parent resolution moves
   entirely to the observer's internal maps within each event
   handler's scope. _OpenSpan no longer carries an OTel context
   token; spans open with
   context=set_span_in_context(parent_span) directly. The
   round-7 try/except ValueError guards on cross-event
   detach() are deleted because the underlying hazard goes
   away — tokens never cross event boundaries.
3. §5.5 LLM-span parent attribution. Three new ContextVars in
   observability/correlation.py (current_namespace_prefix,
   current_fan_out_index, current_attempt_index) carry the
   calling node's identity to _make_llm_event;
   _LlmEventState gains corresponding calling_* fields; the
   observer's _resolve_llm_parent looks up the calling node's
   span by the full _StackKey triple. Under concurrent fan-out
   • retry the LLM span parents under exactly the right calling
     node regardless of dispatch ordering or which sibling instance
     happens to be on the OTel current-span stack.

The three are landed together because they share the same
primitive: parents come from the observer's internal maps within a
single event handler's scope. Splitting would ship intermediate
states that fail their own correctness story (see the spec agent's
ack in
openarmature-coord/threads/phase-6-1-observability-conformance-fillin/02-spec-plan-review.md
for the precondition framing).

Tests

Four new tests in tests/unit/test_observability_otel.py:

• test_shared_observer_concurrent_invocations_dont_collide —
  asyncio.gather([invoke()] * 5) on a single shared observer;
  asserts N distinct trace_id values and that each trace's
  span set is exactly the expected three nodes (invocation +
  node_a + node_b).
• test_concurrent_fan_out_no_lifo_violation — the explicit
  regression check spec asked for. Drives 5-item fan-out under
  warnings.catch_warnings("error") so the path that previously
  needed the round-7 try/except ValueError: pass guards
  surfaces as a fail rather than a silent swallow if anything in
  the new architecture were to trigger it. Stays quiet — the
  underlying hazard is gone.
• test_concurrent_fan_out_llm_spans_parent_under_calling_instance —
  4-instance fan-out, each instance's body calls
  OpenAIProvider.complete() (mocked via httpx.MockTransport);
  asserts every LLM span parents under its own ask instance,
  never a sibling, with N distinct parent span_ids for N calls.
• test_llm_call_inside_retried_node_parents_per_attempt —
  the test from spec's attempt_index flag in
  04-spec-attempt-index-flag.md. Node wrapped with
  RetryMiddleware(max_attempts=3); body calls complete() on
  each attempt (two failures + one success); asserts each LLM span
  parents under THAT attempt's flaky span, not a hardcoded
  attempt_index=0.

Test plan

• uv run pytest -q — 387 passed, 9 skipped
• uv run pyright — 0 errors
• uv run ruff check . && uv run ruff format — clean
• Existing 31 unit tests + 5 conformance fixtures preserved

[Copilot]

Pull request overview

This PR implements Phase 6.1 of the observability refactor by making OTelObserver safe to share across concurrent invocations (scoping internal span state by invocation_id), removing cross-event OpenTelemetry context attach/detach tokens, and adding calling-node identity propagation via new ContextVars so §5.5 LLM spans parent under the correct node attempt under fan-out + retry.

Changes:

• Refactor OTelObserver to store all per-run span tracking in per-invocation_id state containers and resolve parents from internal maps (no cross-event OTel tokens).
• Add ContextVars (current_namespace_prefix, current_fan_out_index, current_attempt_index) set by the engine to carry calling-node identity into LLM provider events.
• Add/expand unit tests covering concurrent invocations, fan-out interleaving, LLM parent attribution under fan-out, and per-attempt attribution under retry.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
tests/unit/test_observability_otel.py	Adds Phase 6.1 regression + concurrency tests for span isolation and correct LLM parenting.
src/openarmature/observability/otel/observer.py	Refactors OTel observer to per-invocation state, removes attach/detach, and implements LLM parent resolution via calling identity.
src/openarmature/observability/correlation.py	Introduces new calling-node ContextVars and exports them.
src/openarmature/observability/init.py	Re-exports the new correlation/identity accessors.
src/openarmature/llm/providers/openai.py	Extends _LlmEventState with calling-node identity and populates it from the new ContextVars.
src/openarmature/graph/compiled.py	Sets/resets calling-node identity ContextVars around node execution and per-attempt attempt_index scope.

Comments suppressed due to low confidence (1)

src/openarmature/observability/otel/observer.py:339

• checkpoint_saved spans are currently parented via _resolve_parent_context, but the triggering node span has already been popped from inv_state.open_spans in _handle_completed. As a result, openarmature.checkpoint.save will fall back to the invocation/subgraph span instead of the node that triggered the save (contradicting this method’s docstring). Consider storing the just-completed node span context (or span_id) long enough for the subsequent checkpoint_saved event to attach correctly, e.g., keep a short-lived map keyed by _StackKey or emit the save span directly from _handle_completed when appropriate.

        invocation_id = current_invocation_id()
        if invocation_id is None:
            return
        inv_state = self._inv_states.get(invocation_id)
        if inv_state is None:
            return
        parent_ctx = self._resolve_parent_context(inv_state, invocation_id, event)
        attrs: dict[str, Any] = {
            "openarmature.checkpoint.save_node": event.node_name,
        }
        cid = current_correlation_id()
        if cid is not None:
            attrs["openarmature.correlation_id"] = cid
        span = self._tracer.start_span(
            name="openarmature.checkpoint.save",
            context=cast("Any", parent_ctx),
            kind=SpanKind.INTERNAL,
            attributes=attrs,
        )

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.