removed label & passing shared metrics aggregator from top level

Author: priyapande

cmd/glbc/main.go

@@ -69,6 +69,7 @@ import (
 	_ "k8s.io/ingress-gce/pkg/klog"
 	"k8s.io/ingress-gce/pkg/neg"
 	"k8s.io/ingress-gce/pkg/neg/metrics"
+	syncMetrics "k8s.io/ingress-gce/pkg/neg/metrics/metricscollector"
 	"k8s.io/ingress-gce/pkg/neg/syncers/labels"
 	negtypes "k8s.io/ingress-gce/pkg/neg/types"
 	"k8s.io/ingress-gce/pkg/utils/zonegetter"
@@ -686,7 +687,12 @@ func createNEGController(ctx *ingctx.ControllerContext, systemHealth *systemheal
 		adapter = ctx.Cloud
 	}
 
+	// register NEG prometheus metrics
+	metrics.RegisterMetrics()
+	syncMetrics.RegisterMetrics()
+
 	negMetrics := metrics.NewNegMetrics("")
+	syncerMetrics := syncMetrics.NewNegMetricsCollector(flags.F.NegMetricsExportInterval, logger, negMetrics.ProviderConfigID)
 
 	// TODO: Refactor NEG to use cloud mocks so ctx.Cloud can be referenced within NewController.
 	negController, err := neg.NewController(
@@ -725,6 +731,7 @@ func createNEGController(ctx *ingctx.ControllerContext, systemHealth *systemheal
 		stopCh,
 		logger,
 		negMetrics,
+		syncerMetrics,
 	)
 	if err != nil {
 		return nil, fmt.Errorf("failed to create NEG controller: %w", err)

pkg/multiproject/neg/neg.go

@@ -17,6 +17,7 @@ import (
 	"k8s.io/ingress-gce/pkg/multiproject/filteredinformer"
 	"k8s.io/ingress-gce/pkg/neg"
 	"k8s.io/ingress-gce/pkg/neg/metrics"
+	syncMetrics "k8s.io/ingress-gce/pkg/neg/metrics/metricscollector"
 	"k8s.io/ingress-gce/pkg/neg/syncers/labels"
 	negtypes "k8s.io/ingress-gce/pkg/neg/types"
 	"k8s.io/ingress-gce/pkg/network"
@@ -29,6 +30,12 @@ import (
 	"k8s.io/klog/v2"
 )
 
+func init() {
+	// register prometheus metrics
+	metrics.RegisterMetrics()
+	syncMetrics.RegisterMetrics()
+}
+
 // StartNEGController creates and runs a NEG controller for the specified ProviderConfig.
 // The returned channel is closed by StopControllersForProviderConfig to signal a shutdown
 // specific to this ProviderConfig's controller.
@@ -294,7 +301,7 @@ func createNEGController(
 	noDefaultBackendServicePort := utils.ServicePort{}
 	var noNodeTopologyInformer cache.SharedIndexInformer
 	negMetrics := metrics.NewNegMetrics(providerConfigID)
-
+	syncerMetrics := syncMetrics.NewNegMetricsCollector(flags.F.NegMetricsExportInterval, logger, negMetrics.ProviderConfigID)
 	negController, err := neg.NewController(
 		kubeClient,
 		svcNegClient,
@@ -331,6 +338,7 @@ func createNEGController(
 		stopCh,
 		logger,
 		negMetrics,
+		syncerMetrics,
 	)
 	if err != nil {
 		return nil, fmt.Errorf("failed to create NEG controller: %w", err)

pkg/neg/controller.go

@@ -55,12 +55,6 @@ import (
 	"k8s.io/klog/v2"
 )
 
-func init() {
-	// register prometheus metrics
-	metrics.RegisterMetrics()
-	syncMetrics.RegisterMetrics()
-}
-
 // Controller is network endpoint group controller.
 // It determines whether NEG for a service port is needed, then signals NegSyncerManager to sync it.
 type Controller struct {
@@ -163,6 +157,7 @@ func NewController(
 	stopCh <-chan struct{},
 	logger klog.Logger,
 	negMetrics *metrics.NegMetrics,
+	syncerMetrics *syncMetrics.SyncerMetrics,
 ) (*Controller, error) {
 	if svcNegClient == nil {
 		return nil, fmt.Errorf("svcNegClient is nil")
@@ -188,8 +183,6 @@ func NewController(
 	recorder := eventBroadcaster.NewRecorder(negScheme,
 		apiv1.EventSource{Component: "neg-controller"})
 
-	syncerMetrics := syncMetrics.NewNegMetricsCollector(flags.F.NegMetricsExportInterval, logger, negMetrics.ProviderConfigID)
-
 	manager := newSyncerManager(
 		namer,
 		l4Namer,

pkg/neg/controller_test.go

@@ -165,6 +165,7 @@ func newTestControllerWithParamsAndContext(kubeClient kubernetes.Interface, test
 		make(<-chan struct{}),
 		klog.TODO(),
 		testContext.NegMetrics,
+		metricscollector.FakeSyncerMetrics(),
 	)
 }
 

pkg/neg/metrics/metrics.go

@@ -66,11 +66,10 @@ var (
 			Buckets: prometheus.ExponentialBuckets(1, 2, 13),
 		},
 		[]string{
-			"operation",         // endpoint operation
-			"neg_type",          // type of neg
-			"api_version",       // GCE API version
-			"result",            // result of the sync
-			"providerconfig_id", // provider config ID if multi-project NEG
+			"operation",   // endpoint operation
+			"neg_type",    // type of neg
+			"api_version", // GCE API version
+			"result",      // result of the sync
 		},
 	)
 
@@ -83,10 +82,9 @@ var (
 			Buckets: prometheus.ExponentialBuckets(1, 2, 13),
 		},
 		[]string{
-			"operation",         // endpoint operation
-			"neg_type",          // type of neg
-			"result",            // result of the sync
-			"providerconfig_id", // provider config ID if multi-project NEG
+			"operation", // endpoint operation
+			"neg_type",  // type of neg
+			"result",    // result of the sync
 		},
 	)
 
@@ -102,7 +100,6 @@ var (
 			"neg_type",                 //type of neg
 			"endpoint_calculator_mode", // type of endpoint calculator used
 			"result",                   // result of the sync
-			"providerconfig_id",        // provider config ID if multi-project NEG
 		},
 	)
 
@@ -115,23 +112,19 @@ var (
 			Buckets: prometheus.ExponentialBuckets(1, 2, 13),
 		},
 		[]string{
-			"process",           // type of manager process loop
-			"result",            // result of the process
-			"providerconfig_id", // provider config ID if multi-project NEG
+			"process", // type of manager process loop
+			"result",  // result of the process
 		},
 	)
 
-	InitializationLatency = prometheus.NewHistogramVec(
+	InitializationLatency = prometheus.NewHistogram(
 		prometheus.HistogramOpts{
 			Subsystem: negControllerSubsystem,
 			Name:      "neg_initialization_duration_seconds",
 			Help:      "Initialization latency of a NEG",
 			// custom buckets - [1s, 2s, 4s, 8s, 16s, 32s, 64s, 128s, 256s(~4min), 512s(~8min), 1024s(~17min), 2048 (~34min), 4096(~68min), +Inf]
 			Buckets: prometheus.ExponentialBuckets(1, 2, 13),
 		},
-		[]string{
-			"providerconfig_id", // provider config ID if multi-project NEG
-		},
 	)
 
 	LastSyncTimestamp = prometheus.NewGauge(
@@ -173,9 +166,8 @@ var (
 			Buckets: append([]float64{0}, prometheus.ExponentialBuckets(1, 2, 20)...),
 		},
 		[]string{
-			"neg_type",          // type of neg
-			"endpoint_type",     // type of endpoint
-			"providerconfig_id", // provider config ID if multi-project NEG
+			"neg_type",      // type of neg
+			"endpoint_type", // type of endpoint
 		},
 	)
 
@@ -187,9 +179,7 @@ var (
 			Name:      "error_count",
 			Help:      "Counts of server errors and NEG controller errors.",
 		},
-		[]string{"error_type",
-			"providerconfig_id", // provider config ID if multi-project NEG
-		},
+		[]string{"error_type"},
 	)
 
 	LabelNumber = prometheus.NewHistogram(
@@ -228,7 +218,7 @@ var (
 			Name:      "gce_request_count",
 			Help:      "Number of requests sent by NEG Controller to Arcus.",
 		},
-		[]string{"request", "result", "providerconfig_id"},
+		[]string{"request", "result"},
 	)
 
 	// GCERequestLatency tracks the latency of GCE requests the neg controller sends to the NEG API
@@ -240,7 +230,7 @@ var (
 			// custom buckets - [0.001, 0.01, 0.1, 1, 2, 4, 8, 16, 32, 64, 128, 256, 512, 1024, 2048, 4096, 8192, 16384, 32768, 65536, 131072, 262144, 524288, +Inf]
 			Buckets: append([]float64{0.001, 0.01, 0.1}, prometheus.ExponentialBuckets(1, 2, 20)...),
 		},
-		[]string{"request", "result", "providerconfig_id"},
+		[]string{"request", "result"},
 	)
 
 	// K8sRequestCount tracks the number of K8s requests the neg controller sends to the K8s API
@@ -250,7 +240,7 @@ var (
 			Name:      "k8s_request_count",
 			Help:      "Number of requests sent by NEG Controller to Kubernetes API Server.",
 		},
-		[]string{"request", "result", "providerconfig_id"},
+		[]string{"request", "result"},
 	)
 
 	// K8sRequestLatency tracks the latency of K8s requests the neg controller sends to the K8s API
@@ -262,7 +252,7 @@ var (
 			// custom buckets - [0.001, 0.01, 0.1, 1, 2, 4, 8, 16, 32, 64, 128, 256, 512, 1024, 2048, 4096, 8192, 16384, 32768, 65536, 131072, 262144, 524288, +Inf]
 			Buckets: append([]float64{0.001, 0.01, 0.1}, prometheus.ExponentialBuckets(1, 2, 20)...),
 		},
-		[]string{"request", "result", "providerconfig_id"},
+		[]string{"request", "result"},
 	)
 )
 
@@ -305,26 +295,26 @@ func NewNegMetrics(providerConfigID string) *NegMetrics {
 func (m *NegMetrics) PublishNegOperationMetrics(operation, negType, apiVersion string, err error, numEndpoints int, start time.Time) {
 	result := getResult(err)
 
-	NegOperationLatency.WithLabelValues(operation, negType, apiVersion, result, m.ProviderConfigID).Observe(time.Since(start).Seconds())
-	NegOperationEndpoints.WithLabelValues(operation, negType, result, m.ProviderConfigID).Observe(float64(numEndpoints))
+	NegOperationLatency.WithLabelValues(operation, negType, apiVersion, result).Observe(time.Since(start).Seconds())
+	NegOperationEndpoints.WithLabelValues(operation, negType, result).Observe(float64(numEndpoints))
 }
 
 // PublishNegSyncMetrics publishes collected metrics for the sync of NEG
 func (m *NegMetrics) PublishNegSyncMetrics(negType, endpointCalculator string, err error, start time.Time) {
 	result := getResult(err)
 
-	SyncerSyncLatency.WithLabelValues(negType, endpointCalculator, result, m.ProviderConfigID).Observe(time.Since(start).Seconds())
+	SyncerSyncLatency.WithLabelValues(negType, endpointCalculator, result).Observe(time.Since(start).Seconds())
 }
 
 // PublishNegManagerProcessMetrics publishes collected metrics for the neg manager loops
 func (m *NegMetrics) PublishNegManagerProcessMetrics(process string, err error, start time.Time) {
 	result := getResult(err)
-	ManagerProcessLatency.WithLabelValues(process, result, m.ProviderConfigID).Observe(time.Since(start).Seconds())
+	ManagerProcessLatency.WithLabelValues(process, result).Observe(time.Since(start).Seconds())
 }
 
 // PublishNegInitializationMetrics publishes collected metrics for time from request to initialization of NEG
 func (m *NegMetrics) PublishNegInitializationMetrics(latency time.Duration) {
-	InitializationLatency.WithLabelValues(m.ProviderConfigID).Observe(latency.Seconds())
+	InitializationLatency.Observe(latency.Seconds())
 }
 
 func PublishNegSyncerStalenessMetrics(syncerStaleness time.Duration) {
@@ -338,7 +328,7 @@ func PublishNegEPSStalenessMetrics(epsStaleness time.Duration) {
 // PublishDegradedModeCorrectnessMetrics publishes collected metrics
 // of the correctness of degraded mode calculations compared with the current one
 func (m *NegMetrics) PublishDegradedModeCorrectnessMetrics(count int, endpointType string, negType string) {
-	DegradeModeCorrectness.WithLabelValues(negType, endpointType, m.ProviderConfigID).Observe(float64(count))
+	DegradeModeCorrectness.WithLabelValues(negType, endpointType).Observe(float64(count))
 }
 
 // PublishNegControllerErrorCountMetrics publishes collected metrics
@@ -347,8 +337,8 @@ func (m *NegMetrics) PublishNegControllerErrorCountMetrics(err error, isIgnored
 	if err == nil {
 		return
 	}
-	NegControllerErrorCount.WithLabelValues(totalNegError, m.ProviderConfigID).Inc()
-	NegControllerErrorCount.WithLabelValues(getErrorLabel(err, isIgnored), m.ProviderConfigID).Inc()
+	NegControllerErrorCount.WithLabelValues(totalNegError).Inc()
+	NegControllerErrorCount.WithLabelValues(getErrorLabel(err, isIgnored)).Inc()
 }
 
 // PublishLabelPropagationError publishes error occured during label propagation.
@@ -374,8 +364,8 @@ func (m *NegMetrics) PublishGCERequestCountMetrics(start time.Time, requestType
 			result = otherError
 		}
 	}
-	GCERequestLatency.WithLabelValues(requestType, result, m.ProviderConfigID).Observe(time.Since(start).Seconds())
-	GCERequestCount.WithLabelValues(requestType, result, m.ProviderConfigID).Inc()
+	GCERequestLatency.WithLabelValues(requestType, result).Observe(time.Since(start).Seconds())
+	GCERequestCount.WithLabelValues(requestType, result).Inc()
 }
 
 // PublishK8sRequestCountMetrics publishes collected metrics for K8s Request Counts
@@ -390,8 +380,8 @@ func (m *NegMetrics) PublishK8sRequestCountMetrics(start time.Time, requestType
 			result = otherError
 		}
 	}
-	K8sRequestLatency.WithLabelValues(requestType, result, m.ProviderConfigID).Observe(time.Since(start).Seconds())
-	K8sRequestCount.WithLabelValues(requestType, result, m.ProviderConfigID).Inc()
+	K8sRequestLatency.WithLabelValues(requestType, result).Observe(time.Since(start).Seconds())
+	K8sRequestCount.WithLabelValues(requestType, result).Inc()
 }
 
 func getResult(err error) string {

pkg/neg/metrics/metricscollector/metrics.go

@@ -88,7 +88,7 @@ var (
 			Name:      "number_of_endpoints",
 			Help:      "The total number of endpoints",
 		},
-		[]string{"feature", "provider_config_id"},
+		[]string{"feature"},
 	)
 
 	DualStackMigrationFinishedDurations = prometheus.NewHistogram(
@@ -116,7 +116,7 @@ var (
 			Name:      "syncer_count_by_endpoint_type",
 			Help:      "Number of Syncers managing NEGs containing endpoint of a particular kind",
 		},
-		[]string{"endpoint_type", "provider_config_id"},
+		[]string{"endpoint_type"},
 	)
 
 	DualStackMigrationServiceCount = prometheus.NewGauge(
@@ -134,7 +134,7 @@ var (
 			Name:      "sync_result",
 			Help:      "Current count for each sync result",
 		},
-		[]string{"result", "provider_config_id"},
+		[]string{"result"},
 	)
 
 	negsManagedCount = prometheus.NewGaugeVec(
@@ -143,14 +143,14 @@ var (
 			Name:      "managed_neg_count",
 			Help:      "Number of NEGs the Neg Controller Manages",
 		},
-		[]string{"location", "endpoint_type", "provider_config_id"},
+		[]string{"location", "endpoint_type"},
 	)
 
 	networkEndpointGroupCount = prometheus.NewGaugeVec(
 		prometheus.GaugeOpts{
 			Name: "number_of_negs",
 			Help: "Number of NEGs",
 		},
-		[]string{"feature", "provider_config_id"},
+		[]string{"feature"},
 	)
 )

pkg/neg/metrics/metricscollector/metrics_collector.go

@@ -133,8 +133,8 @@ func (sm *SyncerMetrics) Run(stopCh <-chan struct{}) {
 // export exports syncer metrics.
 func (sm *SyncerMetrics) export() {
 	lpMetrics := sm.computeLabelMetrics()
-	NumberOfEndpoints.WithLabelValues(totalEndpoints, sm.providerConfigID).Set(float64(lpMetrics.NumberOfEndpoints))
-	NumberOfEndpoints.WithLabelValues(epWithAnnotation, sm.providerConfigID).Set(float64(lpMetrics.EndpointsWithAnnotation))
+	NumberOfEndpoints.WithLabelValues(totalEndpoints).Set(float64(lpMetrics.NumberOfEndpoints))
+	NumberOfEndpoints.WithLabelValues(epWithAnnotation).Set(float64(lpMetrics.EndpointsWithAnnotation))
 
 	stateCount, syncerCount := sm.computeSyncerStateMetrics()
 	//Reset metric so non-existent keys are now 0
@@ -155,7 +155,7 @@ func (sm *SyncerMetrics) export() {
 	//Clear existing metrics (ensures that keys that don't exist anymore are reset)
 	negsManagedCount.Reset()
 	for key, count := range negCounts {
-		negsManagedCount.WithLabelValues(key.location, key.endpointType, sm.providerConfigID).Set(float64(count))
+		negsManagedCount.WithLabelValues(key.location, key.endpointType).Set(float64(count))
 	}
 
 	sm.logger.V(3).Info("Exporting syncer related metrics", "Syncer count", syncerCount,
@@ -173,7 +173,7 @@ func (sm *SyncerMetrics) export() {
 
 	syncerCountByEndpointType, migrationEndpointCount, migrationServicesCount := sm.computeDualStackMigrationCounts()
 	for endpointType, count := range syncerCountByEndpointType {
-		SyncerCountByEndpointType.WithLabelValues(endpointType, sm.providerConfigID).Set(float64(count))
+		SyncerCountByEndpointType.WithLabelValues(endpointType).Set(float64(count))
 	}
 	syncerEndpointState.WithLabelValues(string(negtypes.DualStackMigration)).Set(float64(migrationEndpointCount))
 	DualStackMigrationServiceCount.Set(float64(migrationServicesCount))
@@ -182,7 +182,7 @@ func (sm *SyncerMetrics) export() {
 
 	negCount := sm.computeNegMetrics()
 	for feature, count := range negCount {
-		networkEndpointGroupCount.WithLabelValues(feature.String(), sm.providerConfigID).Set(float64(count))
+		networkEndpointGroupCount.WithLabelValues(feature.String()).Set(float64(count))
 	}
 	sm.logger.V(3).Info("Exported NEG usage metrics", "NEG count", fmt.Sprintf("%#v", negCount))
 }
@@ -194,7 +194,7 @@ func (sm *SyncerMetrics) UpdateSyncerStatusInMetrics(key negtypes.NegSyncerKey,
 		syncErr := negtypes.ClassifyError(err)
 		reason = syncErr.Reason
 	}
-	syncerSyncResult.WithLabelValues(string(reason), sm.providerConfigID).Inc()
+	syncerSyncResult.WithLabelValues(string(reason)).Inc()
 	sm.mu.Lock()
 	defer sm.mu.Unlock()
 	if sm.syncerStateMap == nil {