Merge branch 'sy-system-note-for-resolving-alert' into 'master'

Create system note for auto-resolution of alerts

See merge request gitlab-org/gitlab!54645

app/services/concerns/alert_management/alert_processing.rb

@@ -41,14 +41,21 @@ module AlertManagement
     end
 
     def process_resolved_alert
+      SystemNoteService.log_resolving_alert(alert, alert_source)
+
       return unless auto_close_incident?
-      return close_issue(alert.issue) if alert.resolve(incoming_payload.ends_at)
 
-      logger.warn(
-        message: 'Unable to update AlertManagement::Alert status to resolved',
-        project_id: project.id,
-        alert_id: alert.id
-      )
+      if alert.resolve(incoming_payload.ends_at)
+        SystemNoteService.change_alert_status(alert, User.alert_bot)
+
+        close_issue(alert.issue)
+      else
+        logger.warn(
+          message: 'Unable to update AlertManagement::Alert status to resolved',
+          project_id: project.id,
+          alert_id: alert.id
+        )
+      end
     end
 
     def process_firing_alert

app/services/system_note_service.rb

@@ -327,6 +327,10 @@ module SystemNoteService
     ::SystemNotes::IncidentService.new(noteable: incident, project: incident.project, author: author).change_incident_severity
   end
 
+  def log_resolving_alert(alert, monitoring_tool)
+    ::SystemNotes::AlertManagementService.new(noteable: alert, project: alert.project).log_resolving_alert(monitoring_tool)
+  end
+
   private
 
   def merge_requests_service(noteable, project, author)

app/services/system_notes/alert_management_service.rb

@@ -62,5 +62,20 @@ module SystemNotes
 
       create_note(NoteSummary.new(noteable, project, author, body, action: 'status'))
     end
+
+    # Called when an alert is resolved due to received resolving alert payload
+    #
+    # alert - AlertManagement::Alert object.
+    #
+    # Example Note text:
+    #
+    #   "changed the status to Resolved by closing issue #17"
+    #
+    # Returns the created Note object
+    def log_resolving_alert(monitoring_tool)
+      body = "logged a resolving alert from **#{monitoring_tool}**"
+
+      create_note(NoteSummary.new(noteable, project, User.alert_bot, body, action: 'new_alert_added'))
+    end
   end
 end

changelogs/unreleased/sy-system-note-for-resolving-alert.yml

+---
+title: Create system note on alert when its auto-resolved via alert integration
+merge_request: 54645
+author:
+type: added

spec/services/alert_management/process_prometheus_alert_service_spec.rb

@@ -11,6 +11,7 @@ RSpec.describe AlertManagement::ProcessPrometheusAlertService do
 
   describe '#execute' do
     let(:service) { described_class.new(project, payload) }
+    let(:source) { 'Prometheus' }
     let(:auto_close_incident) { true }
     let(:create_issue) { true }
     let(:send_email) { true }
@@ -31,7 +32,7 @@ RSpec.describe AlertManagement::ProcessPrometheusAlertService do
     subject(:execute) { service.execute }
 
     context 'when alert payload is valid' do
-      let(:parsed_payload) { Gitlab::AlertManagement::Payload.parse(project, payload, monitoring_tool: 'Prometheus') }
+      let(:parsed_payload) { Gitlab::AlertManagement::Payload.parse(project, payload, monitoring_tool: source) }
       let(:fingerprint) { parsed_payload.gitlab_fingerprint }
       let(:payload) do
         {
@@ -112,9 +113,7 @@ RSpec.describe AlertManagement::ProcessPrometheusAlertService do
             it_behaves_like 'Alert Notification Service sends notification email'
             it_behaves_like 'processes incident issues'
 
-            it 'creates a system note corresponding to alert creation' do
-              expect { subject }.to change(Note, :count).by(1)
-            end
+            it_behaves_like 'creates single system note based on the source of the alert'
 
             context 'when auto-alert creation is disabled' do
               let(:create_issue) { false }
@@ -158,17 +157,20 @@ RSpec.describe AlertManagement::ProcessPrometheusAlertService do
 
       context 'when Prometheus alert status is resolved' do
         let(:status) { 'resolved' }
-        let!(:alert) { create(:alert_management_alert, project: project, fingerprint: fingerprint) }
+        let!(:alert) { create(:alert_management_alert, project: project, fingerprint: fingerprint, monitoring_tool: source) }
 
         context 'when auto_resolve_incident set to true' do
           context 'when status can be changed' do
             it_behaves_like 'Alert Notification Service sends notification email'
             it_behaves_like 'does not process incident issues'
 
-            it 'resolves an existing alert' do
+            it 'resolves an existing alert without error' do
+              expect(Gitlab::AppLogger).not_to receive(:warn)
               expect { execute }.to change { alert.reload.resolved? }.to(true)
             end
 
+            it_behaves_like 'creates status-change system note for an auto-resolved alert'
+
             context 'existing issue' do
               let!(:alert) { create(:alert_management_alert, :with_issue, project: project, fingerprint: fingerprint) }
 
@@ -215,6 +217,8 @@ RSpec.describe AlertManagement::ProcessPrometheusAlertService do
           it 'does not resolve an existing alert' do
             expect { execute }.not_to change { alert.reload.resolved? }
           end
+
+          it_behaves_like 'creates single system note based on the source of the alert'
         end
 
         context 'when emails are disabled' do

spec/services/projects/alerting/notify_service_spec.rb

@@ -119,6 +119,7 @@ RSpec.describe Projects::Alerting::NotifyService do
               end
 
               it_behaves_like 'does not an create alert management alert'
+              it_behaves_like 'creates single system note based on the source of the alert'
 
               context 'auto_close_enabled setting enabled' do
                 let(:auto_close_enabled) { true }
@@ -131,6 +132,8 @@ RSpec.describe Projects::Alerting::NotifyService do
                   expect(alert.ended_at).to eql(ended_at)
                 end
 
+                it_behaves_like 'creates status-change system note for an auto-resolved alert'
+
                 context 'related issue exists' do
                   let(:alert) { create(:alert_management_alert, :with_issue, project: project, fingerprint: fingerprint_sha) }
                   let(:issue) { alert.issue }
@@ -209,10 +212,7 @@ RSpec.describe Projects::Alerting::NotifyService do
               )
             end
 
-            it 'creates a system note corresponding to alert creation' do
-              expect { subject }.to change(Note, :count).by(1)
-              expect(Note.last.note).to include(source)
-            end
+            it_behaves_like 'creates single system note based on the source of the alert'
           end
         end
 

spec/services/system_note_service_spec.rb

@@ -779,4 +779,17 @@ RSpec.describe SystemNoteService do
       described_class.change_incident_severity(incident, author)
     end
   end
+
+  describe '.log_resolving_alert' do
+    let(:alert) { build(:alert_management_alert) }
+    let(:monitoring_tool) { 'Prometheus' }
+
+    it 'calls AlertManagementService' do
+      expect_next_instance_of(SystemNotes::AlertManagementService) do |service|
+        expect(service).to receive(:log_resolving_alert).with(monitoring_tool)
+      end
+
+      described_class.log_resolving_alert(alert, monitoring_tool)
+    end
+  end
 end

spec/services/system_notes/alert_management_service_spec.rb

@@ -59,4 +59,17 @@ RSpec.describe ::SystemNotes::AlertManagementService do
       expect(subject.note).to eq("changed the status to **Resolved** by closing issue #{issue.to_reference(project)}")
     end
   end
+
+  describe '#log_resolving_alert' do
+    subject { described_class.new(noteable: noteable, project: project).log_resolving_alert('Some Service') }
+
+    it_behaves_like 'a system note' do
+      let(:author) { User.alert_bot }
+      let(:action) { 'new_alert_added' }
+    end
+
+    it 'has the appropriate message' do
+      expect(subject.note).to eq('logged a resolving alert from **Some Service**')
+    end
+  end
 end

spec/support/shared_examples/alert_notification_service_shared_examples.rb

@@ -27,3 +27,18 @@ RSpec.shared_examples 'Alert Notification Service sends no notifications' do |ht
     end
   end
 end
+
+RSpec.shared_examples 'creates status-change system note for an auto-resolved alert' do
+  it 'has 2 new system notes' do
+    expect { subject }.to change(Note, :count).by(2)
+    expect(Note.last.note).to include('Resolved')
+  end
+end
+
+# Requires `source` to be defined
+RSpec.shared_examples 'creates single system note based on the source of the alert' do
+  it 'has one new system note' do
+    expect { subject }.to change(Note, :count).by(1)
+    expect(Note.last.note).to include(source)
+  end
+end