Repository: koordinator-sh/koordinator

Issue:
[proposal] add metrics for reservation
<!-- Please only use this template for submitting proposal -->

**What is your proposal**:
add metrics for reservation

**Why is this needed**:
When using the reservation feature, our SRE wants to focus on the metrics about the reservation status. We don’t want too many reservation failures. We need metrics monitoring

**Is there a suggested solution, if so, please add it**:

like this below: 
```go
        ReservationFailedTotal = metrics.NewCounterVec(
		&metrics.CounterOpts{
			Subsystem: schedulermetrics.SchedulerSubsystem,
			Name:      "reservation_failed_total",
			Help:      `The total number of failed Reservation`,
		}, []string{"name"},
	)

	ReservationSucceededTotal = metrics.NewCounterVec(
		&metrics.CounterOpts{
			Subsystem: schedulermetrics.SchedulerSubsystem,
			Name:      "reservation_successed_total",
			Help:      `The total number of successed Reservation`,
		}, []string{"name"},
	)
```

Hints from issue comments:
@googs1025: /assign@googs1025: In addition, I think whether other features also need to increase metrics, this may be discussed. 🤔 

Solution patch:
diff --git a/pkg/scheduler/metrics/metrics.go b/pkg/scheduler/metrics/metrics.go
index 073f6b75..d2640d57 100644
--- a/pkg/scheduler/metrics/metrics.go
+++ b/pkg/scheduler/metrics/metrics.go
@@ -20,9 +20,13 @@ import (
 	"sync"
 	"time"
 
+	"github.com/prometheus/client_golang/prometheus"
+
 	"k8s.io/component-base/metrics"
 	"k8s.io/component-base/metrics/legacyregistry"
 	schedulermetrics "k8s.io/kubernetes/pkg/scheduler/metrics"
+
+	utilmetrics "github.com/koordinator-sh/koordinator/pkg/util/metrics"
 )
 
 // All the histogram based metrics have 1ms as size for the smallest bucket.
@@ -34,7 +38,12 @@ var (
 			Help:           "The currently scheduled Pod exceeds the maximum acceptable time interval",
 			StabilityLevel: metrics.STABLE,
 		}, []string{"profile"})
-
+	ReservationStatusPhase = utilmetrics.NewGCGaugeVec("reservation_status_phase", prometheus.NewGaugeVec(
+		prometheus.GaugeOpts{
+			Subsystem: schedulermetrics.SchedulerSubsystem,
+			Name:      "reservation_status_phase",
+			Help:      `The current number of reservations in each status phase (e.g. Pending, Available, Succeeded, Failed)`,
+		}, []string{"name", "phase"}))
 	ElasticQuotaProcessLatency = metrics.NewHistogramVec(
 		&metrics.HistogramOpts{
 			Subsystem: schedulermetrics.SchedulerSubsystem,
@@ -49,8 +58,27 @@ var (
 		SchedulingTimeout,
 		ElasticQuotaProcessLatency,
 	}
+
+	gcMetricsList = []prometheus.Collector{
+		ReservationStatusPhase.GetGaugeVec(),
+	}
+)
+
+const (
+	reservationNameKey  = "name"
+	reservationPhaseKey = "phase"
 )
 
+// RecordReservationPhase records the phase of a reservation as a metric.
+// It uses the provided name, phase, and value to set the metric with specific labels.
+func RecordReservationPhase(name string, phase string, value float64) {
+	labels := prometheus.Labels{
+		reservationNameKey:  name,
+		reservationPhaseKey: phase,
+	}
+	ReservationStatusPhase.WithSet(labels, value)
+}
+
 var registerMetrics sync.Once
 
 // Register all metrics.
@@ -58,6 +86,7 @@ func Register() {
 	// Register the metrics.
 	registerMetrics.Do(func() {
 		RegisterMetrics(metricsList...)
+		RegisterGCMetrics(gcMetricsList...)
 	})
 }
 
@@ -69,6 +98,13 @@ func RegisterMetrics(extraMetrics ...metrics.Registerable) {
 	}
 }
 
+// RegisterGCMetrics registers garbage collection metrics.
+func RegisterGCMetrics(gcMetrics ...prometheus.Collector) {
+	for _, metric := range gcMetrics {
+		legacyregistry.RawMustRegister(metric)
+	}
+}
+
 func RecordElasticQuotaProcessLatency(operation string, latency time.Duration) {
 	ElasticQuotaProcessLatency.WithLabelValues(operation).Observe(latency.Seconds())
 }
diff --git a/pkg/scheduler/plugins/reservation/controller/controller.go b/pkg/scheduler/plugins/reservation/controller/controller.go
index 9d393c59..e9681d25 100644
--- a/pkg/scheduler/plugins/reservation/controller/controller.go
+++ b/pkg/scheduler/plugins/reservation/controller/controller.go
@@ -44,6 +44,7 @@ import (
 	"github.com/koordinator-sh/koordinator/pkg/scheduler/apis/config"
 	"github.com/koordinator-sh/koordinator/pkg/scheduler/frameworkext"
 	frameworkexthelper "github.com/koordinator-sh/koordinator/pkg/scheduler/frameworkext/helper"
+	"github.com/koordinator-sh/koordinator/pkg/scheduler/metrics"
 	reservationutil "github.com/koordinator-sh/koordinator/pkg/util/reservation"
 )
 
@@ -185,15 +186,6 @@ func (c *Controller) sync(reservationName string) (result, error) {
 	}
 
 	reservation = reservation.DeepCopy()
-
-	if isReservationNeedExpiration(reservation) {
-		return result{}, c.expireReservation(reservation)
-	}
-
-	if reservation.Status.NodeName != "" && missingNode(reservation, c.nodeLister) {
-		return result{}, c.expireReservation(reservation)
-	}
-
 	if err := c.syncStatus(reservation); err != nil {
 		return result{}, err
 	}
@@ -203,14 +195,23 @@ func (c *Controller) sync(reservationName string) (result, error) {
 
 func (c *Controller) expireReservation(reservation *schedulingv1alpha1.Reservation) error {
 	reservationutil.SetReservationExpired(reservation)
-	_, err := c.koordClientSet.SchedulingV1alpha1().Reservations().UpdateStatus(context.TODO(), reservation, metav1.UpdateOptions{})
-	return err
+	return c.updateReservationStatus(reservation)
 }
 
 func (c *Controller) syncStatus(reservation *schedulingv1alpha1.Reservation) error {
+	if isReservationNeedExpiration(reservation) {
+		return c.expireReservation(reservation)
+	}
+
+	if reservation.Status.NodeName != "" && missingNode(reservation, c.nodeLister) {
+		return c.expireReservation(reservation)
+	}
+
 	if reservation.Status.NodeName == "" {
+		RecordReservationPhases(reservation)
 		return nil
 	}
+
 	var actualOwners []corev1.ObjectReference
 	var actualAllocated corev1.ResourceList
 	pods := c.getPods(reservation.Status.NodeName)
@@ -248,6 +249,11 @@ func (c *Controller) syncStatus(reservation *schedulingv1alpha1.Reservation) err
 		reservationutil.SetReservationSucceeded(reservation)
 	}
 
+	return c.updateReservationStatus(reservation)
+}
+
+func (c *Controller) updateReservationStatus(reservation *schedulingv1alpha1.Reservation) error {
+	RecordReservationPhases(reservation)
 	_, err := c.koordClientSet.SchedulingV1alpha1().Reservations().UpdateStatus(context.TODO(), reservation, metav1.UpdateOptions{})
 	if err == nil {
 		klog.V(4).InfoS("Successfully sync reservation status", "reservation", klog.KObj(reservation))
@@ -289,3 +295,38 @@ func nextSyncTime(r *schedulingv1alpha1.Reservation) time.Duration {
 	}
 	return duration
 }
+
+// RecordReservationPhases records all possible phases of a reservation as metrics.
+// For each phase, it sets the value to 1.0 if it matches the current phase of the reservation,
+// otherwise, it sets the value to 0.0.
+func RecordReservationPhases(reservation *schedulingv1alpha1.Reservation) {
+	allPhases := []struct {
+		name string
+	}{
+		{string(schedulingv1alpha1.ReservationPending)},
+		{string(schedulingv1alpha1.ReservationAvailable)},
+		{string(schedulingv1alpha1.ReservationSucceeded)},
+		{string(schedulingv1alpha1.ReservationFailed)},
+	}
+
+	boolFloat64 := func(b bool) float64 {
+		if b {
+			return 1.0
+		}
+		return 0.0
+	}
+
+	currentPhase := reservation.Status.Phase
+	for _, phase := range allPhases {
+		isCurrentPhase := false
+		if currentPhase == "" {
+			// If the reservation doesn't have a phase yet,
+			// consider the pending phase to be the current phase.
+			isCurrentPhase = phase.name == string(schedulingv1alpha1.ReservationPending)
+		} else {
+			isCurrentPhase = currentPhase == schedulingv1alpha1.ReservationPhase(phase.name)
+		}
+		// Record the phase with a value of 1.0 if it's the current phase, otherwise 0.0.
+		metrics.RecordReservationPhase(reservation.Name, phase.name, boolFloat64(isCurrentPhase))
+	}
+}



Test patch:
diff --git a/pkg/scheduler/plugins/reservation/controller/controller_test.go b/pkg/scheduler/plugins/reservation/controller/controller_test.go
index 3fec1375..f4120389 100644
--- a/pkg/scheduler/plugins/reservation/controller/controller_test.go
+++ b/pkg/scheduler/plugins/reservation/controller/controller_test.go
@@ -20,6 +20,7 @@ import (
 	"context"
 	"fmt"
 	"sort"
+	"strings"
 	"testing"
 	"time"
 
@@ -31,6 +32,8 @@ import (
 	"k8s.io/apimachinery/pkg/util/uuid"
 	"k8s.io/client-go/informers"
 	kubefake "k8s.io/client-go/kubernetes/fake"
+	basemetrics "k8s.io/component-base/metrics"
+	"k8s.io/component-base/metrics/testutil"
 	"k8s.io/utils/pointer"
 
 	apiext "github.com/koordinator-sh/koordinator/apis/extension"
@@ -38,6 +41,7 @@ import (
 	koordfake "github.com/koordinator-sh/koordinator/pkg/client/clientset/versioned/fake"
 	koordinformers "github.com/koordinator-sh/koordinator/pkg/client/informers/externalversions"
 	"github.com/koordinator-sh/koordinator/pkg/scheduler/apis/config"
+	"github.com/koordinator-sh/koordinator/pkg/scheduler/metrics"
 	reservationutil "github.com/koordinator-sh/koordinator/pkg/util/reservation"
 )
 
@@ -217,6 +221,10 @@ func TestSyncStatus(t *testing.T) {
 	fakeKoordClientSet := koordfake.NewSimpleClientset()
 	sharedInformerFactory := informers.NewSharedInformerFactory(fakeClientSet, 0)
 	koordSharedInformerFactory := koordinformers.NewSharedInformerFactory(fakeKoordClientSet, 0)
+	// register metrics
+	metricsRegistry := basemetrics.NewKubeRegistry()
+	metrics.ReservationStatusPhase.GetGaugeVec().Reset()
+	metricsRegistry.Registerer().MustRegister(metrics.ReservationStatusPhase.GetGaugeVec())
 
 	reservation := &schedulingv1alpha1.Reservation{
 		ObjectMeta: metav1.ObjectMeta{
@@ -298,6 +306,7 @@ func TestSyncStatus(t *testing.T) {
 	assert.NoError(t, err)
 
 	expectReservation := reservation.DeepCopy()
+	expectReservation.Status.Phase = schedulingv1alpha1.ReasonReservationSucceeded
 	reservationutil.SetReservationSucceeded(expectReservation)
 	expectReservation.Status.Allocated = corev1.ResourceList{
 		corev1.ResourceCPU:    resource.MustParse("8000m"),
@@ -318,4 +327,25 @@ func TestSyncStatus(t *testing.T) {
 		cond.LastTransitionTime = metav1.Time{}
 	}
 	assert.Equal(t, expectReservation, got)
+	// check metrics
+	expectAvailableMetricCount := 0
+	expectFailedMetricCount := 0
+	expectPendingMetricCount := 0
+	expectSucceededMetricCount := 1
+	expectedSuccessedMetrics := fmt.Sprintf(`
+				# HELP scheduler_reservation_status_phase The current number of reservations in each status phase (e.g. Pending, Available, Succeeded, Failed)
+				# TYPE scheduler_reservation_status_phase gauge
+				scheduler_reservation_status_phase{name="%s",phase="Available"} %v
+				scheduler_reservation_status_phase{name="%s",phase="Failed"} %v
+				scheduler_reservation_status_phase{name="%s",phase="Pending"} %v
+				scheduler_reservation_status_phase{name="%s",phase="Succeeded"} %v
+				`,
+		"normalReservation", expectAvailableMetricCount,
+		"normalReservation", expectFailedMetricCount,
+		"normalReservation", expectPendingMetricCount,
+		"normalReservation", expectSucceededMetricCount,
+	)
+	if err := testutil.GatherAndCompare(metricsRegistry, strings.NewReader(expectedSuccessedMetrics), "scheduler_reservation_status_phase"); err != nil {
+		t.Error(err)
+	}
 }