feat(ai-statistics): add request_model to context #2624
Conversation
新增请求模型上下文存储及日志写入逻辑优化变更概述新功能
重构
性能优化
变更文件
时序图sequenceDiagram
participant HTTP as HttpContext
participant Logger as LogWriter
participant Metrics as MetricWriter
HTTP->>Logger: WriteUserAttributeToLogWithKey
Logger-->>HTTP: 忽略返回值处理
HTTP->>Metrics: writeMetric
Metrics->>HTTP: 执行指标统计
HTTP-->>Metrics: 传递转换后的数值
💡 小贴士与 lingma-agents 交流的方式📜 直接回复评论
📜 在代码行处标记
📜 在讨论中提问
Change OverviewNew Features
Refactor
Performance Optimization
Change file
Sequence chartsequenceDiagram
participant HTTP as HttpContext
participant Logger as LogWriter
participant Metrics as MetricWriter
HTTP->>Logger: WriteUserAttributeToLogWithKey
Logger-->>HTTP: Ignore return value processing
HTTP->>Metrics: writeMetric
Metrics->>HTTP: Execution metric statistics
HTTP-->>Metrics: Pass the converted value
💡 TipsHow to communicate with lingma-agents📜 Reply to comments directly
**📜 Mark ** at line of code
📜 Ask a question during discussion
|
![lingma-agents[bot]](https://avatars.githubusercontent.com/in/1198481?s=60&v=4){kind=small}
There was a problem hiding this comment.
🔎 代码评审报告
🎯 评审意见概览
| 严重度 | 数量 | 说明 |
|---|---|---|
| 🔴 Blocker | 0 | 阻断性问题，需立即修复。例如：系统崩溃、关键功能不可用或严重安全漏洞。 |
| 🟠 Critical | 0 | 严重问题，高优先级修复。例如：核心功能异常或性能瓶颈影响用户体验。 |
| 🟡 Major | 3 | 主要问题，建议修复。例如：非核心功能缺陷或代码维护性较差。 |
| 🟢 Minor | 0 | 次要问题，酬情优化。例如：代码格式不规范或注释缺失。 |
总计: 3 个问题
📋 评审意见详情
💡 代码实现建议
以下是文件级别的代码建议，聚焦于代码的可读性、可维护性和潜在问题。
🔹 plugins/wasm-go/extensions/ai-statistics/main.go (3 💬)
- 忽略日志写入错误可能掩盖潜在问题 (L250)
- 类型转换函数`convertToUInt`未处理所有可能类型 (L540)
- 忽略`SetProperty`错误可能导致配置未生效 (L540)
🚀 架构设计建议
以下是对代码架构和设计的综合分析，聚焦于跨文件交互、系统一致性和潜在优化空间。
🔍1. 新增请求模型头未进行输入验证存在安全风险
在onHttpRequestHeaders函数中新增的x-higress-llm-model请求头直接存入Context，但未对用户输入进行长度限制、非法字符过滤等安全验证。若该值后续用于日志记录或构造响应，可能引发注入攻击或日志污染风险。需在存储前增加输入校验逻辑。
📌 关键代码
+ if requestModel, _ := proxywasm.GetHttpRequestHeader("x-higress-llm-model"); requestModel != "" {
+ ctx.SetContext(tokenusage.CtxKeyRequestModel, requestModel)
+ }恶意用户可能注入非法模型名称，导致日志泄露敏感信息或影响后续处理逻辑
🔍2. 错误忽略模式导致日志功能可靠性下降
在onHttpRequestBody、onHttpStreamingBody等4个函数中，调用ctx.WriteUserAttributeToLogWithKey时统一忽略了返回错误。若日志写入失败将无法感知，可能造成关键业务数据丢失。需统一处理日志写入错误或建立错误上报机制。
📌 关键代码
250 + _ = ctx.WriteUserAttributeToLogWithKey(wrapper.AILogKey)生产环境中日志采集失效导致故障排查困难
🔍3. 泛型类型使用增加运行时风险
新增的extractStreamingBodyByJsonPath函数返回any类型，且在setAttributeBySource等调用处未做类型断言。这种类型擦除设计可能导致运行时类型转换错误，增加维护成本。建议使用类型安全的返回结构或明确类型约束。
📌 关键代码
426 +func extractStreamingBodyByJsonPath(data []byte, jsonPath string, rule string) any {类型不匹配导致panic或不可预期的程序行为
🔍4. 配置扩展性不足影响未来功能扩展
在setAttributeBySource函数中硬编码处理配置项的逻辑，未采用策略模式或插件化设计。当需要新增配置属性类型时，需修改核心代码而非通过配置扩展，违背开闭原则。建议将处理逻辑抽象为可注册的处理器接口。
📌 关键代码
+ var value any
if source == attribute.ValueSource {功能扩展时需频繁修改核心代码，增加维护复杂度
🔍5. 跨模块数据一致性风险
新增的CtxKeyRequestModel上下文键未检查现有模块的使用情况。需确认下游组件（如metrics统计、tracing系统）是否能正确解析该字段，避免出现数据孤岛或处理逻辑缺失。
📌 关键代码
ctx.SetContext(tokenusage.CtxKeyRequestModel, requestModel)关键业务数据未被下游系统正确消费导致统计不准确
审查详情
📒 文件清单 （1 个文件）
📝 变更: 1 个文件
📝 变更文件:
plugins/wasm-go/extensions/ai-statistics/main.go
💡 小贴士
与 lingma-agents 交流的方式
📜 直接回复评论
直接回复本条评论，lingma-agents 将自动处理您的请求。例如：
-
在当前代码中添加详细的注释说明。
-
请详细介绍一下你说的 LRU 改造方案，并使用伪代码加以说明。
📜 在代码行处标记
在文件的特定位置创建评论并 @lingma-agents。例如：
-
@lingma-agents 分析这个方法的性能瓶颈并提供优化建议。
-
@lingma-agents 对这个方法生成优化代码。
📜 在讨论中提问
在任何讨论中 @lingma-agents 来获取帮助。例如：
-
@lingma-agents 请总结上述讨论并提出解决方案。
-
@lingma-agents 请根据讨论内容生成优化代码。
| @@ -244,7 +247,7 @@ func onHttpRequestBody(ctx wrapper.HttpContext, config AIStatisticsConfig, body | |||
| ctx.SetUserAttribute(ChatRound, userPromptCount) | |||
|
|
|||
| // Write log | |||
| ctx.WriteUserAttributeToLogWithKey(wrapper.AILogKey) | |||
| _ = ctx.WriteUserAttributeToLogWithKey(wrapper.AILogKey) | |||
There was a problem hiding this comment.
忽略日志写入错误可能掩盖潜在问题
🟡 Major | 🧹 Code Smells
📋 问题详情
在多个位置直接忽略WriteUserAttributeToLogWithKey的返回值，可能导致日志写入失败时未被及时发现，影响系统监控和故障排查能力。
💡 解决方案
显式处理错误或记录日志：
- _ = ctx.WriteUserAttributeToLogWithKey(wrapper.AILogKey)
+ if err := ctx.WriteUserAttributeToLogWithKey(wrapper.AILogKey); err != nil {
+ // 记录错误日志（如：log.Errorf("写入日志失败: %v", err)）
+ }您的反馈对我们很重要！(建议右键在新标签页中打开以下链接)
| @@ -534,7 +537,7 @@ func writeMetric(ctx wrapper.HttpContext, config AIStatisticsConfig) { | |||
| } | |||
| } | |||
|
|
|||
| func convertToUInt(val interface{}) (uint64, bool) { | |||
| func convertToUInt(val any) (uint64, bool) { | |||
There was a problem hiding this comment.
类型转换函数convertToUInt未处理所有可能类型
🟡 Major | 🐞 Bugs
📋 问题详情
在convertToUInt函数中，仅处理了float32类型，但可能还有float64、int等类型未被覆盖，导致转换失败时返回false但未明确错误原因。
💡 解决方案
扩展类型支持并明确默认返回：
+ case float64:
+ return uint64(v), true
+ case int:
+ return uint64(v), true
+ default:
+ return 0, false您的反馈对我们很重要！(建议右键在新标签页中打开以下链接)
| @@ -534,7 +537,7 @@ func writeMetric(ctx wrapper.HttpContext, config AIStatisticsConfig) { | |||
| } | |||
| } | |||
|
|
|||
| func convertToUInt(val interface{}) (uint64, bool) { | |||
| func convertToUInt(val any) (uint64, bool) { | |||
There was a problem hiding this comment.
忽略SetProperty错误可能导致配置未生效
🟡 Major | 🧹 Code Smells
📋 问题详情
在setSpanAttribute函数中，proxywasm.SetProperty的错误返回未被处理，可能导致追踪标签未正确设置而无法追踪问题。
💡 解决方案
添加错误日志记录：
- if e := proxywasm.SetProperty([]string{traceSpanTag}, []byte(fmt.Sprint(value))); e != nil {
+ if e := proxywasm.SetProperty([]string{traceSpanTag}, []byte(fmt.Sprint(value))); e != nil {
+ log.Errorf("设置追踪标签失败: %v", e)您的反馈对我们很重要！(建议右键在新标签页中打开以下链接)
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2624 +/- ##
===========================================
+ Coverage 35.91% 46.18% +10.27%
===========================================
Files 69 81 +12
Lines 11576 13047 +1471
===========================================
+ Hits 4157 6026 +1869
+ Misses 7104 6676 -428
- Partials 315 345 +30 🚀 New features to boost your workflow:
|
| @@ -191,6 +191,9 @@ func onHttpRequestHeaders(ctx wrapper.HttpContext, config AIStatisticsConfig) ty | |||
| if consumer, _ := proxywasm.GetHttpRequestHeader(ConsumerKey); consumer != "" { | |||
| ctx.SetContext(ConsumerKey, consumer) | |||
| } | |||
| if requestModel, _ := proxywasm.GetHttpRequestHeader("x-higress-llm-model"); requestModel != "" { | |||
| ctx.SetContext(tokenusage.CtxKeyRequestModel, requestModel) | |||
There was a problem hiding this comment.
| ctx.SetContext(tokenusage.CtxKeyRequestModel, requestModel) | |
| ctx.SetContext(tokenusage.CtxKeyModel, requestModel) |
变量名是不是不对？
There was a problem hiding this comment.
变量名是对的，wasm-go 那边我也同步添加了
这个新变量是用于记录 request 中的model名称
There was a problem hiding this comment.
Ok, 在 PR 中留了几个建议：higress-group/wasm-go#11
daixijun
changed the title
daixijun
force-pushed
the
feat/ai-statistics
branch
from
6531733 to
5467339
Compare
daixijun
changed the title
Signed-off-by: Xijun Dai <[email protected]>
daixijun
force-pushed
the
feat/ai-statistics
branch
from
5467339 to
00b6303
Compare
Ⅰ. Describe what this PR did
OpenAI 的 image 接口不返回使用的 model, 所以得要先从 HttpRequetHeader 中获取到模型存放在 context,后续在 tokenusage 中获取
Ⅱ. Does this pull request fix one issue?
Ⅲ. Why don't you add test cases (unit test/integration test)?
Ⅳ. Describe how to verify it
Ⅴ. Special notes for reviews
Ⅰ. Describe what this PR did
OpenAI's image interface does not return the used model, so you have to first get the model stored in the context from HttpRequetHeader, and then get it in tokenusage
Ⅱ. Does this pull request fix one issue?
Ⅲ. Why don't you add test cases (unit test/integration test)?
Ⅳ. Describe how to verify it
V. Special notes for reviews