Conversation
pikachuren
left a comment
There was a problem hiding this comment.
🙏 感谢 @suantea 提交!
🤖 AI 自动审核声明:本评审报告由 AI 自动生成,当前使用 DeepSeek-V4.1-Flash 模型进行分析。
📖 PR #105 — feat(linereg): line latency trend chart — history API + ECharts popup
🎯 结论
结论:🔄 Request Changes
评分:功能 ⭐⭐⭐ | 最小改动 ⭐⭐ | 前向兼容 ⭐⭐⭐⭐ | 方案设计 ⭐⭐⭐
变更:7 文件(+163 / −1)
行号说明:以下行号均为本 PR 自身改动的新文件行号(
+侧)。
结论中引用的既有实现(tunservice的 history、model.ProbeHistory、Caddy/echarts 依赖行为)均已读取分支与origin/main上的完整文件确认。
整体方案(2-3 句)
后端新增只读接口 GET /v1/linereg/line/:line_id/history(Manager.History 取最近 limit 条后反转成正序),前端把 TunService 线路表「趋势」列上的 Sparkline 包成按钮,点开 Modal 用 ECharts 画「延迟折线 + 失败散点」。分层与命名与项目既有风格一致,查询有上限、line_id 有索引、权限挂在 auth 组,安全面基本没事。但后端这一层与既有的 GET /v1/tunservice/:id/history 高度重复(同一张表、同样的 limit 语义、同一段「倒序取再反转」的代码,而抽屉打开时前端手里已经有这份数据),而图表本身存在一个「有失败点就画错」的时间轴对齐问题;另外 ECharts 的引入方式与 #111/#121 的按需方案不一致,合并顺序会互相牵制。
核心改动清单
webpage/src/pages/TunService.tsx:638-647— 「趋势」列的纯 Sparkline 改为可点击按钮 → 打开趋势弹窗(onClick={() => loadLineTrend(row.id)})。TunService.tsx:248-262—loadLineTrend:打开弹窗、请求 100 条、loading 与错误提示。TunService.tsx:265-326—buildTrendOption:延迟折线 + 失败散点 + 空数据标题。TunService.tsx:662-684— 趋势弹窗(width 720、destroyOnClose、notMerge)。backend/service/linereg/linereg.go:666-681—History(lineID, limit):limit<=0 || >500 → 100,ORDER BY id DESC LIMIT后反转成正序。backend/api/handlers/linereg.go:205-220+backend/api/router.go:220-221— 新路由,与linereg/config等同挂auth组。
问题清单
-
[P1]
TunService.tsx:308(配合:288-290、:295)— 折线数据与时间轴错位:xAxis是type:'category'且data: times(全部 N 个点),而 line series 的data只用successData(已剔除失败点)的裸数值数组;ECharts 对 category 轴按下标对齐,所以只要序列中间出现一个失败点,其后所有点都会左移错位(例如第 3 点失败,第 4 点的值会画在第 3 个刻度上),而失败散点用的是[time, value](按名字对齐)→ 同一张图上折线与红点互相矛盾。这恰好发生在本功能最想表达的场景(中间有抖动/失败),属于会误导排障的展示错误。修复建议(保持长度一致 + 散点按索引对齐):// 折线:失败/无数据用 null 占位,保证与 xAxis 下标一一对应 data: data.map((_, i) => (data[i].available !== false ? latencies[i] : null)), // 散点:改用索引对齐,避免同名时间戳(同秒多条)时按名字匹配不到 data: data.map((p, i) => (p.available === false ? [i, maxLat * 1.2] : null)).filter(Boolean),
-
[P1]
backend/service/linereg/linereg.go:666-681与backend/service/tunservice/manager.go:172-194— 两个History实现(同表、同limit规则、同一段反转循环)+ 两个接口(既有的GET /v1/tunservice/:id/history,backend/api/handlers/tunservice.go:117-125)能力重合。更关键的是:TunService.tsx:233-246的showDetail已经调用tunserviceApi.history(id, 100)把该服务每条线路的数据放进history[row.id],而弹窗入口所在表格的dataSource就是detail.lines(TunService.tsx:625-637)——也就是说,图表直接渲染history[row.id]就能达到同样的产品效果,后端可以 0 改动(约省 40 行)。想确认一下新接口是否有本 PR 之外的计划(例如未来支持「服务外线路」的趋势视图)?如果有,建议在描述里写清与/tunservice/:id/history的分工;如果没有,建议改为复用已有数据,避免两套几乎相同的查询长期并存、后续改表时漏改一处。 -
[P1](跨 PR,合并前需定序)
TunService.tsx:31— 这里用的是裸入口import ReactECharts from 'echarts-for-react',而 #111/#121 已经把MonitorDashboard.tsx、MonitorProbes.tsx改成echarts-for-react/lib/core+echarts/core按需注册(两者都在各自 PR 的 own-delta 里,且与本 PR 不共享文件,因此不会文本冲突、但会互相否掉效果):① 只要仓库里还剩一处裸 import,全量 echarts 仍会进 bundle,#111/#121 想拿到的体积收益会被回收(也直接抵消 #108 ROADMAP 里「echarts 按需引入省 ~800KB」这条);② 据 #111 报告的本地实测,裸入口会走esm路径依赖顶层tslib,而 #111 把它从 lockfile 里删了、#121 又补回来——若 #111 先合入而本 PR 保持原样,vite build会直接报Rollup failed to resolve import "tslib"(CI 拦不住这类只在构建期出现的问题)。建议本 PR 直接对齐按需写法(记得传echarts={echarts},echarts-for-react/lib/core缺这个 prop 会在打开弹窗时运行时报错、图表空白):import ReactECharts from 'echarts-for-react/lib/core' import * as echarts from 'echarts/core' import { LineChart, ScatterChart } from 'echarts/charts' import { GridComponent, TooltipComponent, LegendComponent } from 'echarts/components' import { CanvasRenderer } from 'echarts/renderers' echarts.use([LineChart, ScatterChart, GridComponent, TooltipComponent, LegendComponent, CanvasRenderer]) // <ReactECharts echarts={echarts} option={...} notMerge />
-
[P1]
TunService.tsx:293、:305、:318— legend/series 名称硬编码中文'延迟 (ms)'、'失败点',而同一函数的空数据文案已经走t('tunservice.noHistory');项目是 zh/en 双语,本 PR 也补了lineTrendTitle/lineTrendLoadFailed,英文界面下会出现中文图例,属于中英文不齐。建议把这两个系列名也加进 i18n(tunservice.seriesLatency/tunservice.seriesFail)。 -
[P1]
TunService.tsx:275-290— 只覆盖了latency <= 0 → null与available === false两种情况,于是**「available为 true 但延迟为 0」的记录既不在折线上、也不在失败散点上**(model/models.go:613-627里Available: r.Err == nil,而HTTPLatency在未做 HTTP 探测时就是 0)→ 这类点会静默消失,看起来像「历史数据丢了」。建议区分「无数据(null,折线断开)」与「失败(红点)」两种语义,并明确 0 值的展示策略(例如「仅 TCP 探测」时用tcp_latency兜底,或标注为无数据点)。 -
[P2]
TunService.tsx:249-262— 连续点击不同线路时没有请求竞态保护:慢的响应会覆盖新的选择,出现「标题是 A 线路、图是 B 线路」的瞬时错配(setLineTrendLineId与setLineTrendData是两次独立 setState)。建议用一个递增的 requestId(或AbortController)丢弃过期响应。 -
[P2]
TunService.tsx:279-282— x 轴只显示toLocaleTimeString()(无日期):探测间隔可配(backend/service/linereg/linereg.go:33,43,49:默认 60s、下限 5s,上限更大),100 个点可能跨天,跨天时刻度无法区分;另外p.created_at || p.id * 1000的兜底把自增 ID 当作秒级时间戳,算出的是 1970 年附近的错误时间。建议按跨度切换「时:分」与「日 时:分」(或加dataZoom),并去掉这个容易误导的兜底(缺created_at时直接判空)。 -
[P2]
TunService.tsx:638-647— 现在是把一张 240px 宽的 SVG 塞进size="small"的 Button 里:按钮自带 padding/居中,迷你图容易被挤压变形;ReloadOutlined是「刷新」语义,用在「查看趋势」上也容易被误解为「刷新数据」。建议换成普通可点击容器(或style={{padding: 0, height: 'auto'}})并加一句Tooltip(「点击查看趋势」),图标可换成LineChartOutlined。 -
[P2]
TunService.tsx:673、:678-682— 实例生命周期这块做对了:destroyOnClose保证弹窗关闭即卸载,而echarts-for-react的 React 封装在卸载时会自行 dispose,本 PR 也没有手动echarts.init,实例泄漏风险低。想确认的是每次打开都重建实例 + 弹窗内 720px 宽度的尺寸自适应在真机(含缩放/侧边栏折叠)上是否正常,可顺手核对一次 resize 后的重绘。 -
[P2]
backend/api/handlers/linereg.go:208-213— 直接把err.Error()(DB 错误文本)返回给前端,且与既有的tunservice.History(handlers/tunservice.go:121-123用 404)语义不一致。建议统一成对外固定文案 + 服务端日志。 -
[P2]
backend/service/linereg/linereg.go:666-668— 注释写「limit <= 0 或 > 500 时默认取 100」,但实现是「> 500 才回落 100」,即limit=300会真的取 300(而每线路只保留 200 条,maxHistoryPerLine=200,见linereg.go:618-619)。顺带把注释与实现对齐一下。
产品视角评估(3-5 句)
从迷你 Sparkline 升级到可看完整曲线的趋势图,方向是对的:运维排障时确实需要「刚才那波抖动是几点、有多严重」的完整视图,入口就贴在线路行上、不新增菜单、不新增配置,属于增量而非冗余的交互升级(数据本来就在抽屉里)。真正值得商榷的是决策价值:现在是一条裸曲线加几个红点,看到「19:31 抖动到 200ms」之后,用户仍要自己判断是否超阈值、是否该切线路。若能顺手把线路策略里的失败阈值画成参考线、或在异常点标注「此处触发过切换/重绑」,这张图才能直接支撑「要不要切」的动作,否则更像仪表盘装饰。另外它与 #87(线路健康检查,未合并)在「线路质量观测」上有交集,建议明确分工:趋势图只负责展示,判定与告警交给 #87,避免两边各做一套判定标准。
兼容性/迁移风险
无 DB schema 变更,也没有破坏性接口调整:ProbeHistory(backend/model/models.go:612-627)字段与 json tag(line_id/tool/layer/address/tcp_latency/http_latency/available/error_msg)都是既有的,前端读取的字段名已核对存在,line_id 上的 index 也已有,新增的只是只读接口 + 一条路由(挂 auth 组,与 linereg/config 同级,未放宽权限)。风险集中在与 #111/#121 在 echarts 入口 / tslib 上的耦合(见 P1),以及新增接口与 /v1/tunservice/:id/history 的长期维护成本。
值得肯定的点(1-3 条)
- 查询边界收得很干净:
limit<=0 || >500 → 100、ORDER BY id DESC LIMIT、line_id上本来就有索引,不会出现大表全扫或无上限返回(表本身也按maxHistoryPerLine=200每线路裁剪,见linereg.go:653-664),后端这一步是安全的。 - 只读接口挂在
auth组,与既有linereg路由一致,没有额外放宽;返回内容与既有/tunservice/:id/history同源,没有引入新的敏感字段(token/密钥都不在ProbeHistory里)。 - 空数据与单点数据都兜住了(
buildTrendOption的空标题分支、Sparkline的<2分支),配合notMerge+destroyOnClose,不会白屏或残留上一条线路的数据。
建议操作理由(1 句)
数据与权限处理没问题,但折线按 index 对齐会在「有失败点」时画错、后端这层与既有接口重复度高,建议先修对齐、复用已有数据,并把 echarts 入口对齐到 #111/#121 的按需方案再合入。
6af336f to
d3aed2a
Compare
pikachuren
left a comment
There was a problem hiding this comment.
🙏 感谢 @suantea 提交!
🤖 AI 自动审核声明。
🎯 结论
🔄 Request Changes — 前端图表存在 2 处真实渲染 bug
📖 概要
线路延迟趋势图 · history API + ECharts 弹窗
🚨 关键问题
P1:
- 💡
TunService.tsx用p.id * 1000作为缺失created_at时的时间戳回退,把自增主键当秒级时间戳使用,会生成完全错误的时间。 - 💡 折线系列数据用过滤后的紧凑数组,但 x 轴用未过滤的全量数组,含失败点时会导致图表时间轴整体错位。
P2:strconv.Atoi 的 limit 参数错误被忽略;图例文案硬编码中文未走 i18n。
后端 History() 的条数钳制(100/500上限)做得对,无安全问题。
🎯 结论:🔄 Request Changes — 前端两处渲染 bug 影响功能核心价值,建议修复后合并
- linereg.Manager.History(lineID, limit): probe history in ascending time order for the trend chart (default 100, cap 500 records) - API: GET /v1/linereg/line/:line_id/history - Frontend: trend column in TunService detail drawer becomes a button opening an ECharts modal with TCP/HTTP latency lines and failure markers; i18n keys lineTrendTitle / lineTrendLoadFailed
d3aed2a to
36abe5b
Compare
Summary
Adds a latency trend chart for probe lines:
linereg.Manager.History(lineID, limit): returns probe history for a line in ascending time order for the trend chart (default 100 records, capped at 500)GET /v1/linereg/line/:line_id/historylineTrendTitle/lineTrendLoadFailed(en/zh)Verification
go build ./...OKgo vet ./service/linereg/ ./api/...OKgo test ./service/linereg/ -raceOKnpm run build(webpage) OK