Skip to content

fix: put each turn's spans in one Langfuse trace - #243

Merged
eandualem merged 1 commit into
developfrom
fix/langfuse-one-trace-per-turn
Oct 6, 2026
Merged

eandualem merged 1 commit into
developfrom
fix/langfuse-one-trace-per-turn

Conversation

@eandualem

@eandualem eandualem commented Oct 6, 2026 •

Copy link
Copy Markdown
Owner

Part of #83

Problem

With Langfuse tracing enabled, one turn produced three unrelated traces:

  • agent-request, the runtime's own span, with the session id;
  • invoke_agent and its chat spans from Pydantic AI, with the session id and all prompt, tool and usage data, but not under agent-request;
  • agent-setup, a root of its own, without the session id.

The runner opens agent-request with set_current_observation=False, so the async generator never attaches an OpenTelemetry context it cannot detach at shutdown. Nothing was parented to it, and agent-setup ran before any trace existed.

Change

  • create_request_trace(set_current_observation=False) now makes the request span current for the turn. It is attached in the same way and place as the Langfuse propagated attributes (session, tags, metadata), which already spanned the turn's yields. The observation is still ended manually.
  • create_span yields a no-op when no trace is current, instead of starting a trace of its own without the session. The setup time that agent-setup recorded is now agent_setup_ms in the request trace's metadata.

No API, event, configuration or schema change. The runtime trace_id on assistant:final_response is unchanged.

Verification

  • An offline probe ran one turn through create_runtime with FunctionModel, Langfuse writing to an in-memory OpenTelemetry exporter and no network. Before: three trace ids. After: one trace id, with agent-request as root and invoke_agent → chat beneath it, all with the session id, and no context-detach errors.
  • New unit tests: the manual-mode trace is the current span inside its block and not after it, and create_span does nothing without a current trace. Both fail on develop.
  • make check from a clean copy without .env: 3012 passed, 1 skipped (needs Postgres); Ruff and format clean. tests/compatibility: 204 passed.

Review

Independent review before opening: Codex gpt-6-astra, high effort, separate read-only context, 1 round, 0 findings (it read the diff and ran the tracing tests). CodeRabbit follows on this PR.

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Improvements
    • Request traces now include agent setup time, rounded to the nearest millisecond.
    • Trace context is handled more consistently, helping maintain correct span relationships and restore the previous context after a request.
    • Span creation now safely does nothing when no active trace is available.

A turn produced three unrelated Langfuse traces: the runtime's
agent-request span, the Pydantic AI agent run (which carries the prompt,
tool and usage data) and an agent-setup span without the session id.
The runner starts agent-request without making it current, so nothing
was parented to it, and agent-setup ran before any trace existed.

The request trace's span is now made current for the turn, in the same
way its propagated attributes already were, while it is still ended
manually for async generator shutdown. create_span no longer starts a
trace of its own when none is current; the setup time it recorded is now
agent_setup_ms on the request trace.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Oct 6, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: defaults
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: e402f118-328b-4f2f-bded-2e1d4d06e79c
📥 Commits

Reviewing files that changed from the base of the PR and between a561b49 and ae544d7.

📒 Files selected for processing (3)
  • src/assistant_runtime/app/streaming/_runner.py
  • src/assistant_runtime/services/tracing.py
  • tests/unit/services/test_tracing.py

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.


📝 Walkthrough

Walkthrough

Request tracing now includes rounded agent setup duration metadata. Manual request observations are made current in OpenTelemetry until the request context exits. Span creation returns a no-op handle when no trace ID is current.

Changes

Request tracing

Layer / File(s) Summary
Request trace context lifecycle
src/assistant_runtime/services/tracing.py, src/assistant_runtime/app/streaming/_runner.py, tests/unit/services/test_tracing.py
Manual request observations use an OpenTelemetry span context that is exited after setup failure or request completion. Request trace metadata includes rounded agent setup duration. The test checks that the request span is current inside the block and that the prior span is restored after exit.
No-current-trace span guard
src/assistant_runtime/services/tracing.py, tests/unit/services/test_tracing.py
create_span yields a no-op handle and does not start an observation when no current trace ID exists. The test verifies this behavior.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Sequence Diagram(s)

sequenceDiagram
  participant create_request_trace
  participant OpenTelemetry
  participant create_span
  create_request_trace->>OpenTelemetry: Make the request observation span current
  create_span->>OpenTelemetry: Create a nested span under the current span
  create_request_trace->>OpenTelemetry: Exit the span context and restore the prior current span
Loading

Merge Risk: ⚪ Minimal · up to ae544

This change groups each turn's spans under one trace and adds setup timing to the trace metadata. No actionable merge-blocking risk was found.

Security Architecture Review

Security architecture risk: 🔵 Low · up to ae544

The change primarily consolidates existing telemetry rather than expanding access permissions, tool capabilities or data collection. The normal streaming path preserves turn isolation, but exceptional shutdown and tracing-library failure behavior are not fully validated.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • inferred — The inspected change affects telemetry attribution for turns with tracing enabled, through the existing configured tracing client. Prompt, model and tool telemetry already crossed that boundary; consolidating its parent trace does not establish a new recipient or greater tool authority. External trace-store access controls were not available for assessment.

Trust Boundaries and Controls

  • observed — The streaming service passes the application principal into turn planning before invoking the runner. Telemetry user labels remain separately supplied by tracing configuration; the new span-parenting code does not replace the application principal or introduce an authorization decision.

Resilience and Maintainability Implications

  • inferred — The supported streaming path contains interruption risk by giving each turn a producer task, shielding that producer from consumer cancellation and waiting for an earlier same-session turn to drain. Together with explicit trace closure, these controls counter cross-turn attribution concerns during ordinary cancellation and repetition; they do not validate arbitrary forced task shutdown or raw runner-generator closure.

Hardening Proposals

  • proposed — Strengthen the trace-ownership contract by ending any observation created before parent-context setup fails, and validate session attribution and context restoration with real tracing integration across cancellation and repeated or concurrent turns.
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 60.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 10 functions across 3 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly describes the main change: grouping each turn’s spans under one Langfuse trace.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@eandualem

Copy link
Copy Markdown
Owner Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Oct 6, 2026 •

Copy link
Copy Markdown
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@eandualem
eandualem merged commit f0c46c1 into develop Oct 6, 2026
6 checks passed
@eandualem
eandualem deleted the fix/langfuse-one-trace-per-turn branch October 6, 2026 16:47
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