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

Comply with grace condition when repeat alert notifications is enabled #3676

Merged
merged 4 commits into from Apr 3, 2017

Conversation

@edmundoa
Copy link
Member

@edmundoa edmundoa commented Mar 31, 2017

On 2.2.2 we added the possibility to repeat alert notifications, allowing users to keep the old behaviour in their alerts while still having some features of the new stateful alerts.

Unfortunately this option was not taking into account the grace period, so users got notified way too many times, as explained in #3579.

This PR checks the alert history to see when was the last time notifications were send for the alert, and verify that the defined grace period has elapsed before notifying again.

I'm also adding some tests to ensure that all cases (or all I though of) are covered and we don't break any other case by mistake.

Fixes #3579.

edmundoa added 4 commits Mar 29, 2017
Check the time last notification was sent to ensure we don't repeat
notifications too often, complying with the grace period set by the
user.

Fixes #3579
@edmundoa edmundoa added this to the 2.3.0 milestone Mar 31, 2017
@dennisoelkers dennisoelkers self-assigned this Apr 3, 2017
@dennisoelkers dennisoelkers merged commit 260bc95 into master Apr 3, 2017
4 checks passed
4 checks passed
ci-web-linter Jenkins build graylog-pr-linter-check 1488 has succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
licence/cla Contributor License Agreement is signed.
Details
@dennisoelkers dennisoelkers deleted the issue-3579 branch Apr 3, 2017
edmundoa added a commit that referenced this pull request Apr 3, 2017
#3676)

* Remove condition, as it's always true

* Comply with grace period when repeating notifications

Check the time last notification was sent to ensure we don't repeat
notifications too often, complying with the grace period set by the
user.

Fixes #3579

* Fix ForbiddenAPIs errors

* Mock shouldRepeatNotifications method to fix test
dennisoelkers added a commit that referenced this pull request Apr 3, 2017
#3676) (#3686)

* Remove condition, as it's always true

* Comply with grace period when repeating notifications

Check the time last notification was sent to ensure we don't repeat
notifications too often, complying with the grace period set by the
user.

Fixes #3579

* Fix ForbiddenAPIs errors

* Mock shouldRepeatNotifications method to fix test
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants
You can’t perform that action at this time.