执行摘要
- 一句话：在CI审批检查脚本中新增日志相关修改的审批检测逻辑。
- 推荐动作：该PR是典型的CI流程优化，适合基础设施维护者精读以了解审批检查机制。值得关注的设计决策包括：通过git diff过滤实现精准检测、排除脚本自身修改避免循环触发、以及如何平衡检测覆盖与误报风险。对于普通开发者，了解此变更可避免在修改日志代码时意外触发审批要求。
功能与动机
根据PR body说明，动机是确保对日志行为的修改得到适当审查，因为日志变更（如.info/.debug/.error/log_request）可能影响调试、可观测性和系统稳定性。
实现拆解
- 定义日志关键词数组：在
scripts/check_approval.sh中新增LOG_KEYWORDS数组，包含.info(、.debug(、.error(、log_request(、log_request_error五个模式，用于匹配日志调用。
- 检测日志修改：通过
git diff命令获取与上游分支的差异，过滤出新增行（grep -E "^\+"），并排除差异头（grep -vE "^\+\+\+"），然后使用grep -E结合关键词数组检测是否包含日志相关修改。同时通过-- . ':(exclude)scripts/check_approval.sh'排除了脚本自身修改，避免自我触发检测。
- 触发审批检查：如果检测到日志修改且PR ID存在，则输出提示信息并调用
check_approval函数，要求指定RD（xyxinyang、zyyzghb）审批。
- 测试与调试：提交历史显示作者曾添加临时测试文件
tests/test_log_check_tmp.py进行验证，但根据review建议，该文件在最终提交前被移除，未包含在合并版本中。
关键文件：
scripts/check_approval.sh（模块 CI脚本；类别 infra；类型 core-logic）: 这是唯一被修改的文件，实现了日志修改检测的核心逻辑，直接影响CI审批流程。
关键符号：未识别
关键源码片段
scripts/check_approval.sh
这是唯一被修改的文件，实现了日志修改检测的核心逻辑，直接影响CI审批流程。
# 定义日志相关关键词数组，用于匹配常见的日志调用模式
LOG_KEYWORDS=(
"\.info\(" # 匹配 .info() 调用
"\.debug\(" # 匹配 .debug() 调用
"\.error\(" # 匹配 .error() 调用
"log_request\(" # 匹配 log_request() 调用
"log_request_error" # 匹配 log_request_error 字符串（注意：代码库中当前不存在此方法）
)
# 检测PR中是否包含日志相关修改
# 使用git diff获取与上游分支的差异，排除脚本自身修改（避免自我触发）
HAS_LOG_MODIFY=$(git diff upstream/$BRANCH \
-- . ':(exclude)scripts/check_approval.sh' \
| grep -E "^\+" \
| grep -vE "^\+\+\+" \
| grep -E $(printf -- "%s|" "${LOG_KEYWORDS[@]}" | sed 's/|$//') || true)
# 如果检测到日志修改且PR ID存在，则触发审批检查
if [ -n "${HAS_LOG_MODIFY}" ] && [ -n "${PR_ID}" ]; then
# 输出提示信息，说明需要RD审批
echo_line1="You must have one FastDeploy RD (xyxinyang(zhouchong), zyyzghb(zhangyongyue)) approval for modifying logging behavior (.info/.debug/.error/log_request)."
# 调用check_approval函数，传入提示信息和审批者列表
check_approval "$echo_line1" 1 xyxinyang zyyzghb
fi
评论区精华
- 关键词准确性争议：AI reviewer指出
log_request_error在代码库中不存在（只有log_request方法），建议确认是否需要检测或移除。讨论中未明确回应，但最终代码保留了该关键词。
- 测试覆盖建议：AI reviewer建议添加测试用例验证检测逻辑，包括真实日志调用、删除、注释和字符串场景的准确性。作者未直接回应，但通过临时测试文件进行了调试。
- 自我触发风险：AI reviewer指出脚本自身修改可能误触发检测，建议改进过滤逻辑。作者在最终实现中通过
-- . ':(exclude)scripts/check_approval.sh'排除了脚本文件，解决了此问题。
- 临时文件处理：AI reviewer发现提交的临时测试文件
tests/test_log_check_tmp.py不应提交，建议删除。最终提交中该文件已被移除，表明作者采纳了建议。
- 日志关键词准确性 (correctness): 未明确回应，但最终代码保留了该关键词，可能存在冗余。
- 测试覆盖 (testing): 作者通过临时测试文件调试，但最终未提交正式测试，测试覆盖不足。
- 自我触发风险 (design): 作者通过-- . ':(exclude)scripts/check_approval.sh'排除了脚本文件，解决了此问题。
- 临时文件处理 (style): 作者在最终提交中移除了该文件，采纳了建议。
风险与影响
- 风险：1. 误报风险：关键词
log_request_error在代码库中不存在，可能导致检测逻辑冗余或未来误报，但当前影响有限。
2. 漏报风险：检测逻辑依赖git diff和grep模式匹配，若日志调用格式变化（如换行、空格差异）可能漏检，但现有模式覆盖了常见调用。
3. 维护风险：CI脚本变更可能影响所有PR的审批流程，若脚本逻辑错误（如过滤条件不当）可能导致审批要求误触发或缺失。
4. 兼容性风险：无，纯CI脚本变更不影响运行时功能。
- 影响：1. 对团队流程影响：所有涉及日志相关代码修改的PR现在需要指定RD审批，增加了变更控制粒度，可能略微延长合并周期，但提升了代码质量保障。
2. 对系统影响：仅影响CI流程，不改变FastDeploy核心功能、性能或安全性。
3. 对用户影响：无直接影响，用户不可见。
- 风险标记：关键词冗余, 测试覆盖不足
关联脉络
- PR #7190 [Feature] implement log channel separation and request log level system: 该PR涉及日志系统的重大功能变更，与本PR的日志修改审批检测直接相关，体现了对日志变更加强管控的背景。
参与讨论