Skip to content

Commit

Permalink
[BUGFIX] labels: don't modify original labels in DropMetricName (#13845)
Browse files Browse the repository at this point in the history
Restrict the capacity of first argument to `append()` to force an allocation.
This is for the slice implementation only.

Signed-off-by: Domantas Jadenkus <djadenkus@gmail.com>
Signed-off-by: Bryan Boreham <bjboreham@gmail.com>
  • Loading branch information
jDomantas authored and bboreham committed Mar 27, 2024
1 parent d771cab commit 3929d65
Show file tree
Hide file tree
Showing 3 changed files with 26 additions and 2 deletions.
4 changes: 3 additions & 1 deletion model/labels/labels.go
Original file line number Diff line number Diff line change
Expand Up @@ -349,7 +349,9 @@ func (ls Labels) DropMetricName() Labels {
if i == 0 { // Make common case fast with no allocations.
return ls[1:]
}
return append(ls[:i], ls[i+1:]...)
// Avoid modifying original Labels - use [:i:i] so that left slice would not
// have any spare capacity and append would have to allocate a new slice for the result.
return append(ls[:i:i], ls[i+1:]...)
}
}
return ls
Expand Down
6 changes: 5 additions & 1 deletion model/labels/labels_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -450,7 +450,11 @@ func TestLabels_Get(t *testing.T) {
func TestLabels_DropMetricName(t *testing.T) {
require.True(t, Equal(FromStrings("aaa", "111", "bbb", "222"), FromStrings("aaa", "111", "bbb", "222").DropMetricName()))
require.True(t, Equal(FromStrings("aaa", "111"), FromStrings(MetricName, "myname", "aaa", "111").DropMetricName()))
require.True(t, Equal(FromStrings("__aaa__", "111", "bbb", "222"), FromStrings("__aaa__", "111", MetricName, "myname", "bbb", "222").DropMetricName()))

original := FromStrings("__aaa__", "111", MetricName, "myname", "bbb", "222")
check := FromStrings("__aaa__", "111", MetricName, "myname", "bbb", "222")
require.True(t, Equal(FromStrings("__aaa__", "111", "bbb", "222"), check.DropMetricName()))
require.True(t, Equal(original, check))
}

// BenchmarkLabels_Get was written to check whether a binary search can improve the performance vs the linear search implementation
Expand Down
18 changes: 18 additions & 0 deletions promql/engine_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3136,6 +3136,24 @@ func TestRangeQuery(t *testing.T) {
End: time.Unix(120, 0),
Interval: 1 * time.Minute,
},
{
Name: "drop-metric-name",
Load: `load 30s
requests{job="1", __address__="bar"} 100`,
Query: `requests * 2`,
Result: Matrix{
Series{
Floats: []FPoint{{F: 200, T: 0}, {F: 200, T: 60000}, {F: 200, T: 120000}},
Metric: labels.FromStrings(
"__address__", "bar",
"job", "1",
),
},
},
Start: time.Unix(0, 0),
End: time.Unix(120, 0),
Interval: 1 * time.Minute,
},
}
for _, c := range cases {
t.Run(c.Name, func(t *testing.T) {
Expand Down

0 comments on commit 3929d65

Please sign in to comment.