Skip to content

Conversation

@tjungblu
Copy link
Contributor

I've noticed that K8s and apiserver contains several metrics that etcd does not have. This PR adds the missing metrics to the default Prometheus registry.

@k8s-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: tjungblu
Once this PR has been reviewed and has the lgtm label, please assign spzala for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@codecov
Copy link

codecov bot commented Dec 17, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 68.87%. Comparing base (893aab0) to head (1e457e3).
⚠️ Report is 64 commits behind head on main.

Additional details and impacted files
Files with missing lines Coverage Δ
server/etcdserver/metrics.go 86.95% <100.00%> (+3.97%) ⬆️

... and 35 files with indirect coverage changes

@@            Coverage Diff             @@
##             main   #21029      +/-   ##
==========================================
- Coverage   69.18%   68.87%   -0.32%     
==========================================
  Files         422      429       +7     
  Lines       34841    35755     +914     
==========================================
+ Hits        24104    24625     +521     
- Misses       9335     9723     +388     
- Partials     1402     1407       +5     

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 893aab0...1e457e3. Read the comment docs.

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

I've noticed that K8s and apiserver contains several metrics that etcd
does not have. This PR adds the missing metrics to the default
Prometheus registry.

Signed-off-by: Thomas Jungblut <tjungblu@redhat.com>
@fuweid
Copy link
Member

fuweid commented Dec 17, 2025

/retest


func init() {
// register more extensive go runtime metrics, for that we need to unregister the default metrics first
prometheus.Unregister(collectors.NewGoCollector())
Copy link
Member

Choose a reason for hiding this comment

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

I haven't diged into the implementation of prometheus.Unregister, but you unregister a newly created objerct collectors.NewGoCollector() instead of the existing already registered one, are you sure it's correct?

Copy link
Contributor Author

@tjungblu tjungblu Jan 7, 2026

Choose a reason for hiding this comment

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

Yes. The registry works by the metric name, so it will list all the metric names and remove them. It doesn't refer to the metrics by their references or addresses.

The duplication logic that returns errors on duplicate registers also goes via names.

@tjungblu
Copy link
Contributor Author

tjungblu commented Jan 7, 2026

/retest

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Development

Successfully merging this pull request may close these issues.

4 participants