Skip to content

Commit

Permalink
Fix metric name extracting in AggKey
Browse files Browse the repository at this point in the history
Co-authored-by: msaf1980 <[email protected]>
  • Loading branch information
npazosmendez and msaf1980 committed Sep 12, 2022
1 parent 6b03de9 commit d77e3ee
Show file tree
Hide file tree
Showing 6 changed files with 194 additions and 43 deletions.
3 changes: 2 additions & 1 deletion expr/functions/aliasByNode/function.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,8 @@ func (f *aliasByNode) Do(ctx context.Context, e parser.Expr, from, until int64,
results := make([]*types.MetricData, len(args))

for i, a := range args {
r := a.CopyName(helper.AggKey(a, nodesOrTags))
name := helper.AggKey(a, nodesOrTags)
r := a.CopyTag(name, map[string]string{"name": name})
results[i] = r
}

Expand Down
17 changes: 8 additions & 9 deletions expr/functions/aliasByNode/function_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -131,15 +131,14 @@ func TestAliasByNode(t *testing.T) {
},
Want: []*types.MetricData{types.MakeMetricData("base.metric1", []float64{math.NaN(), 1, 1, 1, 1}, 1, now32)},
},
// FIXME: I don't think `metric1.foo==.bar.baz` is a valid metric name, that is making this test fail https://github.com/grafana/carbonapi/issues/68
// {
// Target: "aliasByNode(metric1.fo*.bar.baz,1,3)",
// M: map[parser.MetricRequest][]*types.MetricData{
// {Metric: "metric1.fo*.bar.baz", From: 0, Until: 1}: {types.MakeMetricData("metric1.foo==.bar.baz", []float64{1, 2, 3, 4, 5}, 1, now32)},
// },
// Want: []*types.MetricData{types.MakeMetricData("foo==.baz",
// []float64{1, 2, 3, 4, 5}, 1, now32)},
// },
{
Target: "aliasByNode(metric1.fo*.bar.baz,1,3)",
M: map[parser.MetricRequest][]*types.MetricData{
{Metric: "metric1.fo*.bar.baz", From: 0, Until: 1}: {types.MakeMetricData("metric1.foo==.bar.baz", []float64{1, 2, 3, 4, 5}, 1, now32)},
},
Want: []*types.MetricData{types.MakeMetricData("foo==.baz",
[]float64{1, 2, 3, 4, 5}, 1, now32)},
},
{
Target: `aliasByTags(*, 2, "baz", "foo", 1)`,
M: map[parser.MetricRequest][]*types.MetricData{
Expand Down
61 changes: 30 additions & 31 deletions expr/functions/groupByNode/function_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -168,37 +168,36 @@ func TestGroupByNode(t *testing.T) {
"metric1.foo.bar1.bla.foo": {types.MakeMetricData("metric1.foo.bar1.bla.foo", []float64{1, 2, 3, 4, 5}, 1, now32)},
},
},
// FIXME: I don't think `..foo==.bar` is a valid metric name, that is making these tests fail https://github.com/grafana/carbonapi/issues/68
// {
// Target: "groupByNode(metric1.foo.*.*,2,\"sum\")",
// M: map[parser.MetricRequest][]*types.MetricData{
// mr: {
// types.MakeMetricData("metric1.foo.Ab1==.lag", []float64{1, 2, 3, 4, 5}, 1, now32),
// types.MakeMetricData("metric1.foo.bC2=.lag", []float64{6, 7, 8, 9, 10}, 1, now32),
// types.MakeMetricData("metric1.foo.Ab1==.lag", []float64{11, 12, 13, 14, 15}, 1, now32),
// types.MakeMetricData("metric1.foo.bC2=.lag=", []float64{7, 8, 9, 10, 11}, 1, now32),
// },
// },
// Name: "groupByNode_names_with_special_symbol_equal",
// Results: map[string][]*types.MetricData{
// "Ab1==": {types.MakeMetricData("Ab1==", []float64{12, 14, 16, 18, 20}, 1, now32)},
// "bC2=": {types.MakeMetricData("bC2=", []float64{13, 15, 17, 19, 21}, 1, now32)},
// },
// },
// {
// Target: "groupByNode(metric1.foo.*.*,3,\"sum\")",
// M: map[parser.MetricRequest][]*types.MetricData{
// mr: {
// types.MakeMetricData("metric1.foo.Ab1==.lag;tag1=value1", []float64{1, 2, 3, 4, 5}, 1, now32),
// types.MakeMetricData("metric1.foo.Ab2.lag=;tag1=value1", []float64{1, 0, 3, 4, 5}, 1, now32),
// },
// },
// Name: "groupByNode_tagged_names_with_special_symbol_equal",
// Results: map[string][]*types.MetricData{
// "lag": {types.MakeMetricData("lag", []float64{1, 2, 3, 4, 5}, 1, now32)},
// "lag=": {types.MakeMetricData("lag=", []float64{1, 0, 3, 4, 5}, 1, now32)},
// },
// },
{
Target: "groupByNode(metric1.foo.*.*,2,\"sum\")",
M: map[parser.MetricRequest][]*types.MetricData{
mr: {
types.MakeMetricData("metric1.foo.Ab1==.lag", []float64{1, 2, 3, 4, 5}, 1, now32),
types.MakeMetricData("metric1.foo.bC2=.lag", []float64{6, 7, 8, 9, 10}, 1, now32),
types.MakeMetricData("metric1.foo.Ab1==.lag", []float64{11, 12, 13, 14, 15}, 1, now32),
types.MakeMetricData("metric1.foo.bC2=.lag=", []float64{7, 8, 9, 10, 11}, 1, now32),
},
},
Name: "groupByNode_names_with_special_symbol_equal",
Results: map[string][]*types.MetricData{
"Ab1==": {types.MakeMetricData("Ab1==", []float64{12, 14, 16, 18, 20}, 1, now32)},
"bC2=": {types.MakeMetricData("bC2=", []float64{13, 15, 17, 19, 21}, 1, now32)},
},
},
{
Target: "groupByNode(metric1.foo.*.*,3,\"sum\")",
M: map[parser.MetricRequest][]*types.MetricData{
mr: {
types.MakeMetricData("metric1.foo.Ab1==.lag;tag1=value1", []float64{1, 2, 3, 4, 5}, 1, now32),
types.MakeMetricData("metric1.foo.Ab2.lag=;tag1=value1", []float64{1, 0, 3, 4, 5}, 1, now32),
},
},
Name: "groupByNode_tagged_names_with_special_symbol_equal",
Results: map[string][]*types.MetricData{
"lag": {types.MakeMetricData("lag", []float64{1, 2, 3, 4, 5}, 1, now32)},
"lag=": {types.MakeMetricData("lag=", []float64{1, 0, 3, 4, 5}, 1, now32)},
},
},
}

for _, tt := range tests {
Expand Down
3 changes: 1 addition & 2 deletions expr/helper/helper.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import (
"regexp"
"strings"

"github.com/go-graphite/carbonapi/expr/helper/metric"
"github.com/go-graphite/carbonapi/expr/interfaces"
"github.com/go-graphite/carbonapi/expr/types"
"github.com/go-graphite/carbonapi/pkg/parser"
Expand Down Expand Up @@ -93,7 +92,7 @@ func GetSeriesArgsAndRemoveNonExisting(ctx context.Context, e parser.Expr, from,
func AggKey(arg *types.MetricData, nodesOrTags []parser.NodeOrTag) string {
matched := make([]string, 0, len(nodesOrTags))
metricTags := arg.Tags
name := metric.ExtractMetric(arg.Name)
name := types.ExtractNameTag(arg.Name)
nodes := strings.Split(name, ".")
for _, nt := range nodesOrTags {
if nt.IsTag {
Expand Down
44 changes: 44 additions & 0 deletions expr/types/extract.go
Original file line number Diff line number Diff line change
Expand Up @@ -64,3 +64,47 @@ func ExtractName(s string) string {
start, end := ExtractNameLoc(s)
return s[start:end]
}

// ExtractNameTag extracts name tag out of function list with . Only for use in MetrciData name parse, it's has more allowed chard in names, like =
func ExtractNameTag(s string) string {
// search for a metric name in 's'
// metric name is defined to be a Series of name characters terminated by a ',' or ')'

var w, i, start, braces int
var c rune
FOR:
for i, c = range s {
w = i
switch c {
case '{':
braces++
case '}':
if braces == 0 {
break FOR
}
braces--
case ',':
if braces == 0 {
break FOR
}
case '(':
if i >= 11 {
n := i - 11
if s[n:i] == "seriesByTag" {
end := strings.IndexRune(s[n:], ')')
if end == -1 {
return s[n:len(s)] // broken end of args, can't exist in correctly parsed functions
} else {
return s[n : n+end+1]
}
}
}
start = i + 1
case ')', ';':
break FOR
}
w += utf8.RuneLen(c)
}

return s[start:w]
}
109 changes: 109 additions & 0 deletions expr/types/extract_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,10 @@ func TestExtractName(t *testing.T) {
"sum(seriesByTag('tag2=value*', 'name=metric'))",
"seriesByTag('tag2=value*', 'name=metric')",
},
{
"sum(metric.name;tag=value)",
"metric.name;tag=value",
},
}

for _, tt := range tests {
Expand All @@ -98,3 +102,108 @@ func TestExtractName(t *testing.T) {
})
}
}

func TestExtractNameTag(t *testing.T) {
var tests = []struct {
input string
metric string
}{
{
"f",
"f",
},
{
"func(f)",
"f",
},
{
"foo.bar.baz",
"foo.bar.baz",
},
{
"nonNegativeDerivative(foo.bar.baz)",
"foo.bar.baz",
},
{
"movingAverage(foo.bar.baz,10)",
"foo.bar.baz",
},
{
"scale(scaleToSeconds(nonNegativeDerivative(foo.bar.baz),60),60)",
"foo.bar.baz",
},
{
"divideSeries(foo.bar.baz,baz.qux.zot)",
"foo.bar.baz",
},
{
"{something}",
"{something}",
},
{
"ab=",
"ab=",
},
{
"ab=.c",
"ab=.c",
},
{
"ab==",
"ab==",
},
{
"scale(scaleToSeconds(nonNegativeDerivative(ab==.c),60),60)",
"ab==.c",
},
{
"divideSeries(metric[12])",
"metric[12]",
},
{
"average(metric{1,2}e,'sum')",
"metric{1,2}e",
},
{
"aliasByNode(alias([email protected], 2), 1)",
"[email protected]",
},
{
"aliasByTags(alias([email protected], 2), 1)",
"[email protected]",
},
// non-ASCII symbols
{
"alias(Количество изменений)",
"Количество изменений",
},
{
"some(Количество изменений, Аргумент)",
"Количество изменений",
},
{
"seriesByTag('tag2=value*', 'name=metric')",
"seriesByTag('tag2=value*', 'name=metric')",
},
{
"sum(seriesByTag('tag2=value*', 'name=metric'))",
"seriesByTag('tag2=value*', 'name=metric')",
},
{
"sum(metric.name;tag=value)",
"metric.name",
},
{
"metric1.foo==.bar.baz",
"metric1.foo==.bar.baz",
},
}

for _, tt := range tests {
t.Run(tt.input, func(t *testing.T) {
if m := ExtractNameTag(tt.input); m != tt.metric {
t.Errorf("ExtractNameTag(%q)=%q, want %q", tt.input, m, tt.metric)
}
})
}
}

0 comments on commit d77e3ee

Please sign in to comment.