Skip to content

fix: restore span instrumentation after the base-SDK hook consolidation - #1

Merged
ash-krnl merged 2 commits into
mainfrom
fix/restore-span-instrumentation
Aug 27, 2026
Merged

fix: restore span instrumentation after the base-SDK hook consolidation#1
ash-krnl merged 2 commits into
mainfrom
fix/restore-span-instrumentation

Conversation

@ash-krnl

Copy link
Copy Markdown
Collaborator

Why

setup_instrumentation called openbox_langgraph.otel_setup.setup_opentelemetry_for_governance, which in openbox-langgraph-sdk-python 1.0.0 is no longer an implementation — its whole body raises OpenBoxConfigError. The call sat inside a best-effort except, so every run logged

OTel instrumentation unavailable; spans disabled

and carried on. Policy arms still evaluated, which is why the demo passed 4/4 while recording span_count: 0 on every event — governed at the activity layer, blind below it. No behavior rule could ever match, because there were no spans to match on.

Nothing here was LangGraph-shaped: that SDK was a route to the openbox_core underneath it. >=0.2.0 with no upper bound admitted the 1.0.0 that removed the entry point, so the break arrived with no code change on this side.

What changed

Instrumentation — builds on openbox_core directly: OpenBoxConfig.resolve → private ContextStoreOpenBoxRuntimeInstrumentationManager(extra_ignored_urls={api_url})install(). One runtime per middleware (runtime.close() clears its store, so sharing one lets a teardown blast a concurrent turn). DID and private key threaded through so the runtime's client keeps signing instead of downgrading to bare-Bearer.

Attributionregister_activity binds an ActivityContext on the store; clear_activity resets the token. The WorkflowSpanBuffer per-workflow registration is gone: every span resolves a context already carrying workflow_id, run_id and workflow_type.

span_aliases.py (new) — spans arrived with only url.full / http.request.method, and the status code reached attributes under neither convention. Core stores the attributes JSONB and has no columns for the flat HTTP fields, while every reader wants the legacy keys: the dashboard's tree builds its link from attrs["http.url"] and its badge from attrs["http.response.status_code"], and core's own spanToSpanData reads attrs["http.url"] for fingerprint identity. So spans rendered with no link and no status. This mirrors the flat fields into attributes under the legacy keys — additive, never overwriting. Extended to the db and file families for the same reason.

db_drivers.py (new) — no SQL produced a span at all. install_dbapi patches the governance seam (CursorTracer.traced_execution) but instruments no driver, and a CursorTracer only exists if a driver instrumentor built one. install_redis and install_asyncpg patch theirs; the DB-API family had no equivalent step anywhere. The legacy setup_database_instrumentation did that half.

No more false success — a base SDK installed without its [http] extra arms the runtime, declines every installer and records zero spans. setup_instrumentation now says so loudly.

Dependencyopenbox-sdk-python[http]>=1.2.0,<2, replacing the LangGraph SDK and the langgraph/langchain-core/langsmith it dragged in. [http] is not optional: the opentelemetry-instrumentation-* packages live behind it.

file_spans.py — reports honestly rather than installing nothing. Core's _file_span_name derives the name from the open mode, so writes-filed-as-reads is fixed upstream; per-operation timing is not, so file start/completion pairs still share a timestamp.

Verification

Environment Result
local 4/4 scenarios, spans with url/method/status
staging 4/4 plumbing (agent has no controls attached, so all arms allow)
production 4/4, Send_Invoicerequire_approval, signing accepted by a signing_required agent
  • 87 tests pass
  • A clean venv installs no LangGraph, and import openbox_langgraph raises ModuleNotFoundError while the demo still runs 4/4
  • startedcompleted pairs intact, verified against pre-port sessions in the same database
  • A 24-span matrix run covers every producible type: 5 HTTP verbs incl. a 404, llm_completion, database_{select,insert,update,delete,query}, file_{read,write}

Known limits, unchanged by this PR

  • llm_routing / llm_provenance need openbox.routing.stage=preflight or gen_ai.generation.id; this SDK emits neither, so the routing panel stays empty for Citadel agents
  • llm_embedding / llm_tool_call / mcp_tool_call classify off the span name or an mcp.method attribute
  • file_delete is impossible: file hooks patch only builtins.open/io.open, so os.remove is invisible
  • db_operation is sent but core classifies from the span name — fixed separately in openbox-sdk-python

🤖 Generated with Claude Code

ash-krnl and others added 2 commits August 27, 2026 18:42
…tion

`setup_instrumentation` called `openbox_langgraph.otel_setup.
setup_opentelemetry_for_governance`, which in openbox-langgraph-sdk-python
1.0.0 is no longer an implementation — its whole body raises
`OpenBoxConfigError`. The call sat inside a best-effort `except`, so every run
logged "OTel instrumentation unavailable; spans disabled" and carried on
governing nothing below the activity layer. Policy arms still evaluated, which
is why the demo passed 4/4 while recording `span_count: 0` on every event.

Hook governance now lives in `openbox_core`'s InstrumentationManager, so this
builds on it directly: OpenBoxConfig.resolve -> private ContextStore ->
OpenBoxRuntime -> InstrumentationManager(extra_ignored_urls={api_url}) ->
install(). One runtime per middleware, since `runtime.close()` clears whatever
store it holds and sharing one would let a teardown blast a concurrent turn.
The DID and private key are threaded through so the runtime's own client keeps
signing rather than silently downgrading to bare-Bearer at the trust boundary.

Attribution moves with it. `WorkflowSpanProcessor.set_activity_context` and the
`WorkflowSpanBuffer` registration are gone; `register_activity` now binds an
`ActivityContext` on the store and `clear_activity` resets that token. Fields
with no first-class home ride along in `metadata`. Nothing is pre-declared per
workflow because every span resolves a context already carrying workflow_id,
run_id and workflow_type.

Two dependent breakages surfaced once spans flowed:

* Spans arrived with only current-convention attributes (`url.full`,
  `http.request.method`), and the status code reached attributes under neither
  convention. Core stores the attributes JSONB and has no columns for the flat
  HTTP fields, while every reader wants the legacy keys — the dashboard's tree
  builds its request link from `attrs["http.url"]` and its badge from
  `attrs["http.response.status_code"]`, and core's own `spanToSpanData` reads
  `attrs["http.url"]` for fingerprint identity. So spans rendered with no link
  and no status. `span_aliases.py` mirrors the flat fields into attributes
  under the legacy keys, additively, never overwriting. Extended to the db and
  file families for the same reason.

* No SQL produced a span at all. `install_dbapi` patches the DB-API governance
  seam (`CursorTracer.traced_execution`) but instruments no driver, and a
  CursorTracer only exists if a driver instrumentor built one — `install_redis`
  and `install_asyncpg` patch theirs, the DB-API family had no equivalent step
  anywhere. The legacy `setup_database_instrumentation` this SDK used to call
  did that half. `db_drivers.py` instruments sqlite3/psycopg2/pymysql/mysql,
  each guarded independently.

`setup_instrumentation` also no longer reports success when it patched nothing:
a base SDK installed without its [http] extra arms the runtime, declines every
installer, and records zero spans, which is exactly the failure this SDK exists
to prevent.

The dependency is now the base SDK directly. Nothing here was LangGraph-shaped
— the LangGraph SDK was a route to the `openbox_core` underneath it, and it
dragged langgraph, langchain-core and langsmith along. `>=0.2.0` with no upper
bound also admitted the 1.0.0 that removed the entry point above, so the break
arrived with no code change on this side; hence `[http]` and an upper bound.

`file_spans.py` reports honestly instead of installing nothing: core's
`_file_span_name` derives the name from the open mode, so the writes-filed-as-
reads bug is fixed upstream. Per-operation timing is not — core sends no
duration, so a file start/completion pair still shares a timestamp.

Verified against local, staging and production: 4/4 scenarios, spans recorded
with url/method/status, started -> completed pairs intact, signing accepted by a
`signing_required` agent. 87 tests pass; a clean venv installs no LangGraph.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Three things stood between a run and its dashboard link, and each one reported
a cause that was not the cause.

* The routes do not exist. Both scripts called `GET /agent` and `POST /agent`;
  the controller is `@Controller('agent')` and registers no bare route, so
  listing is `/agent/list` and creating is `/agent/create`. `GET /agent` 404s,
  `resolve_agent_id()` returned "", and every run printed a bare workflow_id
  instead of a link. Not deployment-specific — a local stack 404s identically.

* urllib's default User-Agent is refused at the edge. `Python-urllib/x.y` gets
  a 403 with `error code: 1010` before the request reaches the API, while curl
  and httpx pass. `setup_openbox.py` reported any 403 as a missing permission
  and pointed at SETUP.md's permission list, so the operator went hunting for a
  problem that did not exist — the key was never evaluated. Both scripts now
  send an honest identifier, and a 1010 says what it is.

* The envelope has two `data` layers. Paginated endpoints answer
  `{status, data: {data: [...], total}}`; peeling one leaves the inner dict,
  which iterates as key strings, so every lookup silently found nothing. This
  is why link resolution still failed after the routes were fixed. `rows_of`
  and `find_by` now unwrap until they reach a list.

`serve_demo.py` also stops telling the operator to set OPENBOX_PLATFORM_API_KEY
when it is already set, and points at the real cause instead.

Verified against production and staging: agent resolves by DID, and each
workflow_id maps to its session link.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant