From 0119cfa4314a96409f3b0551904aa2af6e443e13 Mon Sep 17 00:00:00 2001 From: Patrick J Cherry Date: Thu, 16 May 2013 11:47:41 +0100 Subject: Updated Person#should_suppress to take into account alert level when deciding to suppress an alert. --- lib/mauve/person.rb | 85 ++++++++++++++++++++++++----------- test/tc_mauve_person.rb | 115 +++++++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 174 insertions(+), 26 deletions(-) diff --git a/lib/mauve/person.rb b/lib/mauve/person.rb index 1aa27ee..eb6e28b 100644 --- a/lib/mauve/person.rb +++ b/lib/mauve/person.rb @@ -106,39 +106,74 @@ module Mauve # Works out if a notification should be suppressed. If no parameters are supplied, it will # + # @param [Symbol] Level of notification that is being tested # @param [Time] Theoretical time of notification # @param [Time] Current time. # @return [Boolean] If suppression is needed. - def should_suppress?(with_notification_at = nil, now = Time.now) - # - # This is the query we use. It doesn't get polled until later. - # - previous_notifications = History.all(:order => :created_at.desc, :user => self.username, :type => "notification", :event.like => '% succeeded', :fields => [:created_at]) + def should_suppress?(level, with_notification_at = nil, now = Time.now) - # - # Find the latest alert. - # - if with_notification_at.nil? - latest_notification = previous_notifications.first - latest = (latest_notification.nil? ? nil : latest_notification.created_at) - else - latest = with_notification_at - end + self.suppress_notifications_after.any? do |period, number| + # + # When testing if the next alert will suppress, we know that if only + # one alert is needed to suppress, then this function should always + # return true. + # + return true if with_notification_at and number <= 1 - return self.suppress_notifications_after.any? do |period, number| # - # If no notification time has been specified, use the earliest alert time. + # Here are the previous notifications set to this person in the last period. # - if with_notification_at.nil? or number == 0 - earliest_notification = previous_notifications[number-1] + previous_notifications = History.all( + :user => self.username, :type => "notification", + :created_at.gte => now - period, :created_at.lte => now, + :event.like => '% succeeded', + :order => :created_at.desc) + + # + # Defintely not suppressed if no notifications have been found. + # + return false if previous_notifications.count == 0 + + # + # If we're suppressed already, we need to check the time of the last alert sent + # + if @suppressed + + if with_notification_at.is_a?(Time) + latest = with_notification_at + else + latest = previous_notifications.first.created_at + end + + # + # We should not suppress this alert if the last one was sent ages ago + # + if (now - latest) >= period + return false + end + else - earliest_notification = previous_notifications[number-2] + # + # We do not suppress if we can't find a sufficient number of previous alerts + # + if previous_notifications.count < (with_notification_at.nil? ? number : number - 1) + return false + end + end - earliest = (earliest_notification.nil? ? nil : earliest_notification.created_at) + # + # If we're at the lowest level, return true now. + # + return true if !AlertGroup::LEVELS.include?(level) or AlertGroup::LEVELS.index(level) == 0 + + # + # Suppress this notification if all of the preceeding notifications were of the same or higher level. + # + return previous_notifications.alerts.all? do |a| + AlertGroup::LEVELS.index(a.level) >= AlertGroup::LEVELS.index(level) + end - (earliest.is_a?(Time) and (now - earliest) < period) or - (latest.is_a?(Time) and @suppressed and (now - latest) < period) end end @@ -236,11 +271,11 @@ module Mauve def send_alert(level, alert, now=Time.now) was_suppressed = @suppressed - @suppressed = self.should_suppress? - will_suppress = self.should_suppress?(now) + @suppressed = self.should_suppress?(level) + will_suppress = self.should_suppress?(level, now) logger.info "Starting to send notifications again for #{username}." if was_suppressed and not @suppressed - + # # We only suppress notifications if we were suppressed before we started, # and are still suppressed. diff --git a/test/tc_mauve_person.rb b/test/tc_mauve_person.rb index 14798f1..f3910bf 100644 --- a/test/tc_mauve_person.rb +++ b/test/tc_mauve_person.rb @@ -101,7 +101,6 @@ EOF assert_equal(0, notification_buffer.length, "Notification sent when it should not have been at #{Time.now}.") end - logger_pop end @@ -183,6 +182,120 @@ EOF end end + + def test_send_alert_suppression_as_alerts_get_more_urgent + # + # This configuration is a bit different. We only want one alert per + # minute. + # + config =< 1.minute ) +} + +alert_group("low") { + level LOW + includes { alert_id =~ /^low-/ } + + notify("test") { + every 10.seconds + } +} + +alert_group("normal") { + level NORMAL + includes { alert_id =~ /^normal-/ } + + notify("test") { + every 10.seconds + } +} + +alert_group("default") { + level URGENT + + notify("test") { + every 10.seconds + } +} +EOF + + Configuration.current = ConfigurationBuilder.parse(config) + notification_buffer = Configuration.current.notification_methods["email"].deliver_to_queue + Server.instance.setup + + person = Configuration.current.people["test"] + + alerts = [ + Alert.new( + :alert_id => "low-test", + :source => "test", + :subject => "test" + ), + Alert.new( + :alert_id => "normal-test", + :source => "test", + :subject => "test" + ), + Alert.new( + :alert_id => "urgent-test", + :source => "test", + :subject => "test" + ) + ] + + # + # Raise the alerts + # + alerts.each{|a| a.raise!} + assert_equal(false, person.suppressed?, "Person suppressed before we even begin!") + + assert_equal(:low, alerts[0].level) + assert_equal(:normal, alerts[1].level) + assert_equal(:urgent, alerts[2].level) + + start_time = Time.now + + # + # + # + [ [0, true, alerts.first ], + [1, true, alerts.first ], + [2, false, alerts.first ], + [3, false, alerts.first ], + [4, true, alerts[1]], + [5, false, alerts.first], + [6, true, alerts[1]], + ].each do |offset, notification_sent, alert| + # + # Advance in to the future! + # + Timecop.freeze(start_time + offset) + + person.send_alert(alert.level, alert) + + if notification_sent + assert_equal(1, notification_buffer.length, "#{alert.level.to_s.capitalize} notification not sent when it should have been at #{Time.now}.") + # + # Pop the notification off the buffer. + # + notification_buffer.pop + else + assert_equal(0, notification_buffer.length, "#{alert.level.to_s.capitalize} notification sent when it should not have been at #{Time.now}.") + end + + logger_pop + end + + end def test_current_alerts -- cgit v1.2.1