[SLI-Metrics] kuberay_service_ready

[owenowenisme]

Why are these changes needed?

Added kuberay_service_ready as written in KubeRay v1.4.0 SLI proposal

❯ curl -s localhost:8080/metrics | grep  kuberay_
# HELP kuberay_service_ready RayServiceReady means users can send requests to the underlying cluster and the number of serve endpoints is greater than 0.
# TYPE kuberay_service_ready gauge
kuberay_service_ready{condition="false",name="rayservice-sample",namespace="default"} 1
❯ curl -s localhost:8080/metrics | grep  kuberay_
# HELP kuberay_service_ready RayServiceReady means users can send requests to the underlying cluster and the number of serve endpoints is greater than 0.
# TYPE kuberay_service_ready gauge
kuberay_service_ready{condition="true",name="rayservice-sample",namespace="default"} 1

Related issue number

Checks

• I've made sure the tests are passing.
• Testing Strategy
  • Unit tests
  • Manual tests
  • This PR is not tested :(

[owenowenisme]

@win5923 @troychiu

[win5923]

LGTM

[owenowenisme]

@troychiu @kevin85421 Would you mind taking a look?

[troychiu]

Can't we simply reuse collectRayServiceInfo?

kuberay/ray-operator/controllers/ray/metrics/ray_service_metrics.go

Line 54 in d9a1801

func (c *RayServiceMetricsManager) collectRayServiceInfo(service *rayv1.RayService, ch chan<- prometheus.Metric) {

[owenowenisme]

RayServiceInfo is Desc while RayServiceReady is GaugeVec I don't think we should mix them together.

[troychiu]

RayServiceInfo is a Gauge type of metric which is the same as RayServiceReady. Desc is not a type of metric but a prometheus descriptor. What I meant is we can use the same way to collect RayServiceReady as we did for RayServiceInfo

[owenowenisme]

@troychiu I get what you meant now.

func (c *RayServiceMetricsManager) Collect(ch chan<- prometheus.Metric) {
	var rayServiceList rayv1.RayServiceList
	if err := c.client.List(context.Background(), &rayServiceList); err != nil {
		c.log.Error(err, "Failed to list RayServices")
		return
	}
	for _, rayService := range rayServiceList.Items {
		c.collectRayServiceInfo(&rayService, ch)
	}
	c.RayServiceReady.Collect(ch)
}

func (c *RayServiceMetricsManager) collectRayServiceInfo(service *rayv1.RayService, ch chan<- prometheus.Metric) {
	ch <- prometheus.MustNewConstMetric(
		c.rayServiceInfo,
		prometheus.GaugeValue,
		1,
		service.Name,
		service.Namespace,
	)
	ready := meta.IsStatusConditionTrue(service.Status.Conditions, string(rayv1.RayServiceReady))
	c.RayServiceReady.WithLabelValues(service.Name, service.Namespace, strconv.FormatBool(!ready)).Set(0)
	c.RayServiceReady.WithLabelValues(service.Name, service.Namespace, strconv.FormatBool(ready)).Set(1)
}

But just to make sure, is this what you're thinking in mind?
BTW, I think this is a nice approach! Because we don't have to modify controller with this method. And can apply to rayCluster and rayJob.

[troychiu]

Yeah that's it. Actually we can make it simpler.
you can simply do

ch <- prometheus.MustNewConstMetric(
        c.rayServiceReady,
        prometheus.GaugeValue,
        1,
        service.Name,
        service.Namespace,
        condition, // strconv.FormatBool(...) in this case
)

and

for _, rayService := range rayServiceList.Items {
        c.collectRayServiceInfo(&rayService, ch)
        c.collectRayServiceReady(&rayService, ch)
}

[troychiu]

can we use kuberay_service_condition_ready?

[owenowenisme]

kuberay/ray-operator/apis/ray/v1/rayservice_types.go

Line 135 in 33ee672

RayServiceReady RayServiceConditionType = "Ready"

kuberay_service_ready should be enough?
But, I think it's just a matter of choice. Both are fine to me.

[troychiu]

I think it would be more clear. In kube-state-metrics, they use kube_pod_info and kube_pod_status_ready

[owenowenisme]

Updated.

[owenowenisme]

Cc @kevin85421 PTAL🙏🙏