Skip to content

feat: support openai-api parse#11754

Draft
lzf575 wants to merge 1 commit into
v7.1from
v71-openai-api
Draft

feat: support openai-api parse#11754
lzf575 wants to merge 1 commit into
v7.1from
v71-openai-api

Conversation

@lzf575
Copy link
Copy Markdown
Contributor

@lzf575 lzf575 commented May 20, 2026

This PR is for:

@lzf575 lzf575 requested a review from kylewanginchina May 20, 2026 08:35
@lzf575
Copy link
Copy Markdown
Contributor Author

lzf575 commented May 20, 2026

@claude review下这个pr,使用中文

@lzf575 lzf575 marked this pull request as draft May 20, 2026 08:36
@claude
Copy link
Copy Markdown
Contributor

claude Bot commented May 20, 2026

Claude finished @lzf575's task in 5m 38s —— View job


任务进度

  • 检查仓库说明与PR上下文
  • 审查核心代码改动
  • 更新PR评论反馈

Review 结论

我看下来这个 PR 的目标比较清晰:在现有 HTTP 日志解析之上增加 OpenAI-compatible API 的业务增强,补充流式/非流式 usage、TTFT/TPOT 以及业务维度提取,整体设计思路是合理的,测试样例也覆盖了几种典型场景。

但目前我认为还不建议直接合并,主要有 2 个比较实质的问题:

主要问题

  • 1. 文档声明支持 HTTP/2,但实现目前只接到了 HTTP/1 分支,存在功能与文档不一致的问题
    server/agent_config/README.md:8627 / server/agent_config/README-CH.md:8437 明确写了“HTTP/1 and HTTP/2 traffic matching the configured paths”都会被识别为 OpenAI API。
    但实际接入只发生在 agent/src/flow_generator/protocol_logs/http.rs:1119-1141 这一段 HTTP/1 解析成功后的 handle_openai_http1(...),HTTP/2 分支 agent/src/flow_generator/protocol_logs/http.rs:1145+ 没有对应的 OpenAI 增强逻辑。
    这意味着:即使配置开启了 openai_api,HTTP/2 的 OpenAI 请求目前也不会产出文档承诺的增强字段/指标。
    这个问题属于对外能力不符合文档描述,建议在合并前二选一:

    1. 补齐 HTTP/2 接入;或
    2. 先把 README / template 的描述收窄为“当前仅支持 HTTP/1”。
      Fix this →
  • 2. 业务维度属性会重复写入 attributes,可能导致下游看到重复 key
    在 request 阶段,代码会先把 biz_org_path / biz_user_id / biz_app_id 直接写入 info.attributes,见 agent/src/flow_generator/protocol_logs/http.rs:1465-1483
    但最终发送日志时,又会在 agent/src/flow_generator/protocol_logs/openai_api.rs:388-452populate_log() 中再次把同一组属性写入 attrs
    对于最终会 merge 成 session 的流式场景,这些 request-side attributes 还会通过 agent/src/flow_generator/protocol_logs/http.rs:654 被拼接进最终日志,因此最终一条日志里很可能出现两份相同 key。
    这会增加下游解析歧义:如果消费端默认假设 key 唯一,可能取到第一份、最后一份,或者行为依赖实现细节。
    更稳妥的做法是:业务维度只保留一个写入来源,建议统一在最终 openai_session.populate_log() 时输出,request 中只保留 session 状态,不提前写入 attributes。
    Fix this →

其他观察

  • agent/src/flow_generator/protocol_logs/openai_api.rs 里的状态机和单测整体写得还不错,SSE 分片、Responses API completed event、usage 缺失时 TPOT 不输出这些边界都有覆盖。
  • 配置项定义和 LogParserConfig 透传基本是对齐的,agent/src/config/config.rs:2046-2085agent/src/config/handler.rs:2389-2395 这部分没有看到明显遗漏。
  • 我尝试补充跑 OpenAI 相关测试,但当前环境里的测试命令需要额外权限,未能实际执行;因此上面的判断主要基于代码路径与数据流审阅。

总体建议

这个 PR 的方向是好的,但建议至少先修掉上面两个问题中的第 1 个;第 2 个虽然不一定立刻导致 agent 崩溃,但会给日志消费端留下不确定性。修完后我觉得就更稳了。

@lzf575 lzf575 force-pushed the v71-openai-api branch 23 times, most recently from 3fd22ea to a40a192 Compare May 21, 2026 15:45
@lzf575
Copy link
Copy Markdown
Contributor Author

lzf575 commented May 22, 2026

@claude review下这个pr,使用中文

@claude
Copy link
Copy Markdown
Contributor

claude Bot commented May 22, 2026

Claude finished @lzf575's task in 7m 47s —— View job


- 事件类型:PR 上的 issue comment,触发词是 “@claude review下这个pr,使用中文”。 - 请求类型:代码审查,不是实现修改;需要用中文输出审查结果。 - 关键信息:目标分支是 `origin/v7.1`,当前 PR 为 `v71-openai-api -> v7.1`,改动较大(13 files,+3623/-206),核心在 agent 侧新增 OpenAI API 解析与配置说明。 - 主要任务:读取仓库 `CLAUDE.md` 指令;查看 diff 与关键文件;检查正确性、性能、测试覆盖和潜在风险;把结果写回当前 Claude 评论。 - 潜在挑战:PR 较大,`openai_api.rs` 与 `http.rs` 改动集中,需重点核对 HTTP/1/HTTP/2 接入路径、流式聚合逻辑、属性写入与文档一致性;如果无法运行测试,需要在评论中说明。 - 计划:先读仓库说明,再用 `git diff origin/v7.1...HEAD` 和 `Read` 审阅关键文件,最后用中文更新评论,并附上 job run 链接。

任务进度

  • 检查仓库 CLAUDE.md 与评审约束
  • 审阅 PR diff 与关键代码路径
  • 整理中文 review 结论并更新评论

Review 结论

我看下来这个 PR 的目标比较清晰:在现有 HTTP 日志解析上增加 OpenAI-compatible API 的业务增强,补充流式/非流式 usage、TTFT/TPOT、SSE 状态机以及业务维度提取,整体结构也比较完整,agent/src/flow_generator/protocol_logs/openai_api.rs 里的单测覆盖了不少典型流式场景。

但当前我仍然不建议直接合并,主要有 2 个还比较实质的问题。

主要问题

  • 1. 文档/模板声明支持 HTTP/2,但实现仍然只接在 HTTP/1 路径上
    文档和模板都明确写了命中配置路径的 HTTP/1 和 HTTP/2 都会做 OpenAI API 增强:server/agent_config/README.md:8627server/agent_config/README-CH.md:8437server/agent_config/template.yaml:6146
    但实际代码里只有 HTTP/1 在成功解析后调用了 handle_openai_http1(...)agent/src/flow_generator/protocol_logs/http.rs:1183-1184。HTTP/2/Grpc/Triple 分支 agent/src/flow_generator/protocol_logs/http.rs:1209-1286 没有任何对应的 OpenAI 接入逻辑。
    这会导致配置开启后,HTTP/2 的 OpenAI 请求并不会产出文档承诺的增强字段/指标,属于比较直接的能力不一致。建议二选一:

    1. 补齐 HTTP/2 的接入;或
    2. 先把 README / template 收窄成“当前仅支持 HTTP/1”。
      Fix this →
  • 2. “request_body_max_bytes 会缓存并解析请求体以提取业务维度”的文档描述,与当前实现不符
    文档把 request_body_max_bytes 描述成“请求体的最大缓存和解析字节数,用于提取 stream 标志和业务维度字段”:server/agent_config/README.md:8755-8756server/agent_config/README-CH.md:8562server/agent_config/template.yaml:6221-6224
    但实现里并没有请求体重组/缓存逻辑:OpenAISession 只有 SSE 缓冲,没有任何 request-body buffer,见 agent/src/flow_generator/protocol_logs/openai_api.rs:246-291。请求体分段时,handle_openai_sse_continuation() 只是把当前这个 segment 直接喂给 parse_request_body()agent/src/flow_generator/protocol_logs/http.rs:1782-1795;而 parse_request_body() 在 JSON 解析失败时只做了对 "stream": true 的字节级兜底,业务维度 JSON path 提取不会生效:agent/src/flow_generator/protocol_logs/openai_api.rs:730-746agent/src/flow_generator/protocol_logs/openai_api.rs:757-767
    也就是说,只要请求体跨包且单个 segment 不是完整 JSON,stream 还有机会被识别,但 biz_dimension_extractors.*.json_paths 基本拿不到值。这和当前配置说明里的“缓存并解析请求体以提取业务维度”不一致。建议二选一:

    1. 真正实现 request body 的缓存/重组后再做 JSON path 提取;或
    2. 先把文档收窄为“分段请求体下仅尽力识别 stream,不保证提取 body 内业务维度字段”。
      Fix this →

其他观察

  • agent/src/flow_generator/protocol_logs/http.rs:924-934 这里已经对 request 阶段提前写入的 biz attributes 做了去重,再调用 populate_log() 补最终值,所以我之前担心的“重复业务维度 key”问题,在当前分支里已经基本处理掉了。
  • agent/src/flow_generator/protocol_logs/openai_api.rs 的 SSE 解析、Responses API completed event、chunked SSE 解码这些路径整体写得不错,边界考虑也比较细。
  • UsageStatus::StreamInterrupted 目前我只看到定义,没有看到任何状态流转落到这个值;这不是 blocker,但如果后面希望下游区分“正常缺失 usage”和“流异常中断”,现在这条状态链路还没真正接起来。

总体建议

这个 PR 的方向是对的,核心状态机实现也比我预期完整;但上面两个问题都属于“对外能力声明和真实行为不一致”。我建议至少先把它们中的文档/实现差异收敛掉,再合并会更稳。

补充说明:这次结论主要基于代码路径审阅,我没有在当前环境里实际跑测试命令验证这些路径。

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant