已合并
Revert "[RFC]: MindIE-LLM 日志模块整改——cpp框架日志接口替换" #547
KaiMa创建于 3月10日
Revert "[RFC]: MindIE-LLM 日志模块整改——cpp框架日志接口替换" #547
已合并
KaiMa创建于 3月10日
KaiMa
KaiMa成员
3月10日

合入背景

请描述为什么要做这个PR内的改动。
如涉及,请关联前序PR或同特性/需求下的其他PR。
如果是修复之前PR引入的问题,请关联引入问题的PR。
注意:Fixes #ISSUE ID会自动关闭issue,如问题部分解决请不要使用Fixes,可以用Fix part of #ISSUE ID替代.

修改内容

请描述修改内容的具体实现,涉及哪些组件之间进行交互,可以用1、2、3、...进行罗列。
如果是需求或者重构类的PR,需要补充详细设计文档(说明上下游组件关系、时序图、类图、DFX能力等内容)。

资料变更

请确认是否涉及资料变更。如涉及,需要在PR中体现,并简要说明修改内容。如不涉及,需填写“不涉及”。

接口变更

请确认是否涉及跨代码仓或者客户面可见的接口变更。如涉及,需要详细说明接口以及对应的变更内容,同时需要在资料中体现。如不涉及,需填写“不涉及”。

测试结果

请说明测试场景,测试方法以及测试结果。
测试用例设计时需考虑硬件、部署方式、功能、性能、精度、显存等维度。

CheckList

PR提交人对以下CheckList自检项进行全量自检,自检通过或不涉及,均修改 [ ] 为 [x]。

likedislike
Pull Request已成功合入, 合并人@纪涛
(感谢 KaiMa 的贡献)
ascend-robot
ascend-robot成员
3月10日 评论:

Thanks for your pull-request.
The full list of commands accepted by me can be found at here
You can get sig-info at here


PR Approval Progress

⚠️ This PR does not yet meet the following requirements:lgtm (requires ≥ 2 person(s) per module)、approve (requires ≥ 1 person(s) per module)

Module Approval Details

module lgtm status approve status
repo-Ascend/MindIE-LLM ❌ (0/2)(You can also ask: chuyuelin, maoxx241, wu_yushan, yht1024, chenzhinan1212) ❌ (0/1)(You can also ask: jyoung6652, niushiya, you-zhiyuan-0511, maoxx241, senxiangms)

💡 Tip:

  • Committer can comment /approve or /lgtm
  • Commenting /approve implies both code review (lgtm) and intent to merge (approve)
likedislike
ascend-robotascend-robot成员
3月10日 添加了label:ascend-cla/yes
ascend-robot
ascend-robot成员
3月10日 评论:

CLA Signature Pass

KaiMa, thanks for your pull request. All authors of the commits have signed the CLA. 👍

likedislike
ascend-robotascend-robot成员
3月10日 添加了label:needs-issue
ascend-robot
ascend-robot成员
3月10日 评论:

Linking Issue Notice

@KaiMa , the pull request must be linked to at least one issue.
If an issue has already been linked, but the needs-issue label remains, you can remove the label by commenting /check-issue .

likedislike
纪涛纪涛成员
3月10日 合入了pull request
ascend-robot
ascend-robot成员
3月10日 评论:

问题/功能描述

本次PR对项目中的日志系统进行了大规模的统一重构和功能增强。主要解决了项目中日志接口不统一、依赖分散、格式不一致的问题,旨在提升代码的可维护性、可观测性和规范性。具体包括:将多个C++组件原有的自定义 system_log 库替换为统一的 mindie_llm_log 库;将流式日志宏调用(如 LOG_ERROR_LLM <<)更新为函数式宏调用(如 MINDIE_LLM_LOG_ERROR);并引入了动态日志级别管理功能,增强了关键路径(如请求处理、配置管理、通信模块)的调试日志。

修改方案描述

修改方案涉及项目中的CMake构建文件和大量C++源文件。核心内容包括:1) 在CMakeLists.txt中更新链接库依赖,将 system_log 替换为 mindie_llm_logmindie_llm_utils;2) 在源代码中将 #include “system_log.h” 统一替换为 #include “log.h”;3) 将所有旧的 LOG_*_LLM 宏调用更新为对应的 MINDIE_LLM_LOG_* 宏调用,部分配置管理模块的日志则改为使用 std::cout 输出以简化依赖;4) 在日志初始化逻辑中增加了动态日志级别检查器,支持运行时调整日志级别;5) 在多个模块的关键操作点补充了调试和状态日志,以增强系统运行时的可见性。

likedislike
ascend-robot
ascend-robot成员3月10日进行代码检视1
mindie_llm/text_generator/cpp/sampler/cpu_logits_handler/thread_pool.cpp
@@ -45,4 +43,4 @@
4543 }
4644 
4745 for (int i = 0; i < m_threadNum; ++i) {
4846 ret = pthread_create(&m_threadIDs[i], nullptr, Worker, this);
ascend-robot
ascend-robot3月10日评论:
拼写错误: 代码第39行错误信息中拼写错误:'Init mutext or condition failed'。正确的拼写应该是'mutex'而不是'mutext'。这个拼写错误出现在错误日志和抛出的异常信息中,虽然不影响功能,但影响代码专业性。同样的错误也出现在第42行的异常消息中。
问题类型: 拼写错误
文件路径: mindie_llm/text_generator/cpp/sampler/cpu_logits_handler/thread_pool.cpp
行号: 41
问题代码:
MINDIE_LLM_LOG_ERROR("Init mutext or condition failed");
throw runtime_error("Init mutext or condition failed.");
修改建议:
修正拼写错误,将'mutext'改为'mutex'。修改为:
```cpp
MINDIE_LLM_LOG_ERROR("Init mutex or condition failed");
throw runtime_error("Init mutex or condition failed.");
```
---
此评论由代码审查工具自动生成
likedislike
ascend-robot
ascend-robot成员3月10日进行代码检视1
src/config_manager/backend_config.cpp
@@ -119,4 +118,4 @@
119118 if (backendConfigData.contains("kvPoolConfig")) {
120119 Json& kvPoolConfig = backendConfigData["kvPoolConfig"];
121120 if (kvPoolConfig.contains("backend")) {
122121 backend = kvPoolConfig["backend"];
ascend-robot
ascend-robot3月10日评论:
安全缺陷: 代码将日志输出从专用的日志系统(LOG_ERROR_LLM)改为了直接使用std::cout。这违反了安全最佳实践,因为std::cout通常输出到标准输出(如控制台),在生产环境中可能被重定向到不安全的日志文件或完全丢失,导致安全事件(如TLS配置初始化失败)无法被审计追踪。此外,直接使用std::cout可能无法提供日志级别、时间戳、进程ID等关键上下文信息,降低了日志的可读性和可维护性。
问题类型: 安全缺陷
文件路径: src/config_manager/backend_config.cpp
行号: 68
问题代码:
        std::cout << "interNodeTlsCaFiles init error" << std::endl;
修改建议:
恢复使用专用的日志系统(如LOG_ERROR_LLM)进行错误日志记录。如果必须修改,应确保新的日志机制至少提供与原有系统相当的安全性、可靠性和信息完整性。例如,使用一个包装函数,确保日志被写入安全的、受访问控制的审计日志文件中。
---
此评论由代码审查工具自动生成
likedislike
ascend-robot
ascend-robot成员3月10日进行代码检视1
src/config_manager/backend_config.cpp
@@ -129,4 +128,4 @@
129128};
130129 
131130void BackendConfigManager::InitKvPoolConfigFromJson(Json &backendConfigData)
132131{
ascend-robot
ascend-robot3月10日评论:
安全缺陷: 同第65行问题,TLS CA文件大小验证失败的关键安全事件日志被降级为std::cout输出,削弱了安全监控和事件响应能力。
问题类型: 安全缺陷
文件路径: src/config_manager/backend_config.cpp
行号: 78
问题代码:
        std::cout << "interNodeTlsCaFiles size is invalid" << std::endl;
修改建议:
恢复使用专用的错误日志宏(如LOG_ERROR_LLM)来记录此安全配置错误。
---
此评论由代码审查工具自动生成
likedislike
ascend-robot
ascend-robot成员3月10日进行代码检视1
src/config_manager/backend_config.cpp
@@ -135,4 +134,4 @@
135134 if (backendConfigData.contains("kvPoolConfig")) {
136135 Json& kvPoolConfig = backendConfigData["kvPoolConfig"];
137136 if (kvPoolConfig.contains("backend")) {
138137 backend = kvPoolConfig["backend"];
ascend-robot
ascend-robot3月10日评论:
安全缺陷: TLS CRL文件数组初始化失败的日志被改为std::cout,影响安全审计。CRL(证书吊销列表)是TLS安全的重要组成部分,其配置失败应被明确记录。
问题类型: 安全缺陷
文件路径: src/config_manager/backend_config.cpp
行号: 84
问题代码:
        std::cout << "interNodeTlsCrlFiles init error" << std::endl;
修改建议:
恢复使用专用的错误日志记录此TLS配置错误。
---
此评论由代码审查工具自动生成
likedislike
ascend-robot
ascend-robot成员3月10日进行代码检视1
src/config_manager/backend_config.cpp
@@ -157,4 +156,4 @@
157156 if (kvPoolConfig.contains("backend")) {
158157 backend = kvPoolConfig["backend"];
159158 }
160159 if (kvPoolConfig.contains("configPath")) {
ascend-robot
ascend-robot3月10日评论:
安全缺陷: 获取Home路径失败(可能涉及TLS证书文件路径解析)的日志被改为std::cout。这是一个系统级错误,对后续TLS配置有根本性影响,需要被可靠地记录。
问题类型: 安全缺陷
文件路径: src/config_manager/backend_config.cpp
行号: 104
问题代码:
        std::cout << "Failed to get home path" << std::endl;
修改建议:
恢复使用专用的错误日志记录此系统调用失败。
---
此评论由代码审查工具自动生成
likedislike
ascend-robot
ascend-robot成员3月10日进行代码检视1
src/config_manager/backend_config.cpp
@@ -269,4 +268,4 @@
269268 if (backendConfig_.interNodeTlsCrlFilesVec.size() > MAX_FILE_LIST_SIZE) {
270269 LOG_ERROR_LLM << "interNodeTlsCrlFiles size is invalid";
271270 return false;
272271 }
ascend-robot
ascend-robot3月10日评论:
功能缺陷: 模型实例数量与NPU设备ID数组大小不匹配的配置错误日志被改为std::cout。这是一个关键的配置一致性错误,影响系统部署和资源分配,需要明确的错误日志以便调试和运维。
问题类型: 功能缺陷
文件路径: src/config_manager/backend_config.cpp
行号: 165
问题代码:
        std::cout << "The size of npuDeviceIds does not equal to modelInstanceNumber" << std::endl;
修改建议:
恢复使用专用的错误日志记录此配置验证错误。
---
此评论由代码审查工具自动生成
likedislike
ascend-robot
ascend-robot成员3月10日进行代码检视1
src/config_manager/backend_config.cpp
@@ -275,4 +274,4 @@
275274 
276275bool BackendConfigManager::CheckInterTlsParam()
277276{
278277 std::string homePath{};
ascend-robot
ascend-robot3月10日评论:
功能缺陷: NPU设备ID子集大小与模型世界大小不匹配的配置错误日志被改为std::cout。这关系到分布式推理的正确性,需要清晰的错误提示。
问题类型: 功能缺陷
文件路径: src/config_manager/backend_config.cpp
行号: 171
问题代码:
        std::cout << "The size of npuDeviceIds (subset) does not equal to worldSize" << std::endl;
修改建议:
恢复使用专用的错误日志记录此配置验证错误。
---
此评论由代码审查工具自动生成
likedislike
ascend-robot
ascend-robot成员3月10日进行代码检视1
src/config_manager/backend_config.cpp
@@ -313,4 +312,4 @@
313312 // check cert
314313 std::string tlsCertPath = homePath + backendConfig_.interNodeTlsCert;
315314 CHECK_CONFIG_VALIDATION(checkRes,
316315 ParamChecker::CheckPath(tlsCertPath, homePath, "backendConfig_.interNodeTlsCert"));
ascend-robot
ascend-robot3月10日评论:
安全缺陷: TLS配置初始化失败的关键错误日志被改为std::cout。TLS启用状态下的配置失败是一个严重的安全事件,必须被可靠地审计记录。
问题类型: 安全缺陷
文件路径: src/config_manager/backend_config.cpp
行号: 195
问题代码:
                std::cout << "Failed to init tls cfg" << std::endl;
修改建议:
恢复使用专用的错误日志记录TLS初始化失败。
---
此评论由代码审查工具自动生成
likedislike
ascend-robot
ascend-robot成员3月10日进行代码检视1
src/config_manager/backend_config.cpp
@@ -317,4 +316,4 @@
317316 std::string tlsPkPath = homePath + backendConfig_.interNodeTlsPk;
318317 CHECK_CONFIG_VALIDATION(checkRes, ParamChecker::CheckPath(tlsPkPath, homePath, "backendConfig_.interNodeTlsPk"));
319318 // check ca
320319 std::string interNodeTlsCaPath = homePath + backendConfig_.interNodeTlsCaPath;
ascend-robot
ascend-robot3月10日评论:
安全缺陷: JSON解析异常(type_error)的捕获日志被改为std::cout,并且异常信息(e.what())被直接输出。虽然当前代码片段中异常信息被输出到std::cout,但结合安全规则第37条(禁止把异常堆栈直接返回给前端),需要注意在更广泛的上下文中,此类输出是否可能泄露给用户。使用专用日志系统通常能更好地控制敏感信息的输出。此外,日志内容增加了类名和方法名,这是一个好的实践,但输出通道降级了。
问题类型: 安全缺陷
文件路径: src/config_manager/backend_config.cpp
行号: 199
问题代码:
            std::cout << "Failed to init tls cfg. [BackendConfigManager::InitFromJson] " << e.what() << std::endl;
修改建议:
恢复使用专用的错误日志记录此异常,并确保e.what()中的信息不会包含敏感的内部数据结构。可以保留增加的上下文信息([BackendConfigManager::InitFromJson])。
---
此评论由代码审查工具自动生成
likedislike
ascend-robot
ascend-robot成员3月10日进行代码检视1
src/config_manager/backend_config.cpp
@@ -356,4 +355,4 @@
356355 checkRes, ParamChecker::CheckPath(interNodeTlsCrlPath, homePath, "backendConfig_.interNodeTlsCrlPath"));
357356 for (const std::string &interNodeCrlFile : backendConfig_.interNodeTlsCrlFilesVec) {
358357 std::string interNodetlsCrlFilePath = interNodeTlsCrlPath + interNodeCrlFile;
359358 CHECK_CONFIG_VALIDATION(checkRes, ParamChecker::CheckPath(interNodetlsCrlFilePath, homePath,
ascend-robot
ascend-robot3月10日评论:
功能缺陷: NPU设备ID集合大小检查失败的日志被改为std::cout。此检查用于确保配置一致性,其失败应被明确记录。
问题类型: 功能缺陷
文件路径: src/config_manager/backend_config.cpp
行号: 223
问题代码:
            std::cout << "npuDeviceID does not allow repetitive element" << std::endl;
修改建议:
恢复使用专用的错误日志记录此验证失败。注意:原日志信息“npuDeviceID does not allow repetitive element”与代码实际检查的条件(npuDeviceId.size() != backendConfig_.worldSize)不完全匹配,可能是一个错误。建议同时审查此日志信息的准确性。
---
此评论由代码审查工具自动生成
likedislike