Merge pull request #2994 from swetharepakula/separate-psc-metrics

Move PSC related metrics into PSC metrics directory

Author: k8s-ci-robot

pkg/metrics/features.go

@@ -102,12 +102,6 @@ const (
 	// FrontendConfig Features
 	sslPolicy      = feature("SSLPolicy")
 	httpsRedirects = feature("HTTPSRedirects")
-
-	// PSC Features
-	sa          = feature("ServiceAttachments")
-	saInSuccess = feature("ServiceAttachmentInSuccess")
-	saInError   = feature("ServiceAttachmentInError")
-	services    = feature("Services")
 )
 
 // featuresForIngress returns the list of features for given ingress.

pkg/metrics/metrics.go

@@ -56,20 +56,6 @@ var (
 		},
 		[]string{label},
 	)
-	serviceAttachmentCount = prometheus.NewGaugeVec(
-		prometheus.GaugeOpts{
-			Name: "number_of_service_attachments",
-			Help: "Number of Service Attachments",
-		},
-		[]string{label},
-	)
-	serviceCount = prometheus.NewGaugeVec(
-		prometheus.GaugeOpts{
-			Name: "number_of_services",
-			Help: "Number of Services",
-		},
-		[]string{label},
-	)
 	componentVersion = prometheus.NewGaugeVec(
 		prometheus.GaugeOpts{
 			Name: "component_version",
@@ -91,10 +77,6 @@ func init() {
 	klog.V(3).Infof("Registering Ingress usage metrics %v and %v", ingressCount, servicePortCount)
 	prometheus.MustRegister(ingressCount, servicePortCount)
 
-	klog.V(3).Infof("Registering PSC usage metrics %v", serviceAttachmentCount)
-	prometheus.MustRegister(serviceAttachmentCount)
-	prometheus.MustRegister(serviceCount)
-
 	klog.V(3).Infof("Registering metric export failures count %v", MetricExportFailureCount)
 	prometheus.MustRegister(MetricExportFailureCount)
 
@@ -189,48 +171,6 @@ func (im *ControllerMetrics) DeleteIngress(ingKey string) {
 	delete(im.ingressMap, ingKey)
 }
 
-// SetServiceAttachment adds sa state to the map to be counted during metrics computation.
-// SetServiceAttachment implements PSCMetricsCollector.
-func (im *ControllerMetrics) SetServiceAttachment(saKey string, state pscmetrics.PSCState) {
-	im.Lock()
-	defer im.Unlock()
-
-	if im.pscMap == nil {
-		klog.Fatalf("PSC Metrics failed to initialize correctly.")
-	}
-	im.pscMap[saKey] = state
-}
-
-// DeleteServiceAttachment removes sa state to the map to be counted during metrics computation.
-// DeleteServiceAttachment implements PSCMetricsCollector.
-func (im *ControllerMetrics) DeleteServiceAttachment(saKey string) {
-	im.Lock()
-	defer im.Unlock()
-
-	delete(im.pscMap, saKey)
-}
-
-// SetService adds the service to the map to be counted during metrics computation.
-// SetService implements PSCMetricsCollector.
-func (im *ControllerMetrics) SetService(serviceKey string) {
-	im.Lock()
-	defer im.Unlock()
-
-	if im.serviceMap == nil {
-		klog.Fatalf("PSC Metrics failed to initialize correctly.")
-	}
-	im.serviceMap[serviceKey] = struct{}{}
-}
-
-// DeleteService removes the service from the map to be counted during metrics computation.
-// DeleteService implements PSCMetricsCollector.
-func (im *ControllerMetrics) DeleteService(serviceKey string) {
-	im.Lock()
-	defer im.Unlock()
-
-	delete(im.serviceMap, serviceKey)
-}
-
 // export computes and exports ingress usage metrics.
 func (im *ControllerMetrics) export() {
 	defer func() {
@@ -251,21 +191,6 @@ func (im *ControllerMetrics) export() {
 	}
 
 	im.logger.V(3).Info("Ingress usage metrics exported")
-
-	saCount := im.computePSCMetrics()
-	im.logger.V(3).Info("Exporting PSC Usage Metrics", "serviceAttachmentsCount", saCount)
-	for feature, count := range saCount {
-		serviceAttachmentCount.With(prometheus.Labels{label: feature.String()}).Set(float64(count))
-	}
-	im.logger.V(3).Info("Exported PSC Usage Metrics", "serviceAttachmentsCount", saCount)
-
-	services := im.computeServiceMetrics()
-	im.logger.V(3).Info("Exporting Service Metrics", "serviceCount", serviceCount)
-	for feature, count := range services {
-		serviceCount.With(prometheus.Labels{label: feature.String()}).Set(float64(count))
-	}
-	im.logger.V(3).Info("Exported Service Metrics", "serviceCount", serviceCount)
-
 }
 
 // computeIngressMetrics traverses all ingresses and computes,
@@ -323,34 +248,6 @@ func (im *ControllerMetrics) computeIngressMetrics() (map[feature]int, map[featu
 	return ingCount, svcPortCount
 }
 
-func (im *ControllerMetrics) computePSCMetrics() map[feature]int {
-	im.Lock()
-	defer im.Unlock()
-	im.logger.V(4).Info("Compute PSC Usage metrics from psc state map", "pscStateMap", im.pscMap)
-
-	counts := map[feature]int{
-		sa:          0,
-		saInSuccess: 0,
-		saInError:   0,
-	}
-
-	for _, state := range im.pscMap {
-		counts[sa]++
-		if state.InSuccess {
-			counts[saInSuccess]++
-		} else {
-			counts[saInError]++
-		}
-	}
-	return counts
-}
-
-func (im *ControllerMetrics) computeServiceMetrics() map[feature]int {
-	return map[feature]int{
-		services: len(im.serviceMap),
-	}
-}
-
 // initializeCounts initializes feature count maps for ingress and service ports.
 // This is required in order to reset counts for features that do not exist now
 // but existed before.

pkg/metrics/metrics_test.go

@@ -18,7 +18,6 @@ package metrics
 
 import (
 	"fmt"
-	"strconv"
 	"testing"
 
 	"k8s.io/klog/v2"
@@ -30,7 +29,6 @@ import (
 	"k8s.io/ingress-gce/pkg/annotations"
 	backendconfigv1 "k8s.io/ingress-gce/pkg/apis/backendconfig/v1"
 	frontendconfigv1beta1 "k8s.io/ingress-gce/pkg/apis/frontendconfig/v1beta1"
-	pscmetrics "k8s.io/ingress-gce/pkg/psc/metrics"
 	"k8s.io/ingress-gce/pkg/utils"
 	"k8s.io/utils/ptr"
 )
@@ -1242,157 +1240,3 @@ func TestComputeIngressMetrics(t *testing.T) {
 		})
 	}
 }
-
-func TestComputePSCMetrics(t *testing.T) {
-	t.Parallel()
-	for _, tc := range []struct {
-		desc     string
-		saStates []pscmetrics.PSCState
-		// service attachments to delete
-		deleteStates  []string
-		expectSACount map[feature]int
-	}{
-		{
-			desc:     "empty input",
-			saStates: []pscmetrics.PSCState{},
-			expectSACount: map[feature]int{
-				sa:          0,
-				saInSuccess: 0,
-				saInError:   0,
-			},
-		},
-		{
-			desc: "one service attachment",
-			saStates: []pscmetrics.PSCState{
-				newPSCState(true),
-			},
-			expectSACount: map[feature]int{
-				sa:          1,
-				saInSuccess: 1,
-				saInError:   0,
-			},
-		},
-		{
-			desc: "one service attachment in error",
-			saStates: []pscmetrics.PSCState{
-				newPSCState(false),
-			},
-			expectSACount: map[feature]int{
-				sa:          1,
-				saInSuccess: 0,
-				saInError:   1,
-			},
-		},
-		{
-			desc: "many service attachments, some in error",
-			saStates: []pscmetrics.PSCState{
-				newPSCState(true),
-				newPSCState(true),
-				newPSCState(true),
-				newPSCState(false),
-				newPSCState(false),
-			},
-			expectSACount: map[feature]int{
-				sa:          5,
-				saInSuccess: 3,
-				saInError:   2,
-			},
-		},
-		{
-			desc: "some additions, and some deletions",
-			saStates: []pscmetrics.PSCState{
-				newPSCState(true),
-				newPSCState(true),
-				newPSCState(true),
-				newPSCState(false),
-				newPSCState(false),
-			},
-			deleteStates: []string{"0", "3"},
-			expectSACount: map[feature]int{
-				sa:          3,
-				saInSuccess: 2,
-				saInError:   1,
-			},
-		},
-	} {
-		tc := tc
-		t.Run(tc.desc, func(t *testing.T) {
-			t.Parallel()
-			newMetrics := FakeControllerMetrics()
-			for i, serviceState := range tc.saStates {
-				newMetrics.SetServiceAttachment(strconv.Itoa(i), serviceState)
-			}
-
-			for _, key := range tc.deleteStates {
-				newMetrics.DeleteServiceAttachment(key)
-			}
-			got := newMetrics.computePSCMetrics()
-			if diff := cmp.Diff(tc.expectSACount, got); diff != "" {
-				t.Fatalf("Got diff for service attachment counts (-want +got):\n%s", diff)
-			}
-		})
-	}
-}
-
-func newPSCState(inSuccess bool) pscmetrics.PSCState {
-	return pscmetrics.PSCState{
-		InSuccess: inSuccess,
-	}
-}
-
-func TestComputeServiceMetrics(t *testing.T) {
-	t.Parallel()
-	for _, tc := range []struct {
-		desc           string
-		services       []string
-		deleteServices []string
-		expectSACount  map[feature]int
-	}{
-		{
-			desc: "empty input",
-			expectSACount: map[feature]int{
-				services: 0,
-			},
-		},
-		{
-			desc:     "one service",
-			services: []string{"service-1"},
-			expectSACount: map[feature]int{
-				services: 1,
-			},
-		},
-		{
-			desc:     "many services",
-			services: []string{"service-1", "service-2", "service-3", "service-4", "service-5", "service-6"},
-			expectSACount: map[feature]int{
-				services: 6,
-			},
-		},
-		{
-			desc:           "some additions, and some deletions",
-			services:       []string{"service-1", "service-2", "service-3", "service-4", "service-5", "service-6"},
-			deleteServices: []string{"service-2", "service-5"},
-			expectSACount: map[feature]int{
-				services: 4,
-			},
-		},
-	} {
-		tc := tc
-		t.Run(tc.desc, func(t *testing.T) {
-			t.Parallel()
-			newMetrics := FakeControllerMetrics()
-			for _, service := range tc.services {
-				newMetrics.SetService(service)
-			}
-
-			for _, service := range tc.deleteServices {
-				newMetrics.DeleteService(service)
-			}
-
-			got := newMetrics.computeServiceMetrics()
-			if diff := cmp.Diff(tc.expectSACount, got); diff != "" {
-				t.Fatalf("Got diff for service counts (-want +got):\n%s", diff)
-			}
-		})
-	}
-}

pkg/psc/controller.go

@@ -44,6 +44,7 @@ import (
 	"k8s.io/ingress-gce/pkg/context"
 	"k8s.io/ingress-gce/pkg/flags"
 	"k8s.io/ingress-gce/pkg/psc/metrics"
+	"k8s.io/ingress-gce/pkg/psc/metrics/metricscollector"
 	serviceattachmentclient "k8s.io/ingress-gce/pkg/serviceattachment/client/clientset/versioned"
 	"k8s.io/ingress-gce/pkg/utils"
 	"k8s.io/ingress-gce/pkg/utils/namer"
@@ -56,6 +57,7 @@ import (
 func init() {
 	// register prometheus metrics
 	metrics.RegisterMetrics()
+	metricscollector.RegisterMetrics()
 }
 
 const (
@@ -92,7 +94,7 @@ type Controller struct {
 	svcAttachmentLister cache.Indexer
 	serviceLister       cache.Indexer
 	recorder            func(string) record.EventRecorder
-	collector           metrics.PSCMetricsCollector
+	collector           *metricscollector.PSCMetricsCollector
 
 	hasSynced func() bool
 
@@ -116,6 +118,7 @@ type Controller struct {
 func NewController(ctx *context.ControllerContext, stopCh <-chan struct{}, logger klog.Logger) *Controller {
 	logger = logger.WithName("PSCController")
 	saNamer := namer.NewServiceAttachmentNamer(ctx.ClusterNamer, string(ctx.KubeSystemUID))
+	metricsCollector := metricscollector.NewPSCMetricsCollector(flags.F.MetricsExportInterval, logger)
 	controller := &Controller{
 		cloud:               ctx.Cloud,
 		saClient:            ctx.SAClient,
@@ -125,7 +128,7 @@ func NewController(ctx *context.ControllerContext, stopCh <-chan struct{}, logge
 		serviceLister:       ctx.ServiceInformer.GetIndexer(),
 		hasSynced:           ctx.HasSynced,
 		recorder:            ctx.Recorder,
-		collector:           ctx.ControllerMetrics,
+		collector:           metricsCollector,
 		clusterName:         flags.F.GKEClusterName,
 		regionalCluster:     ctx.RegionalCluster,
 		readOnlyMode:        ctx.ReadOnlyMode,
@@ -183,6 +186,7 @@ func (c *Controller) Run() {
 		time.Sleep(ServiceAttachmentGCPeriod)
 		wait.Until(c.garbageCollectServiceAttachments, ServiceAttachmentGCPeriod, c.stopCh)
 	}()
+	go c.collector.Run(c.stopCh)
 
 	<-c.stopCh
 }