Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

✨ Support RateLimiterLatency & RequestLatency for client-go metrics (client, workqueue, reflector). #2986

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 35 additions & 1 deletion pkg/metrics/client_go_adapter.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ package metrics

import (
"context"
"net/url"
"time"

"github.com/prometheus/client_golang/prometheus"
clientmetrics "k8s.io/client-go/tools/metrics"
Expand All @@ -30,6 +32,26 @@ import (
var (
// client metrics.

requestLatency = prometheus.NewHistogramVec(
prometheus.HistogramOpts{
Name: "rest_client_request_duration_seconds",
Help: "Request latency in seconds. Broken down by verb and URL.",
Buckets: []float64{0.005, 0.01, 0.025, 0.05, 0.1, 0.15, 0.2, 0.25, 0.3, 0.35, 0.4, 0.45, 0.5, 0.6, 0.7, 0.8, 0.9, 1.0,
1.25, 1.5, 1.75, 2.0, 2.5, 3.0, 3.5, 4.0, 4.5, 5, 6, 7, 8, 9, 10, 15, 20, 25, 30, 40, 50, 60},
},
[]string{"verb", "host", "path"},
)

rateLimiterLatency = prometheus.NewHistogramVec(
prometheus.HistogramOpts{
Name: "rest_client_rate_limiter_duration_seconds",
Help: "Client side rate limiter latency in seconds. Broken down by verb and URL.",
Buckets: []float64{0.005, 0.01, 0.025, 0.05, 0.1, 0.15, 0.2, 0.25, 0.3, 0.35, 0.4, 0.45, 0.5, 0.6, 0.7, 0.8, 0.9, 1.0,
Copy link
Member

Choose a reason for hiding this comment

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

These have a ton of buckets and dimensions that include the path which means things like object names will be in there, this will result in a huge cardinality, we can not include something like this by default

Copy link
Author

@fengruotj fengruotj Oct 17, 2024

Choose a reason for hiding this comment

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

My primary requirement is to register some custom metrics for client-go/tools/metrics. However, when using controller-runtime, I am unable to customize the registration of client-go metrics because registerClientMetrics() has a higher precedence. Therefore, I need to add some additional metrics. Perhaps we could reduce the number of buckets and some tags metrics in this case, or using prometheus Summary metrics instead?

1.25, 1.5, 1.75, 2.0, 2.5, 3.0, 3.5, 4.0, 4.5, 5, 6, 7, 8, 9, 10, 15, 20, 25, 30, 40, 50, 60},
},
[]string{"verb", "host", "path"},
)

requestResult = prometheus.NewCounterVec(
prometheus.CounterOpts{
Name: "rest_client_requests_total",
Expand All @@ -47,10 +69,14 @@ func init() {
func registerClientMetrics() {
// register the metrics with our registry
Registry.MustRegister(requestResult)
Registry.MustRegister(rateLimiterLatency)
Registry.MustRegister(requestLatency)

// register the metrics with client-go
clientmetrics.Register(clientmetrics.RegisterOpts{
RequestResult: &resultAdapter{metric: requestResult},
RequestResult: &resultAdapter{metric: requestResult},
RateLimiterLatency: &latencyAdapter{metric: rateLimiterLatency},
RequestLatency: &latencyAdapter{metric: requestLatency},
})
}

Expand All @@ -69,3 +95,11 @@ type resultAdapter struct {
func (r *resultAdapter) Increment(_ context.Context, code, method, host string) {
r.metric.WithLabelValues(code, method, host).Inc()
}

type latencyAdapter struct {
metric *prometheus.HistogramVec
}

func (l *latencyAdapter) Observe(_ context.Context, verb string, u url.URL, latency time.Duration) {
l.metric.WithLabelValues(verb, u.Host, u.Path).Observe(latency.Seconds())
}