Keep track of exactly which collectors are registered for logging warnings to unsupported volumes.

Change-Id: Ie7ce9cf87e47410afb2493f0a0de8ab6d0c3e34b

Author: hime

pkg/metrics/metrics.go

@@ -29,6 +29,8 @@ import (
 	"sync"
 	"time"
 
+	"k8s.io/apimachinery/pkg/util/sets"
+
 	"github.com/googlecloudplatform/gcs-fuse-csi-driver/pkg/cloud_provider/clientset"
 	"github.com/googlecloudplatform/gcs-fuse-csi-driver/pkg/util"
 	"github.com/prometheus/client_golang/prometheus"
@@ -59,7 +61,7 @@ type manager struct {
 	clientset       clientset.Interface
 
 	maximumNumberOfCollectors int
-	registeredCollectorsCount int
+	volumesRegistered         sets.Set[string]
 	mutex                     sync.Mutex
 }
 
@@ -69,6 +71,7 @@ func NewMetricsManager(metricsEndpoint, fuseSocketDir string, maximumNumberOfCol
 		metricsEndpoint:           metricsEndpoint,
 		fuseSocketDir:             fuseSocketDir,
 		clientset:                 clientset,
+		volumesRegistered:         sets.Set[string]{},
 		maximumNumberOfCollectors: maximumNumberOfCollectors,
 		mutex:                     sync.Mutex{},
 	}
@@ -123,24 +126,36 @@ func (mm *manager) RegisterMetricsCollector(targetPath, podNamespace, podName, b
 		"pod_uid":        podUID,
 	}, mm.clientset)
 
-	// Lock the number of registered collectors while we attemtp to register a new collector.
+	// Lock the number of registered collectors while we attempt to register a new collector.
 	mm.mutex.Lock()
 	defer mm.mutex.Unlock()
 
-	// We skip registration when we already registered all supported metrics collectors. If a limit is
-	// unset (maximumNumberOfCollectors less than zero), we continue to registration.
-	if mm.maximumNumberOfCollectors >= 0 && mm.registeredCollectorsCount >= mm.maximumNumberOfCollectors {
-		return
+	// Check if we need to register collector. We register a collector when the following are met:
+	// 1. There is space on the metrics pipeline for the collector to be registered.
+	// 2. The metrics collector has not previously been registered.
+	if mm.maximumNumberOfCollectors >= 0 {
+		// If volume is already registered, do not register again. This flow can get triggered
+		//  since CSI driver has republishVolume capability.
+		if mm.volumesRegistered.Has(targetPath) {
+			return
+		}
+		// If collector hasn't been registered and there's no space left, log a warning.
+		if mm.volumesRegistered.Len() >= mm.maximumNumberOfCollectors {
+			klog.Infof("could not register a metrics collector: podUID: %s, volume: %s. there's already %d collectors registered.", podUID, bucketName, mm.volumesRegistered.Len())
+
+			return
+		}
 	}
 
+	// Attempt to register new metrics collector and record success.
 	err = mm.registry.Register(c)
 	if err != nil {
 		if !strings.Contains(err.Error(), prometheus.AlreadyRegisteredError{}.Error()) {
 			klog.Errorf("failed to register metrics collector for pod  %v/%v, volume %q, bucket %q: %v", podNamespace, podName, volumeName, bucketName, err)
 		}
 	} else {
-		mm.registeredCollectorsCount += 1
-		klog.Infof("successfully registered a new metrics collector: podUID: %s, volume: %s. there's %d collectors registerd.", podUID, bucketName, mm.registeredCollectorsCount)
+		mm.volumesRegistered.Insert(targetPath)
+		klog.Infof("successfully registered a new metrics collector: podUID: %s, volume: %s. there's %d collectors registered.", podUID, bucketName, mm.volumesRegistered.Len())
 	}
 }
 
@@ -158,8 +173,8 @@ func (mm *manager) UnregisterMetricsCollector(targetPath string) {
 	if ok := mm.registry.Unregister(c); !ok {
 		klog.Infof("Unregister metrics collector for targetPath %q is not needed since the collector is not registered", targetPath)
 	} else {
-		mm.registeredCollectorsCount -= 1
-		klog.Infof("successfully unregistered a metrics collector: podUID: %s, volume: %s. there's %d collectors registerd.", podUID, volumeName, mm.registeredCollectorsCount)
+		mm.volumesRegistered.Delete(targetPath)
+		klog.Infof("successfully unregistered a metrics collector: podUID: %s, volume: %s. there's %d collectors registered.", podUID, volumeName, mm.volumesRegistered.Len())
 	}
 }