updated llm logs #2297
updated llm logs #2297
Conversation
WalkthroughThe logger module refines internal handling of LLM response data by parsing the original_response from raw strings, stripping sensitive fields (model, data, object), and enriching the logging payload with additional metadata including timestamps, costs, and LLM-specific fields (llm_call_id, llm_provider, model_params, metadata). Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
kairon/shared/llm/logger.py(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Codacy Static Code Analysis
- GitHub Check: Analyze (python)
- GitHub Check: Python CI
🔇 Additional comments (1)
kairon/shared/llm/logger.py (1)
38-45: LGTM with dependency on exception handling.The logging payload correctly passes the cleaned
original_responsealong with enriched metadata. This is consistent with the parsing logic above. However, ensure that the exception handling for JSON parsing (flagged in lines 30-36) is addressed to prevent potential runtime failures.
| raw = kwargs.get("original_response") | ||
| original_response = json.loads(raw) if raw else {} | ||
|
|
||
| if isinstance(original_response, dict): | ||
| original_response.pop("model", None) | ||
| original_response.pop("data", None) | ||
| original_response.pop("object", None) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n kairon/shared/llm/logger.py | head -50Repository: digiteinfotech/kairon
Length of output: 2497
🏁 Script executed:
grep -n "class LLMLogs" kairon/shared/llm/data_objects.pyRepository: digiteinfotech/kairon
Length of output: 91
🏁 Script executed:
sed -n '4,80p' kairon/shared/llm/data_objects.pyRepository: digiteinfotech/kairon
Length of output: 384
🏁 Script executed:
rg "DynamicField" kairon/shared/llm/data_objects.py -B 5 -A 2Repository: digiteinfotech/kairon
Length of output: 283
🏁 Script executed:
rg "json\.loads" kairon/ -B 2 -A 2 | head -60Repository: digiteinfotech/kairon
Length of output: 3699
🏁 Script executed:
rg "json\.loads" kairon/ -B 3 -A 3 | grep -A 3 "try\|except" | head -40Repository: digiteinfotech/kairon
Length of output: 2378
🏁 Script executed:
grep -r "original_response" kairon/shared/llm/ --include="*.py"Repository: digiteinfotech/kairon
Length of output: 619
🏁 Script executed:
python3 << 'EOF'
# Check what MongoEngine's DynamicField accepts
print("DynamicField in mongoengine accepts any Python object type.")
print("It can store: dict, list, string, number, boolean, None, etc.")
print("So non-dict values from json.loads are NOT a problem for LLMLogs.")
print("\nHowever, the json.loads exception handling IS a valid concern.")
EOFRepository: digiteinfotech/kairon
Length of output: 151
Add exception handling for JSON parsing.
json.loads(raw) can raise a JSONDecodeError if raw contains malformed JSON, crashing the logging pipeline and disrupting LLM call tracking. The codebase establishes a pattern of wrapping json.loads calls in try-except blocks.
🔎 Proposed fix with exception handling
raw = kwargs.get("original_response")
-original_response = json.loads(raw) if raw else {}
+try:
+ original_response = json.loads(raw) if raw else {}
+except (json.JSONDecodeError, TypeError) as e:
+ logger.warning(f"Failed to parse original_response as JSON: {e}")
+ original_response = {}
if isinstance(original_response, dict):
original_response.pop("model", None)
original_response.pop("data", None)
original_response.pop("object", None)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| raw = kwargs.get("original_response") | |
| original_response = json.loads(raw) if raw else {} | |
| if isinstance(original_response, dict): | |
| original_response.pop("model", None) | |
| original_response.pop("data", None) | |
| original_response.pop("object", None) | |
| raw = kwargs.get("original_response") | |
| try: | |
| original_response = json.loads(raw) if raw else {} | |
| except (json.JSONDecodeError, TypeError) as e: | |
| logger.warning(f"Failed to parse original_response as JSON: {e}") | |
| original_response = {} | |
| if isinstance(original_response, dict): | |
| original_response.pop("model", None) | |
| original_response.pop("data", None) | |
| original_response.pop("object", None) |
🤖 Prompt for AI Agents
In kairon/shared/llm/logger.py around lines 30 to 36, the direct call to
json.loads(raw) can raise JSONDecodeError and crash the logging pipeline; wrap
the json.loads call in a try/except that catches json.JSONDecodeError (and a
broad Exception fallback) and on failure set original_response = {} (or parse a
best-effort/sanitized value) and log the parsing error for debugging; then
proceed with the existing isinstance(dict) checks and pop calls so malformed
JSON does not break the flow.
1 participant
Summary by CodeRabbit
New Features
Bug Fixes
✏️ Tip: You can customize this high-level summary in your review settings.