Skip to content

Commit

Permalink
pkg/rule: fix /api/v1/rules endpoint
Browse files Browse the repository at this point in the history
Signed-off-by: Simon Pasquier <[email protected]>
  • Loading branch information
simonpasquier committed Nov 21, 2019
1 parent cd6271c commit e57bf1d
Show file tree
Hide file tree
Showing 3 changed files with 77 additions and 73 deletions.
6 changes: 3 additions & 3 deletions pkg/rule/api/v1.go
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ func (api *API) rules(r *http.Request) (interface{}, []error, *qapi.ApiError) {
}

switch rule := r.(type) {
case thanosrule.AlertingRule:
case *rules.AlertingRule:
enrichedRule = alertingRule{
Name: rule.Name(),
Query: rule.Query().String(),
Expand All @@ -95,7 +95,7 @@ func (api *API) rules(r *http.Request) (interface{}, []error, *qapi.ApiError) {
Health: rule.Health(),
LastError: lastError,
Type: "alerting",
PartialResponseStrategy: rule.PartialResponseStrategy.String(),
PartialResponseStrategy: grp.PartialResponseStrategy.String(),
}
case *rules.RecordingRule:
enrichedRule = recordingRule{
Expand All @@ -107,7 +107,7 @@ func (api *API) rules(r *http.Request) (interface{}, []error, *qapi.ApiError) {
Type: "recording",
}
default:
err := fmt.Errorf("failed to assert type of rule '%v'", rule.Name())
err := fmt.Errorf("rule %q: unsupported type %T", r.Name(), rule)
return nil, nil, &qapi.ApiError{Typ: qapi.ErrorInternal, Err: err}
}

Expand Down
119 changes: 62 additions & 57 deletions pkg/rule/api/v1_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -62,8 +62,6 @@ type rulesRetrieverMock struct {
}

func (m rulesRetrieverMock) RuleGroups() []thanosrule.Group {
var ar rulesRetrieverMock
arules := ar.AlertingRules()
storage := newStorage(m.testing)

engineOpts := promql.EngineOpts{
Expand All @@ -83,9 +81,8 @@ func (m rulesRetrieverMock) RuleGroups() []thanosrule.Group {
}

var r []rules.Rule

for _, alertrule := range arules {
r = append(r, alertrule)
for _, ar := range alertingRules() {
r = append(r, ar)
}

recordingExpr, err := promql.ParseExpr(`vector(1)`)
Expand All @@ -96,43 +93,49 @@ func (m rulesRetrieverMock) RuleGroups() []thanosrule.Group {
r = append(r, recordingRule)

group := rules.NewGroup("grp", "/path/to/file", time.Second, r, false, opts)
return []thanosrule.Group{{Group: group}}
return []thanosrule.Group{thanosrule.Group{Group: group}}
}

func (m rulesRetrieverMock) AlertingRules() []thanosrule.AlertingRule {
var ars []thanosrule.AlertingRule
for _, ar := range alertingRules() {
ars = append(ars, thanosrule.AlertingRule{AlertingRule: ar})
}
return ars
}

func alertingRules() []*rules.AlertingRule {
expr1, err := promql.ParseExpr(`absent(test_metric3) != 1`)
if err != nil {
m.testing.Fatalf("unable to parse alert expression: %s", err)
panic(fmt.Sprintf("unable to parse alert expression: %s", err))
}
expr2, err := promql.ParseExpr(`up == 1`)
if err != nil {
m.testing.Fatalf("Unable to parse alert expression: %s", err)
panic(fmt.Sprintf("unable to parse alert expression: %s", err))
}

rule1 := rules.NewAlertingRule(
"test_metric3",
expr1,
time.Second,
labels.Labels{},
labels.Labels{},
labels.Labels{},
true,
log.NewNopLogger(),
)
rule2 := rules.NewAlertingRule(
"test_metric4",
expr2,
time.Second,
labels.Labels{},
labels.Labels{},
labels.Labels{},
true,
log.NewNopLogger(),
)
var r []thanosrule.AlertingRule
r = append(r, thanosrule.AlertingRule{AlertingRule: rule1})
r = append(r, thanosrule.AlertingRule{AlertingRule: rule2})
return r
return []*rules.AlertingRule{
rules.NewAlertingRule(
"test_metric3",
expr1,
time.Second,
labels.Labels{},
labels.Labels{},
labels.Labels{},
true,
log.NewNopLogger(),
),
rules.NewAlertingRule(
"test_metric4",
expr2,
time.Second,
labels.Labels{},
labels.Labels{},
labels.Labels{},
true,
log.NewNopLogger(),
),
}
}

func TestEndpoints(t *testing.T) {
Expand Down Expand Up @@ -171,16 +174,17 @@ func TestEndpoints(t *testing.T) {
}

func testEndpoints(t *testing.T, api *API) {

type test struct {
endpoint qapi.ApiFunc
params map[string]string
query url.Values
response interface{}
endpointFn qapi.ApiFunc
endpointName string
params map[string]string
query url.Values
response interface{}
}
var tests = []test{
{
endpoint: api.rules,
endpointFn: api.rules,
endpointName: "rules",
response: &RuleDiscovery{
RuleGroups: []*RuleGroup{
{
Expand Down Expand Up @@ -232,27 +236,28 @@ func testEndpoints(t *testing.T, api *API) {
request := func(m string, q url.Values) (*http.Request, error) {
return http.NewRequest(m, fmt.Sprintf("http://example.com?%s", q.Encode()), nil)
}
for i, test := range tests {
for _, method := range methods(test.endpoint) {
// Build a context with the correct request params.
ctx := context.Background()
for p, v := range test.params {
ctx = route.WithParam(ctx, p, v)
}
t.Logf("run %d\t%s\t%q", i, method, test.query.Encode())
for _, test := range tests {
for _, method := range methods(test.endpointFn) {
t.Run(fmt.Sprintf("endpoint=%s/method=%s/query=%q", test.endpointName, method, test.query.Encode()), func(t *testing.T) {
// Build a context with the correct request params.
ctx := context.Background()
for p, v := range test.params {
ctx = route.WithParam(ctx, p, v)
}

req, err := request(method, test.query)
if err != nil {
t.Fatal(err)
}
endpoint, errors, apiError := test.endpoint(req.WithContext(ctx))
req, err := request(method, test.query)
if err != nil {
t.Fatal(err)
}
endpoint, errors, apiError := test.endpointFn(req.WithContext(ctx))

if errors != nil {
t.Fatalf("Unexpected errors: %s", errors)
return
}
assertAPIError(t, apiError)
assertAPIResponse(t, endpoint, test.response)
if errors != nil {
t.Fatalf("Unexpected errors: %s", errors)
return
}
assertAPIError(t, apiError)
assertAPIResponse(t, endpoint, test.response)
})
}
}
}
Expand Down
25 changes: 12 additions & 13 deletions pkg/rule/rule.go
Original file line number Diff line number Diff line change
Expand Up @@ -112,8 +112,8 @@ func (r RuleGroup) MarshalYAML() (interface{}, error) {
// special field in RuleGroup file.
func (m *Managers) Update(dataDir string, evalInterval time.Duration, files []string) error {
var (
errs = tsdberrors.MultiError{}
filesMap = map[storepb.PartialResponseStrategy][]string{}
errs = tsdberrors.MultiError{}
filesByStrategy = map[storepb.PartialResponseStrategy][]string{}
)

if err := os.RemoveAll(path.Join(dataDir, tmpRuleDir)); err != nil {
Expand All @@ -138,19 +138,19 @@ func (m *Managers) Update(dataDir string, evalInterval time.Duration, files []st

// NOTE: This is very ugly, but we need to reparse it into tmp dir without the field to have to reuse
// rules.Manager. The problem is that it uses yaml.UnmarshalStrict for some reasons.
mapped := map[storepb.PartialResponseStrategy]*rulefmt.RuleGroups{}
groupsByStrategy := map[storepb.PartialResponseStrategy]*rulefmt.RuleGroups{}
for _, rg := range rg.Groups {
if _, ok := mapped[*rg.PartialResponseStrategy]; !ok {
mapped[*rg.PartialResponseStrategy] = &rulefmt.RuleGroups{}
if _, ok := groupsByStrategy[*rg.PartialResponseStrategy]; !ok {
groupsByStrategy[*rg.PartialResponseStrategy] = &rulefmt.RuleGroups{}
}

mapped[*rg.PartialResponseStrategy].Groups = append(
mapped[*rg.PartialResponseStrategy].Groups,
groupsByStrategy[*rg.PartialResponseStrategy].Groups = append(
groupsByStrategy[*rg.PartialResponseStrategy].Groups,
rg.RuleGroup,
)
}

for s, rg := range mapped {
for s, rg := range groupsByStrategy {
b, err := yaml.Marshal(rg)
if err != nil {
errs = append(errs, err)
Expand All @@ -163,20 +163,19 @@ func (m *Managers) Update(dataDir string, evalInterval time.Duration, files []st
continue
}

filesMap[s] = append(filesMap[s], newFn)
filesByStrategy[s] = append(filesByStrategy[s], newFn)
}

}

for s, fs := range filesMap {
updater, ok := (*m)[s]
for s, fs := range filesByStrategy {
mgr, ok := (*m)[s]
if !ok {
errs = append(errs, errors.Errorf("no updater found for %v", s))
continue
}
// We add external labels in `pkg/alert.Queue`.
// TODO(bwplotka): Investigate if we should put ext labels here or not.
if err := updater.Update(evalInterval, fs, nil); err != nil {
if err := mgr.Update(evalInterval, fs, nil); err != nil {
errs = append(errs, err)
continue
}
Expand Down

0 comments on commit e57bf1d

Please sign in to comment.