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

Give an option to repeat alert notifications #3536

Merged
merged 10 commits into from Feb 24, 2017
Merged

Give an option to repeat alert notifications #3536

merged 10 commits into from Feb 24, 2017

Conversation

@edmundoa
Copy link
Member

@edmundoa edmundoa commented Feb 22, 2017

This PR fixes the issue described in #3511 by adding an option to the alert conditions parameters to repeat notifications.

The repeat_notifications option is disabled by default, so all alerts will only send a notification when a new alert is triggered, the behaviour introduced in 2.2.0.

When users rely on repeated notifications on their alerting, they can enable repeat_notifications and Graylog will send a notification after each positive evaluation of the alert condition, just as we did in previous versions.

Alerts will continue having state in the UI, and users can see all sent notifications in the alert details page.

edmundoa added 7 commits Feb 21, 2017
When user sets the `repeat_notifications` flag in the alert condition,
resend the notifications regardless of the alert state. The alert will
continue to have a state anyway, and all sent notifications will be
visible in the web interface on the alerts detail page.

Refs #3511
- Sort histories by creation datetime
- Add the time were the notification was sent, to help seeing the order
and also that the attempts sending the notification are different
@dennisoelkers dennisoelkers self-assigned this Feb 22, 2017
Copy link
Member

@dennisoelkers dennisoelkers left a comment

Looks good in general, minor comments added.

*/
package org.graylog2.alerts;

import com.google.inject.Inject;

This comment has been minimized.

@dennisoelkers

dennisoelkers Feb 23, 2017
Member

Minor nitpick, but please use javax.inject.Inject instead. com.google.inject.Inject is guice-specific, we should try to avoid that.

This comment has been minimized.

@edmundoa

edmundoa Feb 23, 2017
Author Member

Damn auto imports...

@@ -30,7 +30,9 @@ const AlarmCallbackHistoryOverview = React.createClass({
return <Spinner />;
}

const histories = this.state.histories.map(this._formatHistory);
const histories = this.state.histories
.sort((h1, h2) => h1.created_at.localeCompare(h2.created_at))

This comment has been minimized.

@dennisoelkers

dennisoelkers Feb 23, 2017
Member

Comparing two timestamps lexicographically might not lead to a correct result. We should use moment().isBefore()/isAfter() for that.

This comment has been minimized.

@edmundoa

edmundoa Feb 23, 2017
Author Member

@dennisoelkers dennisoelkers merged commit 003bea9 into master Feb 24, 2017
4 checks passed
4 checks passed
ci-web-linter Jenkins build graylog-pr-linter-check 1378 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-3511 branch Feb 24, 2017
dennisoelkers added a commit that referenced this pull request Feb 24, 2017
* Add option to repeat notifications

Refs #3511

* Resend notifications when user sets the flag

When user sets the `repeat_notifications` flag in the alert condition,
resend the notifications regardless of the alert state. The alert will
continue to have a state anyway, and all sent notifications will be
visible in the web interface on the alerts detail page.

Refs #3511

* Add repetition setting to alert summaries

* Add repetition setting to alert timeline

* Add date to notification histories

- Sort histories by creation datetime
- Add the time were the notification was sent, to help seeing the order
and also that the attempts sending the notification are different

* Do not include ms in history datetime

* Add license header

* Rename variable

* Fix import

* Use moment to sort notifications by date

(cherry picked from commit 003bea9)
joschi added a commit that referenced this pull request Mar 2, 2017
* Add option to repeat notifications

Refs #3511

* Resend notifications when user sets the flag

When user sets the `repeat_notifications` flag in the alert condition,
resend the notifications regardless of the alert state. The alert will
continue to have a state anyway, and all sent notifications will be
visible in the web interface on the alerts detail page.

Refs #3511

* Add repetition setting to alert summaries

* Add repetition setting to alert timeline

* Add date to notification histories

- Sort histories by creation datetime
- Add the time were the notification was sent, to help seeing the order
and also that the attempts sending the notification are different

* Do not include ms in history datetime

* Add license header

* Rename variable

* Fix import

* Use moment to sort notifications by date

(cherry picked from commit 003bea9)
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.