Pull Request已成功合入, 合并人@张雅晴
(感谢 zhaoxi_00622383 的贡献)变更摘要
此 PR 将 A2A Service 和 Versatile Adapter 的日志清理策略从硬编码的固定值改为可按时间和空间自定义配置。核心改动包括:用自定义 retention 回调函数替换 loguru 的简单字符串配置,实现"按保留天数 + 按总空间上限"的双维度清理;新增 common/sdk_log_cleaner.py 模块,将 openjiuwen SDK 的 SafeRotatingFileHandler 替换为 foundation 的 CompressedRotatingFileHandler,使 SDK 日志获得 gzip 压缩和同等清理能力;同时为 VA 增加了启动时清理旧 PID 残留日志文件的机制。
主要改动
-
loguru retention 回调函数
_cleanup_logs/_va_cleanup_logs: 在app.py中新增自定义清理函数,替代 loguru 原有的retention="7 days"字符串配置;函数通过区分活跃文件(无.gz后缀)与归档文件(.gz后缀),先按retention_days删除过期归档,再按max_total_size从最旧归档开始逐一删除直到总空间不超限,活跃文件永不删除。 -
新增
common/sdk_log_cleaner.py模块: 通过继承 SDK 的DefaultLogger并重写_setup_logger(),将 SDK 原生的SafeRotatingFileHandler替换为CleanableCompressedRotatingFileHandler(继承自 foundation 的CompressedRotatingFileHandler);doRollover()中在 gzip 压缩后追加按天数和按空间的清理逻辑。setup_sdk_log_cleaner()遍历全部 21 个 SDK logger 完成替换,并压缩configure_log_config到初始化之间产生的未压缩残留归档。 -
配置项扩展: 在
config.py和.env.example中为两个服务新增log_rotation_size、log_retention_days、log_max_total_size通用日志配置,以及jiuwen_log_backup_count、jiuwen_log_max_bytes、jiuwen_log_retention_days、jiuwen_log_max_total_size等 SDK 专用日志配置,全部支持环境变量覆盖。 -
VA 启动时旧 PID 日志清理
_cleanup_stale_pid_logs: VA 日志文件带 PID 后缀,重启后旧 PID 文件不会被 loguru 的 retention 回调处理;新增的清理函数在setup_logging()中logger.add之前统一按 mtime 扫描并删除超过retention_days的旧 PID 日志文件(含.log和.gz),排除当前进程的文件。 -
单元测试
test_sdk_log_cleaner.py: 为CleanableCompressedRotatingFileHandler新增 8 个测试用例,覆盖按天数清理(过期删除、零天数全清)、按空间清理(超限删最旧、零空间全清、未超限保留)以及OSError异常分支的 stderr 输出验证。


代码审查
我已对所有 8 个文件完成了审查。以下是我的最终汇总。
审查汇总
| 优先级 | 数量 | 说明 |
|---|---|---|
| P2 | 3 | os.getenv 绕过 Settings 导致空字符串崩溃风险 |
| P3 | 1 | TOCTOU 竞态(极低概率) |
各文件审查结论
| 文件 | 结论 |
|---|---|
applications/a2a_service/.env.example |
无问题 — 仅新增文档/示例配置 |
applications/a2a_service/app.py |
2 个问题 — _cleanup_logs 和 lifespan 中绕过 Settings;1 个 TOCTOU 竞态 |
applications/a2a_service/common/sdk_log_cleaner.py |
1 个问题 — 两处 os.getenv 绕过 Settings |
applications/a2a_service/config.py |
无问题 — 新增字段定义规范、类型正确 |
applications/a2a_service/tests/common/test_sdk_log_cleaner.py |
无问题 — 测试覆盖合理 |
applications/versatile_adapter/.env.example |
无问题 — 仅新增文档/示例配置 |
applications/versatile_adapter/app.py |
无独立问题(TOCTOU 竞态与 a2a_service 共享根因,已合并报告) |
applications/versatile_adapter/config.py |
无问题 — 新增字段定义规范、类型正确 |
整体风险判断
低风险。核心逻辑(按天数/空间清理、handler 替换、残留归档压缩)设计合理,功能正确。3 个 P2 问题共享同一根因——在多处使用 os.getenv() + int() 而非复用已定义的 Settings 字段,在极端配置(环境变量设为空字符串)下才会触发。建议统一改用 get_settings() 访问,与 _va_cleanup_logs 的已有实现保持一致。P3 的 TOCTOU 竞态触发概率极低,可在后续迭代中修复。测试覆盖完整,无安全漏洞、数据一致性或破坏性变更问题。
| 类型 | 数量 |
|---|---|
| 🔴 阻塞 | 0 |
| 🟡 建议 | 2 |
💬 仅评论


欢迎来到 openJiuwen 社区
Hey @JiangCaifu , 感谢你对社区的贡献.
机器人使用手册
有关指令的使用,可以点击 此处 查看详情。开发人员可以在每个PR或Issue下方评论特定指令来触发机器人任务。


【openlibing.ci】检测到当前PR中存在代码检查告警抑制 2 处,详情见下表,请Committer检视合理性。 / Detected 2 code check alert suppression(s) in this PR, see table below. Committers please review.
| 文件路径/File | 行号/Line | 代码片段/Snippet | 工具/Tool |
|---|---|---|---|
| applications/a2a_service/ common/sdk_log_cleaner.py |
69 | def doRollover(self) -> None: # noqa: N802 继承自标准库 RotatingFileHandler,必须保持原名 |
flake8,ruff |
| applications/a2a_service/ tests/common/test_sdk_log_cleaner.py |
45 | from common.sdk_log_cleaner import CleanableCompressedRotatingFileHandler # noqa: E402 |
flake8,ruff |


ci-pipeline


ci-pipeline


ci-pipeline


ci-pipeline


🟡 Medium Priority
_cleanup_logs 函数(loguru retention 回调)在第 145-146 行通过 os.getenv("LOG_RETENTION_DAYS", "7") 和 os.getenv("LOG_MAX_TOTAL_SIZE", "524288000") 读取配置,而 Settings 类(config.py 第 94-96 行)已正确定义了 log_retention_days: int = 7 和 log_max_total_size: int = 524288000。
与此形成对比,versatile_adapter/app.py 中的 _va_cleanup_logs(第 82-84 行)正确地使用了 get_settings() 读取配置。
失败模式:如果环境变量被显式设置为空字符串(如 LOG_RETENTION_DAYS=),os.getenv 返回 "",int("") 抛出 ValueError,导致 loguru 轮转回调崩溃,日志清理停止工作。而通过 get_settings() 访问会触发 pydantic 的启动时校验,提前暴露配置错误。
额外影响:默认值 "7" 和 "524288000" 在 _cleanup_logs 和 Settings 中重复定义,后续修改默认值时容易遗漏一侧。
建议:改用 get_settings() 读取这两个配置项,与 _va_cleanup_logs 保持一致的实现风格,避免绕过 pydantic 校验。


🟡 Medium Priority
在 lifespan 函数(第 533-534 行)中,JIUWEN_LOG_BACKUP_COUNT 和 JIUWEN_LOG_MAX_BYTES 通过 os.getenv 读取:
custom_log_config["backup_count"] = int(os.getenv("JIUWEN_LOG_BACKUP_COUNT", 20))
custom_log_config["max_bytes"] = int(os.getenv("JIUWEN_LOG_MAX_BYTES", 20971520))
而 Settings 类(config.py 第 100-101 行)已定义了 jiuwen_log_backup_count: int = 20 和 jiuwen_log_max_bytes: int = 20971520。settings 变量在同一函数中已可用(第 196 行 settings = get_settings())。
失败模式:若环境变量被设为空字符串(如 JIUWEN_LOG_BACKUP_COUNT=),int("") 抛出 ValueError。由于此代码位于 lifespan 的启动路径中,异常会导致服务启动失败,而非静默回退到默认值。
建议:改用 settings.jiuwen_log_backup_count 和 settings.jiuwen_log_max_bytes,复用已有的 pydantic 校验和默认值。


ci-pipeline


/ci-pipeline


What type of PR is this?
/kind
Self-checklist:(请自检,在[ ]内打上x,我们将检视你的完成情况,否则会导致pr无法合入)