add structure file logging with log files rotating #209
Conversation
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull Request Overview
This PR implements structured file logging with log file rotation capabilities by refactoring the logging system to use a custom RotatingBytesLogger that properly handles file rotation and converts all logging calls to async operations.
- Replaces the old
BytesToTextIOWrapperwith a newRotatingBytesLoggerthat properly integrates withRotatingFileHandler - Updates all logging calls throughout the codebase from synchronous to asynchronous operations (e.g.,
logger.info()toawait logger.ainfo()) - Removes unnecessary imports and improves log message formatting
Reviewed Changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| app/utils/logging.py | Implements new rotating logger classes and removes old wrapper implementation |
| app/services/scheduler.py | Converts logging calls to async operations |
| app/services/auth.py | Converts logging calls to async operations |
| app/models/base.py | Converts logging calls to async operations |
| app/main.py | Converts logging calls to async operations |
| app/database.py | Converts logging calls to async operations |
| app/api/user.py | Converts logging calls to async operations |
| app/api/stuff.py | Converts logging calls to async operations and improves log message |
| app/api/health.py | Removes unused import and converts logging calls to async operations |
|
|
||
|
|
||
| @define(slots=True) | ||
| @define |
There was a problem hiding this comment.
The slots=True parameter was removed from the @define decorator. This could impact memory efficiency and attribute access performance for the singleton class. Consider adding it back unless there's a specific reason for removal.
| @define | |
| @define(slots=True) |
Copilot uses AI. Check for mistakes.
| self.msg(message) | ||
|
|
||
| def info(self, message): | ||
| self.msg(message) | ||
|
|
||
| def flush(self): | ||
| self.handler.flush() | ||
| def warning(self, message): | ||
| self.msg(message) | ||
|
|
||
| def error(self, message): | ||
| self.msg(message) | ||
|
|
||
| def critical(self, message): | ||
| self.msg(message) |
There was a problem hiding this comment.
All logging level methods (debug, info, warning, error, critical) delegate to the same msg() method with INFO level. This means debug messages will be logged at INFO level, which could lead to incorrect log filtering and analysis.
Copilot uses AI. Check for mistakes.
| record = logging.LogRecord( | ||
| name="structlog", | ||
| level=logging.INFO, | ||
| pathname="", | ||
| lineno=0, |
There was a problem hiding this comment.
The LogRecord is created with hardcoded values for pathname="" and lineno=0. This removes valuable debugging information about where the log message originated from, making troubleshooting more difficult.
| record = logging.LogRecord( | |
| name="structlog", | |
| level=logging.INFO, | |
| pathname="", | |
| lineno=0, | |
| import inspect | |
| frame = inspect.stack()[1] | |
| record = logging.LogRecord( | |
| name="structlog", | |
| level=logging.INFO, | |
| pathname=frame.filename, | |
| lineno=frame.lineno, |
Copilot uses AI. Check for mistakes.
| @@ -88,7 +87,7 @@ async def smtp_check( | |||
| "subject": subject, | |||
| } | |||
|
|
|||
| logger.info("Sending email with data: %s", email_data) | |||
| await logger.ainfo("Sending email.", email_data=email_data) | |||
There was a problem hiding this comment.
The async logger methods like ainfo do not appear to be defined in the RotatingBytesLogger class. Only synchronous methods (debug, info, warning, error, critical) are implemented, so this call will likely result in an AttributeError.
| await logger.ainfo("Sending email.", email_data=email_data) | |
| logger.info("Sending email.", extra={"email_data": email_data}) |
Copilot uses AI. Check for mistakes.
No description provided.