fix: Workflow Duration metric shouldn't increase after workflow compl…

…ete (#8989) * fix: Workflow Duration metric shouldn't increase after workflow complete Signed-off-by: Saravanan Balasubramanian <[email protected]> * fix: updated Signed-off-by: Saravanan Balasubramanian <[email protected]> * fix: updated Signed-off-by: Saravanan Balasubramanian <[email protected]>
argoproj · Jun 17, 2022 · a1535fa · a1535fa
1 parent 6106ac7
commit a1535fa
Show file tree

Hide file tree

Showing 2 changed files with 29 additions and 0 deletions.
diff --git 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
@@ -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()
 		},
 	}