Skip to content

fix: generate cn monitor service like dn/log#601

Merged
loveRhythm1990 merged 1 commit into
matrixorigin:mainfrom
loveRhythm1990:lr90/cn-metrics-service
Jul 14, 2026
Merged

fix: generate cn monitor service like dn/log#601
loveRhythm1990 merged 1 commit into
matrixorigin:mainfrom
loveRhythm1990:lr90/cn-metrics-service

Conversation

@loveRhythm1990

Copy link
Copy Markdown
Contributor

What type of PR is this?

  • API-change
  • BUG
  • Improvement
  • Documentation
  • Feature
  • Test and CI
  • Code Refactoring

Which issue(s) this PR fixes:

Fixes #600

What this PR does / why we need it:

CNSet 开启 exportToPrometheus: true + promDiscoveryScheme: Service 时,Prometheus(通过 ServiceMonitor / VMServiceScrape)无法发现 CN
的 metrics target,而 DN/Log 可以正常被发现。

根因:CNSet 的 syncServicepkg/controllers/cnset/resource.go)只在已有的 SQL Service 上打了
prometheus.io/scrape/prometheus.io/port annotation,但从未给任何 Service 加上 metric 端口。基于 Service 的 Prometheus 发现机制(如
ServiceMonitor)根本不读 prometheus.io/* annotation——它是按 label selector 匹配 Service,再按端口名(metric)在 spec.ports
里查找。由于没有任何 Service 暴露 metric 端口,CN 从未被发现,尽管 CN 进程本身已经在 :7001 正确暴露了 metrics。

DNSet 和 LogSet 没有这个问题——它们已经会创建独立的 *-dn-metric / *-log-metric ClusterIP Service,带 metric:7001 端口。

本 PR 让 CNSet 对齐 DNSet/LogSet 的实现:

  • 在 CNSet controller(pkg/controllers/cnset/controller.go)新增 syncMetricService,创建/reconcile 一个独立的 <cn>-cn-metric
    ClusterIP Service,带 metric:7001 端口(照抄 dnset/controller.gosyncMetricService 实现)
  • 该 Service 由 CNSet 拥有(owner reference),并在 Finalize 中一并清理
  • 仍然在 PromDiscoveredByService() 为真时给这个 Service 打 prometheus.io/scrape/port annotation,跟 DN/Log
    行为保持一致,也兼容仍在用 annotation-based scrape 的环境——但这两条 annotation 对 ServiceMonitor 发现机制不是必需的
  • 顺手修复了一个既有小问题:exportToPrometheus 关闭时,主 SQL Service 的 annotation 清理此前只删了 prometheus.io/scrape,遗漏了
    prometheus.io/port;现在两者都会被删除

Special notes for your reviewer:

  • 新的 metric Service 是无条件创建的(不管 exportToPrometheus/promDiscoveryScheme 取值),跟现有 DNSet/LogSet 行为一致——只有
    annotation 是有条件的。如果 reviewer 认为 CN 应该在没开 metrics 时完全不创建这个 Service,可以在这里讨论是否要让 CN 的行为跟 DN/Log
    出现分歧
  • operator 全仓库范围内都不会创建 ServiceMonitor/VMServiceScrape 这类
    CRD(已做过全仓库搜索确认),本次修复不涉及;用户需要自行确保现有 ServiceMonitor 的 selector 能覆盖 CNSet 的 matrixorigin.io/*
    标签,并且 endpoint 用 metric 这个端口名
  • ProxySet 存在同样的问题(在已有 Service 上打 annotation,没有创建独立 metric Service),本 PR 未处理,如有需要可以后续单独提 PR
  • 完整背景和根因分析见 docs/dev/issue-600-cn-metrics-service.md

Additional documentation (e.g. design docs, usage docs, etc.):

Not Available

@qodo-code-review

Copy link
Copy Markdown

Qodo reviews are paused for this user.

Troubleshooting steps vary by plan Learn more →

On a Teams plan?
Reviews resume once this user has a paid seat and their Git account is linked in Qodo.
Link Git account →

Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center?
These require an Enterprise plan - Contact us
Contact us →

@mergify

mergify Bot commented Jul 13, 2026

Copy link
Copy Markdown
Contributor

Tick the box to add this pull request to the merge queue (same as @mergifyio queue).

  • Queue this pull request

@loveRhythm1990 loveRhythm1990 merged commit 36014dc into matrixorigin:main Jul 14, 2026
5 of 6 checks passed
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.

[Bug]: CNSet exportToPrometheus=true does not create metric Service for ServiceMonitor discovery

2 participants