Skip to content

Commit

Permalink
7703 metricsql support metric names in quotes as alternative to name …
Browse files Browse the repository at this point in the history 
…metric pattern (#40)

* Initial tests for label keys to be quoted

* a few more tests involving numbers and quotes

* Adds support for quoted UTF8 label names and special case label name without a value to specify metric name

* Fixed bug with not checking for errors before calling isMetricNameFilter()

* Added more testing for quoted labels
Fixed bug with error not reporting because of isPossibleMetric being when a label name was not quoted

* Minor tweaks to comments

* Fixed tests to disregard two metric names
Added tests to ensure setting two metric names threw errors

* Removed extraneous comment

* Duplicate metric names are now allowed
Refactored code checking for quoted strings into isQuoteString
Added some additional tests

* Modified string output to be unescaped UTF8 chars

* Revert "Modified string output to be unescaped UTF8 chars"

This reverts commit 41dc7c1.

* Reverted parser_example_test.go back to using standard format of metric filters

Co-authored-by: Roman Khavronenko <[email protected]>

* Removed / modified some tests to exclude the case of multiple metric names - this case will now throw an error and is tested in expected Fail cases

* minor comments

---------

Co-authored-by: Roman Khavronenko <[email protected]>
  • Loading branch information
@possibull @hagen1778
possibull and hagen1778 authored Jan 24, 2025
1 parent ce4fd13 commit 72b650d
Show file tree
Hide file tree
Showing 3 changed files with 116 additions and 9 deletions.
3 changes: 2 additions & 1 deletion optimizer_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -274,7 +274,8 @@ func TestOptimize(t *testing.T) {

// label_set
f(`label_set(foo, "__name__", "bar") + x`, `label_set(foo, "__name__", "bar") + x`)
f(`label_set(foo, "a", "bar") + x{__name__="y"}`, `label_set(foo, "a", "bar") + x{__name__="y",a="bar"}`)
// No longer a valid test due to errors on two metric names being set
// f(`label_set(foo, "a", "bar") + x{__name__="y"}`, `label_set(foo, "a", "bar") + x{__name__="y",a="bar"}`)
f(`label_set(foo{bar="baz"}, "xx", "y") + a{x="y"}`, `label_set(foo{bar="baz",x="y"}, "xx", "y") + a{bar="baz",x="y",xx="y"}`)
f(`label_set(foo{x="y"}, "q", "b", "x", "qwe") + label_set(bar{q="w"}, "x", "a", "q", "w")`, `label_set(foo{x="y"}, "q", "b", "x", "qwe") + label_set(bar{q="w"}, "x", "a", "q", "w")`)
f(`label_set(foo{a="b"}, "a", "qwe") + bar{a="x"}`, `label_set(foo{a="b"}, "a", "qwe") + bar{a="qwe",a="x"}`)
Expand Down
66 changes: 61 additions & 5 deletions parser.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -833,16 +833,32 @@ func expandWithExpr(was []*withArgExpr, e Expr) (Expr, error) {
// Already expanded.
return t, nil
}
metricName := ""
{
var me MetricExpr
// Populate me.LabelFilterss

for _, lfes := range t.labelFilterss {
var lfsNew []LabelFilter
for _, lfe := range lfes {
if lfe.Value == nil {
// Expand lfe.Label into lfsNew.
wa := getWithArgExpr(was, lfe.Label)
if wa == nil {
// Check to see if this is a possible metric name
// This means label name set and starts and ends with quotes
// but value is nil
if lfe.IsPossibleMetricName {
if metricName == "" {
metricName = lfe.Label
continue
} else {
if metricName != lfe.Label {
return nil, fmt.Errorf("parse error: metric name must not be set twice: %q or %q", metricName, lfe.Label)
}
continue
}
}
return nil, fmt.Errorf("cannot find WITH template for %q inside %q", lfe.Label, t.AppendString(nil))
}
eNew, err := expandWithExprExt(was, wa, []Expr{})
Expand All @@ -867,7 +883,6 @@ func expandWithExpr(was []*withArgExpr, e Expr) (Expr, error) {
}
continue
}

// convert lfe to LabelFilter.
se, err := expandWithExpr(was, lfe.Value)
if err != nil {
Expand All @@ -882,14 +897,34 @@ func expandWithExpr(was []*withArgExpr, e Expr) (Expr, error) {
if err != nil {
return nil, err
}
if lf.isMetricNameFilter() {
if metricName != "" && metricName != lf.Value {
return nil, fmt.Errorf("parse error: metric name must not be set twice: %q or %q", metricName, lf.Value)
}
metricName = lf.Value
continue
}
lfsNew = append(lfsNew, *lf)
}
lfsNew = removeDuplicateLabelFilters(lfsNew)
me.LabelFilterss = append(me.LabelFilterss, lfsNew)
}
// Prepend metric name to latest
if metricName != "" {
lfesCount := len(t.labelFilterss)
for i := 1; i <= lfesCount; i++ {
lfsLastIndex := len(me.LabelFilterss) - i
var lfsNew []LabelFilter
var lfNew LabelFilter
lfNew.Label = "__name__"
lfNew.Value = metricName
lfsNew = append(lfsNew, lfNew)
lfsNew = append(lfsNew, me.LabelFilterss[lfsLastIndex]...)
me.LabelFilterss[lfsLastIndex] = lfsNew
}
}
t = &me
}
metricName := t.getMetricName()
if metricName == "" {
return t, nil
}
Expand Down Expand Up @@ -1347,10 +1382,24 @@ func (p *parser) parseLabelFilters(mf *labelFilterExpr) ([]*labelFilterExpr, err
}
}

func isQuotedString(s string) bool {
if isStringPrefix(s) && isStringPrefix(s[len(s)-1:]) {
return true
}
return false
}

func (p *parser) parseLabelFilterExpr() (*labelFilterExpr, error) {
if !isIdentPrefix(p.lex.Token) {
var isPossibleMetricName bool
if isQuotedString(p.lex.Token) {
// strip quotes
p.lex.Token = p.lex.Token[1 : len(p.lex.Token)-1]
// quoted string could be a metric name: {"metric_name"}
isPossibleMetricName = true
} else if !isIdentPrefix(p.lex.Token) {
return nil, fmt.Errorf(`labelFilterExpr: unexpected token %q; want "ident"`, p.lex.Token)
}

var lfe labelFilterExpr
lfe.Label = unescapeIdent(p.lex.Token)
if err := p.lex.Next(); err != nil {
Expand All @@ -1375,6 +1424,12 @@ func (p *parser) parseLabelFilterExpr() (*labelFilterExpr, error) {
// - {lf or other="filter"}
//
// It must be substituted by complete label filter during WITH template expand.
// If we have a label name that is quoted with a nil value it is possible it's the metric
// name as per Prometheus 3.0 UTF8 quoted label names specifications, this is used later
// in our expanding of the with statements
// https://github.com/prometheus/proposals/blob/main/proposals/2023-08-21-utf8.md
lfe.IsPossibleMetricName = isPossibleMetricName

return &lfe, nil
default:
return nil, fmt.Errorf(`labelFilterExpr: unexpected token %q; want "=", "!=", "=~", "!~", ",", "or", "}"`, p.lex.Token)
Expand All @@ -1401,8 +1456,9 @@ type labelFilterExpr struct {
// Value can be nil if Label contains unexpanded WITH template reference.
Value *StringExpr

IsRegexp bool
IsNegative bool
IsRegexp bool
IsNegative bool
IsPossibleMetricName bool
}

func (lfe *labelFilterExpr) AppendString(dst []byte) []byte {
Expand Down
56 changes: 53 additions & 3 deletions parser_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,30 +40,66 @@ func TestParseSuccess(t *testing.T) {
same(`{}[:3.4s] offset -10y`)
same(`{Foo="bAR"}`)
same(`{foo="bar"}`)
another(`{"foo"="bar"}`, `{foo="bar"}`)
another(`{"foo"="bAR"}`, `{foo="bAR"}`)
another(`{"3foo"="bar"}`, `{\3foo="bar"}`)
another(`{"'3foo'"="bar"}`, `{\'3foo\'="bar"}`)
another(`{'温度{房间="水电费\xF3"}'="1"}[5m] offset 10m`, `{温度\{房间\=\"水电费ó\"\}="1"}[5m] offset 10m`)
same(`{foo="bar"}[5m]`)
another(`{"foo"="bar"}[5m]`, `{foo="bar"}[5m]`)
same(`{foo="bar"}[5m:]`)
another(`{"foo"="bar"}[5m:]`, `{foo="bar"}[5m:]`)
same(`{foo="bar"}[5m:3s]`)
another(`{"foo"="bar"}[5m:3s]`, `{foo="bar"}[5m:3s]`)
same(`{foo="bar"} offset 13.4ms`)
another(`{"foo"="bar"} offset 13.4ms`, `{foo="bar"} offset 13.4ms`)
same(`{foo="bar"}[5w4h-3.4m13.4ms]`)
another(`{"foo"="bar"}[5w4h-3.4m13.4ms]`, `{foo="bar"}[5w4h-3.4m13.4ms]`)
same(`{foo="bar"} offset 10y`)
another(`{"foo"="bar"} offset 10y`, `{foo="bar"} offset 10y`)
same(`{foo="bar"} offset -10y`)
another(`{"foo"="bar"} offset -10y`, `{foo="bar"} offset -10y`)
same(`{foo="bar"}[5m] offset 10y`)
another(`{"foo"="bar"}[5m] offset 10y`, `{foo="bar"}[5m] offset 10y`)
same(`{foo="bar"}[5m:3s] offset 10y`)
another(`{"foo"="bar"}[5m:3s] offset 10y`, `{foo="bar"}[5m:3s] offset 10y`)
another(`{foo="bar"}[5m] oFFSEt 10y`, `{foo="bar"}[5m] offset 10y`)
another(`{"foo"="bar"}[5m] oFFSEt 10y`, `{foo="bar"}[5m] offset 10y`)
another(`{__name__="metric", a="1"}`, `metric{a="1"}`)
another(`{"metric", a="1"}`, `metric{a="1"}`)
another(`{"metric", "a"="1"}`, `metric{a="1"}`)
same("METRIC")
same("metric")
another(`metric{"metric"}`, "metric")
another(`metric{__name__="metric"}`, "metric")
another(`{"metric"}`, `metric`)
another(`{a="1",__name__="metric"}`, `metric{a="1"}`)
another("metric{}", "metric")
same("m_e:tri44:_c123")
another(`{"m_e:tri44:_c123"}`, "m_e:tri44:_c123")
another("-metric", "0 - metric")
another(`-{"metric"}`, "0 - metric")
same(`metric offset 10h`)
another(`{"metric"} offset 10h`, `metric offset 10h`)
same("metric[5m]")
another(`{"metric"}[5m]`, "metric[5m]")
same("metric[5m:3s]")
another(`{"metric"}[5m:3s]`, "metric[5m:3s]")
same("metric[5m] offset 10h")
another(`{"metric"}[5m] offset 10h`, `metric[5m] offset 10h`)
same("metric[5m:3s] offset 10h")
another(`{"metric"}[5m:3s] offset 10h`, "metric[5m:3s] offset 10h")
same("metric[5i:3i] offset 10i")
another(`{"metric"}[5i:3i]`, "metric[5i:3i]")
same(`metric{foo="bar"}`)
another(`{"metric",foo="bar"}`, `metric{foo="bar"}`)
another(`{"metric","foo"="bar"}`, `metric{foo="bar"}`)
same(`metric{foo="bar"} offset 10h`)
another(`{"metric",foo="bar"} offset 10h`, `metric{foo="bar"} offset 10h`)
another(`{"metric","foo"="bar"} offset 10h`, `metric{foo="bar"} offset 10h`)
same(`metric{foo!="bar"}[2d]`)
another(`{"metric",foo!="bar"}[2d]`, `metric{foo!="bar"}[2d]`)
another(`{"metric","foo"!="bar"}[2d]`, `metric{foo!="bar"}[2d]`)
same(`metric{foo="bar"}[2d] offset 10h`)
same(`metric{foo="bar",b="sdfsdf"}[2d:3h] offset 10h`)
same(`metric{foo="bar",b="sdfsdf"}[2d:3h] offset 10`)
Expand All @@ -72,11 +108,20 @@ func TestParseSuccess(t *testing.T) {
same(`metric{foo="bar",b="sdfsdf"}[2.34:5.6] offset 3600.5`)
same(`metric{foo="bar",b="sdfsdf"}[234:56] offset -3600`)
another(` metric { foo = "bar" } [ 2d ] offset 10h `, `metric{foo="bar"}[2d] offset 10h`)
another(` { "metric", foo = "bar" } [ 2d ] offset 10h `, `metric{foo="bar"}[2d] offset 10h`)

// metricExpr with 'or'
same(`metric{foo="bar" or baz="a"}`)
another(`metric{"foo"="bar" or "baz"="a"}`, `metric{foo="bar" or baz="a"}`)
another(`metric{"foo"="bar" or baz="a"}`, `metric{foo="bar" or baz="a"}`)
another(`metric{foo="bar" or "baz"="a"}`, `metric{foo="bar" or baz="a"}`)
another(`{"metric", foo="bar" or "baz"="a"}`, `metric{foo="bar" or baz="a"}`)
same(`metric{foo="bar",x="y" or baz="a",z="q" or a="b"}`)
another(`{"metric", "foo"="bar","x"="y" or "baz"="a","z"="q" or "a"="b"}`, `metric{foo="bar",x="y" or baz="a",z="q" or a="b"}`)
another(`{"metric", foo="bar","x"="y" or baz="a","z"="q" or a="b"}`, `metric{foo="bar",x="y" or baz="a",z="q" or a="b"}`)
same(`{foo="bar",x="y" or baz="a",z="q" or a="b"}`)
another(`{"foo"="bar","x"="y" or "baz"="a","z"="q" or "a"="b"}`, `{foo="bar",x="y" or baz="a",z="q" or a="b"}`)
another(`{"foo"="bar",x="y" or "baz"="a",z="q" or a="b"}`, `{foo="bar",x="y" or baz="a",z="q" or a="b"}`)
another(`metric{foo="bar" OR baz="a"}`, `metric{foo="bar" or baz="a"}`)
another(`{foo="bar" OR baz="a"}`, `{foo="bar" or baz="a"}`)

Expand Down Expand Up @@ -134,8 +179,8 @@ func TestParseSuccess(t *testing.T) {
another(`sum(x) by (b\x7Ca)`, `sum(x) by(b\|a)`)

// Duplicate filters
same(`foo{__name__="bar"}`)
same(`foo{a="b",a="c",__name__="aaa",b="d"}`)
same(`foo{a="b",a="c",b="d"}`)
same(`{a="b",a="c",b="d"}`)

// Metric filters ending with comma
another(`m{foo="bar",}`, `m{foo="bar"}`)
Expand Down Expand Up @@ -623,7 +668,11 @@ func TestParseError(t *testing.T) {
f(`foo{,foo="bar"}`)
f(`foo{foo=}`)
f(`foo{foo="ba}`)
f(`foo{"foo"="bar"}`)
// No longer invalid with Prometheus 3.0 quoted label names
//f(`foo{"foo"="bar"}`)
// Test if two metric names are set
f(`{"foo", "a"="1", "bar"}`)
f(`{"foo", __name__="bar"}`)
f(`foo{$`)
f(`foo{a $`)
f(`foo{a="b",$`)
Expand Down Expand Up @@ -932,6 +981,7 @@ func TestParseError(t *testing.T) {
f(`with (f())`)
f(`with (sum(a,b)=a+b) sum(x)`)
f(`with (rate()=foobar) rate(x)`)
f(`{foo}`)
f(`with (x={y}) x`)

// with template with {lf1 or lf2} isn't supported
Expand Down

0 comments on commit 72b650d

Please sign in to comment.