Skip to content

Commit 51dc599

Browse files
cristiangrecocristiangreco
committed
Handle case of duplicate labels in metrics
1 parent 3188d16 commit 51dc599

File tree

4 files changed

+92
-15
lines changed

4 files changed

+92
-15
lines changed

‎pkg/promutil/migrate.go‎

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -290,13 +290,19 @@ func recordLabelsForMetric(metricName string, labelKeys []string, observedMetric
290290
// EnsureLabelConsistencyAndRemoveDuplicates ensures that every metric has the same set of labels based on the data
291291
// in observedMetricLabels and that there are no duplicate metrics.
292292
// Prometheus requires that all metrics with the same name have the same set of labels and that no duplicates are registered
293-
func EnsureLabelConsistencyAndRemoveDuplicates(metrics []*PrometheusMetric, observedMetricLabels map[string]model.LabelSet) []*PrometheusMetric {
293+
func EnsureLabelConsistencyAndRemoveDuplicates(metrics []*PrometheusMetric, observedMetricLabels map[string]model.LabelSet, logger logging.Logger) []*PrometheusMetric {
294294
metricKeys := make(map[string]struct{}, len(metrics))
295295
output := make([]*PrometheusMetric, 0, len(metrics))
296296

297297
for _, metric := range metrics {
298-
for observedLabels := range observedMetricLabels[metric.Name()] {
299-
metric.AddIfMissingLabelPair(observedLabels, "")
298+
observedLabels := observedMetricLabels[metric.Name()]
299+
for label := range observedLabels {
300+
metric.AddIfMissingLabelPair(label, "")
301+
}
302+
303+
if len(observedLabels) != metric.LabelsLen() {
304+
logger.Warn("metric has duplicate labels", "metric_name", metric.Name(), "observed_labels", len(observedLabels), "labels_len", metric.LabelsLen())
305+
metric.RemoveDuplicateLabels()
300306
}
301307

302308
metricKey := metric.Name() + "-" + strconv.FormatUint(metric.LabelsSignature(), 10)

‎pkg/promutil/migrate_test.go‎

Lines changed: 23 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1289,13 +1289,23 @@ func Test_EnsureLabelConsistencyAndRemoveDuplicates(t *testing.T) {
12891289
NewPrometheusMetric("metric1", []string{"label1", "label2", "label3"}, []string{"", "", ""}, 3.0),
12901290
},
12911291
},
1292+
{
1293+
name: "removes duplicate labels",
1294+
metrics: []*PrometheusMetric{
1295+
NewPrometheusMetric("metric1", []string{"label1", "label1", "label2"}, []string{"value1", "value1", "value2"}, 1.0),
1296+
},
1297+
observedLabels: map[string]model.LabelSet{"metric1": {"label1": {}, "label2": {}}},
1298+
output: []*PrometheusMetric{
1299+
NewPrometheusMetric("metric1", []string{"label1", "label2"}, []string{"value1", "value2"}, 1.0),
1300+
},
1301+
},
12921302
{
12931303
name: "duplicate metric",
12941304
metrics: []*PrometheusMetric{
12951305
NewPrometheusMetric("metric1", []string{"label1"}, []string{"value1"}, 1.0),
12961306
NewPrometheusMetric("metric1", []string{"label1"}, []string{"value1"}, 1.0),
12971307
},
1298-
observedLabels: map[string]model.LabelSet{},
1308+
observedLabels: map[string]model.LabelSet{"metric1": {"label1": {}}},
12991309
output: []*PrometheusMetric{
13001310
NewPrometheusMetric("metric1", []string{"label1"}, []string{"value1"}, 1.0),
13011311
},
@@ -1306,7 +1316,7 @@ func Test_EnsureLabelConsistencyAndRemoveDuplicates(t *testing.T) {
13061316
NewPrometheusMetric("metric1", []string{"label1", "label2"}, []string{"value1", "value2"}, 1.0),
13071317
NewPrometheusMetric("metric1", []string{"label2", "label1"}, []string{"value2", "value1"}, 1.0),
13081318
},
1309-
observedLabels: map[string]model.LabelSet{},
1319+
observedLabels: map[string]model.LabelSet{"metric1": {"label1": {}, "label2": {}}},
13101320
output: []*PrometheusMetric{
13111321
NewPrometheusMetric("metric1", []string{"label1", "label2"}, []string{"value1", "value2"}, 1.0),
13121322
},
@@ -1317,10 +1327,10 @@ func Test_EnsureLabelConsistencyAndRemoveDuplicates(t *testing.T) {
13171327
NewPrometheusMetric("metric1", []string{"label1"}, []string{"value1"}, 1.0),
13181328
NewPrometheusMetric("metric1", []string{"label2"}, []string{"value2"}, 1.0),
13191329
},
1320-
observedLabels: map[string]model.LabelSet{},
1330+
observedLabels: map[string]model.LabelSet{"metric1": {"label1": {}, "label2": {}}},
13211331
output: []*PrometheusMetric{
1322-
NewPrometheusMetric("metric1", []string{"label1"}, []string{"value1"}, 1.0),
1323-
NewPrometheusMetric("metric1", []string{"label2"}, []string{"value2"}, 1.0),
1332+
NewPrometheusMetric("metric1", []string{"label1", "label2"}, []string{"value1", ""}, 1.0),
1333+
NewPrometheusMetric("metric1", []string{"label1", "label2"}, []string{"", "value2"}, 1.0),
13241334
},
13251335
},
13261336
{
@@ -1329,7 +1339,7 @@ func Test_EnsureLabelConsistencyAndRemoveDuplicates(t *testing.T) {
13291339
NewPrometheusMetric("metric1", []string{"label1"}, []string{"value1"}, 1.0),
13301340
NewPrometheusMetric("metric2", []string{"label1"}, []string{"value1"}, 1.0),
13311341
},
1332-
observedLabels: map[string]model.LabelSet{},
1342+
observedLabels: map[string]model.LabelSet{"metric1": {"label1": {}}, "metric2": {"label1": {}}},
13331343
output: []*PrometheusMetric{
13341344
NewPrometheusMetric("metric1", []string{"label1"}, []string{"value1"}, 1.0),
13351345
NewPrometheusMetric("metric2", []string{"label1"}, []string{"value1"}, 1.0),
@@ -1341,7 +1351,7 @@ func Test_EnsureLabelConsistencyAndRemoveDuplicates(t *testing.T) {
13411351
NewPrometheusMetric("metric1", []string{"label1"}, []string{"value1"}, 1.0),
13421352
NewPrometheusMetric("metric2", []string{"label2"}, []string{"value2"}, 1.0),
13431353
},
1344-
observedLabels: map[string]model.LabelSet{},
1354+
observedLabels: map[string]model.LabelSet{"metric1": {"label1": {}}, "metric2": {"label2": {}}},
13451355
output: []*PrometheusMetric{
13461356
NewPrometheusMetric("metric1", []string{"label1"}, []string{"value1"}, 1.0),
13471357
NewPrometheusMetric("metric2", []string{"label2"}, []string{"value2"}, 1.0),
@@ -1356,18 +1366,18 @@ func Test_EnsureLabelConsistencyAndRemoveDuplicates(t *testing.T) {
13561366
NewPrometheusMetric("metric1", []string{"label1"}, []string{"value1"}, 1.0),
13571367
NewPrometheusMetric("metric1", []string{"label1"}, []string{"value1"}, 1.0),
13581368
},
1359-
observedLabels: map[string]model.LabelSet{},
1369+
observedLabels: map[string]model.LabelSet{"metric1": {"label1": {}}, "metric2": {"label1": {}, "label2": {}}},
13601370
output: []*PrometheusMetric{
1361-
NewPrometheusMetric("metric2", []string{"label2"}, []string{"value2"}, 1.0),
1362-
NewPrometheusMetric("metric2", []string{"label1"}, []string{"value1"}, 1.0),
1371+
NewPrometheusMetric("metric2", []string{"label1", "label2"}, []string{"", "value2"}, 1.0),
1372+
NewPrometheusMetric("metric2", []string{"label1", "label2"}, []string{"value1", ""}, 1.0),
13631373
NewPrometheusMetric("metric1", []string{"label1"}, []string{"value1"}, 1.0),
13641374
},
13651375
},
13661376
}
13671377

13681378
for _, tc := range testCases {
13691379
t.Run(tc.name, func(t *testing.T) {
1370-
actual := EnsureLabelConsistencyAndRemoveDuplicates(tc.metrics, tc.observedLabels)
1380+
actual := EnsureLabelConsistencyAndRemoveDuplicates(tc.metrics, tc.observedLabels, logging.NewNopLogger())
13711381
require.ElementsMatch(t, tc.output, actual)
13721382
})
13731383
}
@@ -1381,14 +1391,15 @@ func Benchmark_EnsureLabelConsistencyAndRemoveDuplicates(b *testing.B) {
13811391
NewPrometheusMetric("metric1", []string{"label1"}, []string{"value1"}, 1.0),
13821392
}
13831393
observedLabels := map[string]model.LabelSet{"metric1": {"label1": {}, "label2": {}, "label3": {}}}
1394+
logger := logging.NewNopLogger()
13841395

13851396
var output []*PrometheusMetric
13861397

13871398
b.ReportAllocs()
13881399
b.ResetTimer()
13891400

13901401
for i := 0; i < b.N; i++ {
1391-
output = EnsureLabelConsistencyAndRemoveDuplicates(metrics, observedLabels)
1402+
output = EnsureLabelConsistencyAndRemoveDuplicates(metrics, observedLabels, logger)
13921403
}
13931404

13941405
expectedOutput := []*PrometheusMetric{

‎pkg/promutil/prometheus.go‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -154,6 +154,10 @@ func (p *PrometheusMetric) Labels() ([]string, []string) {
154154
return p.labels.keys, p.labels.vals
155155
}
156156

157+
func (p *PrometheusMetric) LabelsLen() int {
158+
return len(p.labels.keys)
159+
}
160+
157161
func (p *PrometheusMetric) Value() float64 {
158162
return p.value
159163
}
@@ -199,6 +203,21 @@ func (p *PrometheusMetric) AddIfMissingLabelPair(key, val string) {
199203
}
200204
}
201205

206+
func (p *PrometheusMetric) RemoveDuplicateLabels() {
207+
seen := map[string]struct{}{}
208+
idx := 0
209+
for i, key := range p.labels.keys {
210+
if _, ok := seen[key]; !ok {
211+
seen[key] = struct{}{}
212+
p.labels.keys[idx] = key
213+
p.labels.vals[idx] = p.labels.vals[i]
214+
idx++
215+
}
216+
}
217+
p.labels.keys = p.labels.keys[:idx]
218+
p.labels.vals = p.labels.vals[:idx]
219+
}
220+
202221
type PrometheusCollector struct {
203222
metrics []prometheus.Metric
204223
}

‎pkg/promutil/prometheus_test.go‎

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -121,6 +121,47 @@ func TestPromStringTag(t *testing.T) {
121121
}
122122
}
123123

124+
func TestPrometheusMetric(t *testing.T) {
125+
t.Run("NewPrometheusMetric panics with wrong label size", func(t *testing.T) {
126+
require.Panics(t, func() {
127+
NewPrometheusMetric("metric1", []string{"key1"}, []string{}, 1.0)
128+
})
129+
require.Panics(t, func() {
130+
NewPrometheusMetric("metric1", []string{}, []string{"label1"}, 1.0)
131+
})
132+
require.Panics(t, func() {
133+
NewPrometheusMetric("metric1", []string{"key1", "key2"}, []string{"label1"}, 1.0)
134+
})
135+
require.Panics(t, func() {
136+
NewPrometheusMetric("metric1", []string{"key1"}, []string{"label1", "label2"}, 1.0)
137+
})
138+
})
139+
140+
t.Run("NewPrometheusMetric sorts labels", func(t *testing.T) {
141+
metric := NewPrometheusMetric("metric", []string{"key2", "key1"}, []string{"value2", "value1"}, 1.0)
142+
keys, vals := metric.Labels()
143+
require.Equal(t, []string{"key1", "key2"}, keys)
144+
require.Equal(t, []string{"value1", "value2"}, vals)
145+
})
146+
147+
t.Run("AddIfMissingLabelPair keeps labels sorted", func(t *testing.T) {
148+
metric := NewPrometheusMetric("metric", []string{"key2"}, []string{"value2"}, 1.0)
149+
metric.AddIfMissingLabelPair("key1", "value1")
150+
keys, vals := metric.Labels()
151+
require.Equal(t, []string{"key1", "key2"}, keys)
152+
require.Equal(t, []string{"value1", "value2"}, vals)
153+
})
154+
155+
t.Run("RemoveDuplicateLabels", func(t *testing.T) {
156+
metric := NewPrometheusMetric("metric", []string{"key1", "key1"}, []string{"value1", "value2"}, 1.0)
157+
require.Equal(t, 2, metric.LabelsLen())
158+
metric.RemoveDuplicateLabels()
159+
keys, vals := metric.Labels()
160+
require.Equal(t, []string{"key1"}, keys)
161+
require.Equal(t, []string{"value1"}, vals)
162+
})
163+
}
164+
124165
func TestNewPrometheusCollector_CanReportMetricsAndErrors(t *testing.T) {
125166
metrics := []*PrometheusMetric{
126167
NewPrometheusMetric("this*is*not*valid", []string{}, []string{}, 0),

0 commit comments

Comments
 (0)