-
Notifications
You must be signed in to change notification settings - Fork 5
Added logs for Threads and Response Endpoint #311
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
WalkthroughLogging has been systematically enhanced across several backend modules. Consistent log prefixes were introduced in the responses route, extensive logging was added to thread-related endpoints and functions, and thread result CRUD operations now log updates and creations with masked identifiers. No core logic or control flow was changed. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~15 minutes Poem
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
⏰ 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). (1)
✨ Finishing Touches
🧪 Generate 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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
backend/app/api/routes/responses.py(10 hunks)backend/app/api/routes/threads.py(17 hunks)backend/app/crud/thread_results.py(2 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (2)
backend/app/crud/thread_results.py (4)
backend/app/models/threads.py (2)
OpenAIThreadCreate(14-15)OpenAI_Thread(18-21)backend/app/utils.py (1)
mask_string(156-165)backend/app/tests/crud/test_thread_result.py (1)
test_upsert_and_get_thread_result(8-47)backend/app/alembic/versions/79e47bc3aac6_add_threads_table.py (1)
upgrade(20-45)
backend/app/api/routes/responses.py (2)
backend/app/utils.py (1)
mask_string(156-165)backend/app/api/routes/threads.py (1)
send_callback(37-49)
🔇 Additional comments (12)
backend/app/crud/thread_results.py (2)
1-1: LGTM! Proper logging imports and setup.The logging module, mask_string utility, and logger instance are correctly imported and configured.
Also applies to: 5-5, 7-7
20-22: LGTM! Well-implemented logging for thread result operations.The logging statements appropriately track both update and create operations with:
- Clear function context prefixes (
[upsert_thread_result])- Proper masking of sensitive thread IDs using
mask_string- Appropriate info-level logging for successful operations
Also applies to: 26-28
backend/app/api/routes/responses.py (3)
104-105: LGTM! Consistent logging prefixes and proper data masking.The logging statements correctly use consistent function context prefixes and properly mask sensitive assistant IDs using
mask_string.Also applies to: 143-144, 198-199, 202-203, 223-224, 235-236, 266-268
188-190: LGTM! Enhanced error logging with exception details.The addition of
exc_info=Trueto error logging calls will provide valuable stack trace information for debugging OpenAI API errors.Also applies to: 392-395
301-303: LGTM! Consistent error logging prefix.The logging prefix
[response_sync]maintains consistency with other endpoint logging patterns.backend/app/api/routes/threads.py (7)
16-16: LGTM! Proper import of masking utility.The
mask_stringimport is correctly added to support sensitive data masking in log messages.
45-45: LGTM! Enhanced callback logging with error details.The logging additions properly track callback success and failure with appropriate log levels and exception information.
Also applies to: 48-48
88-91: LGTM! Comprehensive error logging in thread setup.The error logging in
setup_threadprovides valuable debugging information with masked thread IDs and exception details usingexc_info=True.Also applies to: 102-104
167-169: LGTM! Thorough logging throughout run processing.The logging statements in
process_run_coreprovide excellent traceability of run lifecycle events with:
- Proper masking of sensitive IDs
- Appropriate log levels for different scenarios
- Detailed error information with exception traces
Also applies to: 203-205, 209-211, 218-221
238-240: LGTM! Clear logging for polling operations.The logging in
poll_run_and_prepare_responseeffectively tracks run execution with masked thread IDs and proper error handling.Also applies to: 252-254, 260-263
293-295: LGTM! Consistent endpoint logging across all thread routes.The logging statements across all thread endpoints maintain excellent consistency with:
- Uniform function context prefixes
- Proper masking of sensitive identifiers
- Appropriate log levels for different conditions
- Valuable context information (organization_id, project_id)
Also applies to: 310-312, 317-319, 347-349, 370-372, 388-390, 437-439, 463-465, 488-490
411-412: LGTM! Clean simplification of synchronous processing.The removal of the explicit finally block is appropriate since the tracer flushing is handled within
process_run_core.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approved
Summary
This PR includes logs only for a subset of endpoints which are threads and response API.
Target issue is #252
Checklist
Before submitting a pull request, please ensure that you mark these task.
fastapi run --reload app/main.pyordocker compose upin the repository root and test.Notes
Please add here if any other information is required for the reviewer.
Summary by CodeRabbit