Expose all Go runtime metrics on /metrics - #22248
Conversation
|
/cc @ahrtr @fuweid @serathius @ivanvc Noticed this while working on kubernetes scalability measurements. I think this could also help with etcd specific benchmarking? |
Codecov Report❌ Patch coverage is
Additional details and impacted files
... and 19 files with indirect coverage changes @@ Coverage Diff @@
## main #22248 +/- ##
==========================================
- Coverage 69.71% 69.67% -0.04%
==========================================
Files 449 449
Lines 38187 38198 +11
==========================================
- Hits 26622 26615 -7
- Misses 10141 10156 +15
- Partials 1424 1427 +3 Continue to review full report in Codecov by Harness.
🚀 New features to boost your workflow:
|
| func init() { | ||
| if prometheus.Unregister(collectors.NewGoCollector()) { | ||
| prometheus.MustRegister(collectors.NewGoCollector( | ||
| collectors.WithGoCollectorRuntimeMetrics(collectors.MetricsAll), | ||
| )) | ||
| } | ||
| } |
There was a problem hiding this comment.
Any applications that import the package or even etcd embed package (e.g. embedded use cases, because the embed imports etcdhttp) will automatically add all these metrics. Would be better to guard it under the existing --metrics extensive (preferred) or a new flag something like --metrics-runtime-all.
There was a problem hiding this comment.
sounds good, folded behind the --metrics extensive flag.
d50c40a to
9ff1918
Compare
|
/retest |
ivanvc
left a comment
ivanvc
left a comment
There was a problem hiding this comment.
LGTM after being guarded by --metrics extensive. Thanks.
| // EnableAllRuntimeMetrics swaps client_golang's default Go collector for one | ||
| // exposing the full runtime/metrics set (scheduler latency, mutex contention, | ||
| // GC CPU share). | ||
| func EnableAllRuntimeMetrics(lg *zap.Logger) { |
There was a problem hiding this comment.
server/etcdserver/metrics.go should be a better place to add this function
Signed-off-by: Jefftree <jeffrey.ying86@live.com>
879890b to
390688e
Compare
|
/retest |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: ahrtr, ivanvc, Jefftree, serathius The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
5 participants
Turns on all the Go runtime/metrics instead of the three the default collector enables, which gives us scheduler latency, mutex contention and GC CPU share for tracking down etcd bottlenecks. 35 -> 224 series.
client_golang registers its own Go collector into the default registry at init, so reconfiguring it means dropping that one first. This is the same pattern as client_golang's own example, prometheus and coredns.
Kubernetes turned the full set on in kubernetes/kubernetes#111910.
Disclaimer: I'm most interested in
go_gc_heap_live_bytesout of the list, but it seems like providing the overall list is a net improvement overall.