From: Michael Friedrich Date: Tue, 4 Nov 2014 21:03:39 +0000 (+0100) Subject: Only notify users on recovery who have been notified on not-OK before X-Git-Tag: v2.2.0~81 X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=885e7704a214783d8dfba191b53741586908198d;p=icinga2 Only notify users on recovery who have been notified on not-OK before Also ensure that type NotificationRecovery always passes the state filter (missing `OK` is totally fine). Also fix that notification delays set the correct next notification time to the begin time window. fixes #7579 fixes #7623 fixes #6547 --- diff --git a/doc/4-monitoring-basics.md b/doc/4-monitoring-basics.md index f37fc16ec..fcca50ba2 100644 --- a/doc/4-monitoring-basics.md +++ b/doc/4-monitoring-basics.md @@ -610,8 +610,6 @@ to the defined notifications. That way you'll save duplicated attributes in each The time period `24x7` is shipped as example configuration with Icinga 2. - - Use the `apply` keyword to create `Notification` objects for your services: apply Notification "notify-cust-xy-mysql" to Service { @@ -628,6 +626,11 @@ Instead of assigning users to notifications, you can also add the `user_groups` attribute with a list of user groups to the `Notification` object. Icinga 2 will send notifications to all group members. +> **Note** +> +> Only users who have been notified of a problem before (`Warning`, `Critical`, `Unknown` +> states for services, `Down` for hosts) will receive `Recovery` notifications. + ### Notification Escalations When a problem notification is sent and a problem still exists at the time of re-notification diff --git a/lib/icinga/notification.cpp b/lib/icinga/notification.cpp index 350a3deae..a5c561aeb 100644 --- a/lib/icinga/notification.cpp +++ b/lib/icinga/notification.cpp @@ -245,6 +245,14 @@ void Notification::BeginExecuteNotification(NotificationType type, const CheckRe if (times && times->Contains("begin") && now < checkable->GetLastHardStateChange() + times->Get("begin")) { Log(LogNotice, "Notification") << "Not sending notifications for notification object '" << GetName() << "': before escalation range"; + + /* we need to adjust the next notification time + * to now + begin delaying the first notification + */ + double nextProposedNotification = now + times->Get("begin") + 1.0; + if (GetNextNotification() > nextProposedNotification) + SetNextNotification(nextProposedNotification); + return; } @@ -262,25 +270,29 @@ void Notification::BeginExecuteNotification(NotificationType type, const CheckRe if (!(ftype & GetTypeFilter())) { Log(LogNotice, "Notification") - << "Not sending notifications for notification object '" << GetName() << "': type filter does not match"; + << "Not sending notifications for notification object '" << GetName() << "': type filter does not match '" + << NotificationTypeToString(type) << "'"; return; } - Host::Ptr host; - Service::Ptr service; - tie(host, service) = GetHostService(checkable); + /* ensure that recovery notifications are always sent, no state filter checks necessary */ + if (type != NotificationRecovery) { + Host::Ptr host; + Service::Ptr service; + tie(host, service) = GetHostService(checkable); - unsigned long fstate; + unsigned long fstate; - if (service) - fstate = ServiceStateToFilter(service->GetState()); - else - fstate = HostStateToFilter(host->GetState()); + if (service) + fstate = ServiceStateToFilter(service->GetState()); + else + fstate = HostStateToFilter(host->GetState()); - if (!(fstate & GetStateFilter())) { - Log(LogNotice, "Notification") - << "Not sending notifications for notification object '" << GetName() << "': state filter does not match"; - return; + if (!(fstate & GetStateFilter())) { + Log(LogNotice, "Notification") + << "Not sending notifications for notification object '" << GetName() << "': state filter does not match"; + return; + } } } @@ -307,19 +319,47 @@ void Notification::BeginExecuteNotification(NotificationType type, const CheckRe Service::OnNotificationSendStart(this, checkable, allUsers, type, cr, author, text); std::set allNotifiedUsers; + BOOST_FOREACH(const User::Ptr& user, allUsers) { - if (!user->GetEnableNotifications() || !CheckNotificationUserFilters(type, user, force)) + String userName = user->GetName(); + + if (!user->GetEnableNotifications()) { + Log(LogNotice, "Notification") + << "Disabled notifications for user '" << userName << "'. Not sending notification."; continue; + } + + if (!CheckNotificationUserFilters(type, user, force)) { + Log(LogNotice, "Notification") + << "Notification filters for user '" << userName << "' not matched. Not sending notification."; + continue; + } + + /* on recovery, check if user was notified before */ + if (type == NotificationRecovery) { + if (m_NotifiedUsers.find(userName) == m_NotifiedUsers.end()) { + Log(LogNotice, "Notification") + << "We did not notify user '" << userName << "' before. Not sending recovery notification."; + continue; + } + } Log(LogInformation, "Notification") - << "Sending notification for user '" << user->GetName() << "'"; + << "Sending notification for user '" << userName << "'"; Utility::QueueAsyncCallback(boost::bind(&Notification::ExecuteNotificationHelper, this, type, user, cr, force, author, text)); /* collect all notified users */ allNotifiedUsers.insert(user); + + /* store all notified users for later recovery checks */ + m_NotifiedUsers.insert(userName); } + /* if this was a recovery notification, reset all notified users */ + if (type == NotificationRecovery) + ResetNotifiedUsers(); + /* used in db_ido for notification history */ Service::OnNotificationSentToAllUsers(this, checkable, allNotifiedUsers, type, cr, author, text); } @@ -347,23 +387,26 @@ bool Notification::CheckNotificationUserFilters(NotificationType type, const Use return false; } - Checkable::Ptr checkable = GetCheckable(); - Host::Ptr host; - Service::Ptr service; - tie(host, service) = GetHostService(checkable); + /* check state filters it this is not a recovery notification */ + if (type != NotificationRecovery) { + Checkable::Ptr checkable = GetCheckable(); + Host::Ptr host; + Service::Ptr service; + tie(host, service) = GetHostService(checkable); - unsigned long fstate; + unsigned long fstate; - if (service) - fstate = ServiceStateToFilter(service->GetState()); - else - fstate = HostStateToFilter(host->GetState()); + if (service) + fstate = ServiceStateToFilter(service->GetState()); + else + fstate = HostStateToFilter(host->GetState()); - if (!(fstate & user->GetStateFilter())) { - Log(LogNotice, "Notification") - << "Not sending notifications for notification object '" - << GetName() << " and user '" << user->GetName() << "': state filter does not match"; - return false; + if (!(fstate & user->GetStateFilter())) { + Log(LogNotice, "Notification") + << "Not sending notifications for notification object '" + << GetName() << " and user '" << user->GetName() << "': state filter does not match"; + return false; + } } } @@ -403,6 +446,11 @@ void Notification::ExecuteNotificationHelper(NotificationType type, const User:: } } +void Notification::ResetNotifiedUsers(void) +{ + m_NotifiedUsers.clear(); +} + int icinga::ServiceStateToFilter(ServiceState state) { switch (state) { @@ -471,4 +519,3 @@ void Notification::ValidateFilters(const String& location, const Dictionary::Ptr location + ": Type filter is invalid."); } } - diff --git a/lib/icinga/notification.hpp b/lib/icinga/notification.hpp index 9d596cfb9..64cd8f7ab 100644 --- a/lib/icinga/notification.hpp +++ b/lib/icinga/notification.hpp @@ -97,6 +97,8 @@ public: bool CheckNotificationUserFilters(NotificationType type, const User::Ptr& user, bool force); + void ResetNotifiedUsers(void); + static String NotificationTypeToString(NotificationType type); static boost::signals2::signal OnNextNotificationChanged; @@ -111,6 +113,8 @@ protected: virtual void Stop(void); private: + std::set m_NotifiedUsers; + void ExecuteNotificationHelper(NotificationType type, const User::Ptr& user, const CheckResult::Ptr& cr, bool force, const String& author = "", const String& text = ""); static void EvaluateApplyRuleOneInstance(const intrusive_ptr& checkable, const String& name, const Dictionary::Ptr& locals, const ApplyRule& rule);