Merge pull request kubernetes#5111 from jbartosik/metric_server_respo…

…nses

Export metric_server_response like other VPA recommender metrics

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.

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.
@@ -70,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,
 	}
 }
 
@@ -87,16 +70,15 @@ func (c *metricsClient) GetContainersMetrics() ([]*ContainerMetricsSnapshot, err
 
 	podMetricsInterface := c.metricsGetter.PodMetricses(c.namespace)
 	podMetricsList, err := podMetricsInterface.List(context.TODO(), metav1.ListOptions{})
+	recommender_metrics.RecordMetricsServerResponse(err, c.clientName)
 	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))
 	for _, podMetrics := range podMetricsList.Items {
 		metricsSnapshotsForPod := createContainerMetricsSnapshots(podMetrics)
 		metricsSnapshots = append(metricsSnapshots, metricsSnapshotsForPod...)
 	}
-	metricServerResponses.WithLabelValues(strconv.FormatBool(false)).Inc()
 	return metricsSnapshots, nil
 }
 

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 {

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)

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(),

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", "client_name"},
+	)
 )
 
 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, clientName string) {
+	metricServerResponses.WithLabelValues(strconv.FormatBool(err != nil), clientName).Inc()
+}
+
 // NewObjectCounter creates a new helper to split VPA objects into buckets
 func NewObjectCounter() *ObjectCounter {
 	obj := ObjectCounter{