Conversation
There was a problem hiding this comment.
Pull request overview
This pull request refactors the endpoint monitoring system to use asynchronous background tasks instead of blocking thread pool execution. The change improves API response times by deferring endpoint usage logging until after the response is sent to the client.
Key changes:
- Replaced synchronous
run_in_threadpoolexecution with Starlette'sBackgroundTaskfor non-blocking endpoint registration - Added comprehensive error handling and logging for endpoint registration failures
- Moved response generation to occur before endpoint logging setup
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/app/middleware/monitor_requests.py | Refactored middleware to use BackgroundTask and moved response generation before logging setup |
| src/app/services/sql_service.py | Added try-except block and error logging to register_endpoint method |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| session_id=session_id, | ||
| http_code=200, | ||
| response.background = BackgroundTask( | ||
| register_endpoint, request.url.path, session_id, 200 |
There was a problem hiding this comment.
The HTTP status code is hardcoded to 200, but response.status_code should be used instead to accurately log the actual response status (e.g., 404, 500). Replace 200 with response.status_code.
| register_endpoint, request.url.path, session_id, 200 | |
| register_endpoint, request.url.path, session_id, response.status_code |
| except Exception as e: | ||
| logger.error(f"Failed to register endpoint {request.url.path}: {e}") |
There was a problem hiding this comment.
This exception handler will never catch exceptions from register_endpoint because BackgroundTask executes after the response is returned. The try-except block should be removed from the middleware since error handling is now in sql_service.py's register_endpoint method.
3 participants
This pull request improves how API endpoint usage is logged and monitored. The main change is that the logging of endpoint requests is now handled asynchronously in the background, which should improve response times and reliability. Additionally, error handling and logging have been enhanced to ensure failures in logging do not affect the main application flow.
Middleware improvements:
MonitorRequestsMiddlewareinmonitor_requests.pyto useBackgroundTaskfor registering endpoint usage asynchronously, instead of blocking the response with a synchronous call. This allows the API to return responses faster and more reliably. [1] [2]Logging and error handling:
sql_service.pyfor failures during endpoint registration, ensuring that issues are captured without impacting the user experience. [1] [2]