Skip to content

fix: incorrect endpoint#11670

Merged
yuanchaoa merged 1 commit into
mainfrom
fix-incorrect-endpoint
Apr 30, 2026
Merged

fix: incorrect endpoint#11670
yuanchaoa merged 1 commit into
mainfrom
fix-incorrect-endpoint

Conversation

@yuanchaoa
Copy link
Copy Markdown
Contributor

@yuanchaoa yuanchaoa commented Apr 27, 2026

This PR is for:

  • Agent

fix: incorrect endpoint

Steps to reproduce the bug

Changes to fix the bug

  • perf_stats函数不再支持乱序的场景,不在缓存响应的指标
  • is_request_of/is_response_of 时间放宽,避免请求响应时间戳ns 转 ms时时间变为相同导致无法聚合-
  • to_owned_segment添加flow id的赋值,避免tcp重组的指标在函数perf_stats里生成错误的key
  • 对于响应能解析出来endpoint的不会再查询endpoint

Affected branches

  • main
  • 7.1
  • 6.6

Checklist

  • Added unit test to verify the fix.
  • Verified eBPF program runs successfully on linux 4.14.x.
  • Verified eBPF program runs successfully on linux 4.19.x.
  • Verified eBPF program runs successfully on linux 5.2.x.

@yuanchaoa yuanchaoa force-pushed the fix-incorrect-endpoint branch 5 times, most recently from 0dce010 to aa4f935 Compare April 30, 2026 09:08
@kylewanginchina
Copy link
Copy Markdown
Contributor

@claude review一下当前pr中的代码修改

@claude
Copy link
Copy Markdown
Contributor

claude Bot commented Apr 30, 2026

Claude finished @kylewanginchina's task in 2m 31s —— View job


进度

  • Update the GitHub comment with the review checklist
  • Inspect the PR diff and relevant Rust files
  • Analyze the changes for correctness and regressions
  • Post the review feedback in the GitHub comment

Review 结论

整体看,这个 PR 的目标比较明确:把 endpoint 的缓存与回填逻辑统一收敛到 load_endpoint_from_cache(),同时减少 LogCache 中 endpoint 的冗余写入与一些分支逻辑。主线改动方向是合理的,agent/src/common/l7_protocol_info.rsagent/src/common/l7_protocol_log.rs 的重构也让 endpoint 的来源更清晰了。

不过我看下来有 1 个比较明确的回归风险,建议在合入前修掉。

发现的问题

  • Wasm 自定义协议的 response endpoint 回填写到了错误字段
    agent/src/flow_generator/protocol_logs/plugin/wasm.rs:70-75,当前逻辑是:response 日志没有 endpoint 时,从 cache 中取 request endpoint,然后写回 i.resp.endpoint
    但同一套 CustomInfo 结构在其他路径里的处理不是这样的:

    • agent/src/flow_generator/protocol_logs/plugin/shared_obj.rs:176-183 写回的是 info.req.endpoint
    • agent/src/flow_generator/protocol_logs/plugin/custom_protocol_policy.rs:193-196 写回的也是 info.req.endpoint

    这说明当前 PR 把 Wasm 路径改成了和其他 custom protocol 路径不一致的语义。若下游序列化/展示仍然默认从 request 侧取 endpoint,那么 Wasm response 日志可能会出现 endpoint 丢失,等价于这次“修 incorrect endpoint”只修好了 SO/custom-policy,反而让 Wasm 变成例外。

    建议把这里改回与另外两条 custom protocol 路径一致的写法,统一写到 request 侧字段。
    Fix this →

其他观察

  • agent/src/common/l7_protocol_info.rs:199-222 里把 endpoint cache 的读取集中到了 trait 默认方法,这个方向是好的,也避免了之前散落在各协议里的重复判断。
  • 从我抽查的内置协议看,例如:
    • agent/src/flow_generator/protocol_logs/sql/mysql.rs:818-821
    • agent/src/flow_generator/protocol_logs/rpc/dubbo.rs:585-588
    • agent/src/flow_generator/protocol_logs/mq/kafka.rs:906-909
      这些协议都还是把 response 缺失的 endpoint 回填到各自统一的 endpoint 字段,语义基本一致。
  • 这次 review 里我没有跑针对 agent 模块的测试/构建;另外,当前环境下完整 git diff 子命令需要额外授权,所以我是基于已给出的 PR 文件列表和相关源码做的静态审查。建议修正后至少在 @agent 下补一轮最小相关验证,例如 cargo fmt 和目标模块的测试/构建检查。

总结

我目前的结论是:改动方向 OK,但建议先修 Wasm custom protocol 的 endpoint 回填字段不一致问题,再合入。

@yuanchaoa yuanchaoa force-pushed the fix-incorrect-endpoint branch from aa4f935 to ab8652a Compare April 30, 2026 10:07
@yuanchaoa
Copy link
Copy Markdown
Contributor Author

@claude 代码修复了,看看是否修复完整

@claude
Copy link
Copy Markdown
Contributor

claude Bot commented Apr 30, 2026

Claude finished @yuanchaoa's task in 2m 0s —— View job


进度

  • Update the GitHub comment with the review checklist
  • Inspect the repaired endpoint changes
  • Check whether the fix is complete and consistent
  • Post the review result in the GitHub comment

Review 结论

我复查了这次修复,前一次指出的 Wasm custom protocol endpoint 回填问题已经修正完整,目前看这块没有遗漏,相关路径现在是一致的。

复查结果

  • agent/src/flow_generator/protocol_logs/plugin/wasm.rs:70-75,response 缺失 endpoint 时,已经改为写回 resp.endpoint,不再是之前的错误字段。
  • 同样的写法现在也出现在:
    • agent/src/flow_generator/protocol_logs/plugin/shared_obj.rs:176-183
    • agent/src/flow_generator/protocol_logs/plugin/custom_protocol_policy.rs:193-196
  • trait 默认回填逻辑 load_endpoint_from_cache() 本身就是给 response 用的,定义也写得比较明确:agent/src/common/l7_protocol_info.rs:199-223

为什么这次修复是完整的

我额外检查了 CustomInfo 的下游消费路径,确认这次把值写到 resp.endpoint 不会导致最终日志丢失:

  • agent/src/plugin/mod.rs:678-680 在生成 L7ProtocolSendLog 时,如果 req.endpoint 为空,会自动回退使用 resp.endpoint
  • 也就是说,对 response 日志来说,这次修复后的写法既符合当前三条 custom protocol 路径的一致性,也能被最终发送链路正确带出。

额外观察

  • agent/src/plugin/mod.rs:649-650CustomInfo::get_endpoint() 仍然只返回 req.endpoint,因此 cache 中保存的 endpoint 仍然来自 request 侧;这和 load_endpoint_from_cache() “从 request cache 给 response 回填”的设计是对齐的,没有发现新的不一致。
  • 我这次主要做的是静态复查,没有跑 @agent 下的 cargo fmt / cargo test / cargo build 验证。如果你准备合入,建议再补一轮最小相关验证。

总结

这次用户提到的修复点我看下来已经修完整了,可以认为前次 review 提出的 Wasm 回归问题已关闭。目前我没有在这块继续发现新的 endpoint 字段不一致问题。

@yuanchaoa yuanchaoa force-pushed the fix-incorrect-endpoint branch from ab8652a to 3e1cd55 Compare April 30, 2026 10:21
@yuanchaoa
Copy link
Copy Markdown
Contributor Author

@claude 代码修复了,看看是否修复完整

@claude
Copy link
Copy Markdown
Contributor

claude Bot commented Apr 30, 2026

Claude finished @yuanchaoa's task in 1m 54s —— View job


进度

  • Update the GitHub comment with the review checklist
  • Inspect the repaired endpoint changes
  • Check whether the fix is complete and consistent
  • Post the review result in the GitHub comment

Review 结论

我复查了这次修复,前一次指出的 Wasm custom protocol endpoint 回填问题已经修正完整,目前看这块没有遗漏,相关路径现在是一致的。

复查结果

  • agent/src/flow_generator/protocol_logs/plugin/wasm.rs:70-75,response 缺失 endpoint 时,已经改为写回 resp.endpoint,并且补上了 is_empty() 判断,避免覆盖已有值。
  • 同样的写法现在也出现在:
    • agent/src/flow_generator/protocol_logs/plugin/shared_obj.rs:176-183
    • agent/src/flow_generator/protocol_logs/plugin/custom_protocol_policy.rs:193-196
  • trait 默认回填逻辑 load_endpoint_from_cache() 的注释也明确写的是给 response 用:agent/src/common/l7_protocol_info.rs:199-223

为什么这次修复是完整的

我额外检查了 CustomInfo 的下游消费路径,确认这次把值写到 resp.endpoint 不会导致最终日志或 cache 丢失:

  • agent/src/plugin/mod.rs:649-656CustomInfo::get_endpoint() 现在会优先取 req.endpoint,为空时回退到 resp.endpoint
  • agent/src/plugin/mod.rs:684-688 在生成 L7ProtocolSendLog 时,也仍然会在 req.endpoint 为空时回退使用 resp.endpoint
  • agent/src/plugin/mod.rs:730-737impl From<&CustomInfo> for LogCache 通过 info.get_endpoint() 存缓存,因此 response 侧回填的 endpoint 后续也能继续参与 cache / metric 流程。
  • agent/src/flow_generator/flow_map.rs:1693-1697 的 L7 指标上报同样走 l7_info.get_endpoint(),因此这一改动对 custom protocol 的指标出口也是对齐的。

目前没有继续发现的新问题

基于这次修复点做静态复查,我没有再看到 custom protocol endpoint 字段写入不一致的问题。现在 Wasm、SO plugin、custom policy 三条路径在 response 回填语义上已经统一。

额外说明

我这次主要做的是静态复查,没有跑 @agent 下的 cargo fmt / cargo test / cargo build。如果准备合入,建议再补一轮最小相关验证,重点关注 agent/src/common/l7_protocol_info.rs 这次对 perf/cache 分支的改动是否符合预期。

总结

这次用户提到的修复点我看下来已经修完整了,可以认为前次 review 提出的 Wasm 回归问题已关闭。目前我没有在这块继续发现新的 endpoint 字段不一致问题。

@yuanchaoa yuanchaoa enabled auto-merge (rebase) April 30, 2026 10:33
@yuanchaoa yuanchaoa merged commit f0b70b4 into main Apr 30, 2026
8 checks passed
@yuanchaoa yuanchaoa deleted the fix-incorrect-endpoint branch April 30, 2026 10:34
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.

2 participants