generic ephemeral volume: simpler metrics

A CounterVector with status as label may create unnecessary overhead
and using the success case with the empty label value wasn't
easy. It's better to have two seperate counters, one for total number
of calls and one for failed calls.

pkg/controller/volume/ephemeral/controller.go

@@ -25,7 +25,6 @@ import (
 
 	v1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/api/errors"
-	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/util/runtime"
 	"k8s.io/apimachinery/pkg/util/wait"
@@ -282,16 +281,10 @@ func (ec *ephemeralController) handleVolume(pod *v1.Pod, vol v1.Volume) error {
 		},
 		Spec: ephemeral.VolumeClaimTemplate.Spec,
 	}
+	ephemeralvolumemetrics.EphemeralVolumeCreateAttempts.Inc()
 	_, err = ec.kubeClient.CoreV1().PersistentVolumeClaims(pod.Namespace).Create(context.TODO(), pvc, metav1.CreateOptions{})
-	reason := ""
-	if err != nil {
-		reason = string(apierrors.ReasonForError(err))
-		if reason == "" {
-			reason = "Unknown"
-		}
-	}
-	ephemeralvolumemetrics.EphemeralVolumeCreate.WithLabelValues(reason).Inc()
 	if err != nil {
+		ephemeralvolumemetrics.EphemeralVolumeCreateFailures.Inc()
 		return fmt.Errorf("create PVC %s: %v", pvcName, err)
 	}
 	return nil

pkg/controller/volume/ephemeral/controller_test.go

@@ -22,7 +22,7 @@ import (
 	"sort"
 	"testing"
 
-	"k8s.io/api/core/v1"
+	v1 "k8s.io/api/core/v1"
 	// storagev1 "k8s.io/api/storage/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	// "k8s.io/apimachinery/pkg/types"
@@ -116,7 +116,7 @@ func TestSyncHandler(t *testing.T) {
 			name:            "create-conflict",
 			pods:            []*v1.Pod{testPodWithEphemeral},
 			podKey:          podKey(testPodWithEphemeral),
-			expectedMetrics: expectedMetrics{0, 1},
+			expectedMetrics: expectedMetrics{1, 1},
 			expectedError:   true,
 		},
 	}
@@ -139,7 +139,7 @@ func TestSyncHandler(t *testing.T) {
 			}
 
 			fakeKubeClient := createTestClient(objects...)
-			if tc.expectedMetrics.numConflicts > 0 {
+			if tc.expectedMetrics.numFailures > 0 {
 				fakeKubeClient.PrependReactor("create", "persistentvolumeclaims", func(action k8stesting.Action) (handled bool, ret runtime.Object, err error) {
 					return true, nil, apierrors.NewConflict(action.GetResource().GroupResource(), "fake name", errors.New("fake conflict"))
 				})
@@ -245,22 +245,22 @@ func createTestClient(objects ...runtime.Object) *fake.Clientset {
 // Metrics helpers
 
 type expectedMetrics struct {
-	numCreated   int
+	numCreated  int
-	numConflicts int
+	numFailures int
 }
 
 func expectMetrics(t *testing.T, em expectedMetrics) {
 	t.Helper()
 
-	actualCreated, err := testutil.GetCounterMetricValue(ephemeralvolumemetrics.EphemeralVolumeCreate.WithLabelValues(""))
+	actualCreated, err := testutil.GetCounterMetricValue(ephemeralvolumemetrics.EphemeralVolumeCreateAttempts)
 	handleErr(t, err, "ephemeralVolumeCreate")
 	if actualCreated != float64(em.numCreated) {
 		t.Errorf("Expected PVCs to be created %d, got %v", em.numCreated, actualCreated)
 	}
-	actualConflicts, err := testutil.GetCounterMetricValue(ephemeralvolumemetrics.EphemeralVolumeCreate.WithLabelValues("Conflict"))
+	actualConflicts, err := testutil.GetCounterMetricValue(ephemeralvolumemetrics.EphemeralVolumeCreateFailures)
 	handleErr(t, err, "ephemeralVolumeCreate/Conflict")
-	if actualConflicts != float64(em.numConflicts) {
+	if actualConflicts != float64(em.numFailures) {
-		t.Errorf("Expected PVCs to have conflicts %d, got %v", em.numConflicts, actualConflicts)
+		t.Errorf("Expected PVCs to have conflicts %d, got %v", em.numFailures, actualConflicts)
 	}
 }
 
@@ -272,6 +272,6 @@ func handleErr(t *testing.T, err error, metricName string) {
 
 func setupMetrics() {
 	ephemeralvolumemetrics.RegisterMetrics()
-	ephemeralvolumemetrics.EphemeralVolumeCreate.Delete(map[string]string{"reason": ""})
+	ephemeralvolumemetrics.EphemeralVolumeCreateAttempts.Reset()
-	ephemeralvolumemetrics.EphemeralVolumeCreate.Delete(map[string]string{"reason": "Conflict"})
+	ephemeralvolumemetrics.EphemeralVolumeCreateFailures.Reset()
 }

pkg/controller/volume/ephemeral/metrics/metrics.go

@@ -27,20 +27,24 @@ import (
 const EphemeralVolumeSubsystem = "ephemeral_volume_controller"
 
 var (
-	// EphemeralVolumeCreate tracks the number of
+	// EphemeralVolumeCreateAttempts tracks the number of
-	// PersistentVolumeClaims().Create calls for each failure
+	// PersistentVolumeClaims().Create calls (both successful and unsuccessful)
-	// reason
+	EphemeralVolumeCreateAttempts = metrics.NewCounter(
-	// (https://pkg.go.dev/k8s.io/apimachinery@v0.20.2/pkg/apis/meta/v1#StatusReason),
-	// with empty for successful calls.
-	EphemeralVolumeCreate = metrics.NewCounterVec(
 		&metrics.CounterOpts{
 			Subsystem:      EphemeralVolumeSubsystem,
-			Name:           "create",
+			Name:           "create_total",
 			Help:           "Number of PersistenVolumeClaims creation requests",
 			StabilityLevel: metrics.ALPHA,
-		},
+		})
-		[]string{"reason"},
+	// EphemeralVolumeCreateFailures tracks the number of unsuccessful
-	)
+	// PersistentVolumeClaims().Create calls
+	EphemeralVolumeCreateFailures = metrics.NewCounter(
+		&metrics.CounterOpts{
+			Subsystem:      EphemeralVolumeSubsystem,
+			Name:           "create_failures_total",
+			Help:           "Number of PersistenVolumeClaims creation requests",
+			StabilityLevel: metrics.ALPHA,
+		})
 )
 
 var registerMetrics sync.Once
@@ -48,6 +52,7 @@ var registerMetrics sync.Once
 // RegisterMetrics registers EphemeralVolume metrics.
 func RegisterMetrics() {
 	registerMetrics.Do(func() {
-		legacyregistry.MustRegister(EphemeralVolumeCreate)
+		legacyregistry.MustRegister(EphemeralVolumeCreateAttempts)
+		legacyregistry.MustRegister(EphemeralVolumeCreateFailures)
 	})
 }