-
Notifications
You must be signed in to change notification settings - Fork 902
feat: ai-token-ratelimit support setting global rate limit thresholds for routes #2667
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: ai-token-ratelimit support setting global rate limit thresholds for routes #2667
Conversation
AI Token限流插件支持全局限流及配置优化变更概述
变更文件
💡 小贴士与 lingma-agents 交流的方式📜 直接回复评论
📜 在代码行处标记
📜 在讨论中提问
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🔎 代码评审报告
🎯 评审意见概览
| 严重度 | 数量 | 说明 |
|---|---|---|
| 🔴 Blocker | 0 | 阻断性问题,需立即修复。例如:系统崩溃、关键功能不可用或严重安全漏洞。 |
| 🟠 Critical | 3 | 严重问题,高优先级修复。例如:核心功能异常或性能瓶颈影响用户体验。 |
| 🟡 Major | 2 | 主要问题,建议修复。例如:非核心功能缺陷或代码维护性较差。 |
| 🟢 Minor | 0 | 次要问题,酬情优化。例如:代码格式不规范或注释缺失。 |
总计: 5 个问题
📋 评审意见详情
💡 代码实现建议
以下是文件级别的代码建议,聚焦于代码的可读性、可维护性和潜在问题。
📖 plugins/wasm-go/extensions/ai-token-ratelimit/README.md (1 💬)
📖 plugins/wasm-go/extensions/ai-token-ratelimit/README_EN.md (1 💬)
🔹 plugins/wasm-go/extensions/ai-token-ratelimit/config/config.go (3 💬)
- 全局限流阈值解析逻辑未验证阈值是否为正数,可能导致无效配置。 (L233-L238)
- 限流键值匹配逻辑存在缺陷,可能导致正则表达式匹配失败。 (L353-L355)
- 单个限流规则项的阈值解析逻辑未验证阈值是否为正数,可能导致无效配置。 (L372-L381)
🚀 架构设计建议
以下是对代码架构和设计的综合分析,聚焦于跨文件交互、系统一致性和潜在优化空间。
🔍1. 全局限流与规则限流的配置互斥逻辑可能导致用户配置错误
当前实现中,global_threshold 和 rule_items 被设计为互斥配置,即两者只能选其一。这种设计虽然简化了限流逻辑,但在实际使用场景中,用户可能期望同时应用全局限流和特定规则限流(例如,先进行全局限流再进行细粒度限流)。建议在文档中明确说明此限制,并考虑未来扩展支持两种模式的组合使用。
📌 关键代码
if !hasGlobal && !hasRule {
return errors.New("at least one of 'global_threshold' or 'rule_items' must be set")
} else if hasGlobal && hasRule {
return errors.New("'global_threshold' and 'rule_items' cannot be set at the same time")
}用户可能会因不了解配置互斥逻辑而产生配置错误,导致限流策略未按预期生效,影响系统稳定性或用户体验。
🔍2. 全局限流阈值未校验是否为正数,存在无效配置风险
在解析 global_threshold 的过程中,虽然检查了时间窗口字段是否存在且大于0,但没有对 Count 字段做正数校验。如果用户配置了一个非正数的 Count 值,可能导致限流逻辑异常或 Redis 错误。建议增加对 Count 的正数校验,确保配置的有效性。
📌 关键代码
func parseGlobalThreshold(item gjson.Result) (*GlobalThreshold, error) {
for timeWindowKey, duration := range timeWindows {
q := item.Get(timeWindowKey)
if q.Exists() && q.Int() > 0 {
return &GlobalThreshold{
Count: q.Int(),
TimeWindow: duration,
}, nil
}
}
return nil, errors.New("one of 'token_per_second', 'token_per_minute', 'token_per_hour', or 'token_per_day' must be set for global_threshold")
}无效的全局限流阈值可能导致限流功能失效或 Redis 操作失败,影响系统的稳定性和可靠性。
🔍3. 限流键值匹配逻辑存在缺陷,正则表达式匹配可能失败
在处理 limit_by_per_* 类型的限流规则时,对于正则表达式的匹配逻辑存在潜在缺陷。如果用户配置的正则表达式不合法或匹配逻辑有误,可能导致限流键值无法正确匹配,从而影响限流效果。建议增强正则表达式校验和匹配逻辑的健壮性。
📌 关键代码
if itemKey == "*" {
itemType = AllType
} else if strings.HasPrefix(itemKey, "regexp:") {
regexpStr := itemKey[len("regexp:"):]
var err error
regexp, err = re.Compile(regexpStr)
if err != nil {
return fmt.Errorf("failed to compile regex for key '%s': %w", itemKey, err)
}
itemType = RegexpType
} else {
return fmt.Errorf("the '%s' restriction must start with 'regexp:' or be exactly '*'", rule.LimitType)
}正则表达式匹配失败可能导致限流键值无法正确识别,使得部分请求绕过限流控制,影响系统的公平性和稳定性。
🔍4. 限流规则项的阈值解析逻辑未验证阈值是否为正数,可能导致无效配置
在解析限流规则项的阈值时,虽然检查了时间窗口字段是否存在且大于0,但没有对 Count 字段做正数校验。如果用户配置了一个非正数的 Count 值,可能导致限流逻辑异常或 Redis 错误。建议增加对 Count 的正数校验,确保配置的有效性。
📌 关键代码
func createConfigItemFromRate(item gjson.Result, itemType LimitConfigItemType, key string, ipNet *iptree.IPTree, regexp *re.Regexp) (*LimitConfigItem, error) {
for timeWindowKey, duration := range timeWindows {
q := item.Get(timeWindowKey)
if q.Exists() && q.Int() > 0 {
return &LimitConfigItem{
ConfigType: itemType,
Key: key,
IpNet: ipNet,
Regexp: regexp,
Count: q.Int(),
TimeWindow: duration,
}, nil
}
}
return nil, errors.New("one of 'token_per_second', 'token_per_minute', 'token_per_hour', or 'token_per_day' must be set for key: " + key)
}无效的限流阈值可能导致限流功能失效或 Redis 操作失败,影响系统的稳定性和可靠性。
🔍5. 缺少对重复限流规则的严格校验,可能导致配置冲突
当前实现中,虽然记录了已出现的 LimitType 和 Key 组合,并在发现重复时发出警告,但并未阻止配置的加载。如果用户配置了多个相同的限流规则,可能导致限流行为不一致或难以调试。建议加强重复规则的校验,避免配置冲突。
📌 关键代码
// 构造LimitType和Key的唯一标识
ruleKey := string(ruleItem.LimitType) + ":" + ruleItem.Key
// 检查是否有重复的LimitType和Key组合
if seenLimitRules[ruleKey] {
log.Warnf("duplicate rule found: %s='%s' in rule_items", ruleItem.LimitType, ruleItem.Key)
} else {
seenLimitRules[ruleKey] = true
}重复的限流规则可能导致限流行为不一致,增加调试难度,影响系统的可维护性和稳定性。
审查详情
📒 文件清单 (13 个文件)
✅ 新增: 3 个文件
❌ 删除: 2 个文件
📝 变更: 8 个文件
✅ 新增文件:
plugins/wasm-go/extensions/ai-token-ratelimit/config/config.goplugins/wasm-go/extensions/ai-token-ratelimit/config/config_test.goplugins/wasm-go/extensions/ai-token-ratelimit/util/utils.go
❌ 删除文件:
plugins/wasm-go/extensions/ai-token-ratelimit/config.goplugins/wasm-go/extensions/ai-token-ratelimit/utils.go
📝 变更文件:
plugins/wasm-go/extensions/ai-token-ratelimit/README.mdplugins/wasm-go/extensions/ai-token-ratelimit/README_EN.mdplugins/wasm-go/extensions/ai-token-ratelimit/go.modplugins/wasm-go/extensions/ai-token-ratelimit/go.sumplugins/wasm-go/extensions/ai-token-ratelimit/main.goplugins/wasm-go/extensions/cluster-key-rate-limit/config/config.goplugins/wasm-go/extensions/cluster-key-rate-limit/go.sumplugins/wasm-go/extensions/cluster-key-rate-limit/main.go
💡 小贴士
与 lingma-agents 交流的方式
📜 直接回复评论
直接回复本条评论,lingma-agents 将自动处理您的请求。例如:
-
在当前代码中添加详细的注释说明。
-
请详细介绍一下你说的 LRU 改造方案,并使用伪代码加以说明。
📜 在代码行处标记
在文件的特定位置创建评论并 @lingma-agents。例如:
-
@lingma-agents 分析这个方法的性能瓶颈并提供优化建议。
-
@lingma-agents 对这个方法生成优化代码。
📜 在讨论中提问
在任何讨论中 @lingma-agents 来获取帮助。例如:
-
@lingma-agents 请总结上述讨论并提出解决方案。
-
@lingma-agents 请根据讨论内容生成优化代码。
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2667 +/- ##
===========================================
+ Coverage 35.91% 46.02% +10.11%
===========================================
Files 69 81 +12
Lines 11576 13020 +1444
===========================================
+ Hits 4157 5993 +1836
+ Misses 7104 6681 -423
- Partials 315 346 +31 🚀 New features to boost your workflow:
|
Ⅰ. Describe what this PR did
1)AI Token限流插件支持针对整个路由设置限流阈值
2)cluster-key-rate-limit和ai-token-ratelimit插件配置多个相同限流类型时日志提示
3)统一cluster-key-rate-limit和ai-token-ratelimit插件的基础逻辑
Ⅱ. Does this pull request fix one issue?
fixes #2659
fixes #2592
Ⅲ. Why don't you add test cases (unit test/integration test)?
Ⅳ. Describe how to verify it
cluster-key-rate-limit相同rule_item日志提示:
1)相同的type+key有warn日志提示
2)相同的type+不同的key没有warn日志提示
ai token限流插件针对整个路由设置限流值:
1)功能回归
2)全局限流功能验证
Ⅴ. Special notes for reviews