From a1535fa446d15bae56656d20577fdbb000353cc2 Mon Sep 17 00:00:00 2001
From: Saravanan Balasubramanian
 <33908564+sarabala1979@users.noreply.github.com>
Date: Fri, 17 Jun 2022 15:28:12 -0700
Subject: [PATCH] fix: Workflow Duration metric shouldn't increase after
 workflow complete (#8989)

* fix: Workflow Duration metric shouldn't increase after workflow complete

Signed-off-by: Saravanan Balasubramanian <sarabala1979@gmail.com>

* fix: updated

Signed-off-by: Saravanan Balasubramanian <sarabala1979@gmail.com>

* fix: updated

Signed-off-by: Saravanan Balasubramanian <sarabala1979@gmail.com>
---
 workflow/controller/operator_metrics_test.go | 26 ++++++++++++++++++++
 workflow/controller/steps.go                 |  3 +++
 2 files changed, 29 insertions(+)

diff --git a/workflow/controller/operator_metrics_test.go b/workflow/controller/operator_metrics_test.go
index ca2ceda568ae..5954e8ab5568 100644
--- a/workflow/controller/operator_metrics_test.go
+++ b/workflow/controller/operator_metrics_test.go
@@ -4,6 +4,7 @@ import (
 	"context"
 	"fmt"
 	"testing"
+	"time"
 
 	"github.com/prometheus/client_golang/prometheus"
 	dto "github.com/prometheus/client_model/go"
@@ -146,6 +147,15 @@ func getMetricStringValue(metric prometheus.Metric) (string, error) {
 	return fmt.Sprintf("%v", metricString), nil
 }
 
+func getMetricGaugeValue(metric prometheus.Metric) (*float64, error) {
+	metricString := &dto.Metric{}
+	err := metric.Write(metricString)
+	if err != nil {
+		return nil, err
+	}
+	return metricString.Gauge.Value, nil
+}
+
 var testMetricEmissionSameOperationCreationAndFailure = `
 apiVersion: argoproj.io/v1alpha1
 kind: Workflow
@@ -476,10 +486,26 @@ func TestRealtimeWorkflowMetric(t *testing.T) {
 
 	metricErrorDesc := woc.wf.Spec.Metrics.Prometheus[0].GetDesc()
 	assert.NotNil(t, controller.metrics.GetCustomMetric(metricErrorDesc))
+	value, err := getMetricGaugeValue(controller.metrics.GetCustomMetric(metricErrorDesc))
+	assert.NoError(t, err)
 	metricErrorCounter := controller.metrics.GetCustomMetric(metricErrorDesc)
 	metricErrorCounterString, err := getMetricStringValue(metricErrorCounter)
 	assert.NoError(t, err)
 	assert.Contains(t, metricErrorCounterString, `label:<name:"workflowName" value:"test-foobar" > gauge:<value:`)
+
+	value1, err := getMetricGaugeValue(controller.metrics.GetCustomMetric(metricErrorDesc))
+	assert.NoError(t, err)
+	assert.Greater(t, *value1, *value)
+	woc.markWorkflowSuccess(ctx)
+	controller.metrics.GetCustomMetric(metricErrorDesc)
+	value2, err := getMetricGaugeValue(controller.metrics.GetCustomMetric(metricErrorDesc))
+	assert.NoError(t, err)
+	time.Sleep(10 * time.Millisecond)
+	controller.metrics.GetCustomMetric(metricErrorDesc)
+	value3, err := getMetricGaugeValue(controller.metrics.GetCustomMetric(metricErrorDesc))
+	assert.NoError(t, err)
+	// Duration should be same after workflow complete
+	assert.Equal(t, *value2, *value3)
 }
 
 var testRealtimeWorkflowMetricWithGlobalParameters = `
diff --git a/workflow/controller/steps.go b/workflow/controller/steps.go
index fa4c93a0ef83..3c55ce1ec186 100644
--- a/workflow/controller/steps.go
+++ b/workflow/controller/steps.go
@@ -513,6 +513,9 @@ func (woc *wfOperationCtx) prepareDefaultMetricScope() (map[string]string, map[s
 
 	var realTimeScope = map[string]func() float64{
 		common.GlobalVarWorkflowDuration: func() float64 {
+			if woc.wf.Status.Phase.Completed() {
+				return woc.wf.Status.FinishedAt.Time.Sub(woc.wf.Status.StartedAt.Time).Seconds()
+			}
 			return time.Since(woc.wf.Status.StartedAt.Time).Seconds()
 		},
 	}