From bd0966ba291f96e0c83a7bb5dbbaab8fa7966ec9 Mon Sep 17 00:00:00 2001 From: Joachim Bartosik Date: Fri, 19 Aug 2022 12:57:41 +0200 Subject: [PATCH 1/2] Export metric_server_response like other VPA recommender metrics --- .../input/metrics/metrics_client.go | 24 ++----------------- .../pkg/recommender/main.go | 2 -- .../utils/metrics/recommender/recommender.go | 16 ++++++++++++- 3 files changed, 17 insertions(+), 25 deletions(-) diff --git a/vertical-pod-autoscaler/pkg/recommender/input/metrics/metrics_client.go b/vertical-pod-autoscaler/pkg/recommender/input/metrics/metrics_client.go index 1a0b3189a453..f348e972f380 100644 --- a/vertical-pod-autoscaler/pkg/recommender/input/metrics/metrics_client.go +++ b/vertical-pod-autoscaler/pkg/recommender/input/metrics/metrics_client.go @@ -18,36 +18,17 @@ package metrics import ( "context" - "strconv" "time" - "github.com/prometheus/client_golang/prometheus" k8sapiv1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/autoscaler/vertical-pod-autoscaler/pkg/recommender/model" - "k8s.io/autoscaler/vertical-pod-autoscaler/pkg/utils/metrics" + recommender_metrics "k8s.io/autoscaler/vertical-pod-autoscaler/pkg/utils/metrics/recommender" klog "k8s.io/klog/v2" "k8s.io/metrics/pkg/apis/metrics/v1beta1" resourceclient "k8s.io/metrics/pkg/client/clientset/versioned/typed/metrics/v1beta1" ) -const ( - metricsNamespace = metrics.TopMetricsNamespace + "metrics-server-client" -) - -var metricServerResponses = prometheus.NewCounterVec( - prometheus.CounterOpts{ - Namespace: metricsNamespace, - Name: "metric_server_responses", - Help: "Count of responses to queries to metrics server", - }, []string{"is_error"}, -) - -// Register initializes all VPA metrics for metrics server client -func Register() { - prometheus.MustRegister(metricServerResponses) -} - // ContainerMetricsSnapshot contains information about usage of certain container within defined time window. type ContainerMetricsSnapshot struct { // ID identifies a specific container those metrics are coming from. @@ -87,8 +68,8 @@ func (c *metricsClient) GetContainersMetrics() ([]*ContainerMetricsSnapshot, err podMetricsInterface := c.metricsGetter.PodMetricses(c.namespace) podMetricsList, err := podMetricsInterface.List(context.TODO(), metav1.ListOptions{}) + recommender_metrics.RecordMetricsServerResponse(err) if err != nil { - metricServerResponses.WithLabelValues(strconv.FormatBool(true)).Inc() return nil, err } klog.V(3).Infof("%v podMetrics retrieved for all namespaces", len(podMetricsList.Items)) @@ -96,7 +77,6 @@ func (c *metricsClient) GetContainersMetrics() ([]*ContainerMetricsSnapshot, err metricsSnapshotsForPod := createContainerMetricsSnapshots(podMetrics) metricsSnapshots = append(metricsSnapshots, metricsSnapshotsForPod...) } - metricServerResponses.WithLabelValues(strconv.FormatBool(false)).Inc() return metricsSnapshots, nil } diff --git a/vertical-pod-autoscaler/pkg/recommender/main.go b/vertical-pod-autoscaler/pkg/recommender/main.go index f2b18def61b6..e5d1fb6abcf9 100644 --- a/vertical-pod-autoscaler/pkg/recommender/main.go +++ b/vertical-pod-autoscaler/pkg/recommender/main.go @@ -23,7 +23,6 @@ import ( apiv1 "k8s.io/api/core/v1" "k8s.io/autoscaler/vertical-pod-autoscaler/common" "k8s.io/autoscaler/vertical-pod-autoscaler/pkg/recommender/input/history" - vpa_metrics_client "k8s.io/autoscaler/vertical-pod-autoscaler/pkg/recommender/input/metrics" "k8s.io/autoscaler/vertical-pod-autoscaler/pkg/recommender/model" "k8s.io/autoscaler/vertical-pod-autoscaler/pkg/recommender/routines" "k8s.io/autoscaler/vertical-pod-autoscaler/pkg/utils/metrics" @@ -82,7 +81,6 @@ func main() { metrics.Initialize(*address, healthCheck) metrics_recommender.Register() metrics_quality.Register() - vpa_metrics_client.Register() useCheckpoints := *storage != "prometheus" recommender := routines.NewRecommender(config, *checkpointsGCInterval, useCheckpoints, *vpaObjectNamespace) diff --git a/vertical-pod-autoscaler/pkg/utils/metrics/recommender/recommender.go b/vertical-pod-autoscaler/pkg/utils/metrics/recommender/recommender.go index c8d564585c01..7516b602e717 100644 --- a/vertical-pod-autoscaler/pkg/utils/metrics/recommender/recommender.go +++ b/vertical-pod-autoscaler/pkg/utils/metrics/recommender/recommender.go @@ -19,6 +19,7 @@ package recommender import ( "fmt" + "strconv" "time" "github.com/prometheus/client_golang/prometheus" @@ -71,6 +72,14 @@ var ( Help: "Number of aggregate container states being tracked by the recommender", }, ) + + metricServerResponses = prometheus.NewCounterVec( + prometheus.CounterOpts{ + Namespace: metricsNamespace, + Name: "metric_server_responses", + Help: "Count of responses to queries to metrics server", + }, []string{"is_error"}, + ) ) type objectCounterKey struct { @@ -88,7 +97,7 @@ type ObjectCounter struct { // Register initializes all metrics for VPA Recommender func Register() { - prometheus.MustRegister(vpaObjectCount, recommendationLatency, functionLatency, aggregateContainerStatesCount) + prometheus.MustRegister(vpaObjectCount, recommendationLatency, functionLatency, aggregateContainerStatesCount, metricServerResponses) } // NewExecutionTimer provides a timer for Recommender's RunOnce execution @@ -106,6 +115,11 @@ func RecordAggregateContainerStatesCount(statesCount int) { aggregateContainerStatesCount.Set(float64(statesCount)) } +// RecordMetricsServerResponse records result of a query to metrics server +func RecordMetricsServerResponse(err error) { + metricServerResponses.WithLabelValues(strconv.FormatBool(err != nil)).Inc() +} + // NewObjectCounter creates a new helper to split VPA objects into buckets func NewObjectCounter() *ObjectCounter { obj := ObjectCounter{ From 2694bbaed2ecba11f443aa4664683a70f8f3748c Mon Sep 17 00:00:00 2001 From: Joachim Bartosik Date: Wed, 24 Aug 2022 17:22:01 +0200 Subject: [PATCH 2/2] Export client name with responses from metrics server In case we instantiate multiple clients and want to distinguish responses they're getting --- .../pkg/recommender/input/cluster_feeder.go | 10 +++++----- .../pkg/recommender/input/metrics/metrics_client.go | 6 ++++-- .../input/metrics/metrics_client_test_util.go | 2 +- .../pkg/recommender/routines/recommender.go | 4 ++-- .../pkg/utils/metrics/recommender/recommender.go | 6 +++--- 5 files changed, 15 insertions(+), 13 deletions(-) diff --git a/vertical-pod-autoscaler/pkg/recommender/input/cluster_feeder.go b/vertical-pod-autoscaler/pkg/recommender/input/cluster_feeder.go index c3015c4d0098..31dabf7da0e5 100644 --- a/vertical-pod-autoscaler/pkg/recommender/input/cluster_feeder.go +++ b/vertical-pod-autoscaler/pkg/recommender/input/cluster_feeder.go @@ -46,7 +46,7 @@ import ( v1lister "k8s.io/client-go/listers/core/v1" "k8s.io/client-go/rest" "k8s.io/client-go/tools/cache" - "k8s.io/klog/v2" + klog "k8s.io/klog/v2" resourceclient "k8s.io/metrics/pkg/client/clientset/versioned/typed/metrics/v1beta1" ) @@ -114,7 +114,7 @@ func (m ClusterStateFeederFactory) Make() *clusterStateFeeder { // NewClusterStateFeeder creates new ClusterStateFeeder with internal data providers, based on kube client config. // Deprecated; Use ClusterStateFeederFactory instead. -func NewClusterStateFeeder(config *rest.Config, clusterState *model.ClusterState, memorySave bool, namespace string) ClusterStateFeeder { +func NewClusterStateFeeder(config *rest.Config, clusterState *model.ClusterState, memorySave bool, namespace, metricsClientName string) ClusterStateFeeder { kubeClient := kube_client.NewForConfigOrDie(config) podLister, oomObserver := NewPodListerAndOOMObserver(kubeClient, namespace) factory := informers.NewSharedInformerFactoryWithOptions(kubeClient, defaultResyncPeriod, informers.WithNamespace(namespace)) @@ -124,7 +124,7 @@ func NewClusterStateFeeder(config *rest.Config, clusterState *model.ClusterState PodLister: podLister, OOMObserver: oomObserver, KubeClient: kubeClient, - MetricsClient: newMetricsClient(config, namespace), + MetricsClient: newMetricsClient(config, namespace, metricsClientName), VpaCheckpointClient: vpa_clientset.NewForConfigOrDie(config).AutoscalingV1(), VpaLister: vpa_api_util.NewVpasLister(vpa_clientset.NewForConfigOrDie(config), make(chan struct{}), namespace), ClusterState: clusterState, @@ -134,9 +134,9 @@ func NewClusterStateFeeder(config *rest.Config, clusterState *model.ClusterState }.Make() } -func newMetricsClient(config *rest.Config, namespace string) metrics.MetricsClient { +func newMetricsClient(config *rest.Config, namespace, clientName string) metrics.MetricsClient { metricsGetter := resourceclient.NewForConfigOrDie(config) - return metrics.NewMetricsClient(metricsGetter, namespace) + return metrics.NewMetricsClient(metricsGetter, namespace, clientName) } // WatchEvictionEventsWithRetries watches new Events with reason=Evicted and passes them to the observer. diff --git a/vertical-pod-autoscaler/pkg/recommender/input/metrics/metrics_client.go b/vertical-pod-autoscaler/pkg/recommender/input/metrics/metrics_client.go index f348e972f380..8a2e24eaafc9 100644 --- a/vertical-pod-autoscaler/pkg/recommender/input/metrics/metrics_client.go +++ b/vertical-pod-autoscaler/pkg/recommender/input/metrics/metrics_client.go @@ -51,15 +51,17 @@ type MetricsClient interface { type metricsClient struct { metricsGetter resourceclient.PodMetricsesGetter namespace string + clientName string } // NewMetricsClient creates new instance of MetricsClient, which is used by recommender. // It requires an instance of PodMetricsesGetter, which is used for underlying communication with metrics server. // namespace limits queries to particular namespace, use k8sapiv1.NamespaceAll to select all namespaces. -func NewMetricsClient(metricsGetter resourceclient.PodMetricsesGetter, namespace string) MetricsClient { +func NewMetricsClient(metricsGetter resourceclient.PodMetricsesGetter, namespace, clientName string) MetricsClient { return &metricsClient{ metricsGetter: metricsGetter, namespace: namespace, + clientName: clientName, } } @@ -68,7 +70,7 @@ func (c *metricsClient) GetContainersMetrics() ([]*ContainerMetricsSnapshot, err podMetricsInterface := c.metricsGetter.PodMetricses(c.namespace) podMetricsList, err := podMetricsInterface.List(context.TODO(), metav1.ListOptions{}) - recommender_metrics.RecordMetricsServerResponse(err) + recommender_metrics.RecordMetricsServerResponse(err, c.clientName) if err != nil { return nil, err } diff --git a/vertical-pod-autoscaler/pkg/recommender/input/metrics/metrics_client_test_util.go b/vertical-pod-autoscaler/pkg/recommender/input/metrics/metrics_client_test_util.go index dbbca473020e..e03912cd4410 100644 --- a/vertical-pod-autoscaler/pkg/recommender/input/metrics/metrics_client_test_util.go +++ b/vertical-pod-autoscaler/pkg/recommender/input/metrics/metrics_client_test_util.go @@ -84,7 +84,7 @@ func (tc *metricsClientTestCase) createFakeMetricsClient() MetricsClient { fakeMetricsGetter.AddReactor("list", "pods", func(action core.Action) (handled bool, ret runtime.Object, err error) { return true, tc.getFakePodMetricsList(), nil }) - return NewMetricsClient(fakeMetricsGetter.MetricsV1beta1(), "") + return NewMetricsClient(fakeMetricsGetter.MetricsV1beta1(), "", "fake") } func (tc *metricsClientTestCase) getFakePodMetricsList() *metricsapi.PodMetricsList { diff --git a/vertical-pod-autoscaler/pkg/recommender/routines/recommender.go b/vertical-pod-autoscaler/pkg/recommender/routines/recommender.go index 27327aa70ac6..5f53b9660c6a 100644 --- a/vertical-pod-autoscaler/pkg/recommender/routines/recommender.go +++ b/vertical-pod-autoscaler/pkg/recommender/routines/recommender.go @@ -35,7 +35,7 @@ import ( "k8s.io/client-go/informers" kube_client "k8s.io/client-go/kubernetes" "k8s.io/client-go/rest" - "k8s.io/klog/v2" + klog "k8s.io/klog/v2" ) const ( @@ -261,7 +261,7 @@ func NewRecommender(config *rest.Config, checkpointsGCInterval time.Duration, us controllerFetcher := controllerfetcher.NewControllerFetcher(config, kubeClient, factory, scaleCacheEntryFreshnessTime, scaleCacheEntryLifetime, scaleCacheEntryJitterFactor) return RecommenderFactory{ ClusterState: clusterState, - ClusterStateFeeder: input.NewClusterStateFeeder(config, clusterState, *memorySaver, namespace), + ClusterStateFeeder: input.NewClusterStateFeeder(config, clusterState, *memorySaver, namespace, "default-metrics-client"), ControllerFetcher: controllerFetcher, CheckpointWriter: checkpoint.NewCheckpointWriter(clusterState, vpa_clientset.NewForConfigOrDie(config).AutoscalingV1()), VpaClient: vpa_clientset.NewForConfigOrDie(config).AutoscalingV1(), diff --git a/vertical-pod-autoscaler/pkg/utils/metrics/recommender/recommender.go b/vertical-pod-autoscaler/pkg/utils/metrics/recommender/recommender.go index 7516b602e717..f48f3ca5c3f8 100644 --- a/vertical-pod-autoscaler/pkg/utils/metrics/recommender/recommender.go +++ b/vertical-pod-autoscaler/pkg/utils/metrics/recommender/recommender.go @@ -78,7 +78,7 @@ var ( Namespace: metricsNamespace, Name: "metric_server_responses", Help: "Count of responses to queries to metrics server", - }, []string{"is_error"}, + }, []string{"is_error", "client_name"}, ) ) @@ -116,8 +116,8 @@ func RecordAggregateContainerStatesCount(statesCount int) { } // RecordMetricsServerResponse records result of a query to metrics server -func RecordMetricsServerResponse(err error) { - metricServerResponses.WithLabelValues(strconv.FormatBool(err != nil)).Inc() +func RecordMetricsServerResponse(err error, clientName string) { + metricServerResponses.WithLabelValues(strconv.FormatBool(err != nil), clientName).Inc() } // NewObjectCounter creates a new helper to split VPA objects into buckets