From f063dea34c52b89d16df90b1479735836e152482 Mon Sep 17 00:00:00 2001
From: Mathieu Lornac <mathieu.lornac@form3.tech>
Date: Thu, 17 Oct 2024 17:54:25 +0200
Subject: [PATCH] fix: map traversing is unstable in go which leaded to a mix
 of labels and values with the current implementation

---
 internal/metrics/metrics.go      | 26 ++++++++++-------
 internal/metrics/metrics_test.go | 49 +++++++++++++++++++-------------
 2 files changed, 45 insertions(+), 30 deletions(-)

diff --git a/internal/metrics/metrics.go b/internal/metrics/metrics.go
index 17faadb0..26f68dc8 100644
--- a/internal/metrics/metrics.go
+++ b/internal/metrics/metrics.go
@@ -2,6 +2,7 @@ package metrics
 
 import (
 	"errors"
+	"sort"
 	"sync"
 
 	"github.com/prometheus/client_golang/prometheus"
@@ -38,7 +39,7 @@ func buildMetrics(staticMetrics map[string]string) *Metrics {
 	percentileObjectives := map[float64]float64{
 		0.5: 0.05, 0.75: 0.05, 0.9: 0.01, 0.95: 0.001, 0.99: 0.001, 0.9999: 0.00001, 1.0: 0.00001,
 	}
-
+	labelKeys := getStaticMetricLabelKeys(staticMetrics)
 	return &Metrics{
 		Setup: prometheus.NewSummaryVec(prometheus.SummaryOpts{
 			Namespace:  metricNamespace,
@@ -46,14 +47,14 @@ func buildMetrics(staticMetrics map[string]string) *Metrics {
 			Name:       "setup",
 			Help:       "Duration of setup functions.",
 			Objectives: percentileObjectives,
-		}, append([]string{TestNameLabel, ResultLabel}, getStaticMetricLabelKeys(staticMetrics)...)),
+		}, append([]string{TestNameLabel, ResultLabel}, labelKeys...)),
 		Iteration: prometheus.NewSummaryVec(prometheus.SummaryOpts{
 			Namespace:  metricNamespace,
 			Subsystem:  metricSubsystem,
 			Name:       "iteration",
 			Help:       "Duration of iteration functions.",
 			Objectives: percentileObjectives,
-		}, append([]string{TestNameLabel, StageLabel, ResultLabel}, getStaticMetricLabelKeys(staticMetrics)...)),
+		}, append([]string{TestNameLabel, StageLabel, ResultLabel}, labelKeys...)),
 	}
 }
 
@@ -118,17 +119,22 @@ func (metrics *Metrics) RecordIterationStage(name string, stage string, result R
 }
 
 func getStaticMetricLabelKeys(staticMetrics map[string]string) []string {
-	data := make([]string, 0, len(staticMetrics))
-	for k := range staticMetrics {
-		data = append(data, k)
-	}
-	return data
+	return sortedKeys(staticMetrics)
 }
 
 func getStaticMetricLabelValues(staticMetrics map[string]string) []string {
 	data := make([]string, 0, len(staticMetrics))
-	for _, v := range staticMetrics {
-		data = append(data, v)
+	for _, v := range sortedKeys(staticMetrics) {
+		data = append(data, staticMetrics[v])
 	}
 	return data
 }
+
+func sortedKeys(staticMetrics map[string]string) []string {
+	keys := make([]string, 0, len(staticMetrics))
+	for k := range staticMetrics {
+		keys = append(keys, k)
+	}
+	sort.Strings(keys)
+	return keys
+}
diff --git a/internal/metrics/metrics_test.go b/internal/metrics/metrics_test.go
index c173d80c..0adfae79 100644
--- a/internal/metrics/metrics_test.go
+++ b/internal/metrics/metrics_test.go
@@ -1,9 +1,10 @@
 package metrics_test
 
 import (
+	"bytes"
+	"fmt"
 	"testing"
-
-	"github.com/prometheus/client_golang/prometheus"
+	
 	"github.com/prometheus/client_golang/prometheus/testutil"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
@@ -13,29 +14,37 @@ import (
 
 func TestMetrics_Init_IsSafe(t *testing.T) {
 	t.Parallel()
-	metrics.InitWithStaticMetrics(true, map[string]string{
-		"product": "fps",
-		"f1_id":   "myid",
-	}) // race detector assertion
+	labels := map[string]string{
+		"product":  "fps",
+		"customer": "fake-customer",
+		"f1_id":    "myid",
+		"labelx":   "vx",
+	}
+	metrics.InitWithStaticMetrics(true, labels) // race detector assertion
 	for range 10 {
 		go func() {
-			metrics.InitWithStaticMetrics(true, map[string]string{
-				"product": "fps",
-				"f1_id":   "myid",
-			})
+			metrics.InitWithStaticMetrics(true, labels)
 		}()
 	}
 	assert.True(t, metrics.Instance().IterationMetricsEnabled)
 	metrics.Instance().RecordIterationResult("test1", metrics.SuccessResult, 1)
 	assert.Equal(t, 1, testutil.CollectAndCount(metrics.Instance().Iteration, "form3_loadtest_iteration"))
-	o, err := metrics.Instance().Iteration.MetricVec.GetMetricWith(prometheus.Labels{
-		metrics.TestNameLabel: "test1",
-		metrics.StageLabel:    metrics.IterationStage,
-		metrics.ResultLabel:   metrics.SuccessResult.String(),
-		"product":             "fps",
-		"f1_id":               "myid",
-	})
-	require.NoError(t, err)
-	assert.Contains(t, o.Desc().String(), "product")
-	assert.Contains(t, o.Desc().String(), "f1_id")
+
+	expected := `
+        	     # HELP form3_loadtest_iteration Duration of iteration functions.
+        	     # TYPE form3_loadtest_iteration summary
+				`
+	quantileFormat := `
+				form3_loadtest_iteration{customer="fake-customer",f1_id="myid",labelx="vx",product="fps",result="success",stage="iteration",test="test1",quantile="%s"} 1
+				`
+	for _, quantile := range []string{"0.5", "0.75", "0.9", "0.95", "0.99", "0.9999", "1.0"} {
+		expected += fmt.Sprintf(quantileFormat, quantile)
+	}
+
+	expected += `
+        	      form3_loadtest_iteration_sum{customer="fake-customer",f1_id="myid",labelx="vx",product="fps",result="success",stage="iteration",test="test1"} 1
+        	      form3_loadtest_iteration_count{customer="fake-customer",f1_id="myid",labelx="vx",product="fps",result="success",stage="iteration",test="test1"} 1
+				`
+	r := bytes.NewReader([]byte(expected))
+	require.NoError(t, testutil.CollectAndCompare(metrics.Instance().Iteration, r))
 }