Merge branch 'sy-refactor-prometheus-notify-service' into 'master'

Use AlertManagement::Payload for processing Prometheus notification

See merge request gitlab-org/gitlab!39271

app/services/alert_management/process_prometheus_alert_service.rb

@@ -6,7 +6,7 @@ module AlertManagement
     include ::IncidentManagement::Settings
 
     def execute
-      return bad_request unless parsed_alert.valid?
+      return bad_request unless incoming_payload.has_required_attributes?
 
       process_alert_management_alert
 
@@ -15,22 +15,17 @@ module AlertManagement
 
     private
 
-    delegate :firing?, :resolved?, :gitlab_fingerprint, :ends_at, to: :parsed_alert
-
-    def parsed_alert
-      strong_memoize(:parsed_alert) do
-        Gitlab::Alerting::Alert.new(project: project, payload: params)
-      end
-    end
-
     def process_alert_management_alert
-      process_firing_alert_management_alert if firing?
-      process_resolved_alert_management_alert if resolved?
+      if incoming_payload.resolved?
+        process_resolved_alert_management_alert
+      else
+        process_firing_alert_management_alert
+      end
     end
 
     def process_firing_alert_management_alert
-      if am_alert.present?
-        am_alert.register_new_event!
+      if alert.persisted?
+        alert.register_new_event!
         reset_alert_management_alert_status
       else
         create_alert_management_alert
@@ -40,48 +35,42 @@ module AlertManagement
     end
 
     def reset_alert_management_alert_status
-      return if am_alert.trigger
+      return if alert.trigger
 
       logger.warn(
         message: 'Unable to update AlertManagement::Alert status to triggered',
         project_id: project.id,
-        alert_id: am_alert.id
+        alert_id: alert.id
       )
     end
 
     def create_alert_management_alert
-      new_alert = AlertManagement::Alert.new(am_alert_params.merge(ended_at: nil))
-      if new_alert.save
-        new_alert.execute_services
-        @am_alert = new_alert
-        SystemNoteService.create_new_alert(new_alert, Gitlab::AlertManagement::AlertParams::MONITORING_TOOLS[:prometheus])
+      if alert.save
+        alert.execute_services
+        SystemNoteService.create_new_alert(alert, Gitlab::AlertManagement::AlertParams::MONITORING_TOOLS[:prometheus])
         return
       end
 
       logger.warn(
         message: 'Unable to create AlertManagement::Alert',
         project_id: project.id,
-        alert_errors: new_alert.errors.messages
+        alert_errors: alert.errors.messages
       )
     end
 
-    def am_alert_params
-      Gitlab::AlertManagement::AlertParams.from_prometheus_alert(project: project, parsed_alert: parsed_alert)
-    end
-
     def process_resolved_alert_management_alert
-      return if am_alert.blank?
+      return unless alert.persisted?
       return unless auto_close_incident?
 
-      if am_alert.resolve(ends_at)
-        close_issue(am_alert.issue)
+      if alert.resolve(incoming_payload.ends_at)
+        close_issue(alert.issue)
         return
       end
 
       logger.warn(
         message: 'Unable to update AlertManagement::Alert status to resolved',
         project_id: project.id,
-        alert_id: am_alert.id
+        alert_id: alert.id
       )
     end
 
@@ -96,19 +85,44 @@ module AlertManagement
     end
 
     def process_incident_alert
-      return unless am_alert
-      return if am_alert.issue
+      return unless alert.persisted?
+      return if alert.issue
 
-      IncidentManagement::ProcessAlertWorker.perform_async(nil, nil, am_alert.id)
+      IncidentManagement::ProcessAlertWorker.perform_async(nil, nil, alert.id)
     end
 
     def logger
       @logger ||= Gitlab::AppLogger
     end
 
-    def am_alert
-      strong_memoize(:am_alert) do
-        AlertManagement::Alert.not_resolved.for_fingerprint(project, gitlab_fingerprint).first
+    def alert
+      strong_memoize(:alert) do
+        existing_alert || new_alert
+      end
+    end
+
+    def existing_alert
+      strong_memoize(:existing_alert) do
+        AlertManagement::Alert.not_resolved.for_fingerprint(project, incoming_payload.gitlab_fingerprint).first
+      end
+    end
+
+    def new_alert
+      strong_memoize(:new_alert) do
+        AlertManagement::Alert.new(
+          **incoming_payload.alert_params,
+          ended_at: nil
+        )
+      end
+    end
+
+    def incoming_payload
+      strong_memoize(:incoming_payload) do
+        Gitlab::AlertManagement::Payload.parse(
+          project,
+          params,
+          monitoring_tool: Gitlab::AlertManagement::Payload::MONITORING_TOOLS[:prometheus]
+        )
       end
     end
 

lib/gitlab/alert_management/payload/base.rb

@@ -20,6 +20,7 @@ module Gitlab
           :alert_markdown,
           :alert_title,
           :annotations,
+          :description,
           :ends_at,
           :environment,
           :environment_name,
@@ -29,11 +30,12 @@ module Gitlab
           :gitlab_fingerprint,
           :gitlab_prometheus_alert_id,
           :gitlab_y_label,
-          :description,
+          :has_required_attributes?,
           :hosts,
           :metric_id,
           :metrics_dashboard_url,
           :monitoring_tool,
+          :resolved?,
           :runbook,
           :service,
           :severity,
@@ -121,6 +123,14 @@ module Gitlab
           end
         end
 
+        def resolved?
+          status == 'resolved'
+        end
+
+        def has_required_attributes?
+          true
+        end
+
         private
 
         def plain_gitlab_fingerprint; end

lib/gitlab/alert_management/payload/prometheus.rb

@@ -61,6 +61,10 @@ module Gitlab
           )
         end
 
+        def has_required_attributes?
+          project && title && starts_at_raw
+        end
+
         private
 
         def plain_gitlab_fingerprint

spec/lib/gitlab/alert_management/payload/base_spec.rb

@@ -175,4 +175,36 @@ RSpec.describe Gitlab::AlertManagement::Payload::Base do
       it { is_expected.to eq(environment) }
     end
   end
+
+  describe '#resolved?' do
+    before do
+      allow(parsed_payload).to receive(:status).and_return(status)
+    end
+
+    subject { parsed_payload.resolved? }
+
+    context 'when status is not defined' do
+      let(:status) { nil }
+
+      it { is_expected.to be_falsey }
+    end
+
+    context 'when status is not resovled' do
+      let(:status) { 'firing' }
+
+      it { is_expected.to be_falsey }
+    end
+
+    context 'when status is resovled' do
+      let(:status) { 'resolved' }
+
+      it { is_expected.to be_truthy }
+    end
+  end
+
+  describe '#has_required_attributes?' do
+    subject { parsed_payload.has_required_attributes? }
+
+    it { is_expected.to be(true) }
+  end
 end

spec/lib/gitlab/alert_management/payload/prometheus_spec.rb

@@ -204,4 +204,37 @@ RSpec.describe Gitlab::AlertManagement::Payload::Prometheus do
       it { is_expected.to be_nil }
     end
   end
+
+  describe '#has_required_attributes?' do
+    let(:starts_at) { Time.current.change(usec: 0).utc }
+    let(:raw_payload) { { 'annotations' => { 'title' => 'title' }, 'startsAt' => starts_at.rfc3339 } }
+
+    subject { parsed_payload.has_required_attributes? }
+
+    it { is_expected.to be_truthy }
+
+    context 'without project' do
+      let(:parsed_payload) { described_class.new(project: nil, payload: raw_payload) }
+
+      it { is_expected.to be_falsey }
+    end
+
+    context 'without title' do
+      let(:raw_payload) { { 'startsAt' => starts_at.rfc3339 } }
+
+      it { is_expected.to be_falsey }
+    end
+
+    context 'without startsAt' do
+      let(:raw_payload) { { 'annotations' => { 'title' => 'title' } } }
+
+      it { is_expected.to be_falsey }
+    end
+
+    context 'without payload' do
+      let(:parsed_payload) { described_class.new(project: project, payload: nil) }
+
+      it { is_expected.to be_falsey }
+    end
+  end
 end

spec/services/alert_management/process_prometheus_alert_service_spec.rb

@@ -13,7 +13,8 @@ RSpec.describe AlertManagement::ProcessPrometheusAlertService do
     subject(:execute) { described_class.new(project, nil, payload).execute }
 
     context 'when alert payload is valid' do
-      let(:parsed_alert) { Gitlab::Alerting::Alert.new(project: project, payload: payload) }
+      let(:parsed_payload) { Gitlab::AlertManagement::Payload.parse(project, payload, monitoring_tool: 'Prometheus') }
+      let(:fingerprint) { parsed_payload.gitlab_fingerprint }
       let(:payload) do
         {
           'status' => status,
@@ -39,25 +40,25 @@ 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, project: project, fingerprint: parsed_alert.gitlab_fingerprint) }
+          let!(:alert) { create(:alert_management_alert, project: project, fingerprint: fingerprint) }
 
           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) }
+            let!(:alert) { create(:alert_management_alert, :resolved, project: project, fingerprint: fingerprint) }
 
             it_behaves_like 'creates an alert management alert'
           end
 
           context 'existing alert is ignored' do
-            let!(:alert) { create(:alert_management_alert, :ignored, project: project, fingerprint: parsed_alert.gitlab_fingerprint) }
+            let!(:alert) { create(:alert_management_alert, :ignored, project: project, fingerprint: fingerprint) }
 
             it_behaves_like 'adds an alert management alert event'
           end
 
           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) }
+            let!(:resolved_alert) { create(:alert_management_alert, :resolved, project: project, fingerprint: fingerprint) }
+            let!(:alert) { create(:alert_management_alert, project: project, fingerprint: fingerprint) }
 
             it_behaves_like 'adds an alert management alert event'
           end
@@ -99,18 +100,15 @@ RSpec.describe AlertManagement::ProcessPrometheusAlertService do
           end
 
           context 'when alert cannot be created' do
-            let(:errors) { double(messages: { hosts: ['hosts array is over 255 chars'] })}
-            let(:am_alert) { instance_double(AlertManagement::Alert, save: false, errors: errors) }
-
             before do
-              allow(AlertManagement::Alert).to receive(:new).and_return(am_alert)
+              payload['annotations']['title'] = 'description' * 50
             end
 
             it 'writes a warning to the log' do
               expect(Gitlab::AppLogger).to receive(:warn).with(
                 message: 'Unable to create AlertManagement::Alert',
                 project_id: project.id,
-                alert_errors: { hosts: ['hosts array is over 255 chars'] }
+                alert_errors: { title: ["is too long (maximum is 200 characters)"] }
               )
 
               execute
@@ -130,7 +128,7 @@ 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: parsed_alert.gitlab_fingerprint) }
+        let!(:alert) { create(:alert_management_alert, project: project, fingerprint: fingerprint) }
 
         context 'when auto_resolve_incident set to true' do
           let_it_be(:operations_settings) { create(:project_incident_management_setting, project: project, auto_close_incident: true) }
@@ -146,7 +144,7 @@ RSpec.describe AlertManagement::ProcessPrometheusAlertService do
                   stub_feature_flags(track_resource_state_change_events: state_tracking_enabled)
                 end
 
-                let!(:alert) { create(:alert_management_alert, :with_issue, project: project, fingerprint: parsed_alert.gitlab_fingerprint) }
+                let!(:alert) { create(:alert_management_alert, :with_issue, project: project, fingerprint: fingerprint) }
 
                 it 'closes the issue' do
                   issue = alert.issue