Skip to content

Commit

Permalink
Fix parser.VectorSelector.String() with empty name matcher (#14015)
Browse files Browse the repository at this point in the history
The check fell into "this matcher equals vector selector's name" case when vector selector doesn't have a name and the matcher is an explicit matcher for an empty __name__ label.

To provide some context about why this is important: some downstream projects use the promql.Parse(expr.String()) to clone an expression's AST, and with this bug that matcher disappears in the cloning.

Signed-off-by: Oleg Zaytsev <mail@olegzaytsev.com>
  • Loading branch information
colega authored May 6, 2024
1 parent 93be830 commit 4b7a44c
Show file tree
Hide file tree
Showing 2 changed files with 15 additions and 2 deletions.
4 changes: 2 additions & 2 deletions promql/parser/printer.go
Original file line number Diff line number Diff line change
Expand Up @@ -204,8 +204,8 @@ func (node *VectorSelector) String() string {
labelStrings = make([]string, 0, len(node.LabelMatchers)-1)
}
for _, matcher := range node.LabelMatchers {
// Only include the __name__ label if its equality matching and matches the name.
if matcher.Name == labels.MetricName && matcher.Type == labels.MatchEqual && matcher.Value == node.Name {
// Only include the __name__ label if its equality matching and matches the name, but don't skip if it's an explicit empty name matcher.
if matcher.Name == labels.MetricName && matcher.Type == labels.MatchEqual && matcher.Value == node.Name && matcher.Value != "" {
continue
}
labelStrings = append(labelStrings, matcher.String())
Expand Down
13 changes: 13 additions & 0 deletions promql/parser/printer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,9 @@ func TestExprString(t *testing.T) {
{
in: `a[1m] @ end()`,
},
{
in: `{__name__="",a="x"}`,
},
}

for _, test := range inputs {
Expand Down Expand Up @@ -216,6 +219,16 @@ func TestVectorSelector_String(t *testing.T) {
},
expected: `{__name__="foobar"}`,
},
{
name: "empty name matcher",
vs: VectorSelector{
LabelMatchers: []*labels.Matcher{
labels.MustNewMatcher(labels.MatchEqual, labels.MetricName, ""),
labels.MustNewMatcher(labels.MatchEqual, "a", "x"),
},
},
expected: `{__name__="",a="x"}`,
},
} {
t.Run(tc.name, func(t *testing.T) {
require.Equal(t, tc.expected, tc.vs.String())
Expand Down

0 comments on commit 4b7a44c

Please sign in to comment.