fix(tracing): make a fleet agent's trace whole when it runs in its own Langfuse project#1893
Merged
Merged
Conversation
…e is whole When an agent runs in a *different* Langfuse project than the LiteLLM gateway (e.g. a dedicated fleet project), the gateway's success callback logs the full-detail generation into the GATEWAY's project — leaving the agent's own trace with a hole where its model call should be. Only the structural spans (a2a-stream, tool:*) land in the agent's project, so a no-tool turn shows as an empty wrapper and the trace is un-whole. TraceContextMiddleware already sees every model call (it stamps the trace context onto the request). Extend it to ALSO emit, into the agent's own Langfuse project, a lightweight generation — model + token usage + cost, no prompt/completion payload — via a new tracing.trace_generation() helper. The heavy IO stays in the gateway project (unchanged); the two are joinable by trace_id. When agent and gateway share a project (the common case) this is just a small summary node alongside the gateway's detailed one. Best-effort throughout: no-op when tracing is disabled, handles both the ModelResponse and bare-AIMessage return shapes, never raises. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
👀 Quinn is reviewing — verdict (PASS / WARN / FAIL) + findings to follow. |
WalkthroughAdds a ChangesFleet Generation Tracing
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant Caller
participant TraceContextMiddleware
participant Handler
participant Tracing as observability.tracing
Caller->>TraceContextMiddleware: wrap_model_call/awrap_model_call(request)
TraceContextMiddleware->>Handler: handler(_with_trace(request))
Handler-->>TraceContextMiddleware: response (AIMessage, usage, model)
TraceContextMiddleware->>TraceContextMiddleware: compute duration_ms via time.monotonic()
TraceContextMiddleware->>Tracing: _emit_fleet_generation(response, duration_ms)
Tracing->>Tracing: cost_usd via pricing.cost_usd(model, usage)
Tracing->>Tracing: trace_generation(name, model, usage, cost_usd, duration_ms)
TraceContextMiddleware-->>Caller: response
Possibly related issues
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![protoquinn[bot]](https://avatars.githubusercontent.com/in/3282126?s=60&v=4){kind=small}
There was a problem hiding this comment.
QA Audit — PR #1893 | fix(tracing): make a fleet agent's trace whole when it runs in its own Langfuse project
VERDICT: WARN (pending CI — all 8 checks queued/in_progress; approve-on-green policy promotes to APPROVED automatically once every check is terminal-green, no re-review needed)
CI Status
- build: queued
- Fleet integration (multi-instance): queued
- Python tests: queued
- Lint (ruff + import contracts): queued
- A2A live smoke (lean tier): queued
- Web E2E smoke: queued
- Verify workspace config: queued
- gitleaks (tree): in_progress
Diff Review
- 3 files, +203/−2 —
trace_context.py,tracing.py, and tests. Clean implementation spanning the middleware seam and the observability layer. _emit_fleet_generationfollows the same defensive pattern as_with_trace:try/except Exception+ debug log, never raises. Correctly handles bothModelResponse.resultand bareAIMessagereturns.trace_generation()inobservability/tracing.pymirrorstrace_tool_call()—start_observation+ immediateendso it nests under the session span without becoming the parent. Usage/cost encoding matches Langfuse conventions.- Timing uses
time.monotonic()(correct clock, immune to NTP/wall adjustments). - 6 new tests cover emission path, bare-AIMessage, disabled-tracing no-op, error resilience, and async passthrough — all well-isolated with
monkeypatch.
Observations
- CLAWPATCH: no findings on the PR's changed files — clean structural pass.
- CodeRabbit: no unresolved threads.
- LOW:
_emit_fleet_generationcallsfrom observability import pricing, tracinginside the method body on every invocation. Python caches imports, so this is a style nit, not a perf issue — but worth noting it's intentional (avoiding module-level import that might load Langfuse SDK before config is settled).
— Quinn, QA Engineer
|
Submitted COMMENT review on #1893. |
|
✅ CI went terminal-green with no blockers on the prior review — promoting it to APPROVED per the approve-on-green policy (#748). |
There was a problem hiding this comment.
CI terminal-green, no blockers on prior review — auto-approving on green (#748).
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@observability/tracing.py`:
- Around line 391-418: Extract the repeated observation lifecycle in
trace_generation into a shared private helper, since it duplicates the same
enable-check/try/_langfuse.start_observation/.end()/except None pattern already
used by trace_tool_call. Add a small helper such as _start_and_end that accepts
as_type, name, and kwargs, performs the start/end/swallows-exceptions flow, and
have trace_generation call it while keeping its usage_details, cost_details, and
metadata assembly intact.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 3524d89e-6355-484f-8371-09d80d60eee3
📒 Files selected for processing (3)
graph/middleware/trace_context.pyobservability/tracing.pytests/test_trace_context_middleware.py
Comment on lines
+391
to
+418
| try: | ||
| usage_details = None | ||
| if usage: | ||
| candidates = { | ||
| "input": usage.get("input_tokens"), | ||
| "output": usage.get("output_tokens"), | ||
| "total": usage.get("total_tokens"), | ||
| } | ||
| usage_details = {k: int(v) for k, v in candidates.items() if v} or None | ||
| span = _langfuse.start_observation( | ||
| name=name, | ||
| as_type="generation", | ||
| model=model or None, | ||
| usage_details=usage_details, | ||
| cost_details=({"total": float(cost_usd)} if cost_usd else None), | ||
| metadata={ | ||
| "session_id": session_id, | ||
| "trace_id": _trace_id_ctx.get(), | ||
| "duration_ms": duration_ms, | ||
| # Full prompt/completion IO is logged by the gateway's LiteLLM | ||
| # callback into the gateway Langfuse project (joined by trace_id). | ||
| "detail_in": "gateway_project", | ||
| }, | ||
| ) | ||
| span.end() | ||
| return span | ||
| except Exception: | ||
| return None |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
Consider extracting the shared "start_observation → end → swallow" scaffold.
trace_generation duplicates the enable-check/try/start_observation/.end()/except-None skeleton already used by trace_tool_call (lines 332-359). A small private helper (e.g., _start_and_end(as_type, name, **kwargs)) would remove this duplication as more observation-emitting helpers are added.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@observability/tracing.py` around lines 391 - 418, Extract the repeated
observation lifecycle in trace_generation into a shared private helper, since it
duplicates the same enable-check/try/_langfuse.start_observation/.end()/except
None pattern already used by trace_tool_call. Add a small helper such as
_start_and_end that accepts as_type, name, and kwargs, performs the
start/end/swallows-exceptions flow, and have trace_generation call it while
keeping its usage_details, cost_details, and metadata assembly intact.
![protoreview[bot]](https://avatars.githubusercontent.com/in/4233969?s=60&v=4){kind=small}
There was a problem hiding this comment.
QA panel review — WARN
code-review-structural · head abe3b7a83b48 · shadow — comment-only
[review-synthesizer completed: workflow code-review-structural:report]
Prose brief
Overall risk: Low. All findings are minor or nit — the two correctness items (adjacent truthiness checks in tracing.py) are the same root cause: if v / if cost_usd silently discard legitimate zero values from generation traces. No data is lost outside observability, but downstream trace consumers would see gaps where zero-token or zero-cost calls happened.
Fix first: The two falsy-guard bugs in observability/tracing.py (lines ~393 and ~398). A single PR changing both if v → if v is not None and if cost_usd else None → unconditional cost_details construction repairs both. The session_id omission in _emit_fleet_generation is the next priority — it's a one-line kwarg addition.
Disagreements: None — the panel was a single structural-analysis pass, and the verifier confirmed every finding against the diff and surrounding code. No LLM-based review findings were submitted to merge or reconcile.
Verification impact: All six findings survived; none were refuted. The verifier strengthened each with concrete notes (e.g., pricing.cost_usd docstring confirming 0.0 for empty usage, the test_trace_context_middleware.py monkeypatch confirming the real trace_generation path is never exercised).
Gaps: No LLM review-finder passes contributed findings — the entire panel consists of structural-analysis output. The structural pass covered correctness, cross-file consistency, test coverage, and conventions; no security, removed-behavior, or broader integration lanes were examined.
[
{
"file": "observability/tracing.py",
"line": 393,
"severity": "minor",
"category": "correctness",
"claim": "The dict comprehension `{k: int(v) for k, v in candidates.items() if v}` filters out zero-valued token counts (0 is falsy), silently dropping genuine zero-token metrics when a model reports them.",
"evidence": "`usage.get(\"input_tokens\")` returns `None` for missing keys and `0` for present-but-zero keys; `if v` treats both as falsy, so a legitimate `input_tokens: 0` is discarded. Fix: `if v is not None`.",
"verdict": "confirmed",
"note": "PR diff line ~393 shows `if v` — Python treats both `None` and `0` as falsy, so a model reporting `input_tokens: 0` is silently discarded alongside missing keys. `if v is not None` would fix it.",
"source": "structural"
},
{
"file": "observability/tracing.py",
"line": 398,
"severity": "minor",
"category": "correctness",
"claim": "The expression `({\"total\": float(cost_usd)} if cost_usd else None)` treats `0.0` as falsy, suppressing a legitimately-zero cost from the generation span.",
"evidence": "`pricing.cost_usd` returns `0.0` for empty usage and can return `0.0` after rounding for tiny-usage calls (e.g. protolabs/nano at $3e-8/input token). The `if cost_usd` false-branch sets `cost_details=None`, indistinguishable from 'cost was never computed.' Fix: `if cost_usd is not None` (or compute cost_details unconditionally).",
"verdict": "confirmed",
"note": "PR diff line ~398 shows `if cost_usd else None`. `pricing.cost_usd` docstring says '0.0 for empty usage' and `round(3e-8, 6)` = `0.0` for protolabs/nano single-token calls. `0.0` is falsy, so cost_details becomes `None`.",
"source": "structural"
},
{
"file": "graph/middleware/trace_context.py",
"line": 109,
"severity": "minor",
"category": "cross-file",
"claim": "_emit_fleet_generation calls trace_generation() without passing session_id, so generation observations always carry session_id=\"\" while trace_tool_call observations in the same trace carry the real session ID — inconsistent metadata within one trace.",
"evidence": "In `_emit_fleet_generation` (line 109): `tracing.trace_generation(name=name, model=model, usage=usage, cost_usd=cost, duration_ms=duration_ms)` — no `session_id` kwarg. But `trace_generation`'s signature accepts `session_id: str = \"\"` and stamps it into metadata, mirroring `trace_tool_call` which always receives the real session_id from its callers.",
"verdict": "confirmed",
"note": "PR diff shows `trace_generation(...)` call at ~line 109 without `session_id`. `audit.py` always passes `session_id=tracing.current_session_id()` to `trace_tool_call`. `trace_generation`'s signature defaults `session_id=\"\"` and stamps it into metadata. Confirmed inconsistency.",
"source": "structural"
},
{
"file": "observability/tracing.py",
"line": 362,
"severity": "minor",
"category": "tests",
"claim": "New public function trace_generation() has no unit test in test_tracing.py, breaking the module's convention that every public function has a dedicated test and leaving its real implementation path unexercised.",
"evidence": "Every other public function in tracing.py (trace_session, trace_tool_call, score_current_trace, trace_span, flush, current_trace_context) has a dedicated test in tests/test_tracing.py. trace_generation is only exercised indirectly through middleware tests that monkeypatch it away with a lambda — the real usage_details construction, cost_details conditional, and start_observation/end wiring are never executed by any test.",
"verdict": "confirmed",
"note": "Read full `tests/test_tracing.py` — it tests trace_session, trace_tool_call, score_current_trace, trace_span, flush, current_trace_context but has zero coverage for trace_generation. Middleware tests in `test_trace_context_middleware.py` monkeypatch `tracing.trace_generation` with a lambda, so the real implementation path (usage_details, cost_details, start_observation/end) is never executed.",
"source": "structural"
},
{
"file": "graph/middleware/trace_context.py",
"line": 1,
"severity": "nit",
"category": "cross-file",
"claim": "The module docstring describes only trace-stamping behavior and does not mention the new fleet-generation emit behavior now performed by wrap_model_call and awrap_model_call.",
"evidence": "The docstring says the middleware 'stamps a fresh trace context' and is 'No-op ... when tracing is disabled' — but after this PR, both wrapper methods also time the handler call and emit a generation span via _emit_fleet_generation, which is unmentioned.",
"verdict": "confirmed",
"note": "Read `trace_context.py` module docstring from default branch (identical in PR diff — no docstring change). It describes only `_with_trace` stamping behavior. PR adds timing + `_emit_fleet_generation` call in both `wrap_model_call` and `awrap_model_call` — docstring was not updated.",
"source": "structural"
},
{
"file": "graph/middleware/trace_context.py",
"line": 71,
"severity": "nit",
"category": "conventions",
"claim": "The private method name '_emit_fleet_generation' implies fleet-only (multi-project) behavior, but it is called unconditionally from both wrap_model_call and awrap_model_call on every model call regardless of project configuration.",
"evidence": "Called on lines 113 and 119 without any fleet/project guard: `self._emit_fleet_generation(response, …)`. The PR description confirms it runs even when 'agent and gateway share a project (the common case)' — making 'fleet' in the name misleading to readers scanning the call site.",
"verdict": "confirmed",
"note": "PR diff shows `_emit_fleet_generation()` called unconditionally in both `wrap_model_call` (~line 113) and `awrap_model_call` (~line 119) with no project/fleet guard. The method's own docstring describes it as for the 'different project' case, yet it fires always. Name is misleading but behavior is intentional.",
"source": "structural"
}
]
protoquinn Bot
pushed a commit
that referenced
this pull request
Jul 7, 2026
….96.0 cut (#1895) Nine PRs merged since v0.95.1 shipped without a changelog entry, so the generated release notes (updater + Discord, single-source from CHANGELOG) would have omitted a headline feature and a security-relevant default change. Backfill before rolling the release: - Added: Hermes first-class runtime (#1889 + hardening #1891) - Changed: filesystem.allow_run defaults OFF on the headless tier (#1888) - Fixed: fleet members no longer inherit hub identity (#1886/#1882/#1881), whole trace on a dedicated Langfuse project (#1893), background knowledge ingest wakes the agent (#1887), quieter plugin hot-reload logs (#1885) The already-present entries (#1880 tracing, #1890 plugin-view 401, #1879 subagent max_turns) are unchanged. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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
1 participant
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.
Closes #1892.
Problem
When a fleet agent is pointed at a dedicated Langfuse project (separate from the shared LiteLLM gateway's project), a single turn's trace fragments across two projects:
a2a-stream,tool:*) → the agent's project (agent's own OTLP key). ✅<agent>-turn, the LLM call with model/tokens/cost) → the gateway's project — because the gateway's LiteLLM success callback logs every generation with the gateway's Langfuse key.Same
trace_id, twoproject_ids. A no-tool turn then shows in the agent's project as just an emptya2a-streamwrapper — the actual model call is invisible there.TraceContextMiddleware's docstring even names the now-broken assumption: "the gateway runs its Langfuse callback in the SAME project."Fix
TraceContextMiddleware.wrap_model_callis the single seam that sees every model call. It already stamps the trace context onto the request; now it also emits — into the agent's own project — a lightweight generation via a newobservability.tracing.trace_generation()helper: model + token usage + cost + duration, no prompt/completion payload. The full IO stays in the gateway project (unchanged), joinable bytrace_id.This deliberately keeps the heavy LLM detail in the gateway project (that's where LLM-observability for all consumers lives) while making the fleet trace stand on its own.
Tests
tests/test_trace_context_middleware.py— 6 new cases: records model/usage/cost, handles bothModelResponse.resultand bare-AIMessagereturns, no-op when tracing disabled, never raises on garbage/sink-blowup, andawrap_model_callemits + passes the response through untouched. Full file green (13) + adjacent trace/subagent suites (18 total).Context
Homelab fleet dogfooding — jon/roxy/frank/vera moved to a dedicated
protoLabs.studioLangfuse project (homelab-iac #185); the gateway stays on its own project for external consumers. Requires a base rebuild + fleet redeploy to activate.Summary by CodeRabbit
New Features
Bug Fixes