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

Rule component: Add a new API /api/v1/alerts/:state for pull alerts from Thanos Rule #901

Closed
wants to merge 4 commits into from

Conversation

wklken
Copy link

@wklken wklken commented Mar 9, 2019

We want to use a pull monitoring system to replace the alertmanager, which needs to fetch the firing alerts from Thanos Rule. So, after the rule apis added by this issue #851 from V0.3.2, we add one more API to fetch the alerts in specific state, like firing

Changes

  • /api/v1/alerts/:state

Verification

  • Tested query and rule components locally
  • Tested in a Thanos cluster which monitoring a lot of k8s clusters

BTW, while the AlertingRule.active is not public, it's difficult to mock the Alerts and do the unittest here, still trying:)

{
endpoint: api.stateAlerts,
params: map[string]string{"state": "firing"},
response: &AlertDiscovery{Alerts: []*Alert{}},
Copy link
Member

Choose a reason for hiding this comment

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

Could you add proper tests mock some firing alerts and check that they are returned currectly

@brancz
Copy link
Member

brancz commented May 6, 2019

How is this different from querying the ALERTS time-series?

change to 0.4.0  new func call
@povilasv
Copy link
Member

povilasv commented May 9, 2019

@wklken does Prometheus / alertmanager have a similiar api? Couldn't find this exact api.

@wklken
Copy link
Author

wklken commented May 9, 2019

@povilasv

  • alertmanger has the api
  • we want to pull the firing alerts from thanos-rule directly, without alertmanger been deployed.
    -since /alerts is supported by rule-api now, it's reasonable to add one more API to filter the firing alerts

@povilasv
Copy link
Member

could you link me to that api? I couldn't find the :state part.

@wklken
Copy link
Author

wklken commented May 11, 2019

https://github.com/prometheus/alertmanager/blob/master/api/v2/openapi.yaml#L130

/alerts, in query active default is true;

In rule, /alerts will return all alerts, include pending/firing;

@povilasv
Copy link
Member

povilasv commented May 21, 2019

I think we want to mimic Prometheus API, so I'm against introducing new API if it's not in Prometheus.
I would suggest doing this in Prometheus first and then we could port it here.

Alertmanager is a different beast all together.

Thoughts about this @bwplotka ?

@stale
Copy link

stale bot commented Jan 11, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Jan 11, 2020
@stale stale bot closed this Jan 18, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants