Resolve comments

Signed-off-by: win5923 <[email protected]>

Author: win5923

ray-operator/controllers/ray/metrics/ray_cluster_metrics.go

@@ -4,24 +4,35 @@ import (
 	"strconv"
 
 	"github.com/prometheus/client_golang/prometheus"
-	"github.com/prometheus/client_golang/prometheus/promauto"
 	"sigs.k8s.io/controller-runtime/pkg/metrics"
+
+	rayv1 "github.com/ray-project/kuberay/ray-operator/apis/ray/v1"
+	"github.com/ray-project/kuberay/ray-operator/controllers/ray/utils"
 )
 
-var rayClustersCreatedCounter = promauto.NewCounterVec(
+var rayClustersCreatedCounter = prometheus.NewCounterVec(
 	prometheus.CounterOpts{
 		Name: "ray_clusters_created_total",
 		Help: "The total number of RayClusters created",
 	},
 	[]string{"namespace", "created_by_ray_job", "created_by_ray_service"},
 )
 
-// CreatedRayClustersCounterInc increments the counter for RayClusters created
-func CreatedRayClustersCounterInc(namespace string, createdByRayJob bool, createdByRayService bool) {
-	rayClustersCreatedCounter.WithLabelValues(namespace, strconv.FormatBool(createdByRayJob), strconv.FormatBool(createdByRayService)).Inc()
-}
-
 func registerRayClusterMetrics() {
 	// Register custom metrics with the global prometheus registry
 	metrics.Registry.MustRegister(rayClustersCreatedCounter)
 }
+
+// CreatedRayClustersCounterInc increments the counter for ray_clusters_created_total metric based on the creator CRD type.
+func CreatedRayClustersCounterInc(enabled bool, namespace string, instance *rayv1.RayCluster) {
+	if !enabled {
+		return
+	}
+
+	creatorCRDType := utils.GetCRDType(instance.Labels[utils.RayOriginatedFromCRDLabelKey])
+
+	isfromRayJob := creatorCRDType == utils.RayJobCRD
+	isfromRayService := creatorCRDType == utils.RayServiceCRD
+
+	rayClustersCreatedCounter.WithLabelValues(namespace, strconv.FormatBool(isfromRayJob), strconv.FormatBool(isfromRayService)).Inc()
+}

ray-operator/controllers/ray/metrics/ray_cluster_metrics_test.go

@@ -5,20 +5,38 @@ import (
 	"testing"
 
 	"github.com/prometheus/client_golang/prometheus/testutil"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+
+	rayv1 "github.com/ray-project/kuberay/ray-operator/apis/ray/v1"
+	"github.com/ray-project/kuberay/ray-operator/controllers/ray/utils"
 )
 
 func TestCreatedRayClustersCounterInc(t *testing.T) {
-	CreatedRayClustersCounterInc("default", true, false)
-	CreatedRayClustersCounterInc("default", false, true)
-	CreatedRayClustersCounterInc("test", false, false)
-	CreatedRayClustersCounterInc("test", false, false)
+	CreatedRayClustersCounterInc(true, "default", &rayv1.RayCluster{})
+	CreatedRayClustersCounterInc(true, "default", &rayv1.RayCluster{})
+	CreatedRayClustersCounterInc(true, "test", &rayv1.RayCluster{})
+	CreatedRayClustersCounterInc(true, "test", &rayv1.RayCluster{
+		ObjectMeta: metav1.ObjectMeta{
+			Labels: map[string]string{
+				utils.RayOriginatedFromCRDLabelKey: "RayJob",
+			},
+		},
+	})
+	CreatedRayClustersCounterInc(true, "test", &rayv1.RayCluster{
+		ObjectMeta: metav1.ObjectMeta{
+			Labels: map[string]string{
+				utils.RayOriginatedFromCRDLabelKey: "RayService",
+			},
+		},
+	})
 
 	expected := `
 	# HELP ray_clusters_created_total The total number of RayClusters created
 	# TYPE ray_clusters_created_total counter
-	ray_clusters_created_total{created_by_ray_job="true",created_by_ray_service="false",namespace="default"} 1
-	ray_clusters_created_total{created_by_ray_job="false",created_by_ray_service="true",namespace="default"} 1
-	ray_clusters_created_total{created_by_ray_job="false",created_by_ray_service="false",namespace="test"} 2
+	ray_clusters_created_total{created_by_ray_job="false",created_by_ray_service="false",namespace="default"} 2
+	ray_clusters_created_total{created_by_ray_job="false",created_by_ray_service="false",namespace="test"} 1
+	ray_clusters_created_total{created_by_ray_job="true",created_by_ray_service="false",namespace="test"} 1
+	ray_clusters_created_total{created_by_ray_job="false",created_by_ray_service="true",namespace="test"} 1
 	`
 	if err := testutil.CollectAndCompare(rayClustersCreatedCounter, strings.NewReader(expected)); err != nil {
 		t.Errorf("unexpected collecting result:\n%s", err)

ray-operator/controllers/ray/raycluster_controller.go

@@ -740,21 +740,11 @@ func (r *RayClusterReconciler) reconcilePods(ctx context.Context, instance *rayv
 		// Create head Pod if it does not exist.
 		logger.Info("reconcilePods: Found 0 head Pods; creating a head Pod for the RayCluster.")
 
-		if r.EnableMetrics {
-			creatorCRDType := getCreatorCRDType(*instance)
-			// Increment the counter for ray_clusters_created_total metric based on the creator CRD type.
-			if creatorCRDType == utils.RayClusterCRD {
-				metrics.CreatedRayClustersCounterInc(instance.Namespace, false, false)
-			} else if creatorCRDType == utils.RayJobCRD {
-				metrics.CreatedRayClustersCounterInc(instance.Namespace, true, false)
-			} else if creatorCRDType == utils.RayServiceCRD {
-				metrics.CreatedRayClustersCounterInc(instance.Namespace, false, true)
-			}
-		}
-
 		if err := r.createHeadPod(ctx, *instance); err != nil {
 			return errstd.Join(utils.ErrFailedCreateHeadPod, err)
 		}
+
+		metrics.CreatedRayClustersCounterInc(r.EnableMetrics, instance.Namespace, instance)
 	} else if len(headPods.Items) > 1 { // This should never happen. This protects against the case that users manually create headpod.
 		correctHeadPodName := instance.Name + "-head"
 		headPodNames := make([]string, len(headPods.Items))