Skip to content

Commit

Permalink
Fix the parsing of threshold name tags containing tokens
Browse files Browse the repository at this point in the history
@efdknittlfrank pointed out that v0.38.0 presented a bug when thresholds
were defined for URL grouping. Indeed, using colons and curly braces
inside of a thresholds tags definition would confuse the parser, and
lead to an unclear error.

This commit ensures that we parse thresholds tags properly:
* closing curly brace is now parsed from the right
* we ensure the closing curly brace is the last character in the string.
* knowing the position of the opening an closing tag definition curly
braces, we extract the whole definition's substring based on that.
  • Loading branch information
oleiade committed May 5, 2022
1 parent e4b75cd commit 7b57242
Show file tree
Hide file tree
Showing 2 changed files with 37 additions and 14 deletions.
32 changes: 18 additions & 14 deletions metrics/metric.go
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,7 @@ var ErrMetricNameParsing = errors.New("parsing metric name failed")
// of "key:value" strings. On failure, it returns an error containing the `ErrMetricNameParsing` in its chain.
func ParseMetricName(name string) (string, []string, error) {
openingTokenPos := strings.IndexByte(name, '{')
closingTokenPos := strings.IndexByte(name, '}')
closingTokenPos := strings.LastIndexByte(name, '}')
containsOpeningToken := openingTokenPos != -1
containsClosingToken := closingTokenPos != -1

Expand All @@ -150,25 +150,29 @@ func ParseMetricName(name string) (string, []string, error) {
return "", nil, fmt.Errorf("%w; reason: unmatched opening/close curly brace", ErrMetricNameParsing)
}

// If the closing brace token appears before the opening one,
// the expression is malformed
if closingTokenPos < openingTokenPos {
return "", nil, fmt.Errorf("%w; reason: closing curly brace appears before opening one", ErrMetricNameParsing)
}

parserFn := func(c rune) bool {
return c == '{' || c == '}'
// If the last character is not a closing brace token,
// the expression is malformed.
if closingTokenPos != (len(name) - 1) {
err := fmt.Errorf(
"%w; reason: missing closing curly brace in last position"+
"of the threshold expression",
ErrMetricNameParsing,
)
return "", nil, err
}

// Split the metric_name{tag_key:tag_value,...} expression
// into two "metric_name" and "tag_key:tag_value,..." strings.
parts := strings.FieldsFunc(name, parserFn)
if len(parts) == 0 || len(parts) > 2 {
return "", nil, ErrMetricNameParsing
}

// Split the tag key values
tags := strings.Split(parts[1], ",")
// We already know the position of the opening and closing curly brace
// tokens. Thus, we extract the string in between them, and split its
// content to obtain the tags key values.
tags := strings.Split(name[openingTokenPos+1:closingTokenPos], ",")

// For each tag definition, ensure
// For each tag definition, ensure it is correctly formed
for i, t := range tags {
keyValue := strings.SplitN(t, ":", 2)

Expand All @@ -179,5 +183,5 @@ func ParseMetricName(name string) (string, []string, error) {
tags[i] = strings.TrimSpace(t)
}

return parts[0], tags, nil
return name[0:openingTokenPos], tags, nil
}
19 changes: 19 additions & 0 deletions metrics/metric_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,20 @@ func TestParseMetricName(t *testing.T) {
wantTags: []string{"group:::mygroup"},
wantErr: false,
},
{
name: "metric name with valid name and repeated curly braces tokens in tags definition",
metricNameExpression: "http_req_duration{name:http://${}.com}",
wantMetricName: "http_req_duration",
wantTags: []string{"name:http://${}.com"},
wantErr: false,
},
{
name: "metric name with valid name and repeated curly braces and colon tokens in tags definition",
metricNameExpression: "http_req_duration{name:http://${}.com,url:ssh://github.com:grafana/k6}",
wantMetricName: "http_req_duration",
wantTags: []string{"name:http://${}.com", "url:ssh://github.com:grafana/k6"},
wantErr: false,
},
{
name: "metric name with tag definition missing `:value`",
metricNameExpression: "test_metric{easyas}",
Expand Down Expand Up @@ -146,6 +160,11 @@ func TestParseMetricName(t *testing.T) {
metricNameExpression: "test_metric}abc{bar",
wantErr: true,
},
{
name: "metric name with valid name and trailing characters after closing curly brace in tags definition",
metricNameExpression: "test_metric{foo:ba}r",
wantErr: true,
},
}
for _, tt := range tests {
tt := tt
Expand Down

0 comments on commit 7b57242

Please sign in to comment.