Skip to content

feat: add MLflow tracing to all agent templates - #34

Merged
mpk-droid merged 9 commits into
red-hat-data-services:mainfrom
sanafayyaz315:mlflow-integration-v2
Mar 25, 2026
Merged

mpk-droid merged 9 commits into
red-hat-data-services:mainfrom
sanafayyaz315:mlflow-integration-v2

Conversation

@sanafayyaz315

Copy link
Copy Markdown
Contributor

Summary

Add optional MLflow tracing integration to all 4 agent templates
(CrewAI, LangGraph, LlamaIndex, OpenAI Responses).

What's included

1. MLflow tracing integration for all 4 frameworks

  • CrewAI: mlflow.crewai.autolog() + provider-specific autolog
    (openai)
  • LangGraph: mlflow.langchain.autolog()
  • LlamaIndex: mlflow.llama_index.autolog()
  • OpenAI Responses: mlflow.openai.autolog() + manual
    wrap_func_with_mlflow_trace for agent/tool spans

2. Per-template changes

  • tracing.py (new) — enable_tracing(), check_mlflow_health(),
    conditional autolog
  • main.py — call enable_tracing() in lifespan
  • template.env — MLflow config vars (local + OpenShift cluster)
  • README.md — MLflow setup instructions

3. Fixes

  • CrewAI: wrap_func_with_mlflow_trace on tool _run for explicit
    TOOL spans

4. Design decisions

  • MLflow is NOT a dependency in pyproject.toml — user
    installs manually
  • Tracing is optional — app works normally without
    MLFLOW_TRACKING_URI
  • If tracing is enabled but server is unreachable, app crashes at
    startup (fail-fast)

@coderabbitai

coderabbitai Bot commented Mar 23, 2026 •

Copy link
Copy Markdown
Contributor

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

Adds MLflow tracing: new tracing modules, startup calls to enable_tracing(), conditional wrapping of tool/agent callables for MLflow spans, updates to environment templates, and expanded README tracing documentation for Local and OpenShift deployments.

Changes

Cohort / File(s) Summary
Version Control & Root Configuration
/.gitignore
Added ignore patterns: *.db, CLAUDE.md, and agents/demo/.
Documentation Updates
agents/crewai/websearch_agent/README.md, agents/langgraph/react_agent/README.md, agents/llamaindex/websearch_agent/README.md, agents/vanilla_python/openai_responses_agent/README.md
Added MLflow "Tracing" sections, Local/OpenShift env examples, optional install/start instructions, and switched .env examples to ini fences.
Application Startup Integration
agents/crewai/websearch_agent/main.py, agents/langgraph/react_agent/main.py, agents/llamaindex/websearch_agent/main.py, agents/vanilla_python/openai_responses_agent/main.py
Imported and invoked enable_tracing() in FastAPI lifespan startup so tracing is attempted before agents/tools are constructed.
MLflow Tracing Modules
agents/crewai/websearch_agent/src/crewai_web_search/tracing.py, agents/langgraph/react_agent/src/react_agent/tracing.py, agents/llamaindex/websearch_agent/src/websearch_agent/tracing.py, agents/vanilla_python/openai_responses_agent/src/openai_responses_agent/tracing.py
New tracing modules: check_mlflow_health() (health-check + retry), enable_tracing() (dotenv, set tracking URI/experiment, enable async logging, framework autolog), and wrap_func_with_mlflow_trace() (conditionally wrap callables with mlflow.trace spans where implemented).
CrewAI Tool Integration
agents/crewai/websearch_agent/src/crewai_web_search/crew.py
Builds tools list up-front and wraps each tool’s _run with wrap_func_with_mlflow_trace(..., span_type="tool", name=tool.name) before Agent creation.
Vanilla Python Agent Changes
agents/vanilla_python/openai_responses_agent/src/openai_responses_agent/agent.py, agents/vanilla_python/openai_responses_agent/main.py
Wrapped adapter tool callables and agent.query with tracing wrappers (span_type "tool"/"agent"); adjusted message→Responses mapping and made CSV argument parsing resilient to empty rows.
Environment Templates
agents/crewai/websearch_agent/template.env, agents/langgraph/react_agent/template.env, agents/llamaindex/websearch_agent/template.env, agents/vanilla_python/openai_responses_agent/template.env
Updated API_KEY placeholder to <your-api-key> with guidance, added LLM_PROVIDER where applicable, and appended commented MLflow tracing variables for Local and OpenShift (URI, experiment, token, TLS, workspace, timeouts/retries).

Sequence Diagram(s)

mermaid
sequenceDiagram
participant Startup as "App Startup"
participant Tracing as "Tracing Module"
participant MLflow as "MLflow Server"
participant Agent as "Agent / Tools"
Startup->>Tracing: call enable_tracing()
Tracing->>MLflow: GET {MLFLOW_TRACKING_URI}/health (with retry)
MLflow-->>Tracing: 200 OK
Tracing->>Tracing: configure mlflow, set experiment, enable autolog
Startup->>Agent: build agent and register tools
Agent->>Tracing: wrap tool._run / agent.query with mlflow.trace
Note over Agent,MLflow: During runtime
Agent->>MLflow: traced tool/agent spans logged

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately summarizes the main change: adding MLflow tracing integration to all four agent templates (CrewAI, LangGraph, LlamaIndex, OpenAI Responses).
Description check ✅ Passed The description comprehensively documents what is included in the PR: MLflow tracing for each framework, per-template changes (tracing.py, main.py, template.env, README), design decisions regarding optional tracing and manual MLflow installation, and the graceful degradation behavior when the server is unreachable.
Docstring Coverage ✅ Passed Docstring coverage is 86.36% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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 and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 7

🧹 Nitpick comments (3)
agents/vanilla_python/openai_responses_agent/src/openai_responses_agent/agent.py (1)

69-69: Unused attribute _mlflow_enabled.

This attribute is set but never referenced in this file. Consider removing it if it's not needed elsewhere.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@agents/vanilla_python/openai_responses_agent/src/openai_responses_agent/agent.py`
at line 69, The attribute self._mlflow_enabled is assigned using
getenv("MLFLOW_TRACKING_URI") in the constructor but never used elsewhere;
either remove this unused attribute assignment or wire it into logic that needs
it (e.g., replace other MLflow checks with self._mlflow_enabled or guard
MLflow-related behavior using this flag). Locate the assignment to
self._mlflow_enabled in the class constructor in agent.py and delete that line
if not needed, or update any MLflow-related methods to reference
self._mlflow_enabled consistently.
agents/crewai/websearch_agent/pyproject.toml (1)

11-11: Consider adding a version constraint for stability.

Removing all version constraints from crewai[litellm] may lead to unexpected breakages when upstream releases breaking changes. Consider specifying at least a minimum version (e.g., crewai[litellm]>=1.10.0) to document the tested baseline.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@agents/crewai/websearch_agent/pyproject.toml` at line 11, The pyproject
dependency "crewai[litellm]" lacks a version constraint which can cause
breakages; update the dependency string in
agents/crewai/websearch_agent/pyproject.toml (the "crewai[litellm]" entry) to
include a minimum tested version (for example "crewai[litellm]>=1.10.0") or
another appropriate version range to pin a stable baseline.
agents/llamaindex/websearch_agent/README.md (1)

48-53: Add language specifiers to fenced code blocks.

The environment variable configuration blocks are missing language specifiers. Adding bash or ini helps with syntax highlighting and satisfies markdown linting.

📝 Proposed fix
-```
+```bash
 MLFLOW_TRACKING_URI="http://localhost:5000"
-```
+```bash
 MLFLOW_TRACKING_URI="https://<openshift-dashboard-url>/mlflow"

Also applies to: 85-91

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@agents/llamaindex/websearch_agent/README.md` around lines 48 - 53, Add
language specifiers (e.g., bash or ini) to the fenced code blocks that show
environment variables in agents/llamaindex/websearch_agent/README.md so markdown
linting and syntax highlighting work; update the three backtick fences
surrounding the MLFLOW_* blocks (the blocks around the sample env lines at the
shown sections, including the second block at lines ~85-91) to start with
```bash (or ```ini) instead of just ```, ensuring each opening fence includes
the language token and the closing fence remains ```; no other content changes
are required.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@agents/crewai/websearch_agent/src/crewai_web_search/tracing.py`:
- Around line 38-44: The warning log in tracing.py uses response.reason_phrase
which doesn't exist on requests.Response; update the log to use response.reason
instead (keep response.status_code, response.text[:500], mlflow_url and logger
as-is) so the logger.warning call references response.reason instead of
response.reason_phrase.
- Around line 99-107: The except block in tracing.py is catching HTTPException
but check_mlflow_health raises RuntimeError, so the handler is unreachable;
update the exception handler to catch RuntimeError (e.g., change "except
HTTPException as e" to "except RuntimeError as e") and ensure you use the caught
exception (e) when logging and when re-raising the RuntimeError with the
existing message; locate this in the block that calls check_mlflow_health and
updates logger.info/logger.warning.

In `@agents/langgraph/react_agent/src/react_agent/tracing.py`:
- Around line 20-54: check_mlflow_health currently uses raw requests and misses
MLflow client settings (MLFLOW_TRACKING_TOKEN, MLFLOW_TRACKING_INSECURE_TLS,
MLFLOW_TRACKING_AUTH, MLFLOW_HTTP_REQUEST_TIMEOUT,
MLFLOW_HTTP_REQUEST_MAX_RETRIES) and references response.reason_phrase
(invalid); update it to use the MLflow client or replicate its settings: create
an mlflow.tracking.MlflowClient (or configure requests.Session with token/auth,
verify flag from MLFLOW_TRACKING_INSECURE_TLS, timeout from
MLFLOW_HTTP_REQUEST_TIMEOUT and retries from MLFLOW_HTTP_REQUEST_MAX_RETRIES) to
call the server health endpoint, replace response.reason_phrase with
response.reason, and keep the same retry loop and logging in check_mlflow_health
so the probe behaves identically to the real MLflow client.

In `@agents/llamaindex/websearch_agent/src/websearch_agent/tracing.py`:
- Around line 38-44: The logger.warning call is using the non-existent attribute
response.reason_phrase which raises AttributeError; update the log to use
response.reason instead (leave response.status_code, response.text[:500], and
mlflow_url as-is) so the warning message accesses the correct requests.Response
attribute (locate the logger.warning block that references
response.reason_phrase and replace that symbol with response.reason).
- Around line 77-85: The except block is catching HTTPException although
check_mlflow_health() raises RuntimeError, so replace the except HTTPException
as e handler with except RuntimeError as e to ensure the handler runs; update
the logger.warning to include the original error info and re-raise the
RuntimeError with the wrapped message using exception chaining (raise
RuntimeError(...) from e) so the original traceback is preserved; refer to the
check_mlflow_health call and the surrounding logger.info/logger.warning and the
existing raise RuntimeError block when making the change.

In
`@agents/vanilla_python/openai_responses_agent/src/openai_responses_agent/agent.py`:
- Around line 111-116: The _messages_to_responses_input function currently never
sets instructions and treats system messages as input_text; update it to detect
messages with role "system", set the instructions variable to that message's
content (use m.get("content","") or "" as a fallback), skip adding that message
to input_items, and continue processing other messages (mapping "assistant" to
content_type "output_text" and all other roles to "input_text"); return the
instructions string and the constructed input_items list.

In
`@agents/vanilla_python/openai_responses_agent/src/openai_responses_agent/tracing.py`:
- Around line 20-54: The check_mlflow_health function currently uses raw
requests and response.reason_phrase which breaks configured MLflow
tracing/auth/tls settings and raises AttributeError; update check_mlflow_health
to use the MLflow client (mlflow.tracking.MlflowClient) by setting
mlflow.set_tracking_uri(mlflow_tracking_uri) and invoking the client to probe
the /health endpoint (or use the client's internal endpoint caller) so all
MLFLOW_* settings (MLFLOW_TRACKING_TOKEN, MLFLOW_TRACKING_INSECURE_TLS,
MLFLOW_TRACKING_AUTH, MLFLOW_HTTP_REQUEST_TIMEOUT,
MLFLOW_HTTP_REQUEST_MAX_RETRIES) are honored, and replace any use of
response.reason_phrase with response.reason if you keep any raw requests usage.

---

Nitpick comments:
In `@agents/crewai/websearch_agent/pyproject.toml`:
- Line 11: The pyproject dependency "crewai[litellm]" lacks a version constraint
which can cause breakages; update the dependency string in
agents/crewai/websearch_agent/pyproject.toml (the "crewai[litellm]" entry) to
include a minimum tested version (for example "crewai[litellm]>=1.10.0") or
another appropriate version range to pin a stable baseline.

In `@agents/llamaindex/websearch_agent/README.md`:
- Around line 48-53: Add language specifiers (e.g., bash or ini) to the fenced
code blocks that show environment variables in
agents/llamaindex/websearch_agent/README.md so markdown linting and syntax
highlighting work; update the three backtick fences surrounding the MLFLOW_*
blocks (the blocks around the sample env lines at the shown sections, including
the second block at lines ~85-91) to start with ```bash (or ```ini) instead of
just ```, ensuring each opening fence includes the language token and the
closing fence remains ```; no other content changes are required.

In
`@agents/vanilla_python/openai_responses_agent/src/openai_responses_agent/agent.py`:
- Line 69: The attribute self._mlflow_enabled is assigned using
getenv("MLFLOW_TRACKING_URI") in the constructor but never used elsewhere;
either remove this unused attribute assignment or wire it into logic that needs
it (e.g., replace other MLflow checks with self._mlflow_enabled or guard
MLflow-related behavior using this flag). Locate the assignment to
self._mlflow_enabled in the class constructor in agent.py and delete that line
if not needed, or update any MLflow-related methods to reference
self._mlflow_enabled consistently.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: bd40c6ff-5599-4e59-9d2b-5e4a1aae9857

📥 Commits

Reviewing files that changed from the base of the PR and between e530dea and 0e4d57d.

📒 Files selected for processing (20)
  • .gitignore
  • agents/crewai/websearch_agent/README.md
  • agents/crewai/websearch_agent/main.py
  • agents/crewai/websearch_agent/pyproject.toml
  • agents/crewai/websearch_agent/src/crewai_web_search/crew.py
  • agents/crewai/websearch_agent/src/crewai_web_search/tracing.py
  • agents/crewai/websearch_agent/template.env
  • agents/langgraph/react_agent/README.md
  • agents/langgraph/react_agent/main.py
  • agents/langgraph/react_agent/src/react_agent/tracing.py
  • agents/langgraph/react_agent/template.env
  • agents/llamaindex/websearch_agent/README.md
  • agents/llamaindex/websearch_agent/main.py
  • agents/llamaindex/websearch_agent/src/websearch_agent/tracing.py
  • agents/llamaindex/websearch_agent/template.env
  • agents/vanilla_python/openai_responses_agent/README.md
  • agents/vanilla_python/openai_responses_agent/main.py
  • agents/vanilla_python/openai_responses_agent/src/openai_responses_agent/agent.py
  • agents/vanilla_python/openai_responses_agent/src/openai_responses_agent/tracing.py
  • agents/vanilla_python/openai_responses_agent/template.env

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🧹 Nitpick comments (3)
agents/langgraph/react_agent/src/react_agent/tracing.py (1)

79-84: Preserve the original exception cause on startup failure.

At Line 81, the new RuntimeError is raised without chaining, which hides the original stack context from check_mlflow_health().

Proposed fix
     except RuntimeError as e:
         logger.warning(f"[Tracing] MLflow server is unreachable at {tracking_uri}")
         raise RuntimeError(
                     f"MLFLOW_TRACKING_URI is set but server is unreachable: {tracking_uri}. "
                     f"Start the server or check the URI. Error: {e}"
-                )
+                ) from e
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@agents/langgraph/react_agent/src/react_agent/tracing.py` around lines 79 -
84, In the except RuntimeError block inside tracing.py (handling
check_mlflow_health/tracking URI), preserve the original exception by raising
the new RuntimeError using exception chaining (raise RuntimeError(... ) from e)
instead of raising it plain; update the raise in the except block that currently
logs logger.warning(f"[Tracing] MLflow server is unreachable at {tracking_uri}")
to include "from e" so the original stack context is retained for debugging.
agents/crewai/websearch_agent/src/crewai_web_search/tracing.py (1)

19-146: Extract shared MLflow bootstrap logic to a common tracing utility.

check_mlflow_health + startup wiring is duplicated across multiple templates, which increases drift risk and patch overhead. Consider centralizing shared logic and keeping only framework-specific autolog()/span wiring per template.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@agents/crewai/websearch_agent/src/crewai_web_search/tracing.py` around lines
19 - 146, The check_mlflow_health, enable_tracing, and
wrap_func_with_mlflow_trace logic is duplicated across templates — extract the
shared MLflow bootstrap (health check, tracking URI handling,
mlflow.set_tracking_uri, set_experiment, enable_async_logging, and provider
autolog selection logic) into a common utility function/class (e.g.,
tracing.bootstrap or tracing.enable_mlflow) and replace the duplicated blocks in
this file with calls to that utility; keep only template-specific wiring here
(e.g., mlflow.crewai.autolog() or any framework-specific autolog calls) and
ensure you reference the original symbols when refactoring: check_mlflow_health,
enable_tracing, wrap_func_with_mlflow_trace, provider_autolog_map, and
mlflow.crewai.autolog to guide where to move code and what to retain locally.
agents/vanilla_python/openai_responses_agent/src/openai_responses_agent/tracing.py (1)

100-105: Chain the original exception when re-raising.

At Line 102, use from e so the root startup failure cause is preserved.

Proposed fix
     except RuntimeError as e:
         logger.warning(f"[Tracing] MLflow server is unreachable at {tracking_uri}")
         raise RuntimeError(
                     f"MLFLOW_TRACKING_URI is set but server is unreachable: {tracking_uri}. "
                     f"Start the server or check the URI. Error: {e}"
-                )
+                ) from e
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@agents/vanilla_python/openai_responses_agent/src/openai_responses_agent/tracing.py`
around lines 100 - 105, In the except RuntimeError block in tracing.py (the
handler that logs MLflow server unreachability using logger and tracking_uri),
re-raise the new RuntimeError using exception chaining so the original exception
is preserved—replace the current "raise RuntimeError(...)" with "raise
RuntimeError(... ) from e" so the original RuntimeError (variable e) is attached
as the __cause__.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@agents/crewai/websearch_agent/src/crewai_web_search/tracing.py`:
- Around line 55-73: The function wrap_func_with_mlflow_trace shadows the
builtin type() and misdeclares name's type; rename the parameter type to
span_type (or similar) and update any internal references from type to
span_type, change the signature to use name: Optional[str] = None (and import
Optional if not already), and update all call sites of
wrap_func_with_mlflow_trace to pass the renamed argument
(span_type="tool"/"agent") so the comparisons and mlflow.trace calls use
span_type instead of the builtin name.

In
`@agents/vanilla_python/openai_responses_agent/src/openai_responses_agent/tracing.py`:
- Around line 56-74: Rename the parameter in wrap_func_with_mlflow_trace from
type to span_type to avoid shadowing the builtin and change its annotation to
Literal["tool","agent"] so only valid span values are accepted; update all
internal references (the comparisons and error message) to use span_type and add
a static type hint import from typing for Literal, ensuring the function
signature and control flow only allow "tool" or "agent" (raise ValueError for
anything else).

---

Nitpick comments:
In `@agents/crewai/websearch_agent/src/crewai_web_search/tracing.py`:
- Around line 19-146: The check_mlflow_health, enable_tracing, and
wrap_func_with_mlflow_trace logic is duplicated across templates — extract the
shared MLflow bootstrap (health check, tracking URI handling,
mlflow.set_tracking_uri, set_experiment, enable_async_logging, and provider
autolog selection logic) into a common utility function/class (e.g.,
tracing.bootstrap or tracing.enable_mlflow) and replace the duplicated blocks in
this file with calls to that utility; keep only template-specific wiring here
(e.g., mlflow.crewai.autolog() or any framework-specific autolog calls) and
ensure you reference the original symbols when refactoring: check_mlflow_health,
enable_tracing, wrap_func_with_mlflow_trace, provider_autolog_map, and
mlflow.crewai.autolog to guide where to move code and what to retain locally.

In `@agents/langgraph/react_agent/src/react_agent/tracing.py`:
- Around line 79-84: In the except RuntimeError block inside tracing.py
(handling check_mlflow_health/tracking URI), preserve the original exception by
raising the new RuntimeError using exception chaining (raise RuntimeError(... )
from e) instead of raising it plain; update the raise in the except block that
currently logs logger.warning(f"[Tracing] MLflow server is unreachable at
{tracking_uri}") to include "from e" so the original stack context is retained
for debugging.

In
`@agents/vanilla_python/openai_responses_agent/src/openai_responses_agent/tracing.py`:
- Around line 100-105: In the except RuntimeError block in tracing.py (the
handler that logs MLflow server unreachability using logger and tracking_uri),
re-raise the new RuntimeError using exception chaining so the original exception
is preserved—replace the current "raise RuntimeError(...)" with "raise
RuntimeError(... ) from e" so the original RuntimeError (variable e) is attached
as the __cause__.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: a20d39b4-fdfa-4566-b8aa-22ab5e68df6f

📥 Commits

Reviewing files that changed from the base of the PR and between 0e4d57d and 1abe73c.

📒 Files selected for processing (9)
  • agents/crewai/websearch_agent/README.md
  • agents/crewai/websearch_agent/src/crewai_web_search/tracing.py
  • agents/langgraph/react_agent/README.md
  • agents/langgraph/react_agent/src/react_agent/tracing.py
  • agents/llamaindex/websearch_agent/README.md
  • agents/llamaindex/websearch_agent/src/websearch_agent/tracing.py
  • agents/vanilla_python/openai_responses_agent/README.md
  • agents/vanilla_python/openai_responses_agent/src/openai_responses_agent/agent.py
  • agents/vanilla_python/openai_responses_agent/src/openai_responses_agent/tracing.py
✅ Files skipped from review due to trivial changes (4)
  • agents/vanilla_python/openai_responses_agent/README.md
  • agents/langgraph/react_agent/README.md
  • agents/crewai/websearch_agent/README.md
  • agents/llamaindex/websearch_agent/README.md
🚧 Files skipped from review as they are similar to previous changes (1)
  • agents/vanilla_python/openai_responses_agent/src/openai_responses_agent/agent.py

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
agents/crewai/websearch_agent/src/crewai_web_search/tracing.py (1)

101-106: Chain the exception for better tracebacks.

When re-raising an exception within an except block, use raise ... from e to preserve the exception chain for debugging.

♻️ Proposed fix
     except RuntimeError as e:
         logger.warning(f"[Tracing] MLflow server is unreachable at {tracking_uri}")
         raise RuntimeError(
                     f"MLFLOW_TRACKING_URI is set but server is unreachable: {tracking_uri}. "
                     f"Start the server or check the URI. Error: {e}"
-                )
+                ) from e
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@agents/crewai/websearch_agent/src/crewai_web_search/tracing.py` around lines
101 - 106, The except block in tracing.py that catches "RuntimeError as e" and
re-raises a new RuntimeError should chain the original exception to preserve
tracebacks; update the raise in that except handler to use "raise
RuntimeError(... ) from e" (referencing the existing tracking_uri and e
variables) so the original exception is linked to the new one.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@agents/crewai/websearch_agent/src/crewai_web_search/tracing.py`:
- Around line 101-106: The except block in tracing.py that catches "RuntimeError
as e" and re-raises a new RuntimeError should chain the original exception to
preserve tracebacks; update the raise in that except handler to use "raise
RuntimeError(... ) from e" (referencing the existing tracking_uri and e
variables) so the original exception is linked to the new one.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 588457c6-c08e-44f3-b297-923f8f410ab4

📥 Commits

Reviewing files that changed from the base of the PR and between 1abe73c and a81ad79.

📒 Files selected for processing (2)
  • agents/crewai/websearch_agent/src/crewai_web_search/crew.py
  • agents/crewai/websearch_agent/src/crewai_web_search/tracing.py
🚧 Files skipped from review as they are similar to previous changes (1)
  • agents/crewai/websearch_agent/src/crewai_web_search/crew.py

@mpk-droid mpk-droid left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall looks good. Added few comments. Could you also fix "major" coderabbit comments?

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@agents/langgraph/react_agent/src/react_agent/tracing.py`:
- Around line 29-33: Replace use of time.time() and hardcoded timeout=5 in the
health-check loop with a monotonic timer and a per-request remaining budget:
initialize start_time using time.monotonic(), compute remaining = max_wait_time
- (time.monotonic() - start_time) before each iteration, pass timeout=min(5,
remaining) to requests.get(mlflow_url, timeout=...), and when sleeping use
sleep_duration = min(backoff_seconds, remaining) (or skip sleeping if remaining
<= 0) so neither the request nor the sleep can exceed the configured
max_wait_time; apply these changes in the loop that contains start_time, the
while True block, and the requests.get call.
- Around line 72-85: The mlflow imports and timeout parsing in tracing.py can
raise ModuleNotFoundError or ValueError and must be guarded; wrap the "import
mlflow" and "import mlflow.langchain" in a try/except ModuleNotFoundError and
set a flag (e.g., mlflow_available) to False when missing so the tracing block
skips MLflow logic, and when reading MLFLOW_HEALTH_CHECK_TIMEOUT use a safe
parse with a fallback (e.g., default 5) catching ValueError before calling
check_mlflow_health(tracking_uri=tracking_uri,
max_wait_time=health_check_timeout); keep existing RuntimeError handling for
check_mlflow_health and log appropriate messages via logger so startup
gracefully degrades when mlflow is absent or the env var is malformed.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 33ce0614-28a1-4f57-b4ff-466c5926282d

📥 Commits

Reviewing files that changed from the base of the PR and between 827c07c and 4a6898c.

📒 Files selected for processing (4)
  • agents/crewai/websearch_agent/src/crewai_web_search/tracing.py
  • agents/langgraph/react_agent/src/react_agent/tracing.py
  • agents/llamaindex/websearch_agent/src/websearch_agent/tracing.py
  • agents/vanilla_python/openai_responses_agent/src/openai_responses_agent/tracing.py
🚧 Files skipped from review as they are similar to previous changes (2)
  • agents/crewai/websearch_agent/src/crewai_web_search/tracing.py
  • agents/vanilla_python/openai_responses_agent/src/openai_responses_agent/tracing.py

@mpk-droid mpk-droid left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks for fixes. couple more comments.

Comment thread agents/crewai/websearch_agent/README.md Resolved
Comment thread agents/crewai/websearch_agent/README.md Resolved
sanafayyaz315 and others added 9 commits March 24, 2026 13:51
… Responses agent templates

Add optional MLflow tracing support to 4 agent templates. Tracing is
enabled when MLFLOW_TRACKING_URI is set, with health checks and
graceful fallback.

- Add tracing.py with health check, enable_tracing(), and
  framework-specific autolog for each agent
- Add enable_tracing() call in each main.py lifespan function
- Add MLflow env vars (local + OpenShift) to all template.env files
- Add tracing setup docs to all README files
- CrewAI: provider-aware LLM autolog via LLM_PROVIDER env var,
  pin crewai[litellm]==1.9.2
- OpenAI Responses: wrap tools/agent with MLflow spans,
  fix next(reader) StopIteration crash,
  fix _messages_to_responses_input content type for assistant messages
- Add *.db, CLAUDE.md, agents/demo/ to .gitignore

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Add wrap_func_with_mlflow_trace to CrewAI tracing.py for TOOL spans
- Wrap CrewAI tool _run methods with MLflow tracing in crew.py
- Unpin crewai[litellm] version (was ==1.9.2) to fix streaming+tools bug
- Add wrap_func_with_mlflow_trace to OpenAI Responses streaming path
- Remove stale `from anyio import current_time` import in OpenAI agent

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Fix response.reason_phrase → response.reason (requests API)
- Fix except HTTPException → except RuntimeError in health check error handling
- Fix e.detail → str(e) for RuntimeError (no .detail attribute)
- Remove unused HTTPException import from all tracing files
- Extract system messages to instructions param in OpenAI Responses agent
- Remove unused self._mlflow_enabled attribute
- Add crewai[litellm]>=1.10.1 version constraint
- Add ini language specifiers to README code blocks

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…rsions

mlflow.crewai.autolog() does not capture tool spans in CrewAI >=1.10.
Manual wrapping via wrap_func_with_mlflow_trace is needed until this
is fixed. Comments warn to remove manual wrapping if autolog is fixed
in the future to avoid duplicate spans.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…OW_HEALTH_CHECK_TIMEOUT env var

- Rename `type` to `span_type` in wrap_func_with_mlflow_trace to avoid
  shadowing Python builtin, use Literal["tool", "agent"] for type safety
- Fix `name: str = None` to `Optional[str] = None` in CrewAI tracing
- Add MLFLOW_HEALTH_CHECK_TIMEOUT to template.env files (default 5s)
- Update all call sites to use span_type= keyword

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Instead of crashing the app, log a warning with connection timeout
duration and continue without tracing when MLflow is unavailable.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…v var

- Use min(5, remaining) for per-request timeout so it never exceeds
  the configured MLFLOW_HEALTH_CHECK_TIMEOUT budget
- Add ValueError guard for MLFLOW_HEALTH_CHECK_TIMEOUT parsing

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…w version

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@sanafayyaz315
sanafayyaz315 force-pushed the mlflow-integration-v2 branch from 8c55e3c to f98b24e Compare March 24, 2026 13:54

@mpk-droid mpk-droid left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Thanks for pushing all the fixes!

@mpk-droid
mpk-droid merged commit ca1d0df into red-hat-data-services:main Mar 25, 2026
1 check passed
tarun-etikala referenced this pull request in tarun-etikala/agentic-starter-kits Apr 16, 2026
- Return HTTP 503 from /health when agent is uninitialized so K8s
  readiness probes correctly gate traffic (8 agents, audit #5)
- Bind local dev servers to 127.0.0.1 to prevent DNS rebinding
  attacks; container Dockerfiles remain at 0.0.0.0 (audit #36)
- Standardize FastAPI to >=0.135.1 across all agents (audit #33)
- Bump uvicorn in mcp_automl_template to >=0.41.0 (audit #34)
- Remove env:init dependency in react_with_database_memory (audit red-hat-data-services#46)
- Update post-helm-migration-audit.md (14 resolved, 4 accepted)

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
tarun-etikala referenced this pull request in tarun-etikala/agentic-starter-kits Apr 16, 2026
- Return HTTP 503 from /health when agent is uninitialized so K8s
  readiness probes correctly gate traffic (8 agents, audit #5)
- Bind local dev servers to 127.0.0.1 to prevent DNS rebinding
  attacks; container Dockerfiles remain at 0.0.0.0 (audit #36)
- Standardize FastAPI to >=0.135.1 across all agents (audit #33)
- Bump uvicorn in mcp_automl_template to >=0.41.0 (audit #34)
- Remove env:init dependency in react_with_database_memory (audit red-hat-data-services#46)

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
tarun-etikala referenced this pull request in tarun-etikala/agentic-starter-kits Apr 20, 2026
- Return HTTP 503 from /health when agent is uninitialized so K8s
  readiness probes correctly gate traffic (8 agents, audit #5)
- Bind local dev servers to 127.0.0.1 to prevent DNS rebinding
  attacks; container Dockerfiles remain at 0.0.0.0 (audit #36)
- Standardize FastAPI to >=0.135.1 across all agents (audit #33)
- Bump uvicorn in mcp_automl_template to >=0.41.0 (audit #34)
- Remove env:init dependency in react_with_database_memory (audit red-hat-data-services#46)

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
tarun-etikala referenced this pull request in tarun-etikala/agentic-starter-kits Apr 20, 2026
- Return HTTP 503 from /health when agent is uninitialized so K8s
  readiness probes correctly gate traffic (8 agents, audit #5)
- Bind local dev servers to 127.0.0.1 to prevent DNS rebinding
  attacks; container Dockerfiles remain at 0.0.0.0 (audit #36)
- Standardize FastAPI to >=0.135.1 across all agents (audit #33)
- Bump uvicorn in mcp_automl_template to >=0.41.0 (audit #34)
- Remove env:init dependency in react_with_database_memory (audit red-hat-data-services#46)

Co-Authored-By: Claude Opus 4.6 (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.

4 participants