Skip to content

fix: record RPC metrics synchronously to prevent event loss - #3805

Merged
AlexStocks merged 5 commits into
apache:developfrom
dodjdnh:fix/rpc-metrics-event-loss
Nov 13, 2026
Merged

AlexStocks merged 5 commits into
apache:developfrom
dodjdnh:fix/rpc-metrics-event-loss

Conversation

@dodjdnh

@dodjdnh dodjdnh commented Nov 12, 2026

Copy link
Copy Markdown
Contributor

Problem

目前 RPC 指标依赖容量为 1,024 的异步 channel 消费更新。高并发打满队列时,请求开始、完成或过期事件会直接丢弃,导致计数和 RT 样本缺失、processing 指标无法归零;此外,刚初始化完成时的首批请求也可能早于异步订阅就绪,造成漏记。

Changes

  • 核心指标同步记账:在 Metrics.Init 返回前注册同步 RPC recorder。Publish 时直接在当前 goroutine 完成核心指标更新,之后再尝试向异步订阅者发送事件通知。
  • 指标句柄缓存与按需绑定:按角色和现有的 8 个基础标签缓存已注册的指标句柄,带结果和错误类型的指标在首次触发时按需创建。所有指标的名称、类型、HELP、单位和标签定义均保持不变。
  • 边界与并发语义对齐:在请求开始事件中记录初始化状态快照,避免初始化前发起、初始化后才完成的请求只记半边账;延续单次终态约束,请求过期时仅扣减 processing,后续晚到的响应不再重复统计。
  • 自定义 Registry 兼容:对于未实现 GaugeFuncRegistry 的自定义 Registry,保留原有的串行兼容路径;捕获 Registry panic,避免指标异常影响业务结果或阻断用户回调(注:慢 Registry 仍会直接增加调用或回调耗时)。

Behavior

  • Before:核心指标写入依赖异步队列消费,队列打满即丢事件。
  • After:核心指标在 Publish 返回前同步更新,异步通道满仅影响外部通知订阅者(注:Registry 自身故障或进程崩溃仍无法保证记账完整)。

Verification

  • 本地单测与静态检查:当前 HEAD 在 Windows (Go 1.26.5) 下受影响包回归通过;Linux (Go 1.25.14) 下的 race、vet 及定向 lint 检查均已通过。
  • 跨节点 E2E 核账:三轮 1,024 并发突发压测中,双端逐方法计数差额为 0,processing 最终归零;RT 探针、Prometheus 实际采集、跨时间窗口及 30 分钟持续负载核账全部通过。
  • 性能对照验证:7 个性能场景各完成 5 组有效对照,均未触发预设的吞吐、P99 或 CPU 退化阈值。
  • 当前 HEAD 尚未重新部署集群验证(已核对其生产代码、go.mod 和 go.sum 与集群实测版本完全一致;未覆盖场景及已知协议层 race 见折叠详情)。

Review focus

  1. 同步开销与缓存生命周期:同步记账引入的延迟代价是否可接受,完整标签缓存与按需绑定的生命周期设计是否合理。
  2. 并发与状态边界:初始化快照、并发首次绑定,以及“完成 vs 过期”竞态下,是否完整保留了原有的统计和回调语义。
  3. 接口与扩展兼容性:新增的 SubscribeRecorder 接口,以及自定义 Registry 的并发与异常兜底约定是否合适。
Details & Attachments:关联 Issue、验证报告与实现图解

关联 Issue

Fixes #3802

验证报告与实现图解

版本与测试范围说明

  • 基线 commit:6b04ac04bfc868060995dc9fa4e1dcc5138ad02b
  • 当前 PR HEAD:803bd2cda9530362192e911dc4a4492d11f5e883
  • 集群实测 commit:f345d2ace82606b79e5bc209717b16648e3b791c(整理提交时仅移除了实验脚本并补充了测试与文档,框架生产代码与实测版本一致,故当前 HEAD 未重复上集群)。
  • 本地验证记录:此前在 Linux Go 1.25.14 下通过了受影响包的 race 和 vet;2026-11-12 在 Windows Go 1.26.5 下重跑了同范围常规测试并全部通过。改动文件已通过 gofmt、modernize v0.21.1、仓库 imports-formatter 以及 golangci-lint v2.7.2(0 issues),未跑全仓 CI。
# Linux(已有通过记录)
go test -race ./metrics/... ./filter/metrics ./common/rpcerror -count=1
go vet ./metrics/... ./filter/metrics ./common/rpcerror

# Windows 回归
go test -mod=readonly -count=1 -timeout=120s ./metrics/... ./filter/metrics ./common/rpcerror

E2E 与性能摘要

  • 测试环境:Consumer 与 Provider 部署在两台 Kubernetes worker 节点(各限 2 CPU / 2GiB),通过 ClusterIP 跑真实 RPC 流量,由独立 Prometheus 实例拉取原生 Exporter 指标。
  • 正确性核账:
    • 3 轮 1,024 并发突发下,修复版双端各方法计数零丢失、processing 归零;基线版本每轮均有漏记且 processing 出现偏移。
    • 专用 RT 探针跑完 79,050 次调用,双端累计与窗口 RT 的 16 项逐方法样本数断言全部通过;30 分钟持续负载完成 9,624,600 次调用,计数完整且 processing 归零。
  • 性能对照:
    • 7 个场景每轮压测 60 秒,交替运行新旧版本并重启进程,剔除受环境干扰轮次后各取 5 组有效对照。
    • 高并发场景下吞吐中位数提升约 13.8%、P99 降低约 11.9%;80% 固定速率场景下 P99 降低约 38.8%(注:高压下基线版本存在丢事件,双方实际承担的指标处理工作量并不完全对等;且 5 组中位数仅用于回归门禁排查,不代表严格统计显著性)。
    • 所有场景均未触发复查阈值(吞吐下降 >5%、P99 上升 >10% 或 CPU 增加 >10%)。

已知边界与未覆盖项

  • Race 测试说明:受影响包本地 race 及 19,197 次跨节点 race 均通过;额外跑的 Getty / Hessian 协议层 race 在基线和修复版上均失败(涉及连接清理逻辑),不计入通过范围。
  • 未覆盖场景:多实例标签 / Pod 身份轮换、带在途请求的优雅退出、跨节点精确重复异常响应竞争尚未做完整 E2E;部分初始化、Registry 故障及终态竞争的验证依赖单测与集成测试。
  • 运行时特征:Prometheus 并发抓取时可能读到多个关联指标更新之间的瞬时中间态;缓存的指标序列随 Registry 生命周期存活,若业务标签基数持续膨胀仍会增加内存占用。
  • 归档说明:原始逐轮数据、失败日志和实验验证脚本已本地归档,不随本 PR 提交。

Checklist

  • I confirm the target branch is develop
  • I have run make fmt to format my code(已完成改动文件的定向格式化检查,未执行全仓改写)
  • I have run make test to run local tests(已跑受影响包,未跑全仓测试)
  • I have added tests that prove my fix is effective or that my feature works

Add a recorder callback independent of best-effort channel subscriptions. Cover concurrent publication, full channels and recorder removal.

Fixes: apache#3802
Signed-off-by: Kevin <yaojiaangyao@gmail.com>
Install the recorder before Metrics.Init returns, cache complete label identities and bind result series lazily. Preserve readiness snapshots, monitoring expiry and legacy registry behavior. Isolate registry panics from business invocations.

Fixes: apache#3802
Signed-off-by: Kevin <yaojiaangyao@gmail.com>
Exercise the real one-time initialization lifecycle, disabled metrics, full observer queues and custom registry failures. Verify returned errors and real Exchange callbacks remain intact.

Fixes: apache#3802
Signed-off-by: Kevin <yaojiaangyao@gmail.com>
Document readiness, concurrent collection, registry compatibility and recording limits, with commands for the regression suite.

Fixes: apache#3802
Signed-off-by: Kevin <yaojiaangyao@gmail.com>
@codecov-commenter

codecov-commenter commented Nov 12, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 97.71429% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 58.91%. Comparing base (6b04ac0) to head (8c0407c).
⚠️ Report is 3 commits behind head on develop.

Files with missing lines Patch % Lines
metrics/rpc/collector.go 80.95% 4 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #3805      +/-   ##
===========================================
+ Coverage    58.70%   58.91%   +0.20%     
===========================================
  Files          492      493       +1     
  Lines        40377    40495     +118     
===========================================
+ Hits         23702    23856     +154     
+ Misses       14901    14866      -35     
+ Partials      1774     1773       -1     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Extract initialization, registry-failure callbacks, concurrent recording and
metric assertions into focused test helpers. Preserve the existing scenarios,
concurrency levels and assertions while reducing cognitive complexity.

Fixes: apache#3802
Signed-off-by: Kevin <yaojiaangyao@gmail.com>
Comment thread metrics/rpc/collector.go
}
return
}
c.getSeries(e).record(e)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P2] 新的同步记账入口没有承接 RT 直方图记录,与 #3791 合并后 Prometheus 部署的直方图会停止采样

触发条件:#3791 在 afterInvokeHandler 末尾新增 c.reportRTHistogram(role, labels, event.costTime.Seconds())(该 PR head 221c96e2 的 metrics/rpc/collector.go:127),并在 newRPCCollector 注入 histogramRegistry。本 PR 删除了 rpcMetricsChan 与 go rc.start(),afterInvokeHandler 只在「注册表未实现 GaugeFuncRegistry」的兼容分支内被调用(metrics/rpc/collector.go:81-91,metrics/api.go:343 是同一判据的另一处)。promMetricRegistry 实现了 GaugeFunc(metrics/prometheus/registry.go:96),因此默认 Prometheus 部署的每个完成事件都走本行的 c.getSeries(e).record(e),reportRTHistogram 不再可达。

实际影响:两个 PR 都合入后,dubbo_provider_rt_milliseconds_histogram_seconds / dubbo_consumer_rt_milliseconds_histogram_seconds 只会暴露空序列(_bucket 各档位、_sum、_count 恒为 0),开启 histogram.enabled=true 的部署拿不到任何延迟分布数据;既有 RT、计数与 processing 指标不受影响。若按相反顺序合并(本 PR 先入、#3791 后 rebase),结论相同,因为新入口是本 PR 引入的。

证据:本 PR head 803bd2cd 的 record() 里,BeforeInvoke/ObservationExpired/AfterInvoke 三个分支均只更新 qps、processing、counter、rt、quantiles,没有任何 HistogramWithOptions(...).Observe(...) 调用;histogram 相关字段也未出现在本 PR 的 rpcCollector 结构体中。

修复方向:在 rpcSeries.record 的 AfterInvoke 分支中同样观察直方图(等价于把 reportRTHistogram 提到 record() 的分支判断之上,用 e.costTime.Seconds()),并补一条断言:完成事件记录后 _bucket 的累计值随观测数增长,而非恒为 0。合并顺序上建议先合 #3791,本 PR rebase 时一并迁移该调用点。

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P2] 新的同步记账入口没有承接 RT 直方图记录,与 #3791 合并后 Prometheus 部署的直方图会停止采样

触发条件:#3791 在 afterInvokeHandler 末尾新增 c.reportRTHistogram(role, labels, event.costTime.Seconds())(该 PR head 221c96e2 的 metrics/rpc/collector.go:127),并在 newRPCCollector 注入 histogramRegistry。本 PR 删除了 rpcMetricsChan 与 go rc.start(),afterInvokeHandler 只在「注册表未实现 GaugeFuncRegistry」的兼容分支内被调用(metrics/rpc/collector.go:81-91,metrics/api.go:343 是同一判据的另一处)。promMetricRegistry 实现了 GaugeFunc(metrics/prometheus/registry.go:96),因此默认 Prometheus 部署的每个完成事件都走本行的 c.getSeries(e).record(e),reportRTHistogram 不再可达。

实际影响:两个 PR 都合入后,dubbo_provider_rt_milliseconds_histogram_seconds / dubbo_consumer_rt_milliseconds_histogram_seconds 只会暴露空序列(_bucket 各档位、_sum、_count 恒为 0),开启 histogram.enabled=true 的部署拿不到任何延迟分布数据;既有 RT、计数与 processing 指标不受影响。若按相反顺序合并(本 PR 先入、#3791 后 rebase),结论相同,因为新入口是本 PR 引入的。

证据:本 PR head 803bd2cd 的 record() 里,BeforeInvoke/ObservationExpired/AfterInvoke 三个分支均只更新 qps、processing、counter、rt、quantiles,没有任何 HistogramWithOptions(...).Observe(...) 调用;histogram 相关字段也未出现在本 PR 的 rpcCollector 结构体中。

修复方向:在 rpcSeries.record 的 AfterInvoke 分支中同样观察直方图(等价于把 reportRTHistogram 提到 record() 的分支判断之上,用 e.costTime.Seconds()),并补一条断言:完成事件记录后 _bucket 的累计值随观测数增长,而非恒为 0。合并顺序上建议先合 #3791,本 PR rebase 时一并迁移该调用点。

明白

@sonarqubecloud

Copy link
Copy Markdown

@dodjdnh dodjdnh changed the title fix: prevent RPC metric loss under observer queue saturation fix: record RPC metrics synchronously to prevent event loss Nov 12, 2026

@Alanxtl Alanxtl left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@Alanxtl Alanxtl added ☢️ Bug 3.3.3 version 3.3.3 labels Nov 13, 2026
@AlexStocks
AlexStocks merged commit cc82a2e into apache:develop Nov 13, 2026
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

3.3.3 version 3.3.3 ☢️ Bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants