fix(runner): framework-agnostic observability naming + MLflow trace IDs#1283
Merged
jeremyeder merged 5 commits intoambient-code:mainfrom Apr 10, 2026
Merged
Conversation
Contributor
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (9)
Cache: Disabled due to data retention organization setting Knowledge base: Disabled due to data retention organization setting 📝 WalkthroughWalkthroughVendor-specific observability artifacts were replaced with neutral ambient-runner naming: turn traces now use Changes
Sequence Diagram(s)sequenceDiagram
participant Runner as Runner
participant ObsMgr as ObservabilityManager
participant Langfuse as Langfuse
participant MLflow as MLflow
Runner->>ObsMgr: start_turn()
alt Langfuse creates turn trace
ObsMgr->>Langfuse: create turn trace (name: "llm_interaction", metadata: {"runner_type": slug})
Langfuse-->>ObsMgr: turn.trace_id
ObsMgr->>MLflow: optionally emit/associate span (include ambient.runner_type)
ObsMgr-->>Runner: return turn context (trace_id from Langfuse)
else Langfuse not producing turn
ObsMgr->>MLflow: get_active_trace_id()
MLflow-->>ObsMgr: mlflow_trace_id
ObsMgr->>ObsMgr: _sync_last_trace_id_from_mlflow() -> set last_trace_id
ObsMgr->>MLflow: create/associate turn span (use constants, include ambient.runner_type)
ObsMgr-->>Runner: return turn context (trace_id from MLflow)
end
🚥 Pre-merge checks | ✅ 5 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
✨ Simplify code
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@components/runners/ambient-runner/ambient_runner/observability.py`:
- Around line 442-446: The documentation in tracing.py that describes the
"Langfuse trace ID" is outdated and the event name ambient:langfuse_trace is
misleading because get_current_trace_id() in observability.py can return either
a Langfuse or MLflow ID; update the comment/text around lines 45-46 to state the
event carries a trace ID from either Langfuse or MLflow depending on active
backend, and either rename the event to ambient:trace_id (and update all usages
and tests) or add an explicit note that ambient:langfuse_trace is historical but
may contain MLflow IDs; ensure you also update any middleware docs and
references that assume only Langfuse IDs and adjust observability.py emit points
to use the new event name if renamed.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 97bae982-5f00-4d3b-b381-e08c25151ab5
📒 Files selected for processing (7)
components/runners/ambient-runner/README.mdcomponents/runners/ambient-runner/ambient_runner/mlflow_observability.pycomponents/runners/ambient-runner/ambient_runner/observability.pycomponents/runners/ambient-runner/tests/test_observability.pycomponents/runners/ambient-runner/tests/test_observability_metrics.pycomponents/runners/ambient-runner/tests/test_observability_mlflow_integration.pycomponents/runners/ambient-runner/tests/test_privacy_masking.py
Contributor
|
ignore the two test failures, not real |
ambient-code
bot
force-pushed
the
fix/observability-naming-mlflow-trace-ids
branch
from
3541d01 to
1609635
Compare
Contributor
Automated fixes applied1. CodeRabbit review comment addressed — renamed
2. Merge conflict partially addressed
Action needed: The PR author (@etirelli) should rebase this branch onto CI failures ( 🤖 Session |
- Turn traces: llm_interaction; session metrics: Session Metrics - Langfuse tag runner:<RUNNER_TYPE> instead of claude-code - Session metric source ambient-runner-metrics + runner_type metadata - MLflow spans: ambient.runner_type; same span names as Langfuse - Docs: README + module docstrings Made-with: Cursor
Use mlflow.get_active_trace_id() for get_current_trace_id and sync last_trace_id after MLflow-only start_turn so middleware, corrections, and feedback can correlate runs without Langfuse. Made-with: Cursor
Re-expand the observability.py header with turn/tool trace hierarchy and Langfuse/MLflow notes, using vendor-neutral naming (llm_interaction). Made-with: Cursor
…e_id The CustomEvent emitted by the tracing middleware can carry either a Langfuse or MLflow trace ID depending on the active backend. Rename the event from ambient:langfuse_trace to ambient:trace_id and update the docstring to reflect this. Addresses CodeRabbit review comment. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
… main Move _privacy_masking_function alias after all imports, matching main's ordering to prevent merge conflict on observability.py. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
jeremyeder
force-pushed
the
fix/observability-naming-mlflow-trace-ids
branch
from
1609635 to
9785493
Compare
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
3 participants
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.
Summary
Follow-up to the merged MLflow observability work:
mainlanded the integration before these two commits could ship. This PR restores them.1. Framework-agnostic tracing names (Langfuse + MLflow)
claude_interaction→llm_interactionclaude-code→runner:<RUNNER_TYPE>(fromRUNNER_TYPE)Claude Code - Session Metrics→Session Metricsclaude-code-metrics→ambient-runner-metricsMigration: Existing Langfuse dashboards, alerts, or saved views that filter on the old trace name or tags need to be updated to the new values.
2. MLflow-only: trace IDs for middleware and feedback
When
OBSERVABILITY_BACKENDS=mlflow(no Langfuse turn), the runner now:mlflow.get_active_trace_id()viaObservabilityManager.get_current_trace_id()last_trace_idafterstart_turnso AG-UI trace events, corrections, and/feedbackresolution behave like the Langfuse path.Testing
pytest(runner):test_observability,test_observability_metrics,test_observability_mlflow_integration,test_privacy_masking(101 tests in local run)Commits
refactor(runner): framework-agnostic observability naming (Gage review)(cherry-picked)fix(runner): expose MLflow trace ids when Langfuse is off(cherry-picked)Summary by CodeRabbit
New Features
Changes
Bug Fixes
Documentation
Tests