Merge branch '223151-open-new-alert-when-existing-are-resolved' into 'master'

Open new alert when existing alert is resolved

See merge request gitlab-org/gitlab!36261

app/models/alert_management/alert.rb

@@ -57,7 +57,7 @@ module AlertManagement
     validates :started_at,      presence: true
     validates :fingerprint,     allow_blank: true, uniqueness: {
       scope: :project,
-      conditions: -> { where.not(status: STATUSES[:resolved]) },
+      conditions: -> { not_resolved },
       message: -> (object, data) { _('Cannot have multiple unresolved alerts') }
     }, unless: :resolved?
     validate :hosts_length
@@ -120,6 +120,7 @@ module AlertManagement
     scope :for_environment, -> (environment) { where(environment: environment) }
     scope :search, -> (query) { fuzzy_search(query, [:title, :description, :monitoring_tool, :service]) }
     scope :open, -> { with_status(:triggered, :acknowledged) }
+    scope :not_resolved, -> { where.not(status: STATUSES[:resolved]) }
     scope :with_prometheus_alert, -> { includes(:prometheus_alert) }
 
     scope :order_start_time,    -> (sort_order) { order(started_at: sort_order) }

app/services/alert_management/process_prometheus_alert_service.rb

@@ -94,7 +94,7 @@ module AlertManagement
     end
 
     def am_alert
-      @am_alert ||= AlertManagement::Alert.for_fingerprint(project, gitlab_fingerprint).first
+      @am_alert ||= AlertManagement::Alert.not_resolved.for_fingerprint(project, gitlab_fingerprint).first
     end
 
     def bad_request

app/services/projects/alerting/notify_service.rb

@@ -55,7 +55,7 @@ module Projects
       def find_alert_by_fingerprint(fingerprint)
         return unless fingerprint
 
-        AlertManagement::Alert.for_fingerprint(project, fingerprint).first
+        AlertManagement::Alert.not_resolved.for_fingerprint(project, fingerprint).first
       end
 
       def send_email?

changelogs/unreleased/223151-open-new-alert-when-existing-are-resolved.yml

+---
+title: Open new alert when existing alert is resolved
+merge_request: 36261
+author:
+type: added

doc/user/project/integrations/generic_alerts.md

@@ -101,4 +101,6 @@ displays a counter on the
 [Alert Management List](../operations/alert_management.md#alert-management-list)
 and details pages.
 
+If the existing alert is already `resolved`, then a new alert will be created instead.
+
 ![Alert Management List](../operations/img/alert_list_v13_1.png)

spec/models/alert_management/alert_spec.rb

@@ -235,6 +235,14 @@ RSpec.describe AlertManagement::Alert do
 
       it { is_expected.to contain_exactly(acknowledged_alert, triggered_alert) }
     end
+
+    describe '.not_resolved' do
+      subject { described_class.not_resolved }
+
+      let!(:acknowledged_alert) { create(:alert_management_alert, :acknowledged, project: project) }
+
+      it { is_expected.to contain_exactly(acknowledged_alert, triggered_alert, ignored_alert) }
+    end
   end
 
   describe '.last_prometheus_alert_by_project_id' do

spec/services/alert_management/process_prometheus_alert_service_spec.rb

@@ -39,22 +39,27 @@ RSpec.describe AlertManagement::ProcessPrometheusAlertService do
 
       context 'when Prometheus alert status is firing' do
         context 'when alert with the same fingerprint already exists' do
-          let!(:alert) { create(:alert_management_alert, :resolved, project: project, fingerprint: parsed_alert.gitlab_fingerprint) }
+          let!(:alert) { create(:alert_management_alert, project: project, fingerprint: parsed_alert.gitlab_fingerprint) }
 
-          it 'increases alert events count' do
-            expect { execute }.to change { alert.reload.events }.by(1)
+          it_behaves_like 'adds an alert management alert event'
+
+          context 'existing alert is resolved' do
+            let!(:alert) { create(:alert_management_alert, :resolved, project: project, fingerprint: parsed_alert.gitlab_fingerprint) }
+
+            it_behaves_like 'creates an alert management alert'
           end
 
-          context 'when status can be changed' do
-            it 'changes status to triggered' do
-              expect { execute }.to change { alert.reload.triggered? }.to(true)
-            end
+          context 'existing alert is ignored' do
+            let!(:alert) { create(:alert_management_alert, :ignored, project: project, fingerprint: parsed_alert.gitlab_fingerprint) }
+
+            it_behaves_like 'adds an alert management alert event'
           end
 
-          it 'does not executes the alert service hooks' do
-            expect(alert).not_to receive(:execute_services)
+          context 'two existing alerts, one resolved one open' do
+            let!(:resolved_alert) { create(:alert_management_alert, :resolved, project: project, fingerprint: parsed_alert.gitlab_fingerprint) }
+            let!(:alert) { create(:alert_management_alert, project: project, fingerprint: parsed_alert.gitlab_fingerprint) }
 
-            subject
+            it_behaves_like 'adds an alert management alert event'
           end
 
           context 'when status change did not succeed' do
@@ -73,23 +78,11 @@ RSpec.describe AlertManagement::ProcessPrometheusAlertService do
               execute
             end
           end
-
-          it { is_expected.to be_success }
         end
 
         context 'when alert does not exist' do
           context 'when alert can be created' do
-            it 'creates a new alert' do
-              expect { execute }.to change { AlertManagement::Alert.where(project: project).count }.by(1)
-            end
-
-            it 'executes the alert service hooks' do
-              slack_service = create(:service, type: 'SlackService', project: project, alert_events: true, active: true)
-
-              subject
-
-              expect(ProjectServiceWorker).to have_received(:perform_async).with(slack_service.id, an_instance_of(Hash))
-            end
+            it_behaves_like 'creates an alert management alert'
           end
 
           context 'when alert cannot be created' do

spec/services/projects/alerting/notify_service_spec.rb

@@ -64,12 +64,6 @@ RSpec.describe Projects::Alerting::NotifyService do
     end
   end
 
-  shared_examples 'NotifyService does not create alert' do
-    it 'does not create alert' do
-      expect { subject }.not_to change(AlertManagement::Alert, :count)
-    end
-  end
-
   describe '#execute' do
     let(:token) { 'invalid-token' }
     let(:starts_at) { Time.current.change(usec: 0) }
@@ -107,62 +101,64 @@ RSpec.describe Projects::Alerting::NotifyService do
         end
 
         context 'with valid payload' do
+          shared_examples 'assigns the alert properties' do
+            it 'ensure that created alert has all data properly assigned' do
+              subject
+
+              expect(last_alert_attributes).to match(
+                project_id: project.id,
+                title: payload_raw.fetch(:title),
+                started_at: Time.zone.parse(payload_raw.fetch(:start_time)),
+                severity: payload_raw.fetch(:severity),
+                status: AlertManagement::Alert::STATUSES[:triggered],
+                events: 1,
+                hosts: payload_raw.fetch(:hosts),
+                payload: payload_raw.with_indifferent_access,
+                issue_id: nil,
+                description: payload_raw.fetch(:description),
+                monitoring_tool: payload_raw.fetch(:monitoring_tool),
+                service: payload_raw.fetch(:service),
+                fingerprint: Digest::SHA1.hexdigest(fingerprint),
+                ended_at: nil,
+                prometheus_alert_id: nil,
+                environment_id: nil
+              )
+            end
+          end
+
           let(:last_alert_attributes) do
             AlertManagement::Alert.last.attributes
               .except('id', 'iid', 'created_at', 'updated_at')
               .with_indifferent_access
           end
 
-          it 'creates AlertManagement::Alert' do
-            expect { subject }.to change(AlertManagement::Alert, :count).by(1)
-          end
-
-          it 'created alert has all data properly assigned' do
-            subject
-
-            expect(last_alert_attributes).to match(
-              project_id: project.id,
-              title: payload_raw.fetch(:title),
-              started_at: Time.zone.parse(payload_raw.fetch(:start_time)),
-              severity: payload_raw.fetch(:severity),
-              status: AlertManagement::Alert::STATUSES[:triggered],
-              events: 1,
-              hosts: payload_raw.fetch(:hosts),
-              payload: payload_raw.with_indifferent_access,
-              issue_id: nil,
-              description: payload_raw.fetch(:description),
-              monitoring_tool: payload_raw.fetch(:monitoring_tool),
-              service: payload_raw.fetch(:service),
-              fingerprint: Digest::SHA1.hexdigest(fingerprint),
-              ended_at: nil,
-              prometheus_alert_id: nil,
-              environment_id: nil
-            )
-          end
-
-          it 'executes the alert service hooks' do
-            slack_service = create(:service, type: 'SlackService', project: project, alert_events: true, active: true)
-            subject
-
-            expect(ProjectServiceWorker).to have_received(:perform_async).with(slack_service.id, an_instance_of(Hash))
-          end
+          it_behaves_like 'creates an alert management alert'
+          it_behaves_like 'assigns the alert properties'
 
           context 'existing alert with same fingerprint' do
             let(:fingerprint_sha) { Digest::SHA1.hexdigest(fingerprint) }
-            let!(:existing_alert) { create(:alert_management_alert, project: project, fingerprint: fingerprint_sha) }
+            let!(:alert) { create(:alert_management_alert, project: project, fingerprint: fingerprint_sha) }
+
+            it_behaves_like 'adds an alert management alert event'
+
+            context 'existing alert is resolved' do
+              let!(:alert) { create(:alert_management_alert, :resolved, project: project, fingerprint: fingerprint_sha) }
 
-            it 'does not create AlertManagement::Alert' do
-              expect { subject }.not_to change(AlertManagement::Alert, :count)
+              it_behaves_like 'creates an alert management alert'
+              it_behaves_like 'assigns the alert properties'
             end
 
-            it 'increments the existing alert count' do
-              expect { subject }.to change { existing_alert.reload.events }.from(1).to(2)
+            context 'existing alert is ignored' do
+              let!(:alert) { create(:alert_management_alert, :ignored, project: project, fingerprint: fingerprint_sha) }
+
+              it_behaves_like 'adds an alert management alert event'
             end
 
-            it 'does not executes the alert service hooks' do
-              subject
+            context 'two existing alerts, one resolved one open' do
+              let!(:resolved_existing_alert) { create(:alert_management_alert, :resolved, project: project, fingerprint: fingerprint_sha) }
+              let!(:alert) { create(:alert_management_alert, project: project, fingerprint: fingerprint_sha) }
 
-              expect(ProjectServiceWorker).not_to have_received(:perform_async)
+              it_behaves_like 'adds an alert management alert event'
             end
           end
 
@@ -174,9 +170,7 @@ RSpec.describe Projects::Alerting::NotifyService do
               }
             end
 
-            it 'creates AlertManagement::Alert' do
-              expect { subject }.to change(AlertManagement::Alert, :count).by(1)
-            end
+            it_behaves_like 'creates an alert management alert'
 
             it 'created alert has all data properly assigned' do
               subject
@@ -218,19 +212,19 @@ RSpec.describe Projects::Alerting::NotifyService do
             end
 
             it_behaves_like 'does not process incident issues due to error', http_status: :bad_request
-            it_behaves_like 'NotifyService does not create alert'
+            it_behaves_like 'does not an create alert management alert'
           end
 
           context 'when alert already exists' do
             let(:fingerprint_sha) { Digest::SHA1.hexdigest(fingerprint) }
-            let!(:existing_alert) { create(:alert_management_alert, project: project, fingerprint: fingerprint_sha) }
+            let!(:alert) { create(:alert_management_alert, project: project, fingerprint: fingerprint_sha) }
 
             context 'when existing alert does not have an associated issue' do
               it_behaves_like 'processes incident issues'
             end
 
             context 'when existing alert has an associated issue' do
-              let!(:existing_alert) { create(:alert_management_alert, :with_issue, project: project, fingerprint: fingerprint_sha) }
+              let!(:alert) { create(:alert_management_alert, :with_issue, project: project, fingerprint: fingerprint_sha) }
 
               it_behaves_like 'does not process incident issues'
             end
@@ -246,14 +240,14 @@ RSpec.describe Projects::Alerting::NotifyService do
 
       context 'with invalid token' do
         it_behaves_like 'does not process incident issues due to error', http_status: :unauthorized
-        it_behaves_like 'NotifyService does not create alert'
+        it_behaves_like 'does not an create alert management alert'
       end
 
       context 'with deactivated Alerts Service' do
         let!(:alerts_service) { create(:alerts_service, :inactive, project: project) }
 
         it_behaves_like 'does not process incident issues due to error', http_status: :forbidden
-        it_behaves_like 'NotifyService does not create alert'
+        it_behaves_like 'does not an create alert management alert'
       end
     end
   end

spec/support/shared_examples/services/alert_management_shared_examples.rb

+# frozen_string_literal: true
+
+RSpec.shared_examples 'creates an alert management alert' do
+  it { is_expected.to be_success }
+
+  it 'creates AlertManagement::Alert' do
+    expect { subject }.to change(AlertManagement::Alert, :count).by(1)
+  end
+
+  it 'executes the alert service hooks' do
+    slack_service = create(:service, type: 'SlackService', project: project, alert_events: true, active: true)
+
+    subject
+
+    expect(ProjectServiceWorker).to have_received(:perform_async).with(slack_service.id, an_instance_of(Hash))
+  end
+end
+
+RSpec.shared_examples 'does not an create alert management alert' do
+  it 'does not create alert' do
+    expect { subject }.not_to change(AlertManagement::Alert, :count)
+  end
+end
+
+RSpec.shared_examples 'adds an alert management alert event' do
+  it { is_expected.to be_success }
+
+  it 'does not create an alert' do
+    expect { subject }.not_to change(AlertManagement::Alert, :count)
+  end
+
+  it 'increases alert events count' do
+    expect { subject }.to change { alert.reload.events }.by(1)
+  end
+
+  it 'does not executes the alert service hooks' do
+    expect(alert).not_to receive(:execute_services)
+
+    subject
+  end
+end