Fix Metric tekton_pipelines_controller_pipelinerun_count

Added condition check to avoid recount issue.
tektoncd · Jan 12, 2022 · e4ad6bf · e4ad6bf
1 parent 097eeb4
commit e4ad6bf
Show file tree

Hide file tree

Showing 3 changed files with 12 additions and 4 deletions.
diff --git a/pkg/pipelinerunmetrics/metrics.go b/pkg/pipelinerunmetrics/metrics.go
@@ -31,6 +31,7 @@ import (
 	"go.opencensus.io/tag"
 	"go.uber.org/zap"
 	corev1 "k8s.io/api/core/v1"
+	"k8s.io/apimachinery/pkg/api/equality"
 	"k8s.io/apimachinery/pkg/labels"
 	"knative.dev/pkg/apis"
 	"knative.dev/pkg/logging"
@@ -209,7 +210,14 @@ func nilInsertTag(task, taskrun string) []tag.Mutator {
 // DurationAndCount logs the duration of PipelineRun execution and
 // count for number of PipelineRuns succeed or failed
 // returns an error if its failed to log the metrics
-func (r *Recorder) DurationAndCount(pr *v1beta1.PipelineRun) error {
+func (r *Recorder) DurationAndCount(pr *v1beta1.PipelineRun, beforeCondition *apis.Condition) error {
+
+	afterCondition := pr.Status.GetCondition(apis.ConditionSucceeded)
+	// To avoid recount
+	if equality.Semantic.DeepEqual(beforeCondition, afterCondition) && afterCondition != nil {
+		return nil
+	}
+
 	r.mutex.Lock()
 	defer r.mutex.Unlock()
 

diff --git a/pkg/pipelinerunmetrics/metrics_test.go b/pkg/pipelinerunmetrics/metrics_test.go
@@ -58,7 +58,7 @@ func getConfigContext() context.Context {
 func TestUninitializedMetrics(t *testing.T) {
 	metrics := Recorder{}
 
-	if err := metrics.DurationAndCount(&v1beta1.PipelineRun{}); err == nil {
+	if err := metrics.DurationAndCount(&v1beta1.PipelineRun{}, nil); err == nil {
 		t.Error("DurationAndCount recording expected to return error but got nil")
 	}
 	if err := metrics.RunningPipelineRuns(nil); err == nil {
@@ -248,7 +248,7 @@ func TestRecordPipelineRunDurationCount(t *testing.T) {
 				t.Fatalf("NewRecorder: %v", err)
 			}
 
-			if err := metrics.DurationAndCount(test.pipelineRun); err != nil {
+			if err := metrics.DurationAndCount(test.pipelineRun, nil); err != nil {
 				t.Errorf("DurationAndCount: %v", err)
 			}
 			metricstest.CheckLastValueData(t, "pipelinerun_duration_seconds", test.expectedTags, test.expectedDuration)

diff --git a/pkg/reconciler/pipelinerun/pipelinerun.go b/pkg/reconciler/pipelinerun/pipelinerun.go
@@ -196,7 +196,7 @@ func (c *Reconciler) ReconcileKind(ctx context.Context, pr *v1beta1.PipelineRun)
 			return c.finishReconcileUpdateEmitEvents(ctx, pr, before, err)
 		}
 		go func(metrics *pipelinerunmetrics.Recorder) {
-			err := metrics.DurationAndCount(pr)
+			err := metrics.DurationAndCount(pr, before)
 			if err != nil {
 				logger.Warnf("Failed to log the metrics : %v", err)
 			}