-
Notifications
You must be signed in to change notification settings - Fork 2.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add matcher support to Query Rules endpoint #5111
Changes from 2 commits
cdc3c2a
91002dc
4a1dc37
2120a1c
9e25ab2
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -7,9 +7,12 @@ import ( | |||||
"context" | ||||||
"sort" | ||||||
"sync" | ||||||
"text/template" | ||||||
"text/template/parse" | ||||||
|
||||||
"github.com/pkg/errors" | ||||||
"github.com/prometheus/prometheus/model/labels" | ||||||
"github.com/prometheus/prometheus/promql/parser" | ||||||
"github.com/prometheus/prometheus/storage" | ||||||
|
||||||
"github.com/thanos-io/thanos/pkg/rules/rulespb" | ||||||
|
@@ -58,6 +61,16 @@ func (rr *GRPCClient) Rules(ctx context.Context, req *rulespb.RulesRequest) (*ru | |||||
return nil, nil, errors.Wrap(err, "proxy Rules") | ||||||
} | ||||||
|
||||||
var matcherSets [][]*labels.Matcher | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can we pre-allocate here? We know the final slice's length, right? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, will pre-allocate! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done! |
||||||
for _, s := range req.MatcherString { | ||||||
matchers, err := parser.ParseMetricSelector(s) | ||||||
if err != nil { | ||||||
return nil, nil, errors.Wrap(err, "proxy Rules") | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: Should make the error description just a tiny bit clearer?
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah! Nice catch, will change this! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done! |
||||||
} | ||||||
matcherSets = append(matcherSets, matchers) | ||||||
} | ||||||
|
||||||
resp.groups = filterRules(resp.groups, matcherSets) | ||||||
// TODO(bwplotka): Move to SortInterface with equal method and heap. | ||||||
resp.groups = dedupGroups(resp.groups) | ||||||
for _, g := range resp.groups { | ||||||
|
@@ -67,6 +80,52 @@ func (rr *GRPCClient) Rules(ctx context.Context, req *rulespb.RulesRequest) (*ru | |||||
return &rulespb.RuleGroups{Groups: resp.groups}, resp.warnings, nil | ||||||
} | ||||||
|
||||||
// filterRules filters rules in a group according to given matcherSets. | ||||||
func filterRules(ruleGroups []*rulespb.RuleGroup, matcherSets [][]*labels.Matcher) []*rulespb.RuleGroup { | ||||||
if len(matcherSets) == 0 || len(ruleGroups) == 0 { | ||||||
onprem marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
return ruleGroups | ||||||
} | ||||||
|
||||||
for _, g := range ruleGroups { | ||||||
filteredRules := []*rulespb.Rule{} | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. A small advice: use There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done! |
||||||
for _, r := range g.Rules { | ||||||
rl := r.GetLabels() | ||||||
if matches(matcherSets, rl) { | ||||||
filteredRules = append(filteredRules, r) | ||||||
} | ||||||
} | ||||||
g.Rules = filteredRules | ||||||
} | ||||||
|
||||||
return ruleGroups | ||||||
} | ||||||
|
||||||
// matches returns whether the non-templated labels satisfy all the matchers in matcherSets. | ||||||
func matches(matcherSets [][]*labels.Matcher, l labels.Labels) bool { | ||||||
if len(matcherSets) == 0 { | ||||||
return true | ||||||
} | ||||||
|
||||||
var nonTemplatedLabels labels.Labels | ||||||
labelTemplate := template.New("label") | ||||||
for _, label := range l { | ||||||
t, err := labelTemplate.Parse(label.Value) | ||||||
// Label value is non-templated if it is one node of type NodeText. | ||||||
if err == nil && len(t.Root.Nodes) == 1 && t.Root.Nodes[0].Type() == parse.NodeText { | ||||||
nonTemplatedLabels = append(nonTemplatedLabels, label) | ||||||
} | ||||||
} | ||||||
|
||||||
for _, matchers := range matcherSets { | ||||||
for _, m := range matchers { | ||||||
if v := nonTemplatedLabels.Get(m.Name); !m.Matches(v) { | ||||||
return false | ||||||
} | ||||||
} | ||||||
} | ||||||
return true | ||||||
} | ||||||
|
||||||
// dedupRules re-sorts the set so that the same series with different replica | ||||||
// labels are coming right after each other. | ||||||
func dedupRules(rules []*rulespb.Rule, replicaLabels map[string]struct{}) []*rulespb.Rule { | ||||||
|
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -40,6 +40,7 @@ message RulesRequest { | |
} | ||
Type type = 1; | ||
PartialResponseStrategy partial_response_strategy = 2; | ||
repeated string MatcherString = 3; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should we keep this field name There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sure, let me change! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done! |
||
} | ||
|
||
message RulesResponse { | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A small nit: add dot at the line end.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done! Also, have added tests! 🙂