Skip to content

fix: 避免引擎日志重复写入 - #848

Merged
dengyh merged 1 commit into
TencentBlueKing:masterfrom
dengyh:ai/fix-duplicate-engine-logs
Aug 3, 2026
Merged

fix: 避免引擎日志重复写入#848
dengyh merged 1 commit into
TencentBlueKing:masterfrom
dengyh:ai/fix-duplicate-engine-logs

Conversation

@dengyh

@dengyh dengyh commented Aug 3, 2026

Copy link
Copy Markdown
Collaborator

问题原因

bamboo_enginepipeline_enginepipeline 在自身绑定 root Handler 后仍向父 logger 传播,导致同一个 Handler 沿 logger 继承链被重复调用。PaaS3 日志模式下,pipeline.eri.log 的数据库 Handler 被移除后又补上 root Handler,最多会将同一条日志写入三次。

修改内容

  • bamboo_enginepipeline_enginepipeline 设为对应日志链路的传播终点。
  • PaaS3 模式下允许 pipeline.eri.log 使用空 Handler 并传播到 pipeline,避免重复绑定 root
  • 增加 DATABASE/PaaS3 两种模式的日志路由回归测试。
  • 确认数据库节点日志 Handler 在 DATABASE 模式下仍调用一次,在 PaaS3 模式下仍按原逻辑关闭。

影响范围

仅调整 Python logging Handler 的传播路径,不修改日志内容、级别、格式、Trace 信息、节点信息或日志查询接口。合法的重复业务日志不会被过滤。

验证

  • pytest tests/project_settings tests/engine/task/test_node_log.py -q --no-cov:25 passed。
  • pre-commit run --files config/default.py tests/project_settings/test_logging_settings.py:全部通过。
  • 更大范围 engine 测试中,156 个无数据库依赖用例通过;其余 222 个因本机未启动 MySQL 而无法执行。

@dengyh
dengyh force-pushed the ai/fix-duplicate-engine-logs branch from 2fb326d to 89891fe Compare August 3, 2026 07:18

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

代码审查总结

这个 PR 修复了引擎日志重复写入 root handler 的问题,思路清晰,实现正确:

  1. propagate: Falsepipelinebamboo_enginepipeline_engine 这三个 logger 已经显式包含 root handler,设置 propagate: False 避免日志再通过传播链到达 root logger 造成重复输出。
  2. 移除空 handler 回退逻辑 — 在非 DATABASE 模式下,pipeline.eri.log 的 handler 被过滤为空后,会通过 propagate: True 传播到父级 pipeline(已有 root handler),不需要额外回退。
  3. 新增测试覆盖 — 测试用例验证了所有引擎 logger 在 DATABASE 和 PaaS3 模式下均只写入 root handler 一次,且 DATABASE 模式下专用 handler 保持正常工作。

✅ 逻辑正确,无 Critical/Important 级别问题。代码简洁,测试充分。

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

增量审查

本次 synchronize 事件触发后重新审查了 PR diff,当前变更内容与上次审查一致(单次提交 89891fe)。

上次审查结论仍然有效:

  1. propagate: False 设置正确,三个 logger 已显式绑定 root handler,关闭传播避免重复写入
  2. ✅ 移除空 handler 回退逻辑合理——PaaS3 模式下 pipeline.eri.log 的 handler 被过滤为空后,通过 propagate: True 传播到父级 pipeline(已有 root handler)
  3. ✅ 测试覆盖充分,验证了 DATABASE/PaaS3 两种模式下的日志路由正确性

无新增 Critical/Important 问题。

@codecov-commenter

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 83.12%. Comparing base (63b6bb1) to head (89891fe).
⚠️ Report is 2 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #848      +/-   ##
==========================================
+ Coverage   83.10%   83.12%   +0.02%     
==========================================
  Files         307      307              
  Lines       18167    18165       -2     
==========================================
+ Hits        15098    15100       +2     
+ Misses       3069     3065       -4     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@dengyh
dengyh merged commit dabe598 into TencentBlueKing:master Aug 3, 2026
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants