-
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
Use proto rules API instead of struct; Moved as much as possible to promclient; Added rulesAPI RPC to sidecar. #2243
Conversation
299c29d
to
bac1909
Compare
This should be ready to go @s-urbaniak! |
PTAL |
pkg/promclient/promclient.go
Outdated
} | ||
defer runutil.ExhaustCloseWithLogOnErr(logger, resp.Body, "metrics body") | ||
fmt.Println(matcher) |
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.
Remove this?
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
ff9d8a7
to
ac89485
Compare
if err != nil { | ||
return nil, nil, errors.Wrapf(err, "perform GET request against %s", u.String()) | ||
return nil, nil, err |
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.
no wrapping is necessary any more?
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.
correct
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.
TODO: Use with Stack on command error.
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.
Fatalf("%+v", err)
pkg/errors
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.
wow, this is huge 😲 |
Happy to have a quick pair-review @s-urbaniak if needed |
7616456
to
5d0c93f
Compare
if err != nil { | ||
return nil, nil, errors.Wrapf(err, "perform GET request against %s", u.String()) | ||
return nil, nil, err |
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.
TODO: Use with Stack on command error.
if err != nil { | ||
return nil, nil, errors.Wrapf(err, "perform GET request against %s", u.String()) | ||
return nil, nil, err |
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.
Fatalf("%+v", err)
pkg/errors
PartialResponseStrategy string `json:"partialResponseStrategy"` | ||
} | ||
|
||
type AlertingRule struct { |
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.
Upstream make it public?
lgtm 👍 |
looking at the CI failure 👀 |
yay, I can repro locally, let me try to fix this so I'll learn 👷♂️ |
Are you on it @s-urbaniak ? If not, I can fix |
Signed-off-by: Bartlomiej Plotka <bwplotka@gmail.com>
Signed-off-by: Bartlomiej Plotka <bwplotka@gmail.com>
Fixed hopefully. |
@bwplotka i was on it, but my fix was related to the test itself, i will continue from here for the remaining failures. |
…romclient; Added rulesAPI RPC to sidecar. (#2291) * rules_custom_test: fix asserting labels Signed-off-by: Sergiusz Urbaniak <sergiusz.urbaniak@gmail.com> * TestPrometheusStore_Rules_e2e: fix test fixture Signed-off-by: Sergiusz Urbaniak <sergiusz.urbaniak@gmail.com>
Thanks @s-urbaniak for help! Let's move it forward. We still need:
|
@bwplotka on it 👍 |
…romclient; Added rulesAPI RPC to sidecar. (#2243) * Use proto rules API instead of struct; Added rulesAPI RPC to sidecar. Signed-off-by: Bartlomiej Plotka <bwplotka@gmail.com> * Fixed broken test. Signed-off-by: Bartlomiej Plotka <bwplotka@gmail.com> * Use proto rules API instead of struct; Moved as much as possible to promclient; Added rulesAPI RPC to sidecar. (#2291) * rules_custom_test: fix asserting labels Signed-off-by: Sergiusz Urbaniak <sergiusz.urbaniak@gmail.com> * TestPrometheusStore_Rules_e2e: fix test fixture Signed-off-by: Sergiusz Urbaniak <sergiusz.urbaniak@gmail.com> Co-authored-by: Sergiusz Urbaniak <sergiusz.urbaniak@gmail.com>
) * Added RulesAPI. Signed-off-by: Bartlomiej Plotka <bwplotka@gmail.com> * Added warnings. Signed-off-by: Bartlomiej Plotka <bwplotka@gmail.com> * Added Type to rules requests as it is on HTTP API. (#2201) Signed-off-by: Bartlomiej Plotka <bwplotka@gmail.com> * pkg/store/storepb: fix wrong rule reference (#2237) * pkg/store/storepb: fix wrong rule reference Currently we recursively reference rules instead of recording rules. Signed-off-by: Sergiusz Urbaniak <sergiusz.urbaniak@gmail.com> * proto: regenerate Signed-off-by: Sergiusz Urbaniak <sergiusz.urbaniak@gmail.com> * Made storepb.RuleGroups a source of truth for rules API (Go, JSON, proto). Added tests. (#2242) Signed-off-by: Bartlomiej Plotka <bwplotka@gmail.com> * Use proto rules API instead of struct; Moved as much as possible to promclient; Added rulesAPI RPC to sidecar. (#2243) * Use proto rules API instead of struct; Added rulesAPI RPC to sidecar. Signed-off-by: Bartlomiej Plotka <bwplotka@gmail.com> * Fixed broken test. Signed-off-by: Bartlomiej Plotka <bwplotka@gmail.com> * Use proto rules API instead of struct; Moved as much as possible to promclient; Added rulesAPI RPC to sidecar. (#2291) * rules_custom_test: fix asserting labels Signed-off-by: Sergiusz Urbaniak <sergiusz.urbaniak@gmail.com> * TestPrometheusStore_Rules_e2e: fix test fixture Signed-off-by: Sergiusz Urbaniak <sergiusz.urbaniak@gmail.com> Co-authored-by: Sergiusz Urbaniak <sergiusz.urbaniak@gmail.com> * cmd/thanos/query: add initial rules support (#2240) * cmd/thanos/query: add initial rules support Signed-off-by: Sergiusz Urbaniak <sergiusz.urbaniak@gmail.com> * pkg/query/api/v1: initial implementation Signed-off-by: Sergiusz Urbaniak <sergiusz.urbaniak@gmail.com> * e2e: initial implementation and fixes Signed-off-by: Sergiusz Urbaniak <sergiusz.urbaniak@gmail.com> * pkg/query: fix racy access to assert rules API store Signed-off-by: Sergiusz Urbaniak <sergiusz.urbaniak@gmail.com> * Refactored proto generation and separated store from rules APIs. (#2558) * Refactored proto generation and separate store from rules APIs. Signed-off-by: Bartlomiej Plotka <bwplotka@gmail.com> * Addressed comments. Signed-off-by: Bartlomiej Plotka <bwplotka@gmail.com> * Fixed proto gen. Signed-off-by: Bartlomiej Plotka <bwplotka@gmail.com> * Addressed Serg comments. Signed-off-by: Bartlomiej Plotka <bwplotka@gmail.com> * Added Ruler support for RulesAPI; Refactored Manager. (#2562) As per: https://thanos.io/proposals/202003_thanos_rules_federation.md/ Signed-off-by: Bartlomiej Plotka <bwplotka@gmail.com> * Small fixes to changelog and flags. Do not add any. Signed-off-by: Bartlomiej Plotka <bwplotka@gmail.com> * Fixed after rebase. Signed-off-by: Bartlomiej Plotka <bwplotka@gmail.com> Co-authored-by: Sergiusz Urbaniak <sergiusz.urbaniak@gmail.com>
Signed-off-by: Bartlomiej Plotka bwplotka@gmail.com
cc @s-urbaniak