Fix Tempo tracing field name and null data handling in API response

[Sandeep20013]

Pull Request

Summary

Fix two issues related to Tempo tracing and API handling:

1. Corrected CRD field name from enableTracing to enableTraces to match the expected schema.
2. Fixed handling of null response from Tempo services API. Previously, the code assumed the data field would default to an empty list via .get("data", []). However, when the API returns "data": null, this caused runtime issues. Updated logic to use or [] so it safely falls back to an empty list.

Related Issues

• JIRA: https://redhat.atlassian.net/browse/RHOAIENG-54322

Please review and indicate how it has been tested

• Locally
• Jenkins

Summary by CodeRabbit

• Tests
  • Updated test telemetry configuration to improve compatibility with tracing verification.
  • Made API response handling in tests more defensive, ensuring stable behavior when expected response fields are empty or null.

[github-actions]

The following are automatically added/executed:

• PR size label.
• Run pre-commit
• Run tox
• Add PR author as the PR assignee
• Build image based on the PR

Available user actions:

• To mark a PR as WIP, add /wip in a comment. To remove it from the PR comment /wip cancel to the PR.
• To block merging of a PR, add /hold in a comment. To un-block merging of PR comment /hold cancel.
• To mark a PR as approved, add /lgtm in a comment. To remove, add /lgtm cancel.
  lgtm label removed on each new commit push.
• To mark PR as verified comment /verified to the PR, to un-verify comment /verified cancel to the PR.
  verified label removed on each new commit push.
• To Cherry-pick a merged PR /cherry-pick <target_branch_name> to the PR. If <target_branch_name> is valid,
  and the current PR is merged, a cherry-picked PR would be created and linked to the current PR.
• To build and push image to quay, add /build-push-pr-image in a comment. This would create an image with tag
  pr-<pr_number> to quay repository. This image tag, however would be deleted on PR merge or close action.

Supported labels

{'/lgtm', '/wip', '/build-push-pr-image', '/cherry-pick', '/verified', '/hold'}

[coderabbitai]

📝 Walkthrough

Walkthrough

Two test files updated: tests/fixtures/guardrails.py renames OTEL exporter option enableTracing → enableTraces in the guardrails_orchestrator fixture; tests/model_explainability/guardrails/test_guardrails.py makes Tempo /api/services parsing defensive by using .get("data") or [].

Changes

Cohort / File(s)	Summary
Fixture Configuration tests/fixtures/guardrails.py	OTEL exporter config key changed from enableTracing to enableTraces when constructing GuardrailsOrchestrator. Verify consumers expect the new key and no silent misconfiguration occurs.
API Response Handling tests/model_explainability/guardrails/test_guardrails.py	Polling helper now uses .get("data") or [] to treat falsy data (e.g., null) as empty list. Confirms downstream filtering and retry logic handle empty lists consistently.

Security notes (actionable):

• Validate the config-key rename does not disable tracing in environments reliant on the old key; loss of tracing can impede incident detection and response (relevant class: Improper Input/Handling — CWE-20). Action: audit config consumers and CI/test environments.
• Ensure the .get("data") or [] change does not mask malformed or unexpected API responses; add explicit logging/metrics for unexpected response shapes to avoid silent failures (improper handling of exceptional conditions — CWE-703).

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

🚥 Pre-merge checks | ✅ 2

✅ Passed checks (2 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately describes both main changes: the Tempo field name correction and null data handling fix in the API response.
Description check	✅ Passed	The description includes all required sections with substantive content: summary explains both issues clearly, related JIRA issue is linked, and testing status is documented.

_{✏️ 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

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

tests/model_explainability/guardrails/test_guardrails.py (1)

390-403: ⚠️ Potential issue | 🟠 Major

Add explicit HTTP timeouts (CWE-400) and handle transient request failures to allow retries.

Lines 392 and 399 call requests.get() without timeout, risking indefinite hangs and resource exhaustion during polling. More critically, unhandled requests.RequestException and JSON decode errors will propagate immediately instead of triggering the @retry mechanism—the decorator only retries on False returns, not exceptions. Without try-except blocks, any network or parsing failure breaks the retry loop entirely.

Suggested fix

     `@retry`(wait_timeout=Timeout.TIMEOUT_1MIN, sleep=5)
     def check_traces():
-            services = requests.get(f"{tempo_traces_service_portforward}/api/services").json().get("data") or []
+            try:
+                services_resp = requests.get(
+                    f"{tempo_traces_service_portforward}/api/services",
+                    timeout=10,
+                )
+                services_resp.raise_for_status()
+                services = services_resp.json().get("data") or []
+            except (requests.RequestException, ValueError):
+                return False
             guardrails_services = [s for s in services if "guardrails" in s]
             if not guardrails_services:
                 return False

             svc = guardrails_services[0]

-            traces = requests.get(f"{tempo_traces_service_portforward}/api/traces?service={svc}").json()
+            try:
+                traces_resp = requests.get(
+                    f"{tempo_traces_service_portforward}/api/traces?service={svc}",
+                    timeout=10,
+                )
+                traces_resp.raise_for_status()
+                traces = traces_resp.json()
+            except (requests.RequestException, ValueError):
+                return False

             if traces.get("data"):
                 return traces

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@tests/model_explainability/guardrails/test_guardrails.py` around lines 390 -
403, The check_traces() helper currently calls requests.get(...) and .json()
without timeouts or error handling; wrap each HTTP call in try/except catching
requests.RequestException and ValueError/JSONDecodeError, pass a sensible
timeout (e.g., timeout=10) to requests.get, check response.status_code before
.json(), and on any exception or non-2xx status return False so the `@retry`
decorator can retry; update references inside check_traces() where
requests.get() is called (including the services and traces requests) to use
this pattern.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Outside diff comments:
In `@tests/model_explainability/guardrails/test_guardrails.py`:
- Around line 390-403: The check_traces() helper currently calls
requests.get(...) and .json() without timeouts or error handling; wrap each HTTP
call in try/except catching requests.RequestException and
ValueError/JSONDecodeError, pass a sensible timeout (e.g., timeout=10) to
requests.get, check response.status_code before .json(), and on any exception or
non-2xx status return False so the `@retry` decorator can retry; update references
inside check_traces() where requests.get() is called (including the services and
traces requests) to use this pattern.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository YAML (base), Central YAML (inherited), Organization UI (inherited)

Review profile: CHILL

Plan: Pro

Run ID: 137690ea-37de-4e76-83ca-817b40385e5c

📥 Commits

Reviewing files that changed from the base of the PR and between 2d927b5 and 81f9eba.

📒 Files selected for processing (2)

• tests/fixtures/guardrails.py
• tests/model_explainability/guardrails/test_guardrails.py

[dbasunag]

Please fix the PR title.

[kpunwatk]

/lgtm, Thanks for the fix @Sandeep20013

[coderabbitai]

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 `@tests/model_explainability/guardrails/test_guardrails.py`:
- Line 393: The requests.get() calls fetching Tempo data (e.g., the call that
assigns services =
requests.get(f"{tempo_traces_service_portforward}/api/services").json().get("data")
or []) must include an explicit timeout and an HTTP status check to avoid
hanging the retry loop (Timeout.TIMEOUT_1MIN); update those calls in
tests/model_explainability/guardrails/test_guardrails.py to pass a timeout
(e.g., timeout=5) to requests.get and call response.raise_for_status() or check
response.status_code before calling .json(), and handle/raise a clear exception
so the retry logic can proceed instead of blocking.

🪄 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: Repository YAML (base), Central YAML (inherited), Organization UI (inherited)

Review profile: CHILL

Plan: Pro

Run ID: ae9a5d7d-ca60-485a-9c6c-1ed80045fad0

📥 Commits

Reviewing files that changed from the base of the PR and between 81f9eba and 8c3647e.

📒 Files selected for processing (1)

• tests/model_explainability/guardrails/test_guardrails.py

[github-actions]

Status of building tag latest: success.
Status of pushing tag latest to image registry: success.