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

api: support more query filters #1366

Merged
merged 2 commits into from
May 7, 2018

Conversation

simonpasquier
Copy link
Member

Fixes #1359.

This change adds 2 new query filters to the /api/v1/alerts endpoint.

  • active, filter out active alerts when set to 'false' (default: 'true').
  • unprocessed, filter out unprocessed alerts when set to 'false'
    (default: 'true').

The default values ensure that the API behavior remains the same as
before when the query filters aren't provided.

cc @stuartnelson3

This change adds 2 new query filters to the /api/v1/alerts endpoint.

- active, filter out active alerts when set to 'false' (default: 'true').
- unprocessed, filter out unprocessed alerts when set to 'false'
 (default: 'true').

The default values ensure that the API behavior remains the same as
before when the query filters aren't provided.

Signed-off-by: Simon Pasquier <spasquie@redhat.com>
Copy link
Contributor

@stuartnelson3 stuartnelson3 left a comment

Choose a reason for hiding this comment

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

looks great! couple small requests

@@ -20,26 +21,72 @@ import (
"github.com/stretchr/testify/require"
)

// fakeAlerts is a struct implementing the provider.Alerts interface for tests.
Copy link
Contributor

Choose a reason for hiding this comment

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

💯

api/api_test.go Outdated
false,
map[string]string{"filter": "{alertname"},
400,
[]string{"alert1", "alert2", "alert3", "alert4"},
Copy link
Contributor

Choose a reason for hiding this comment

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

this []string{} should probably be empty, since it's not being executed anyway because of the 400.

@@ -88,6 +136,167 @@ func TestAddAlerts(t *testing.T) {
}
}

func TestListAlerts(t *testing.T) {
Copy link
Contributor

Choose a reason for hiding this comment

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

😍

Signed-off-by: Simon Pasquier <spasquie@redhat.com>
@simonpasquier
Copy link
Member Author

@stuartnelson3 thanks for spotting the inconsistency!

@stuartnelson3 stuartnelson3 merged commit 383024e into prometheus:master May 7, 2018
stuartnelson3 added a commit that referenced this pull request May 7, 2018
Support the new filter options in the alerts api
endpoint introduced by #1366

Signed-off-by: stuart nelson <stuartnelson3@gmail.com>
stuartnelson3 added a commit that referenced this pull request May 7, 2018
Support the new filter options in the alerts api
endpoint introduced by #1366

Signed-off-by: stuart nelson <stuartnelson3@gmail.com>
@simonpasquier simonpasquier deleted the add-more-api-filters branch May 7, 2018 19:03
stuartnelson3 added a commit that referenced this pull request May 9, 2018
* cli alert query: Expose --active and --unprocessed

Support the new filter options in the alerts api
endpoint introduced by #1366

Signed-off-by: stuart nelson <stuartnelson3@gmail.com>

* Update comment and client_test

Signed-off-by: stuart nelson <stuartnelson3@gmail.com>
hh pushed a commit to ii/alertmanager that referenced this pull request Jun 4, 2019
Cherry-pick two bug fixes into 0.18.1.

Signed-off-by: Ben Kochie <superq@gmail.com>
hh pushed a commit to ii/alertmanager that referenced this pull request Jun 4, 2019
Cherry-pick two bug fixes into 0.18.1.

Signed-off-by: Ben Kochie <superq@gmail.com>
hh pushed a commit to ii/alertmanager that referenced this pull request Jun 4, 2019
Cherry-pick two bug fixes into 0.18.1.

Signed-off-by: Ben Kochie <superq@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.

2 participants