fix(observability): persist model usage on unattended runs - #230
Open
Caldalis wants to merge 1 commit into
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Intent
Model token usage exists only on spans and reaches the database through a ContextVar sink. Scheduled tasks, team wakeup and TL chat, public API runs, and handoff resume never bound one, so their execution spans — turn planning and task actions — were dropped. Only the post-turn memory-capture job, which binds its own sink, left any trace. Verified on a real scheduled run:
mainrecorded twomemory.captureevents and nothing from the run itself.Changes
api/chat.pyintoobservability.persist_spans.public_api/runs.pyit binds toworker_db, the session the loop actually runs on.llm_call_*events, marking trimmed rows withbodies_omittedso an audit view cannot mistake "not retained" for "the model returned nothing". Other span types pass through unchanged.Risk
llm_callspans also carry the full prompt and response bodies —request_payloadandrequest_messagesmeasured 22 KB each on a real run, and are duplicated across thestartedandfinishedevents. Unattended paths run at machine cadence andagent_eventshas no retention policy, so persisting bodies there would grow without bound and would retain user data indefinitely. Restricting to metrics measured a 28× reduction across eight real events while preserving every field existing consumers read —session_timings._ModelSpanneedstask_frame_id/iteration/json_attempt/json_max_attempts, and the conversation-log view needsrequest_parameters. Row count grows too: a knowledge-retrieving run emits roughly 30 span rows at about 1 KB each, so a five-minute task costs ~8 MB per day against ~187 MB unprojected. A retention policy foragent_eventsis worth considering separately.Writes to the caller's own Session and commits, matching the three existing sinks — and those call sites either commit immediately beforehand or, in the scheduled-task case, commit once per stream event, so transaction semantics are unchanged.
emit_span_eventalready swallows sink errors, so a failed telemetry write cannot fail a run. The existing sinks are untouched.Tests
12 cases in
test_observability_span_persistence.pycover the projectionTests
12 cases in
test_observability_span_persistence.pycover the projection (metrics kept, bodies dropped, trimmed rows marked), the no-op degradation, sink release and failure isolation, and the wiring on the scheduled-task execution path — that last one verified to fail when the instrumentation is removed. Full suite 2018 passed (2006 before).ruff checkclean on changed files.Verified end to end against a real scheduled run:
mainrecorded no execution spans for it, the branch recorded four (1.0–1.4 KB each, no bodies), the pre-existingmemory.capturesink was untouched, and the conversation-log view renders the trimmed rows without error.UI Validation
Not applicable — backend only, no route or role affected. The conversation-log view was exercised anyway to confirm trimmed rows render cleanly.
The non-streaming chat endpoint is out of scope: new sessions there need lazy session-id resolution, and it is an attended path.