Repository navigation
feat(ai): add trace discovery and diagnosis tools for the AI assistant - #1618
Open
ambiguous-pointer wants to merge 2 commits into
Open
ambiguous-pointer wants to merge 2 commits into
ambiguous-pointer wants to merge 2 commits into
Conversation
The assistant could only look up a distributed trace by ID and could not read one without already knowing the ID. That makes every investigation start with a question to the user instead of an observation by the agent. - search_traces queries the provider by service, operation, trace ID, tags, time window and minimum duration, and returns bounded per-trace summaries. The provider interface gains SearchTraces so another backend can be added without touching the tool layer. - analyze_trace turns a trace into a diagnosis: the latency critical path with self time versus wait time per hop, the per-service latency breakdown, the slowest spans, the failed spans with their error classification and root-cause flag, and the caller to callee dependency edges. - The analyser is a pure function over an already normalized trace, so the algorithms are testable without a backend and stay independent of the transport. - Summaries are computed over every raw span. The detail path keeps its existing span budget because it backs get_trace_by_id, whose behaviour is unchanged. Every list and attribute value the tools return is capped, truncation is reported in the payload, and no credential reaches a tool result.
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
The hooks tests fail to compile, and unresolved defects allow reasoning-policy bypass, expose endpoint credentials, and cause Jaeger searches with minimum-duration filters to fail.
12 open issues
What changed in this PR
Adds MCP trace discovery and structured diagnosis so the AI assistant can investigate distributed traces without first receiving a trace ID.
Changes:
- Adds
search_traces, Jaeger search support, and a provider-level search contract. - Adds
analyze_tracewith latency-path, service, error-root-cause, dependency, and GenAI usage analysis. - Adds tool registration, tests, documentation, sanitized failure logging, and standalone reasoning-policy helpers.
This pull request requires changes. Unresolved findings include non-compiling hook tests, Jaeger rejecting duration-filtered searches, credential exposure through analysis errors, and a reasoning-filter bypass. Further findings concern analysis accuracy, search semantics, truncation and response bounds, GenAI accounting, and documentation consistency.
| File | Description |
|---|---|
pkg/mcp/tools/trace/tools_test.go |
Tests trace search and diagnosis handlers. |
pkg/mcp/tools/trace/search.go |
Defines search arguments, validation, and the tool handler. |
pkg/mcp/tools/trace/search_test.go |
Tests query mapping, summaries, limits, and errors; currently asserts the invalid duration format. |
pkg/mcp/tools/trace/provider.go |
Extends the provider interface with trace search. |
pkg/mcp/tools/trace/model.go |
Adds search, analysis, and GenAI response models. |
pkg/mcp/tools/trace/jaeger.go |
Implements Jaeger search, raw-span summaries, and sanitized failure logging; has outstanding query, bounds, truncation, and comment findings. |
pkg/mcp/tools/trace/jaeger_logging_test.go |
Tests failure-log sanitization; includes an imported-package shadowing nit. |
pkg/mcp/tools/trace/genai.go |
Classifies GenAI spans and aggregates usage; findings cover scientific-notation parsing and potential token double-counting. |
pkg/mcp/tools/trace/genai_test.go |
Tests classification, token totals, bounded details, and content exclusion. |
pkg/mcp/tools/trace/analyzer.go |
Implements diagnosis; findings cover error sanitization, trace envelopes, critical paths, cycles, service timing, truncation, error classification, and helper documentation. |
pkg/mcp/tools/trace/analyzer_test.go |
Tests analysis algorithms, malformed traces, and boundary cases. |
pkg/mcp/register.go |
Registers search_traces and analyze_trace. |
pkg/mcp/register_test.go |
Checks tool registration and required arguments. |
pkg/mcp/DEVELOPMENT.md |
Documents tools and response limits; several stated limits differ from implementation. |
docs/ai-trace-observability-design.md |
Describes architecture and contracts; findings address broken formatting, stale implementation status, and overstated critical-path guarantees. |
ai/component/hooks/reasoning.go |
Adds unwired reasoning-policy helpers; escaped JSON can bypass filtering. |
ai/component/hooks/reasoning_test.go |
Adds policy and intended integration tests that reference unavailable APIs and fail to compile. |
Review effort: Max
Contributor
Author
评审意见的处理结果感谢 @copilot-pull-request-reviewer 的评审。12 条意见我逐条复核过，全部成立、无误报，已按 P0 → P1 → P2 处理完毕（修复见 复核时把每条都落到了代码与上游行为上，下面按优先级列出「结论 → 依据 → 处理」。 P0-1 编译失败（#2）+ 死代码（#10）+ 策略绕过（#1）—— 一并处理这三条指向同一个动作：把
P0-2
|
| 意见 | 处理 |
|---|---|
| #6 裁剪后总时长用错 | analysisEnvelope 优先用 provider 的 trace.DurationMicros（它在全部原始 span 上算），只在没有 provider 值时回退到保留 span 的包络。否则被裁剪的链路会用更短的总时长做分母，把各服务延迟占比整体虚高。补了 TestBuildTraceAnalysisUsesProviderEnvelopeForClippedSpans |
| #8 token 被科学计数法吞掉 | genAIIntAttribute 改走 parseIntegralAttribute，先 Atoi，失败再按 float 解析并要求是整数值。已实测 1000000 → "1e+06" → Atoi 失败 → 0，阈值恰为 ≥1e6（999999 正常） |
#9 触达后端 limit 时 truncated 误报 false |
Provider 侧按调用方条数再多取一条做探测，命中即 truncated=true 并裁回调用方条数。这样「刚好装满」不再被误读为「已看全」，同时避免了「达到上限就报截断」带来的假阳性。补了 TestJaegerProviderSearchTracesReportsTruncationAtTheBackendLimit |
#7 slowSpans 排名截断误置 truncated |
排名结果不丢分析数据，因此不再置位。与同文件 critical path 的既有规则一致（TestBuildCriticalPathStopsAtBudgetWithoutTruncation：链条正好在预算处结束不算截断）。补了 TestBuildTraceAnalysisDoesNotReportTruncationForTheSlowSpanRanking |
P2 一致性
- Improvement of dubbo-admin and monitor #11 文档限额：
DEVELOPMENT.md与设计文档 §5.2 的限额表改成与常量一致（摘要 100 / 错误 span 20 / GenAI 20），并补上搜索侧truncated的新语义。 - monitor 2.0 为啥没有 Charts 和Statistics 了 #12 文档注释名：
requestContext→traceRequestContext；顺带把tools.go里重复的 context 回退换成调用同一个 helper——原来那处还少了ctx == nil判断。
额外发现（评审未提）
-
CI 从不在
ai分支上跑。.github/workflows/ci.yml与riscv-build.yml的触发分支只有develop，PR 目标分支是ai，所以这两个 workflow 都不会触发——这也是编译失败能进到 PR 的直接原因。打开之后暴露出一批从未在这条分支上校验过的历史缺陷（lockfile 镜像失效、
ai模块缺 license header、三个文件不是 gofmt 干净、前端源码不是 prettier 干净、license 配置把**/tools/**整条忽略）。这些与本 PR 的改动无关，已经拆到独立分支fix/ai-branch-ci（目标分支ai），不在本 PR 里，免得把一个功能 PR 撑成 60 多个文件。 -
PR 描述里的验证口径漏了
ai模块。ai/是独立 go.mod，go build ./... && go vet ./pkg/... && go test ./pkg/...只覆盖根模块，所以「全绿」是假象。本 PR 补充验证为两个模块分别go build/go vet/go test -short。
评审正文里另外提到、但没有单列行内评论的三处
评审正文的「What changed in this PR」逐文件说明里还提到了三处，本 PR 一并处理：
jaeger_logging_test.go的 import 遮蔽 nit：errors := traceLogErrorFields(t, lines)遮蔽了errors包。已改名为recordedErrors。- 设计文档的实现状态过期：§14.2 的 T7 仍写着「工具返回按设计保留上游错误原文」，与本次对传输错误的净化不符。已更新为「传输错误已净化（请求 URL 不进入工具返回），仍未净化的是上游响应体与 span 内容」。
- 设计文档对关键路径的保证说过头：§5.3.3 原写「恰好满足……链尾的
end等于 trace 的墙钟终点。链路上的selfMicros之和是这条 trace 的不可压缩耗时」。两处都收紧了：span 列表被maxTraceSpans裁剪时关键路径只在保留的 span 上计算，链尾可能早于 provider 的墙钟终点；selfMicros之和是这条关键路径上的不可压缩耗时，并行分支的 self time 不计入，是工作量的下界而非上界。§14.3 的maxTraceSpans风险行同步更新（裁剪不再让占比虚高，但关键路径仍只在保留的 span 上计算）。
评审正文还提到 genai.go 的「token 重复计数」与 analyzer.go 的「环 / 服务计时 / 错误分类」，这两处复核后确认实现本身是对的，无需改动：
- token 重复计数：
gen_ai.usage.total_tokens是非标准属性，设计上只作为单次调用的参考值出现在明细里，聚合的 total 恒等于 input + output，避免出现两种 total 定义；已有用例TestBuildGenAISummaryKeepsProviderTokenTotalAsCallDetailOnly覆盖。 - 环：
computeSelfTimes是迭代实现，hasFailedDescendant用显式栈 + visited，buildCriticalPath用 visited 集合，三处都不会在父子引用成环时挂死；已有用例TestBuildTraceAnalysisSurvivesCyclicReferences覆盖。 - 服务计时 / 错误分类：与 §5.3.2、§5.4 的设计一致，
error.type优先级与根因规则（无失败后代）均按文档实现。
验证
两个模块均通过：
go build ./... && go vet ./... && go test ./... -short -gcflags=-l # 根模块
cd ai && go build ./... && go vet ./... && go test ./... -short -gcflags=-l
本 PR 新增 6 个回归用例（endpoint 校验 2、minDuration wire format、后端 limit 截断、provider 包络、slow span 排名、科学计数法 token），#1616 另加 1 个（JSON 转义绕过），每个都在对应修复前会失败。
未处理
ai模块还有 23 个文件未通过 gofmt（属于 feat(ai): make the agent observable through OTel GenAI conventions #1616 那条线，本 PR 只顺手修了本 PR 触碰到的那个）。把cd ai && go fmt ./...加进make fmt会让 CI 因这些既有文件变红，所以本 PR 没动，留给 feat(ai): make the agent observable through OTel GenAI conventions #1616 收敛。- 设计文档 §14.2 记录的「工具结果中 span 内容未过滤」仍然开放，本次只处理了请求 URL 这一段。
ambiguous-pointer
force-pushed
the
feat/ai_trace_diagnosis
branch
2 times, most recently
from
November 2, 2026 04:17
01fc77d to
24882bb
Compare
ambiguous-pointer
force-pushed
the
feat/ai_trace_diagnosis
branch
from
November 2, 2026 07:12
24882bb to
21272f0
Compare
The review found one defect that blocks the build, one that breaks every duration filtered search against a real Jaeger, one that lets a configured credential reach the model context, and a set of correctness issues in the diagnosis. - The hook tests referenced APIs that do not exist on this branch, so the ai module did not compile. The reasoning policy helpers belong to the companion observability change, where their wiring lives, so they move there. The companion branch also fixes the policy bypass through a JSON escape of the type discriminator. - minDuration is sent as a duration string. Jaeger parses it with time.ParseDuration, so the unit-less microsecond count was rejected with a 400 and every filtered search failed against a real backend. - A credential configured on the endpoint query would reach the tool result through the transport error. The endpoint is validated, the transport error is sanitized for the tool result as well as for the failure log, and the request URL no longer enters the model context. - The diagnosis takes the duration of the complete trace from the provider instead of recomputing it from the retained spans, so a clipped span list no longer shortens the trace and inflates the latency shares. - A search that reached the limit of the provider is now marked as truncated, so the agent does not conclude that it saw every match. - The slow span ranking no longer marks the analysis as truncated, because a ranking loses no analysis data. - Token counts are read from the integral scientific notation too, so a million token call no longer reports zero tokens. - The documented response limits, the helper name and the design document match the implementation again.
ambiguous-pointer
force-pushed
the
feat/ai_trace_diagnosis
branch
from
November 2, 2026 07:28
21272f0 to
bcaf745
Compare
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
概要
让 AI 助手能自主发现并诊断 Dubbo 业务的分布式链路：不需要人先提供 traceId，助手自己按服务与时间窗找出候选链路，再算出关键路径、服务维度耗时、错误根因与调用依赖。
为什么需要
助手能查日志、能查指标，唯独不能查链路——只要问题停在「最近 X 服务有没有失败请求」这一层，它就必须先把问题退回给用户索要 traceId，观测能力形同虚设。
即使拿到了 traceId，原始 span 也是事实而不是结论：
parentSpanId里，没有被提炼出来。Agent 需要的是结论，否则它拿到一堆 span 还是要自己做一遍精确的时间轴推理，而 LLM 做这件事非常不可靠。
改了什么
search_tracesanalyze_traceProvider.SearchTraces关键设计
先发现，后分析。 助手能查日志、能查指标，唯独不能查链路——只要问题停在「最近 X 服务有没有失败请求」这一层，它就必须先把问题退回给用户索要 traceId，观测能力形同虚设。所以第一件事是让 provider 支持检索，而不是先去做更复杂的分析。
诊断逻辑做成纯函数。
BuildTraceAnalysis(trace *Trace) *TraceAnalysis输入是已归一化的Trace，输出是结论。好处有三个：算法可以完全脱离后端做单测；将来接入 Tempo 时分析逻辑一行不用改；算法与传输解耦，不会被某个 provider 的特殊行为污染。self time 用区间并集算，不重���。 一个 span 的总时长里大部分可能是它在等子 span。self time = 总时长减去子 span 覆盖区间的并集（重叠的子 span 不能重复计数）。
关键路径走「结束时间最晚」的那一支。 从「结束最晚的根」出发，每一跳走向「结束最晚的子节点」——那是父 span 内最后完成的活动，决定了父 span 的完成时刻。
错误根因 = 没有失败后代的失败 span。 上游 span 往往因为下游失败而一起标红，但根因只有一个。判据是「它的子树里还有没有别的失败 span」。
摘要基于全部原始 span，细节路径维持原样。
get_trace_by_id的归一化会把 span 裁剪到 500 条并丢弃属性——那是它作为「取原始链路」工具的合理取舍。但检索摘要若复用这条路径，根操作名、错误数、服务集合就都不可信了。所以摘要单独实现，遍历全部原始 span；而get_trace_by_id的行为完全未变，避免对已发布工具造成回归。有界响应。 Agent 的上下文是有限的。所有列表都有上限，所有截断都会在 payload 里显式标记，让 Agent 能区分「数据完整」与「数据不完整」。
凭据不进工具结果。 传输错误在写日志前解包（Go 的
*url.Error会把完整请求 URL 写进 message，其中含检索条件与 traceId），只保留dial tcp ...这类诊断信息。验证
go build ./... && go vet ./pkg/... && go test ./pkg/...全绿，trace 包 71 个用例。覆盖：检索的参数映射与摘要归一化、genai kind 识别、上游错误、结果截断；诊断的线性链、分支场景关键路径落在慢分支、根因归属、服务分解、依赖边、孤儿 span、父子引用成环不挂死、畸形时间戳、错误计数不被列表上限截断；GenAI 归类与 token 口径。
不在本 PR 范围
关联