Skip to content
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

Merged
merged 3 commits into from
Mar 19, 2020

Conversation

bwplotka
Copy link
Member

Signed-off-by: Bartlomiej Plotka bwplotka@gmail.com

cc @s-urbaniak

@bwplotka bwplotka changed the title Use proto rules API instead of struct; Added rulesAPI RPC to sidecar. Use proto rules API instead of struct; Moved as much as possible to promclient; Added rulesAPI RPC to sidecar. Mar 10, 2020
@bwplotka bwplotka force-pushed the api-on-proto branch 2 times, most recently from 299c29d to bac1909 Compare March 10, 2020 19:04
@bwplotka
Copy link
Member Author

This should be ready to go @s-urbaniak!

@bwplotka
Copy link
Member Author

PTAL

}
defer runutil.ExhaustCloseWithLogOnErr(logger, resp.Body, "metrics body")
fmt.Println(matcher)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove this?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

@bwplotka bwplotka force-pushed the api-on-proto branch 2 times, most recently from ff9d8a7 to ac89485 Compare March 11, 2020 11:14
if err != nil {
return nil, nil, errors.Wrapf(err, "perform GET request against %s", u.String())
return nil, nil, err
Copy link
Contributor

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?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

correct

Copy link
Member Author

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.

Copy link
Member Author

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

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@s-urbaniak
Copy link
Contributor

wow, this is huge 😲
i am a bit overwhelmed, but i see RulesServer now being registered, so i am super happy 🎉 😊

@bwplotka
Copy link
Member Author

Happy to have a quick pair-review @s-urbaniak if needed

@bwplotka bwplotka force-pushed the api-on-proto branch 2 times, most recently from 7616456 to 5d0c93f Compare March 11, 2020 12:44
if err != nil {
return nil, nil, errors.Wrapf(err, "perform GET request against %s", u.String())
return nil, nil, err
Copy link
Member Author

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
Copy link
Member Author

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 {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Upstream make it public?

@s-urbaniak
Copy link
Contributor

lgtm 👍

@s-urbaniak
Copy link
Contributor

looking at the CI failure 👀

@s-urbaniak
Copy link
Contributor

yay, I can repro locally, let me try to fix this so I'll learn 👷‍♂️

@bwplotka
Copy link
Member Author

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>
@bwplotka
Copy link
Member Author

Fixed hopefully.

@s-urbaniak
Copy link
Contributor

@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>
@bwplotka bwplotka merged commit 932f2ee into features/rules-proxy Mar 19, 2020
@bwplotka
Copy link
Member Author

Thanks @s-urbaniak for help!

Let's move it forward.

We still need:

  • e2e tests
  • proxy implementation
  • ruler implementation

@bwplotka bwplotka deleted the api-on-proto branch March 19, 2020 11:57
@s-urbaniak
Copy link
Contributor

@bwplotka on it 👍

bwplotka added a commit that referenced this pull request May 22, 2020
…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>
brancz pushed a commit that referenced this pull request May 25, 2020
)

* 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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants