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

add api for /api/v1/alerts #2600

Closed
wants to merge 0 commits into from
Closed

add api for /api/v1/alerts #2600

wants to merge 0 commits into from

Conversation

mg03
Copy link

@mg03 mg03 commented Apr 7, 2017

Hi
this merge contains code to expose /api/v1/alerts . The output is json. Example is
{ "status": "success", "data": { "alerts": [ { "name": "promup", "query": "up{job=\"prometheus\"} != 1", "duration": "1m0s", "labels": { "severity": "critical" }, "annotations": { "dashboard": "http://grafana.com/dashboard/somedashbaord", "description": "lol hahahahha", "summary": "Prom Up" }, "status": "inactive" }, { "name": "promx", "query": "up{job=\"prom\"} < 1", "duration": "1m5s", "labels": { "severity": "critical" }, "annotations": { "dashboard": "http://grafana.com/dashboard/somedashbaord", "description": "sad sad", "summary": "Promxxxxx Up" }, "status": "pending", "activesince": "2017-03-31T19:09:07.808-07:00" }, { "name": "promnoann", "query": "up{job=\"prom\"} < 1", "duration": "10s", "labels": { "severity": "critical" }, "status": "firing", "activesince": "2017-03-31T19:09:07.808-07:00" } ] } }

@tomwilkie
Copy link
Member

Having a quick look at this, it seems you've exposed alert rules, not alerts, so perhaps this should be exposes under /api/v1/alertrules instead?

Also, each alert rule can have multiple active alerts (differentiated by the labels), which currently is partially exposed by this PR by the activesince field. You can access them via the ALERTS timeseries, so they probably don't need exposing here.

Also, you're exposing the parsed alert rule here, which is more than the UI exposes. I wonder whether it would be preferable to just expose the alert rule as some text (as suggested by @brancz in #2467), so we don't lock ourselves into another format. @fabxc WDYT?

This issues seem relevant: #1570

@brancz
Copy link
Member

brancz commented Apr 10, 2017

In terms of exposing alerting rules, I think we should actually have both, parsed and not parsed, whereas what we expose in the UI currently is an odd mix of both (not that it doesn't serve it's purpose well).

I think we should 1) expose the loaded rule files, which are completely unmodified as they came from disk and 2) a parsed representation, so that UIs that can be built on top of the API don't have to implement a parser for alerting rules themselves.

@mg03
Copy link
Author

mg03 commented Apr 11, 2017

@fabxc : could you please provide guidance /direction?

@fabxc
Copy link
Contributor

fabxc commented Apr 12, 2017

I agree with @brancz that we ultimately want both.

Probably one /api/v1/alerts endpoint that has a list of entries where each entry contains a JSON representation of the alerting rules plus a list of firing instances of the alert.

Additionally, we should also have and endpoint exposing configuration and rule files (as mentioned in the past, everything in our UI should be available via API too). Maybe along the lines of:

api/v1/status – things seen on the status page except for config file
api/v1/status/config – plain config file. JSON, raw, both?
api/v1/status/rules – list of rule files and their path

That's not a definitive spec. Just my opinion at this point.

@brancz
Copy link
Member

brancz commented Apr 12, 2017

api/v1/status – things seen on the status page except for config file
api/v1/status/config – plain config file. JSON, raw, both?
api/v1/status/rules – list of rule files and their path

The status endpoint sounds good to me.

Regarding the config I think we should return JSON and YAML (maybe even the pure unparsed yaml additionally).

I don't recall how we hold rules in memory (1 pool of rules, by file they came from, etc.) and with rule groups coming up, we may need to change that, so I'm hesitant to expose anything at this point in a stable API that's not the plaintext filename and content that were loaded. Wdyt @fabxc ?

@brian-brazil
Copy link
Contributor

We'll want raw so comments and the like can be inspected for those using configuration management.

@brancz
Copy link
Member

brancz commented Apr 12, 2017

@brian-brazil agreed!

@mg03
Copy link
Author

mg03 commented Apr 12, 2017

@fabxc @brian-brazil : thank you sirs for yall feedback .... ill work on it

@moolitayer
Copy link

moolitayer commented Jun 1, 2017

hi @mg03, are you still actively working on this PR?
The /api/v1/alerts endpoint is of great interest to me

@bogdanov1609
Copy link

And for us too 👍

@kuppakoffe
Copy link

Hi @mg03 ,
This looks nice, is it going to be a part of any release (prometheus 2.0 or something).

@gmauleon
Copy link

Hey there,

Is someone still working on this proposition (aka exposing prometheus rules as a json)?

I need to extract rules in order to pre-configure an external monitoring system before I send alerts to it via an alertmanager wehook. I was thinking to do a temporary html parser for /rules on the main prometheus UI for now but that's far from ideal :)

Thanks

@i19
Copy link

i19 commented Sep 22, 2017

maybe it should add another column called value, just like `/alerts' does

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.

10 participants