v6.6.50（解决 #59 冲突） #60
Conversation
…pare in hasModelMappingsChanged; removed boolTo01. • Performance: pre-allocate map and regex slice capacities in UpdateMappings. • Verified with amp module tests (all passing)
feat: regex support for model-mappings
…st-id-tracking feat(logging): implement request ID tracking and propagation
Summary of ChangesHello @luispater, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! 此拉取请求主要旨在提升API的可观察性和灵活性。它为AI API调用引入了一个强大的请求ID追踪机制，确保日志和日志文件组织具有更好的可追溯性。此外，它通过增加对正则表达式的支持，显著增强了AMP模型映射功能，从而实现更动态和强大的模型路由配置。此PR也是为了解决先前合并尝试中的冲突而重新提交的。 Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
本次 PR 主要包含了两项重要改进：为日志和请求追踪引入了 request_id，并为模型映射（model mapping）增加了正则表达式支持。这些变更为系统的可观测性和灵活性带来了显著提升。
代码整体质量很高，特别是 request_id 的上下文传递和正则表达式的实现逻辑清晰。
我提出了两点建议：
- 在
internal/api/modules/amp/amp.go中，修复hasModelMappingsChanged函数的一个逻辑缺陷。当前实现忽略了正则表达式规则的顺序，可能导致配置变更无法生效。 - 在
internal/logging/requestid.go中，为GenerateRequestID函数增加了错误日志，以处理罕见但严重的crypto/rand读取失败情况，增强了系统的健壮性。
请查看具体的审查评论以获取详细信息和代码建议。
| // Build map for efficient and robust comparison | ||
| type mappingInfo struct { | ||
| to string | ||
| regex bool | ||
| } | ||
| oldMap := make(map[string]mappingInfo, len(old.ModelMappings)) | ||
| for _, mapping := range old.ModelMappings { | ||
| oldMap[strings.TrimSpace(mapping.From)] = strings.TrimSpace(mapping.To) | ||
| oldMap[strings.TrimSpace(mapping.From)] = mappingInfo{ | ||
| to: strings.TrimSpace(mapping.To), | ||
| regex: mapping.Regex, | ||
| } | ||
| } | ||
|
|
||
| for _, mapping := range new.ModelMappings { | ||
| from := strings.TrimSpace(mapping.From) | ||
| to := strings.TrimSpace(mapping.To) | ||
| if oldTo, exists := oldMap[from]; !exists || oldTo != to { | ||
| if oldVal, exists := oldMap[from]; !exists || oldVal.to != to || oldVal.regex != mapping.Regex { | ||
| return true | ||
| } | ||
| } |
There was a problem hiding this comment.
当前的 hasModelMappingsChanged 实现使用 map 来比较新旧模型映射，这会导致正则表达式映射的顺序信息丢失。根据 internal/api/modules/amp/model_mapping.go 中的实现，正则表达式的匹配顺序至关重要。
如果用户在配置文件中仅仅调整了正则表达式规则的顺序，当前的实现将无法检测到这一变更，导致路由逻辑不会更新，这可能会引发难以调试的非预期路由行为。
建议修改此函数，以确保在比较时能考虑到规则的顺序。一个更简单且健壮的方法是直接按顺序比较 ModelMappings 切片中的每个元素。
// Direct comparison is needed to respect the order of regex mappings.
for i := range new.ModelMappings {
oldMapping := old.ModelMappings[i]
newMapping := new.ModelMappings[i]
if strings.TrimSpace(oldMapping.From) != strings.TrimSpace(newMapping.From) ||
strings.TrimSpace(oldMapping.To) != strings.TrimSpace(newMapping.To) ||
oldMapping.Regex != newMapping.Regex {
return true
}
}| func GenerateRequestID() string { | ||
| b := make([]byte, 4) | ||
| if _, err := rand.Read(b); err != nil { | ||
| return "00000000" | ||
| } | ||
| return hex.EncodeToString(b) | ||
| } |
There was a problem hiding this comment.
在 GenerateRequestID 函数中，当 crypto/rand.Read 失败时，函数会静默地返回一个固定的字符串 "00000000"。虽然 crypto/rand 失败的情况很少见，但一旦发生，这将导致后续生成的所有请求ID都相同，从而严重影响日志的可追溯性。
这是一个关键的系统问题（通常意味着操作系统的熵源出现问题），应该被记录下来以便排查。建议在此处添加日志记录到标准错误流（stderr），以避免与 logrus 记录器产生循环依赖或在记录器尚未初始化时出现问题。
为了使用 fmt.Fprintf 和 os.Stderr，请确保在文件顶部导入 fmt 和 os 包。
func GenerateRequestID() string {
b := make([]byte, 4)
if _, err := rand.Read(b); err != nil {
// This is a critical error, as it means the OS's entropy source is failing.
// Log it to stderr for visibility, as the main logger might not be initialized
// or could cause circular dependencies.
fmt.Fprintf(os.Stderr, "CRITICAL: failed to generate request ID from crypto/rand: %v\n", err)
return "00000000"
}
return hex.EncodeToString(b)
}
基于 PR #59 的内容做冲突解决并重新提交：
internal/logging/global_logger.go的合并冲突（保留 request_id 格式化 + nil Caller 兜底）。go build -o test-output ./cmd/server && rm test-output编译验证。此 PR 用于替代无法直接合并的 #59。