|
| 1 | +# LoongSuite Python 探针代码审查规范 |
| 2 | + |
| 3 | +本代码仓库是基于 OpenTelemetry 针对 Python 应用进行可观测数据采集的探针。请根据以下原则 Review 代码,并给出修改建议。 |
| 4 | + |
| 5 | +--- |
| 6 | + |
| 7 | +## 1. 优先使用框架扩展机制 |
| 8 | + |
| 9 | +埋点时请尽可能使用框架自带的扩展机制,比如 ASGI 的 middleware 机制。 |
| 10 | + |
| 11 | +**避免**:使用 monkey patch 去 patch 某个具体的方法,因为这个方法可能在某个版本存在,在下一个版本就不存在了。 |
| 12 | + |
| 13 | +--- |
| 14 | + |
| 15 | +## 2. 避免引入三方依赖 |
| 16 | + |
| 17 | +* 原则上不要引入三方的依赖库,需要用拷贝源码的方式 shadow 到项目中,但 requests 库例外。 |
| 18 | +* 尽量复用已有的依赖,例如假如依赖 httpx 可以改为依赖 aiohttp,因为 aiohttp 是已有的依赖,而 httpx 需要新增 |
| 19 | + |
| 20 | +--- |
| 21 | + |
| 22 | +## 3. 保留业务语义 |
| 23 | + |
| 24 | +埋点逻辑需要保留用户的业务语义。 |
| 25 | + |
| 26 | +**例如**:在某个地方原本用户要抛出一个异常,探针就不可以吞掉这个异常,必须确保异常能够正常抛出。 |
| 27 | + |
| 28 | +--- |
| 29 | + |
| 30 | +## 4. 性能要求 |
| 31 | + |
| 32 | +埋点的逻辑需要足够轻量,避免占用用户过多的 CPU。 |
| 33 | + |
| 34 | +**禁止**: |
| 35 | +- `deepcopy` 操作 |
| 36 | +- 流量链路禁止使用 info 及以上级别,建议使用 `logger.debug` 级别,仅在开启 DEBUG 模式时输出。原因是频繁打日志会占用用户过多的 CPU 和 I/O 资源。 |
| 37 | + |
| 38 | +--- |
| 39 | + |
| 40 | +## 5. 资源泄漏检查 |
| 41 | + |
| 42 | +需要注意潜在的资源泄漏情况,包括但不限于以下几种: |
| 43 | + |
| 44 | +### 5.1 循环引用 + 不可回收对象(含 `__del__`) |
| 45 | + |
| 46 | +**问题**:引用计数无法处理循环引用,Python 会依靠循环垃圾回收器(gc)来清理。 |
| 47 | +**风险点**:如果循环引用中的对象定义了 `__del__` 方法,Python 2 或早期版本可能无法自动回收(Python 3.4+ 已改进,但仍需注意)。 |
| 48 | +**建议**:避免在可能产生循环引用的对象中定义 `__del__`;或显式断开引用。 |
| 49 | + |
| 50 | +### 5.2 全局变量或模块级缓存无限增长 |
| 51 | + |
| 52 | +**问题**:全局字典、列表、缓存等在程序生命周期内不会被释放。 |
| 53 | + |
| 54 | +**典型场景**: |
| 55 | +- 手动实现的缓存未设置淘汰策略(如 LRU) |
| 56 | +- 使用 `functools.lru_cache` 装饰长期运行的函数,但未限制大小 |
| 57 | + |
| 58 | +**建议**:使用 `functools.lru_cache(maxsize=...)` 或定期清理缓存。 |
| 59 | + |
| 60 | +### 5.3 闭包引用外部变量导致对象无法释放 |
| 61 | + |
| 62 | +**问题**:闭包可能无意中持有了大对象的引用。 |
| 63 | +**建议**:在闭包中显式 `del` 不需要的变量,或重构避免捕获大对象。 |
| 64 | + |
| 65 | +### 5.4 第三方 C 扩展模块的内存管理错误 |
| 66 | + |
| 67 | +**问题**:C 扩展(如某些 numpy、pytorch、opencv 插件)若未正确释放内存,Python 的 gc 无法干预。 |
| 68 | + |
| 69 | +**表现**:`tracemalloc` 或 `pympler` 无法追踪到泄漏,但系统内存持续增长。 |
| 70 | + |
| 71 | +**建议**: |
| 72 | +- 检查 C 扩展文档或更新版本 |
| 73 | +- 使用 `objgraph`、`memory_profiler` 或系统工具(如 valgrind)辅助诊断 |
| 74 | + |
| 75 | +### 5.5 未关闭的资源(文件、连接、线程等) |
| 76 | + |
| 77 | +虽然严格来说不一定是"内存泄漏",但未关闭的资源会持有内存或系统句柄。 |
| 78 | + |
| 79 | +**示例**: |
| 80 | + |
| 81 | +```python |
| 82 | +f = open('huge_file.txt') |
| 83 | +# 忘记 f.close(),文件句柄和缓冲区内存可能长期占用 |
| 84 | +``` |
| 85 | + |
| 86 | +**建议**:始终使用 `with` 语句管理资源。 |
| 87 | + |
| 88 | +### 5.6 异常捕获中保留 traceback |
| 89 | + |
| 90 | +**问题**:`traceback` 对象会持有栈帧,栈帧又引用局部变量,导致整个作用域的对象无法释放。 |
| 91 | + |
| 92 | +**示例**: |
| 93 | + |
| 94 | +```python |
| 95 | +import sys |
| 96 | + |
| 97 | +exc_info = None |
| 98 | +try: |
| 99 | + raise ValueError("oops") |
| 100 | +except Exception as e: |
| 101 | + exc_info = sys.exc_info() # 包含 traceback |
| 102 | +# exc_info 持有栈帧 → 持有所有局部变量 |
| 103 | +``` |
| 104 | + |
| 105 | +**建议**:避免长期保存 `exc_info`,使用后设为 `None`;或使用 `traceback.format_exc()` 代替。 |
| 106 | + |
| 107 | +--- |
| 108 | + |
| 109 | +## 6. 线程和定时任务限制 |
| 110 | + |
| 111 | +**禁止**:创建新的线程以及定时任务。 |
| 112 | + |
| 113 | +如果一定要创建,需要严格评估对应的资源开销。 |
| 114 | + |
| 115 | +--- |
| 116 | + |
| 117 | +## 7. 高基数属性检查 |
| 118 | + |
| 119 | +检查指标中的各个属性是否有可能是高基数的,高基数的属性容易造成发散问题。 |
| 120 | + |
| 121 | +**特别注意以下属性**: |
| 122 | +- `endpoint` |
| 123 | +- `destId` |
| 124 | +- `rpc` |
| 125 | + |
| 126 | +--- |
| 127 | + |
| 128 | +## 8. 插件埋点范围 |
| 129 | + |
| 130 | +请检查插件中是否只对这个插件本身的一些类进行埋点。 |
| 131 | +**禁止**:对插件依赖的类进行埋点。 |
| 132 | +**例如**:CrewAI 依赖了 litellm 框架,在 CrewAI 的埋点里面就不要对 litellm 相关的类进行埋点操作。 |
| 133 | + |
| 134 | +--- |
| 135 | + |
| 136 | +## 9. 异常相关 |
| 137 | + |
| 138 | +建议统一使用 raise 来保留完整的异常堆栈,使用 raise e 会丢失原始堆栈信息。 |
| 139 | + |
| 140 | +--- |
| 141 | + |
| 142 | +## 10. 线程安全 |
| 143 | + |
| 144 | +### 10.1 单例模式必须使用线程锁保护 |
| 145 | +**问题**:在并发场景下,不加锁的单例检查可能会创建多个实例。 |
| 146 | + |
| 147 | +### 10.2 全局变量读写需要注意线程安全 |
| 148 | + |
| 149 | +多线程环境下修改全局变量可能导致竞态条件,建议: |
| 150 | +- 使用线程锁保护读写操作 |
| 151 | +- 或将配置封装到单例类中 |
| 152 | + |
| 153 | +--- |
| 154 | + |
| 155 | +## 11. 导入语句规范 |
| 156 | + |
| 157 | +### 11.1 导入顺序 |
| 158 | + |
| 159 | +按照 PEP 8 规范,导入语句应按以下顺序排列: |
| 160 | +1. 标准库导入 |
| 161 | +2. 第三方库导入 |
| 162 | +3. 本地导入 |
| 163 | + |
| 164 | +各组之间用空行分隔。 |
| 165 | + |
| 166 | +### 11.2 避免重复导入 |
| 167 | + |
| 168 | +同一个模块只应导入一次,检查文件中是否存在重复的 import 语句。 |
| 169 | + |
| 170 | +### 11.3 延迟导入 |
| 171 | + |
| 172 | +对于可能不存在的库(如用户未安装的框架),建议使用延迟导入或 try-except 方式,避免在模块级别直接导入可能不存在的模块,这会导致整个模块加载失败。 |
| 173 | + |
| 174 | +--- |
| 175 | + |
| 176 | +## 12. 异常处理规范 |
| 177 | + |
| 178 | +### 12.1 避免捕获过宽的异常 |
| 179 | +**禁止**:使用 `except BaseException`,这会捕获 `KeyboardInterrupt` 和 `SystemExit`。 |
| 180 | +**建议**:使用 `except Exception`。 |
| 181 | + |
| 182 | +### 12.2 使用 logger 而非 print |
| 183 | + |
| 184 | +--- |
| 185 | + |
| 186 | +## 13. Python 版本兼容性 |
| 187 | + |
| 188 | +* 避免使用 Python 3.8 以上才支持的语法 |
| 189 | + |
| 190 | +--- |
| 191 | + |
| 192 | +## 14. 代码格式规范 |
| 193 | + |
| 194 | +* 文件末尾换行符:根据 PEP 8 规范,所有 Python 文件末尾应该有一个换行符。 |
| 195 | +* 顶级函数和类定义之间应有**两个空行**。 |
| 196 | +* 等号两侧应有空格 |
| 197 | +* 枚举命名风格一致性,符合之前命名的习惯 |
| 198 | +* 变量命名需要比较有意义,不要有一些很简单的字符 |
| 199 | + |
| 200 | +--- |
| 201 | + |
| 202 | +## 15. 单测测试覆盖要求 |
| 203 | + |
| 204 | +* 新增的功能模块应该有对应的单元测试覆盖,特别是: |
| 205 | + - 核心业务逻辑 |
| 206 | + - 边界情况处理 |
| 207 | + - 异常情况处理 |
| 208 | +* 单元测试需要验证到响应的功能修改,例如新增一个插件的单元测试需要包含完成 trace 验证,metrics 验证,并且测试用例尽量真实,不能有太多的 mock 测试。 |
| 209 | + |
| 210 | +--- |
| 211 | + |
| 212 | +## 16. 字典和缓存的内存管理 |
| 213 | + |
| 214 | +### 16.1 防止字典无限增长 |
| 215 | + |
| 216 | +**建议**: |
| 217 | +1. 添加最大容量限制 |
| 218 | +2. 添加定期清理机制(如基于时间的过期) |
| 219 | +3. 使用 `WeakValueDictionary` 允许 GC 自动回收 |
| 220 | + |
| 221 | +--- |
| 222 | + |
| 223 | +## 17. 拼写检查 |
| 224 | + |
| 225 | +* 检查是否存在拼写错误 |
| 226 | + |
| 227 | +--- |
| 228 | + |
| 229 | +## 18. 方法存在性验证 |
| 230 | + |
| 231 | +* 在重构代码后,请确保调用的方法确实存在。常见问题是重构后方法名变更或被移除,导致运行时 `AttributeError`。 |
| 232 | + |
| 233 | +--- |
| 234 | + |
| 235 | +## 19. 插件埋点对称性 |
| 236 | + |
| 237 | +* 在插件的 uninstrument 的实现中,要保持 instrument 实现的对称性,例如在 instrument 中申请的资源,在 uninstrument 中释放 |
| 238 | +* 在 instrument 中实现的逻辑,在 uninstrument 方法中进行倒序的清理。 |
| 239 | + |
| 240 | +--- |
| 241 | + |
| 242 | +## 20. 注释相关 |
| 243 | + |
| 244 | +* 关键的方法,建议给出一些 input/output 的示例,提升代码可读性 |
| 245 | +* 避免引入无意义的注释,例如一些开发过程中的注释,忘记删除 |
| 246 | +* 有些代码被注释掉,这种情况建议直接删除 |
| 247 | + |
| 248 | +--- |
| 249 | + |
| 250 | +## 21. PR 规范 |
| 251 | + |
| 252 | +* PR 的标题需要符合 conventional commits 规范 |
| 253 | +* PR 中需要关联工作项 |
0 commit comments