fix: Update numaplane progressive metrics (#927)

Signed-off-by: chandankumar4 <chandan.kr404@gmail.com>

Author: chandankumar4

internal/controller/isbservicerollout/isbservicerollout_controller.go

@@ -477,9 +477,9 @@ func (r *ISBServiceRolloutReconciler) processExistingISBService(ctx context.Cont
 				// assessmentResult value indicates that the progressive rollout is completed, so we can generate the metrics for the same
 				assessmentResult := metrics.EvaluateSuccessStatusForMetrics(isbServiceRollout.GetUpgradingChildStatus().AssessmentResult)
 				if assessmentResult != "" {
-					r.customMetrics.IncISBSvcProgressiveResults(isbServiceRollout.GetRolloutObjectMeta().GetNamespace(), isbServiceRollout.GetRolloutObjectMeta().GetName(),
+					r.customMetrics.IncISBServiceProgressiveCompleted(isbServiceRollout.GetRolloutObjectMeta().GetNamespace(), isbServiceRollout.GetRolloutObjectMeta().GetName(),
 						isbServiceRollout.GetUpgradingChildStatus().Name, metrics.EvaluateSuccessStatusForMetrics(isbServiceRollout.GetUpgradingChildStatus().BasicAssessmentResult),
-						assessmentResult, isbServiceRollout.GetUpgradingChildStatus().ForcedSuccess, true)
+						assessmentResult, isbServiceRollout.GetUpgradingChildStatus().ForcedSuccess)
 				}
 			}
 		} else {

internal/controller/isbservicerollout/isbservicerollout_progressive.go

@@ -16,7 +16,6 @@ import (
 	"github.com/numaproj/numaplane/internal/util"
 	"github.com/numaproj/numaplane/internal/util/kubernetes"
 	"github.com/numaproj/numaplane/internal/util/logger"
-	"github.com/numaproj/numaplane/internal/util/metrics"
 	apiv1 "github.com/numaproj/numaplane/pkg/apis/numaplane/v1alpha1"
 )
 
@@ -221,15 +220,10 @@ func (r *ISBServiceRolloutReconciler) ProcessUpgradingChildPostUpgrade(
 	return false, nil
 }
 
-func (r *ISBServiceRolloutReconciler) UpdateProgressiveMetrics(rolloutObject progressive.ProgressiveRolloutObject, completed bool) {
+func (r *ISBServiceRolloutReconciler) UpdateProgressiveMetrics(rolloutObject progressive.ProgressiveRolloutObject) {
 	if rolloutObject.GetUpgradingChildStatus() != nil {
 		childName := rolloutObject.GetUpgradingChildStatus().Name
-		successStatus := metrics.EvaluateSuccessStatusForMetrics(rolloutObject.GetUpgradingChildStatus().AssessmentResult)
-		forcedSuccess := rolloutObject.GetUpgradingChildStatus().ForcedSuccess
-		basicAssessmentResult := metrics.EvaluateSuccessStatusForMetrics(rolloutObject.GetUpgradingChildStatus().BasicAssessmentResult)
-
-		r.customMetrics.IncISBSvcProgressiveResults(rolloutObject.GetRolloutObjectMeta().GetNamespace(), rolloutObject.GetRolloutObjectMeta().GetName(),
-			childName, basicAssessmentResult, successStatus, forcedSuccess, completed)
+		r.customMetrics.IncIBSServiceProgressiveStarted(rolloutObject.GetRolloutObjectMeta().GetNamespace(), rolloutObject.GetRolloutObjectMeta().GetName(), childName)
 	}
 }
 

internal/controller/monovertexrollout/monovertexrollout_controller.go

@@ -382,9 +382,9 @@ func (r *MonoVertexRolloutReconciler) processExistingMonoVertex(ctx context.Cont
 				// assessmentResult value indicates that the progressive rollout is completed, so we can generate the metrics for the same
 				assessmentResult := metrics.EvaluateSuccessStatusForMetrics(monoVertexRollout.GetUpgradingChildStatus().AssessmentResult)
 				if assessmentResult != "" {
-					r.customMetrics.IncMonovertexProgressiveResults(monoVertexRollout.GetRolloutObjectMeta().GetNamespace(), monoVertexRollout.GetRolloutObjectMeta().GetName(),
+					r.customMetrics.IncMonovertexProgressiveCompleted(monoVertexRollout.GetRolloutObjectMeta().GetNamespace(), monoVertexRollout.GetRolloutObjectMeta().GetName(),
 						monoVertexRollout.GetUpgradingChildStatus().Name, metrics.EvaluateSuccessStatusForMetrics(monoVertexRollout.GetUpgradingChildStatus().BasicAssessmentResult),
-						assessmentResult, monoVertexRollout.GetUpgradingChildStatus().ForcedSuccess, true)
+						assessmentResult, monoVertexRollout.GetUpgradingChildStatus().ForcedSuccess)
 				}
 			}
 		} else {

internal/controller/monovertexrollout/monovertexrollout_progressive.go

@@ -18,7 +18,6 @@ import (
 	"github.com/numaproj/numaplane/internal/util"
 	"github.com/numaproj/numaplane/internal/util/kubernetes"
 	"github.com/numaproj/numaplane/internal/util/logger"
-	"github.com/numaproj/numaplane/internal/util/metrics"
 	apiv1 "github.com/numaproj/numaplane/pkg/apis/numaplane/v1alpha1"
 
 	"github.com/numaproj/numaplane/internal/common"
@@ -757,14 +756,9 @@ func (r *MonoVertexRolloutReconciler) ProgressiveUnsupported(ctx context.Context
 	return false
 }
 
-func (r *MonoVertexRolloutReconciler) UpdateProgressiveMetrics(rolloutObject progressive.ProgressiveRolloutObject, completed bool) {
+func (r *MonoVertexRolloutReconciler) UpdateProgressiveMetrics(rolloutObject progressive.ProgressiveRolloutObject) {
 	if rolloutObject.GetUpgradingChildStatus() != nil {
 		childName := rolloutObject.GetUpgradingChildStatus().Name
-		successStatus := metrics.EvaluateSuccessStatusForMetrics(rolloutObject.GetUpgradingChildStatus().AssessmentResult)
-		forcedSuccess := rolloutObject.GetUpgradingChildStatus().ForcedSuccess
-		basicAssessmentResult := metrics.EvaluateSuccessStatusForMetrics(rolloutObject.GetUpgradingChildStatus().BasicAssessmentResult)
-
-		r.customMetrics.IncMonovertexProgressiveResults(rolloutObject.GetRolloutObjectMeta().GetNamespace(), rolloutObject.GetRolloutObjectMeta().GetName(),
-			childName, basicAssessmentResult, successStatus, forcedSuccess, completed)
+		r.customMetrics.IncMonovertexProgressiveStarted(rolloutObject.GetRolloutObjectMeta().GetNamespace(), rolloutObject.GetRolloutObjectMeta().GetName(), childName)
 	}
 }

internal/controller/pipelinerollout/pipelinerollout_controller.go

@@ -638,9 +638,9 @@ func (r *PipelineRolloutReconciler) processExistingPipeline(ctx context.Context,
 				// assessmentResult value indicates that the progressive rollout is completed, so we can generate the metrics for the same
 				assessmentResult := metrics.EvaluateSuccessStatusForMetrics(pipelineRollout.GetUpgradingChildStatus().AssessmentResult)
 				if assessmentResult != "" {
-					r.customMetrics.IncPipelineProgressiveResults(pipelineRollout.GetRolloutObjectMeta().GetNamespace(), pipelineRollout.GetRolloutObjectMeta().GetName(),
+					r.customMetrics.IncPipelineProgressiveCompleted(pipelineRollout.GetRolloutObjectMeta().GetNamespace(), pipelineRollout.GetRolloutObjectMeta().GetName(),
 						pipelineRollout.GetUpgradingChildStatus().Name, metrics.EvaluateSuccessStatusForMetrics(pipelineRollout.GetUpgradingChildStatus().BasicAssessmentResult),
-						assessmentResult, pipelineRollout.GetUpgradingChildStatus().ForcedSuccess, true)
+						assessmentResult, pipelineRollout.GetUpgradingChildStatus().ForcedSuccess)
 				}
 			}
 		} else {

internal/controller/pipelinerollout/pipelinerollout_progressive.go

@@ -20,7 +20,6 @@ import (
 	"github.com/numaproj/numaplane/internal/util"
 	"github.com/numaproj/numaplane/internal/util/kubernetes"
 	"github.com/numaproj/numaplane/internal/util/logger"
-	"github.com/numaproj/numaplane/internal/util/metrics"
 	apiv1 "github.com/numaproj/numaplane/pkg/apis/numaplane/v1alpha1"
 )
 
@@ -788,14 +787,9 @@ func (r *PipelineRolloutReconciler) ProgressiveUnsupported(ctx context.Context,
 	return false
 }
 
-func (r *PipelineRolloutReconciler) UpdateProgressiveMetrics(rolloutObject progressive.ProgressiveRolloutObject, completed bool) {
+func (r *PipelineRolloutReconciler) UpdateProgressiveMetrics(rolloutObject progressive.ProgressiveRolloutObject) {
 	if rolloutObject.GetUpgradingChildStatus() != nil {
 		childName := rolloutObject.GetUpgradingChildStatus().Name
-		successStatus := metrics.EvaluateSuccessStatusForMetrics(rolloutObject.GetUpgradingChildStatus().AssessmentResult)
-		forcedSuccess := rolloutObject.GetUpgradingChildStatus().ForcedSuccess
-		basicAssessmentResult := metrics.EvaluateSuccessStatusForMetrics(rolloutObject.GetUpgradingChildStatus().BasicAssessmentResult)
-
-		r.customMetrics.IncPipelineProgressiveResults(rolloutObject.GetRolloutObjectMeta().GetNamespace(), rolloutObject.GetRolloutObjectMeta().GetName(),
-			childName, basicAssessmentResult, successStatus, forcedSuccess, completed)
+		r.customMetrics.IncPipelineProgressiveStarted(rolloutObject.GetRolloutObjectMeta().GetNamespace(), rolloutObject.GetRolloutObjectMeta().GetName(), childName)
 	}
 }

internal/controller/progressive/progressive.go

@@ -88,7 +88,7 @@ type progressiveController interface {
 	// ProgressiveUnsupported checks to see if Full Progressive Rollout (with assessment) is unsupported for this Rollout
 	ProgressiveUnsupported(ctx context.Context, rolloutObject ProgressiveRolloutObject) bool
 
-	UpdateProgressiveMetrics(rolloutObject ProgressiveRolloutObject, completed bool)
+	UpdateProgressiveMetrics(rolloutObject ProgressiveRolloutObject)
 }
 
 // ProgressiveRolloutObject describes a Rollout instance that supports progressive upgrade
@@ -816,7 +816,7 @@ func startUpgradeProcess(
 	}
 
 	// Update progressive metrics after creating the upgrading child
-	controller.UpdateProgressiveMetrics(rolloutObject, false)
+	controller.UpdateProgressiveMetrics(rolloutObject)
 
 	return newUpgradingChildDef, false, nil
 }

internal/controller/progressive/progressive_test.go

@@ -29,7 +29,7 @@ import (
 
 type fakeProgressiveController struct{}
 
-func (fpc fakeProgressiveController) UpdateProgressiveMetrics(rolloutObject ProgressiveRolloutObject, completed bool) {
+func (fpc fakeProgressiveController) UpdateProgressiveMetrics(rolloutObject ProgressiveRolloutObject) {
 }
 
 func (fpc fakeProgressiveController) CreateUpgradingChildDefinition(ctx context.Context, rolloutObject ProgressiveRolloutObject, name string) (*unstructured.Unstructured, error) {

internal/util/metrics/metrics.go

@@ -94,9 +94,12 @@ type CustomMetrics struct {
 	ISBServicePausedSeconds *prometheus.GaugeVec
 
 	// Progressive Rollout Metrics
-	PipelineProgressiveResults   *prometheus.CounterVec
-	IsbSvcProgressiveResults     *prometheus.CounterVec
-	MonoVertexProgressiveResults *prometheus.CounterVec
+	PipelineProgressiveStarted     *prometheus.CounterVec
+	PipelineProgressiveCompleted   *prometheus.CounterVec
+	ISBServiceProgressiveStarted   *prometheus.CounterVec
+	ISBServiceProgressiveCompleted *prometheus.CounterVec
+	MonovertexProgressiveStarted   *prometheus.CounterVec
+	MonovertexProgressiveCompleted *prometheus.CounterVec
 }
 
 const (
@@ -351,26 +354,47 @@ var (
 		ConstLabels: defaultLabels,
 	}, []string{})
 
-	// pipelineProgressiveResults count the total number of pipeline progressive rollout results
-	pipelineProgressiveResults = promauto.NewCounterVec(prometheus.CounterOpts{
-		Name:        "numaplane_pipeline_progressive_results_total",
-		Help:        "The total number of pipeline progressive rollout results",
+	// pipelineProgressiveStarted count the total number of pipeline progressive rollouts started
+	pipelineProgressiveStarted = promauto.NewCounterVec(prometheus.CounterOpts{
+		Name:        "numaplane_pipeline_progressive_started",
+		Help:        "The total number of pipeline progressive rollout started",
 		ConstLabels: defaultLabels,
-	}, []string{LabelNamespace, LabelName, LabelRolloutName, LabelSuccess, LabelForcedSuccess, LabelResourceHealthSuccess, LabelCompleted})
+	}, []string{LabelNamespace, LabelRolloutName, LabelName})
 
-	//ISBSvcProgressiveResults count the total number of isbsvc progressive rollout results
-	isbSvcProgressiveResults = promauto.NewCounterVec(prometheus.CounterOpts{
-		Name:        "numaplane_isbsvc_progressive_results_total",
-		Help:        "The total number of isbsvc progressive rollout results",
+	// pipelineProgressiveResults count the total number of pipeline progressive rollouts completed
+	pipelineProgressiveCompleted = promauto.NewCounterVec(prometheus.CounterOpts{
+		Name:        "numaplane_pipeline_progressive_completed",
+		Help:        "The total number of pipeline progressive rollout completed",
 		ConstLabels: defaultLabels,
-	}, []string{LabelNamespace, LabelName, LabelRolloutName, LabelSuccess, LabelForcedSuccess, LabelResourceHealthSuccess, LabelCompleted})
+	}, []string{LabelNamespace, LabelRolloutName, LabelName, LabelSuccess, LabelForcedSuccess, LabelResourceHealthSuccess})
 
-	// MonoVertexProgressiveResults count the total number of monovertex progressive rollout results
-	monoVertexProgressiveResults = promauto.NewCounterVec(prometheus.CounterOpts{
-		Name:        "numaplane_monovertex_progressive_results_total",
-		Help:        "The total number of monovertex progressive rollout results",
+	// isbServiceProgressiveStarted count the total number of ISB Service progressive rollouts started
+	isbServiceProgressiveStarted = promauto.NewCounterVec(prometheus.CounterOpts{
+		Name:        "numaplane_isbsvc_progressive_started",
+		Help:        "The total number of pipeline isbsvc rollout started",
 		ConstLabels: defaultLabels,
-	}, []string{LabelNamespace, LabelName, LabelRolloutName, LabelSuccess, LabelForcedSuccess, LabelResourceHealthSuccess, LabelCompleted})
+	}, []string{LabelNamespace, LabelRolloutName, LabelName})
+
+	//isbServiceProgressiveCompleted count the total number of isbsvc progressive rollouts completed
+	isbServiceProgressiveCompleted = promauto.NewCounterVec(prometheus.CounterOpts{
+		Name:        "numaplane_isbsvc_progressive_completed",
+		Help:        "The total number of isbsvc progressive rollout completed",
+		ConstLabels: defaultLabels,
+	}, []string{LabelNamespace, LabelRolloutName, LabelName, LabelSuccess, LabelForcedSuccess, LabelResourceHealthSuccess})
+
+	// monovertexProgressiveStarted count the total number of monovertex progressive rollouts started
+	monovertexProgressiveStarted = promauto.NewCounterVec(prometheus.CounterOpts{
+		Name:        "numaplane_monovertex_progressive_started",
+		Help:        "The total number of monovertex rollout started",
+		ConstLabels: defaultLabels,
+	}, []string{LabelNamespace, LabelRolloutName, LabelName})
+
+	// monovertexProgressiveCompleted count the total number of monovertex progressive rollout completed
+	monovertexProgressiveCompleted = promauto.NewCounterVec(prometheus.CounterOpts{
+		Name:        "numaplane_monovertex_progressive_completed",
+		Help:        "The total number of monovertex progressive rollout completed",
+		ConstLabels: defaultLabels,
+	}, []string{LabelNamespace, LabelRolloutName, LabelName, LabelSuccess, LabelForcedSuccess, LabelResourceHealthSuccess})
 )
 
 // RegisterCustomMetrics registers the custom metrics to the existing global prometheus registry for pipelines, ISB service and numaflow controller
@@ -382,8 +406,8 @@ func RegisterCustomMetrics(numaLogger *logger.NumaLogger) *CustomMetrics {
 		numaflowControllerRolloutsHealth, numaflowControllerRolloutsRunning, numaflowControllerRolloutSyncs, numaflowControllerRolloutSyncErrors, numaflowControllerRolloutPausedSeconds,
 		numaflowControllersHealth, numaflowControllerSyncs, numaflowControllerSyncErrors, numaflowControllerKubectlExecutionCounter,
 		reconciliationDuration, kubeRequestCounter, kubeResourceCacheMonitored,
-		kubeResourceCache, clusterCacheError, pipelinePausedSeconds, pipelinePausingSeconds, isbServicePausedSeconds, pipelineProgressiveResults,
-		isbSvcProgressiveResults, monoVertexProgressiveResults)
+		kubeResourceCache, clusterCacheError, pipelinePausedSeconds, pipelinePausingSeconds, isbServicePausedSeconds,
+		pipelineProgressiveStarted, pipelineProgressiveCompleted, isbServiceProgressiveStarted, isbServiceProgressiveCompleted, monovertexProgressiveStarted, monovertexProgressiveCompleted)
 
 	return &CustomMetrics{
 		NumaLogger:                                numaLogger,
@@ -421,9 +445,12 @@ func RegisterCustomMetrics(numaLogger *logger.NumaLogger) *CustomMetrics {
 		PipelinePausedSeconds:                     pipelinePausedSeconds,
 		PipelinePausingSeconds:                    pipelinePausingSeconds,
 		ISBServicePausedSeconds:                   isbServicePausedSeconds,
-		PipelineProgressiveResults:                pipelineProgressiveResults,
-		IsbSvcProgressiveResults:                  isbSvcProgressiveResults,
-		MonoVertexProgressiveResults:              monoVertexProgressiveResults,
+		PipelineProgressiveStarted:                pipelineProgressiveStarted,
+		PipelineProgressiveCompleted:              pipelineProgressiveCompleted,
+		ISBServiceProgressiveStarted:              isbServiceProgressiveStarted,
+		ISBServiceProgressiveCompleted:            isbServiceProgressiveCompleted,
+		MonovertexProgressiveStarted:              monovertexProgressiveStarted,
+		MonovertexProgressiveCompleted:            monovertexProgressiveCompleted,
 	}
 }
 
@@ -600,16 +627,28 @@ func (m *CustomMetrics) IncProgressivePipelineDrains(namespace, pipelineRolloutN
 	m.ProgressivePipelineDrains.WithLabelValues(namespace, pipelineRolloutName, pipelineName, strconv.FormatBool(drainComplete), string(drainResult)).Inc()
 }
 
-func (m *CustomMetrics) IncPipelineProgressiveResults(namespace, name, childName string, basicAssessmentResult, successStatus util.OptionalBoolStr, forcedSuccess, completed bool) {
-	m.PipelineProgressiveResults.WithLabelValues(namespace, childName, name, successStatus.ToString(), strconv.FormatBool(forcedSuccess), basicAssessmentResult.ToString(), strconv.FormatBool(completed)).Inc()
+func (m *CustomMetrics) IncPipelineProgressiveStarted(namespace, rolloutName, pipelineName string) {
+	m.PipelineProgressiveStarted.WithLabelValues(namespace, rolloutName, pipelineName).Inc()
+}
+
+func (m *CustomMetrics) IncPipelineProgressiveCompleted(namespace, rolloutName, pipelineName string, basicAssessmentResult, successStatus util.OptionalBoolStr, forcedSuccess bool) {
+	m.PipelineProgressiveCompleted.WithLabelValues(namespace, rolloutName, pipelineName, successStatus.ToString(), strconv.FormatBool(forcedSuccess), basicAssessmentResult.ToString()).Inc()
+}
+
+func (m *CustomMetrics) IncIBSServiceProgressiveStarted(namespace, rolloutName, isbServiceName string) {
+	m.ISBServiceProgressiveStarted.WithLabelValues(namespace, rolloutName, isbServiceName).Inc()
+}
+
+func (m *CustomMetrics) IncISBServiceProgressiveCompleted(namespace, rolloutName, isbServiceName string, basicAssessmentResult, successStatus util.OptionalBoolStr, forcedSuccess bool) {
+	m.ISBServiceProgressiveCompleted.WithLabelValues(namespace, rolloutName, isbServiceName, successStatus.ToString(), strconv.FormatBool(forcedSuccess), basicAssessmentResult.ToString()).Inc()
 }
 
-func (m *CustomMetrics) IncISBSvcProgressiveResults(namespace, name, childName string, basicAssessmentResult, successStatus util.OptionalBoolStr, forcedSuccess, completed bool) {
-	m.IsbSvcProgressiveResults.WithLabelValues(namespace, childName, name, successStatus.ToString(), strconv.FormatBool(forcedSuccess), basicAssessmentResult.ToString(), strconv.FormatBool(completed)).Inc()
+func (m *CustomMetrics) IncMonovertexProgressiveStarted(namespace, rolloutName, monovertexName string) {
+	m.MonovertexProgressiveStarted.WithLabelValues(namespace, rolloutName, monovertexName).Inc()
 }
 
-func (m *CustomMetrics) IncMonovertexProgressiveResults(namespace, name, childName string, basicAssessmentResult, successStatus util.OptionalBoolStr, forcedSuccess, completed bool) {
-	m.MonoVertexProgressiveResults.WithLabelValues(namespace, childName, name, successStatus.ToString(), strconv.FormatBool(forcedSuccess), basicAssessmentResult.ToString(), strconv.FormatBool(completed)).Inc()
+func (m *CustomMetrics) IncMonovertexProgressiveCompleted(namespace, rolloutName, monovertexName string, basicAssessmentResult, successStatus util.OptionalBoolStr, forcedSuccess bool) {
+	m.MonovertexProgressiveCompleted.WithLabelValues(namespace, rolloutName, monovertexName, successStatus.ToString(), strconv.FormatBool(forcedSuccess), basicAssessmentResult.ToString()).Inc()
 }
 
 func EvaluateSuccessStatusForMetrics(assessmentResult apiv1.AssessmentResult) util.OptionalBoolStr {