Skip to content

Commit

Permalink
fix: map traversing is unstable in go which leaded to a mix of labels…
Browse files Browse the repository at this point in the history
… and values with the current implementation
  • Loading branch information
mlornac committed Oct 17, 2024
1 parent c0020a5 commit d0615f7
Show file tree
Hide file tree
Showing 2 changed files with 47 additions and 29 deletions.
26 changes: 16 additions & 10 deletions internal/metrics/metrics.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package metrics

import (
"errors"
"sort"
"sync"

"github.com/prometheus/client_golang/prometheus"
Expand Down Expand Up @@ -38,22 +39,22 @@ 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,
Subsystem: metricSubsystem,
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...)),
}
}

Expand Down Expand Up @@ -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
}
50 changes: 31 additions & 19 deletions internal/metrics/metrics_test.go
Original file line number Diff line number Diff line change
@@ -1,9 +1,13 @@
package metrics_test

import (
"bytes"
"fmt"
"testing"

"github.com/prometheus/client_golang/prometheus"

"github.com/prometheus/client_golang/prometheus/testutil"

Check failure on line 9 in internal/metrics/metrics_test.go

View workflow job for this annotation

GitHub Actions / Analyze (go)

other declaration of testutil

Check failure on line 9 in internal/metrics/metrics_test.go

View workflow job for this annotation

GitHub Actions / Test

other declaration of testutil

"github.com/prometheus/client_golang/prometheus/testutil"

Check failure on line 11 in internal/metrics/metrics_test.go

View workflow job for this annotation

GitHub Actions / Analyze (go)

testutil redeclared in this block

Check failure on line 11 in internal/metrics/metrics_test.go

View workflow job for this annotation

GitHub Actions / Analyze (go)

"github.com/prometheus/client_golang/prometheus/testutil" imported and not used

Check failure on line 11 in internal/metrics/metrics_test.go

View workflow job for this annotation

GitHub Actions / Test

testutil redeclared in this block

Check failure on line 11 in internal/metrics/metrics_test.go

View workflow job for this annotation

GitHub Actions / Test

"github.com/prometheus/client_golang/prometheus/testutil" imported and not used (typecheck)
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
Expand All @@ -13,29 +17,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))
}

0 comments on commit d0615f7

Please sign in to comment.