Merge branch '224500-replace-process-alert-worker-with-a-new-worker' into 'master'

Replace IncidentManagement::ProcessAlertWorker with a new version

See merge request gitlab-org/gitlab!60285

app/services/concerns/alert_management/alert_processing.rb

@@ -88,7 +88,7 @@ module AlertManagement
     def process_incident_issues
       return if alert.issue || alert.resolved?
 
-      ::IncidentManagement::ProcessAlertWorker.perform_async(nil, nil, alert.id)
+      ::IncidentManagement::ProcessAlertWorkerV2.perform_async(alert.id)
     end
 
     def send_alert_email

app/workers/all_queues.yml

@@ -1056,6 +1056,15 @@
   :weight: 2
   :idempotent:
   :tags: []
+- :name: incident_management:incident_management_process_alert_worker_v2
+  :worker_name: IncidentManagement::ProcessAlertWorkerV2
+  :feature_category: :incident_management
+  :has_external_dependencies:
+  :urgency: :low
+  :resource_boundary: :unknown
+  :weight: 2
+  :idempotent: true
+  :tags: []
 - :name: incident_management:incident_management_process_prometheus_alert
   :worker_name: IncidentManagement::ProcessPrometheusAlertWorker
   :feature_category: :incident_management

app/workers/incident_management/process_alert_worker.rb

@@ -10,6 +10,12 @@ module IncidentManagement
     # `project_id` and `alert_payload` are deprecated and can be removed
     # starting from 14.0 release
     # https://gitlab.com/gitlab-org/gitlab/-/issues/224500
+    #
+    # This worker is not scheduled anymore since
+    # https://gitlab.com/gitlab-org/gitlab/-/merge_requests/60285
+    # and will be removed completely via
+    # https://gitlab.com/gitlab-org/gitlab/-/issues/224500
+    # in 14.0.
     def perform(_project_id = nil, _alert_payload = nil, alert_id = nil)
       return unless alert_id
 

app/workers/incident_management/process_alert_worker_v2.rb

+# frozen_string_literal: true
+
+module IncidentManagement
+  class ProcessAlertWorkerV2 # rubocop:disable Scalability/IdempotentWorker
+    include ApplicationWorker
+
+    queue_namespace :incident_management
+    feature_category :incident_management
+
+    idempotent!
+
+    def perform(alert_id)
+      return unless alert_id
+
+      alert = find_alert(alert_id)
+      return unless alert
+
+      result = create_issue_for(alert)
+      return if result.success?
+
+      log_warning(alert, result)
+    end
+
+    private
+
+    def find_alert(alert_id)
+      AlertManagement::Alert.find_by_id(alert_id)
+    end
+
+    def create_issue_for(alert)
+      AlertManagement::CreateAlertIssueService
+        .new(alert, User.alert_bot)
+        .execute
+    end
+
+    def log_warning(alert, result)
+      issue_id = result.payload[:issue]&.id
+
+      Gitlab::AppLogger.warn(
+        message: 'Cannot process an Incident',
+        issue_id: issue_id,
+        alert_id: alert.id,
+        errors: result.message
+      )
+    end
+  end
+end

spec/support/shared_examples/services/alert_management/alert_processing/incident_creation_shared_examples.rb

@@ -25,9 +25,9 @@ RSpec.shared_examples 'processes incident issues' do |with_issue: false|
   end
 
   specify do
-    expect(IncidentManagement::ProcessAlertWorker)
+    expect(IncidentManagement::ProcessAlertWorkerV2)
       .to receive(:perform_async)
-      .with(nil, nil, kind_of(Integer))
+      .with(kind_of(Integer))
 
     Sidekiq::Testing.inline! do
       expect(subject).to be_success
@@ -45,7 +45,7 @@ end
 
 RSpec.shared_examples 'does not process incident issues' do
   specify do
-    expect(IncidentManagement::ProcessAlertWorker).not_to receive(:perform_async)
+    expect(IncidentManagement::ProcessAlertWorkerV2).not_to receive(:perform_async)
 
     subject
   end

spec/workers/incident_management/process_alert_worker_v2_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe IncidentManagement::ProcessAlertWorkerV2 do
+  let_it_be(:project) { create(:project) }
+  let_it_be(:settings) { create(:project_incident_management_setting, project: project, create_issue: true) }
+
+  describe '#perform' do
+    let_it_be(:started_at) { Time.now.rfc3339 }
+    let_it_be(:payload) { { 'title' => 'title', 'start_time' => started_at } }
+    let_it_be(:alert) { create(:alert_management_alert, project: project, payload: payload, started_at: started_at) }
+
+    let(:created_issue) { Issue.last! }
+
+    subject(:perform_worker) { described_class.new.perform(alert.id) }
+
+    before do
+      allow(Gitlab::AppLogger).to receive(:warn).and_call_original
+
+      allow(AlertManagement::CreateAlertIssueService)
+        .to receive(:new).with(alert, User.alert_bot)
+        .and_call_original
+    end
+
+    shared_examples 'creates issue successfully' do
+      it 'creates an issue' do
+        expect(AlertManagement::CreateAlertIssueService)
+          .to receive(:new).with(alert, User.alert_bot)
+
+        expect { perform_worker }.to change { Issue.count }.by(1)
+      end
+
+      it 'updates AlertManagement::Alert#issue_id' do
+        perform_worker
+
+        expect(alert.reload.issue_id).to eq(created_issue.id)
+      end
+
+      it 'does not write a warning to log' do
+        perform_worker
+
+        expect(Gitlab::AppLogger).not_to have_received(:warn)
+      end
+
+      it_behaves_like 'an idempotent worker' do
+        let(:job_args) { [alert.id] }
+
+        it 'does not create a second issue' do
+          expect { perform_worker }.to change { Issue.count }.by(1)
+        end
+      end
+    end
+
+    context 'with valid alert' do
+      it_behaves_like 'creates issue successfully'
+
+      context 'when alert cannot be updated' do
+        let_it_be(:alert) { create(:alert_management_alert, :with_validation_errors, project: project, payload: payload) }
+
+        it 'updates AlertManagement::Alert#issue_id' do
+          expect { perform_worker }.not_to change { alert.reload.issue_id }
+        end
+
+        it 'logs a warning' do
+          perform_worker
+
+          expect(Gitlab::AppLogger).to have_received(:warn).with(
+            message: 'Cannot process an Incident',
+            issue_id: created_issue.id,
+            alert_id: alert.id,
+            errors: 'Hosts hosts array is over 255 chars'
+          )
+        end
+      end
+
+      context 'prometheus alert' do
+        let_it_be(:alert) { create(:alert_management_alert, :prometheus, project: project, started_at: started_at) }
+
+        it_behaves_like 'creates issue successfully'
+      end
+    end
+
+    context 'with invalid alert' do
+      let(:invalid_alert_id) { non_existing_record_id }
+
+      subject(:perform_worker) { described_class.new.perform(invalid_alert_id) }
+
+      it 'does not create issues' do
+        expect(AlertManagement::CreateAlertIssueService).not_to receive(:new)
+
+        expect { perform_worker }.not_to change { Issue.count }
+      end
+    end
+  end
+end