feat: use loguru for enhanced logging support#5115
Merged
Conversation
dosubot
bot
added
the
size:XL
|
Related Documentation Checked 1 published document(s) in 1 knowledge base(s). No updates required. How did I do? Any feedback? |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Hey - 我发现了 1 个问题，并给出了一些整体性的反馈：
LogManager中的全局_file_sink_id和_trace_sink_id意味着在整个进程中只能有一个普通/trace 文件 sink 处于活动状态；如果不同的 logger 以后需要输出到不同的文件，建议改为按 logger 维度跟踪 sink ID（例如使用以 logger 名称为键的字典），而不是使用全局变量。- 在
_add_file_sink中，对整数调用logging.getLevelName(level)会得到类似'Level 0'的字符串，而这些并不是合法的 Loguru 日志级别名称；更安全的做法是要么直接将整数传给 Loguru，要么在调用loguru.add之前显式将其映射到已知的级别字符串（DEBUG/INFO 等）。
给 AI Agents 的提示
Please address the comments from this code review:
## Overall Comments
- `LogManager` 中的全局 `_file_sink_id` 和 `_trace_sink_id` 意味着在整个进程中只能有一个普通/trace 文件 sink 处于活动状态；如果不同的 logger 以后需要输出到不同的文件，建议改为按 logger 维度跟踪 sink ID（例如使用以 logger 名称为键的字典），而不是使用全局变量。
- 在 `_add_file_sink` 中，对整数调用 `logging.getLevelName(level)` 会得到类似 `'Level 0'` 的字符串，而这些并不是合法的 Loguru 日志级别名称；更安全的做法是要么直接将整数传给 Loguru，要么在调用 `loguru.add` 之前显式将其映射到已知的级别字符串（DEBUG/INFO 等）。
## Individual Comments
### Comment 1
<location> `astrbot/core/log.py:23` </location>
<code_context>
- Args:
- pathname (str): 文件路径
+class _RecordEnricherFilter(logging.Filter):
+ """为 logging.LogRecord 注入 AstrBot 日志字段。"""
</code_context>
<issue_to_address>
**issue (complexity):** 请考虑将计算 AstrBot 特定日志扩展字段的逻辑集中到一个单独的辅助函数中，然后在所有富化路径中复用它，以避免重复和行为漂移。
你目前在三个位置重复了“扩展字段”富化逻辑，从而付出了不必要的复杂度成本：
- `_RecordEnricherFilter.filter`
- `_patch_record`
- `_LoguruInterceptHandler.emit`（构造 `payload` 的地方）
它们在概念上都在计算相同的一组字段：
- `plugin_tag`
- `short_levelname`
- `astrbot_version_tag`
- `source_file`
- `source_line`
- `is_trace`
但通过了不同的代码路径和默认值。这是造成心智负担和行为漂移风险的主要原因。
你可以在保留所有现有特性（loguru 集成、WebUI 颜色、trace 标记等）的前提下，通过把富化逻辑集中到一个辅助函数中，并在 stdlib 和 loguru 的路径里统一使用它，从而简化实现。
### 1. 引入一个统一的富化辅助函数
```python
def _compute_astrbot_extra(
*,
pathname: str | None,
levelname: str,
levelno: int,
lineno: int,
logger_name: str,
is_trace: bool | None = None,
) -> dict:
return {
"plugin_tag": "[Plug]" if _is_plugin_path(pathname) else "[Core]",
"short_levelname": _get_short_level_name(levelname),
"astrbot_version_tag": f" [v{VERSION}]" if levelno >= logging.WARNING else "",
"source_file": _build_source_file(pathname),
"source_line": lineno,
"is_trace": logger_name == "astrbot.trace" if is_trace is None else is_trace,
}
```
### 2. 在 `_RecordEnricherFilter` 中使用它
```python
class _RecordEnricherFilter(logging.Filter):
def filter(self, record: logging.LogRecord) -> bool:
extra = _compute_astrbot_extra(
pathname=record.pathname,
levelname=record.levelname,
levelno=record.levelno,
lineno=record.lineno,
logger_name=record.name,
is_trace=getattr(record, "is_trace", None),
)
for k, v in extra.items():
setattr(record, k, v)
return True
```
### 3. 在 `_LoguruInterceptHandler` 中使用它来替代手动构造 `payload`
```python
class _LoguruInterceptHandler(logging.Handler):
def emit(self, record: logging.LogRecord) -> None:
try:
level: str | int = _loguru.level(record.levelname).name
except ValueError:
level = record.levelno
extra = _compute_astrbot_extra(
pathname=getattr(record, "pathname", None),
levelname=record.levelname,
levelno=record.levelno,
lineno=getattr(record, "lineno", 0),
logger_name=record.name,
is_trace=getattr(record, "is_trace", None),
)
# allow pre-populated attributes (e.g. from custom loggers) to override defaults
for key, value in extra.items():
extra[key] = getattr(record, key, value)
_loguru.bind(**extra).opt(exception=record.exc_info).log(
level,
record.getMessage(),
)
```
### 4. 在 `_patch_record` 中同样复用该辅助函数
```python
def _patch_record(record: "Record") -> None:
extra = record["extra"]
computed = _compute_astrbot_extra(
pathname=record["file"].path,
levelname=record["level"].name,
levelno=record["level"].no,
lineno=record["line"],
logger_name=record["name"],
is_trace=extra.get("is_trace"),
)
for key, value in computed.items():
extra.setdefault(key, value)
```
这样可以保持现有行为不变，同时：
- 去除用于计算同一组字段的重复逻辑。
- 确保 `plugin_tag`、`short_levelname`、`astrbot_version_tag`、`source_file`、`source_line` 和 `is_trace` 在以下场景中以一致的方式计算：
- 纯 `logging` logger → 通过 `_RecordEnricherFilter`
- 从 `logging` 桥接到 loguru → 通过 `_LoguruInterceptHandler`
- 直接使用 loguru → 通过 `_patch_record`
- 使这些字段的未来更改（例如增加 `module`，或修改版本标记规则）集中在一个函数中完成。
</issue_to_address>为了让我变得更有用！请在每条评论上点 👍 或 👎，我会根据你的反馈改进后续评审。
Original comment in English
Hey - I've found 1 issue, and left some high level feedback:
- The global
_file_sink_idand_trace_sink_idinLogManagermean only a single normal/trace file sink can be active across the whole process; if different loggers ever need different file outputs, consider tracking sink IDs per logger (e.g., in a dict keyed by logger name) instead of globally. - In
_add_file_sink, usinglogging.getLevelName(level)on an integer can produce strings like'Level 0'which are not valid Loguru level names; it would be safer to either pass the integer directly to Loguru or map explicitly to known level strings (DEBUG/INFO/etc.) before callingloguru.add.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The global `_file_sink_id` and `_trace_sink_id` in `LogManager` mean only a single normal/trace file sink can be active across the whole process; if different loggers ever need different file outputs, consider tracking sink IDs per logger (e.g., in a dict keyed by logger name) instead of globally.
- In `_add_file_sink`, using `logging.getLevelName(level)` on an integer can produce strings like `'Level 0'` which are not valid Loguru level names; it would be safer to either pass the integer directly to Loguru or map explicitly to known level strings (DEBUG/INFO/etc.) before calling `loguru.add`.
## Individual Comments
### Comment 1
<location> `astrbot/core/log.py:23` </location>
<code_context>
- Args:
- pathname (str): 文件路径
+class _RecordEnricherFilter(logging.Filter):
+ """为 logging.LogRecord 注入 AstrBot 日志字段。"""
</code_context>
<issue_to_address>
**issue (complexity):** Consider centralizing the logic that computes AstrBot-specific log extra fields into a single helper function and reusing it across all enrichment paths to avoid duplication and drift.
You’re paying a complexity cost from duplicating the “extra field” enrichment logic in three places:
- `_RecordEnricherFilter.filter`
- `_patch_record`
- `_LoguruInterceptHandler.emit` (the `payload` construction)
They all compute the same conceptual fields:
- `plugin_tag`
- `short_levelname`
- `astrbot_version_tag`
- `source_file`
- `source_line`
- `is_trace`
but via different code paths and defaults. This is the main source of mental overhead and risk of drift.
You can keep all features (loguru integration, WebUI colors, trace flag, etc.) while simplifying by centralizing the enrichment into a single helper and using it from both stdlib and loguru paths.
### 1. Introduce a single enrichment helper
```python
def _compute_astrbot_extra(
*,
pathname: str | None,
levelname: str,
levelno: int,
lineno: int,
logger_name: str,
is_trace: bool | None = None,
) -> dict:
return {
"plugin_tag": "[Plug]" if _is_plugin_path(pathname) else "[Core]",
"short_levelname": _get_short_level_name(levelname),
"astrbot_version_tag": f" [v{VERSION}]" if levelno >= logging.WARNING else "",
"source_file": _build_source_file(pathname),
"source_line": lineno,
"is_trace": logger_name == "astrbot.trace" if is_trace is None else is_trace,
}
```
### 2. Use it in `_RecordEnricherFilter`
```python
class _RecordEnricherFilter(logging.Filter):
def filter(self, record: logging.LogRecord) -> bool:
extra = _compute_astrbot_extra(
pathname=record.pathname,
levelname=record.levelname,
levelno=record.levelno,
lineno=record.lineno,
logger_name=record.name,
is_trace=getattr(record, "is_trace", None),
)
for k, v in extra.items():
setattr(record, k, v)
return True
```
### 3. Use it in `_LoguruInterceptHandler` instead of manually building `payload`
```python
class _LoguruInterceptHandler(logging.Handler):
def emit(self, record: logging.LogRecord) -> None:
try:
level: str | int = _loguru.level(record.levelname).name
except ValueError:
level = record.levelno
extra = _compute_astrbot_extra(
pathname=getattr(record, "pathname", None),
levelname=record.levelname,
levelno=record.levelno,
lineno=getattr(record, "lineno", 0),
logger_name=record.name,
is_trace=getattr(record, "is_trace", None),
)
# allow pre-populated attributes (e.g. from custom loggers) to override defaults
for key, value in extra.items():
extra[key] = getattr(record, key, value)
_loguru.bind(**extra).opt(exception=record.exc_info).log(
level,
record.getMessage(),
)
```
### 4. Use the same helper in `_patch_record`
```python
def _patch_record(record: "Record") -> None:
extra = record["extra"]
computed = _compute_astrbot_extra(
pathname=record["file"].path,
levelname=record["level"].name,
levelno=record["level"].no,
lineno=record["line"],
logger_name=record["name"],
is_trace=extra.get("is_trace"),
)
for key, value in computed.items():
extra.setdefault(key, value)
```
This keeps behavior intact but:
- Removes duplicated logic for computing the same fields.
- Ensures `plugin_tag`, `short_levelname`, `astrbot_version_tag`, `source_file`, `source_line`, and `is_trace` are computed consistently for:
- pure `logging` loggers → via `_RecordEnricherFilter`
- bridged `logging` → loguru → via `_LoguruInterceptHandler`
- direct loguru usage → via `_patch_record`
- Makes future changes to these fields (e.g. adding `module`, changing version-tag rule) localized to a single function.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
|
|
||
| Args: | ||
| pathname (str): 文件路径 | ||
| class _RecordEnricherFilter(logging.Filter): |
Contributor
There was a problem hiding this comment.
issue (complexity): 请考虑将计算 AstrBot 特定日志扩展字段的逻辑集中到一个单独的辅助函数中，然后在所有富化路径中复用它，以避免重复和行为漂移。
你目前在三个位置重复了“扩展字段”富化逻辑，从而付出了不必要的复杂度成本：
_RecordEnricherFilter.filter_patch_record_LoguruInterceptHandler.emit（构造payload的地方）
它们在概念上都在计算相同的一组字段：
plugin_tagshort_levelnameastrbot_version_tagsource_filesource_lineis_trace
但通过了不同的代码路径和默认值。这是造成心智负担和行为漂移风险的主要原因。
你可以在保留所有现有特性（loguru 集成、WebUI 颜色、trace 标记等）的前提下，通过把富化逻辑集中到一个辅助函数中，并在 stdlib 和 loguru 的路径里统一使用它，从而简化实现。
1. 引入一个统一的富化辅助函数
def _compute_astrbot_extra(
*,
pathname: str | None,
levelname: str,
levelno: int,
lineno: int,
logger_name: str,
is_trace: bool | None = None,
) -> dict:
return {
"plugin_tag": "[Plug]" if _is_plugin_path(pathname) else "[Core]",
"short_levelname": _get_short_level_name(levelname),
"astrbot_version_tag": f" [v{VERSION}]" if levelno >= logging.WARNING else "",
"source_file": _build_source_file(pathname),
"source_line": lineno,
"is_trace": logger_name == "astrbot.trace" if is_trace is None else is_trace,
}2. 在 _RecordEnricherFilter 中使用它
class _RecordEnricherFilter(logging.Filter):
def filter(self, record: logging.LogRecord) -> bool:
extra = _compute_astrbot_extra(
pathname=record.pathname,
levelname=record.levelname,
levelno=record.levelno,
lineno=record.lineno,
logger_name=record.name,
is_trace=getattr(record, "is_trace", None),
)
for k, v in extra.items():
setattr(record, k, v)
return True3. 在 _LoguruInterceptHandler 中使用它来替代手动构造 payload
class _LoguruInterceptHandler(logging.Handler):
def emit(self, record: logging.LogRecord) -> None:
try:
level: str | int = _loguru.level(record.levelname).name
except ValueError:
level = record.levelno
extra = _compute_astrbot_extra(
pathname=getattr(record, "pathname", None),
levelname=record.levelname,
levelno=record.levelno,
lineno=getattr(record, "lineno", 0),
logger_name=record.name,
is_trace=getattr(record, "is_trace", None),
)
# allow pre-populated attributes (e.g. from custom loggers) to override defaults
for key, value in extra.items():
extra[key] = getattr(record, key, value)
_loguru.bind(**extra).opt(exception=record.exc_info).log(
level,
record.getMessage(),
)4. 在 _patch_record 中同样复用该辅助函数
def _patch_record(record: "Record") -> None:
extra = record["extra"]
computed = _compute_astrbot_extra(
pathname=record["file"].path,
levelname=record["level"].name,
levelno=record["level"].no,
lineno=record["line"],
logger_name=record["name"],
is_trace=extra.get("is_trace"),
)
for key, value in computed.items():
extra.setdefault(key, value)这样可以保持现有行为不变，同时：
- 去除用于计算同一组字段的重复逻辑。
- 确保
plugin_tag、short_levelname、astrbot_version_tag、source_file、source_line和is_trace在以下场景中以一致的方式计算：- 纯
logginglogger → 通过_RecordEnricherFilter - 从
logging桥接到 loguru → 通过_LoguruInterceptHandler - 直接使用 loguru → 通过
_patch_record
- 纯
- 使这些字段的未来更改（例如增加
module，或修改版本标记规则）集中在一个函数中完成。
Original comment in English
issue (complexity): Consider centralizing the logic that computes AstrBot-specific log extra fields into a single helper function and reusing it across all enrichment paths to avoid duplication and drift.
You’re paying a complexity cost from duplicating the “extra field” enrichment logic in three places:
_RecordEnricherFilter.filter_patch_record_LoguruInterceptHandler.emit(thepayloadconstruction)
They all compute the same conceptual fields:
plugin_tagshort_levelnameastrbot_version_tagsource_filesource_lineis_trace
but via different code paths and defaults. This is the main source of mental overhead and risk of drift.
You can keep all features (loguru integration, WebUI colors, trace flag, etc.) while simplifying by centralizing the enrichment into a single helper and using it from both stdlib and loguru paths.
1. Introduce a single enrichment helper
def _compute_astrbot_extra(
*,
pathname: str | None,
levelname: str,
levelno: int,
lineno: int,
logger_name: str,
is_trace: bool | None = None,
) -> dict:
return {
"plugin_tag": "[Plug]" if _is_plugin_path(pathname) else "[Core]",
"short_levelname": _get_short_level_name(levelname),
"astrbot_version_tag": f" [v{VERSION}]" if levelno >= logging.WARNING else "",
"source_file": _build_source_file(pathname),
"source_line": lineno,
"is_trace": logger_name == "astrbot.trace" if is_trace is None else is_trace,
}2. Use it in _RecordEnricherFilter
class _RecordEnricherFilter(logging.Filter):
def filter(self, record: logging.LogRecord) -> bool:
extra = _compute_astrbot_extra(
pathname=record.pathname,
levelname=record.levelname,
levelno=record.levelno,
lineno=record.lineno,
logger_name=record.name,
is_trace=getattr(record, "is_trace", None),
)
for k, v in extra.items():
setattr(record, k, v)
return True3. Use it in _LoguruInterceptHandler instead of manually building payload
class _LoguruInterceptHandler(logging.Handler):
def emit(self, record: logging.LogRecord) -> None:
try:
level: str | int = _loguru.level(record.levelname).name
except ValueError:
level = record.levelno
extra = _compute_astrbot_extra(
pathname=getattr(record, "pathname", None),
levelname=record.levelname,
levelno=record.levelno,
lineno=getattr(record, "lineno", 0),
logger_name=record.name,
is_trace=getattr(record, "is_trace", None),
)
# allow pre-populated attributes (e.g. from custom loggers) to override defaults
for key, value in extra.items():
extra[key] = getattr(record, key, value)
_loguru.bind(**extra).opt(exception=record.exc_info).log(
level,
record.getMessage(),
)4. Use the same helper in _patch_record
def _patch_record(record: "Record") -> None:
extra = record["extra"]
computed = _compute_astrbot_extra(
pathname=record["file"].path,
levelname=record["level"].name,
levelno=record["level"].no,
lineno=record["line"],
logger_name=record["name"],
is_trace=extra.get("is_trace"),
)
for key, value in computed.items():
extra.setdefault(key, value)This keeps behavior intact but:
- Removes duplicated logic for computing the same fields.
- Ensures
plugin_tag,short_levelname,astrbot_version_tag,source_file,source_line, andis_traceare computed consistently for:- pure
loggingloggers → via_RecordEnricherFilter - bridged
logging→ loguru → via_LoguruInterceptHandler - direct loguru usage → via
_patch_record
- pure
- Makes future changes to these fields (e.g. adding
module, changing version-tag rule) localized to a single function.
dosubot
bot
added
the
area:core
Soulter
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
using loguru.
Modifications / 改动点
Screenshots or Test Results / 运行截图或测试结果
Checklist / 检查清单
requirements.txt和pyproject.toml文件相应位置。/ I have ensured that no new dependencies are introduced, OR if new dependencies are introduced, they have been added to the appropriate locations inrequirements.txtandpyproject.toml.Summary by Sourcery
在保留 AstrBot 特有的元数据和 WebUI 控制台集成的前提下，将现有的日志设置替换为基于 Loguru 的实现。
新功能：
logging模块桥接到 Loguru，使核心和插件的日志记录都通过统一的日志流水线。增强：
构建：
requirements.txt和pyproject.toml中用loguru替换colorlog依赖。Original summary in English
Summary by Sourcery
Replace the existing logging setup with a Loguru-based implementation while preserving AstrBot-specific metadata and WebUI console integration.
New Features:
Enhancements:
Build: