Merge branch '217048-alert-management-severity-should-be-ingested-by-alerts-endpoint' into 'master'

Resolve "Alert Management - Severity should be ingested by Alerts endpoint"

Closes #217048

See merge request gitlab-org/gitlab!31549

lib/gitlab/alert_management/alert_params.rb

@@ -19,7 +19,8 @@ module Gitlab
           service: annotations[:service],
           hosts: Array(annotations[:hosts]),
           payload: payload,
-          started_at: parsed_payload['startsAt']
+          started_at: parsed_payload['startsAt'],
+          severity: annotations[:severity]
         }
       end
 

lib/gitlab/alerting/notification_payload_parser.rb

@@ -5,7 +5,8 @@ module Gitlab
     class NotificationPayloadParser
       BadPayloadError = Class.new(StandardError)
 
-      DEFAULT_TITLE = 'New: Incident'
+      DEFAULT_TITLE = 'New: Incident'.freeze
+      DEFAULT_SEVERITY = 'critical'.freeze
 
       def initialize(payload)
         @payload = payload.to_h.with_indifferent_access
@@ -30,6 +31,10 @@ module Gitlab
         payload[:title].presence || DEFAULT_TITLE
       end
 
+      def severity
+        payload[:severity].presence || DEFAULT_SEVERITY
+      end
+
       def annotations
         primary_params
           .reverse_merge(flatten_secondary_params)
@@ -43,7 +48,8 @@ module Gitlab
           'description' => payload[:description],
           'monitoring_tool' => payload[:monitoring_tool],
           'service' => payload[:service],
-          'hosts' => hosts.presence
+          'hosts' => hosts.presence,
+          'severity' => severity
         }
       end
 

spec/factories/alert_management/alerts.rb

@@ -25,7 +25,7 @@ FactoryBot.define do
     end
 
     trait :with_host do
-      hosts { FFaker::Internet.ip_v4_address }
+      hosts { [FFaker::Internet.ip_v4_address] }
     end
 
     trait :with_ended_at do
@@ -56,12 +56,17 @@ FactoryBot.define do
       without_ended_at
     end
 
+    trait :low_severity do
+      severity { 'low' }
+    end
+
     trait :all_fields do
       with_issue
       with_fingerprint
       with_service
       with_monitoring_tool
       with_host
+      low_severity
     end
   end
 end

spec/lib/gitlab/alert_management/alert_params_spec.rb

@@ -7,7 +7,7 @@ describe Gitlab::AlertManagement::AlertParams do
 
   describe '.from_generic_alert' do
     let(:started_at) { Time.current.change(usec: 0).rfc3339 }
-    let(:payload) do
+    let(:default_payload) do
       {
         'title' => 'Alert title',
         'description' => 'Description',
@@ -18,6 +18,7 @@ describe Gitlab::AlertManagement::AlertParams do
         'some' => { 'extra' => { 'payload' => 'here' } }
       }
     end
+    let(:payload) { default_payload }
 
     subject { described_class.from_generic_alert(project: project, payload: payload) }
 
@@ -28,12 +29,21 @@ describe Gitlab::AlertManagement::AlertParams do
         description: 'Description',
         monitoring_tool: 'Monitoring tool name',
         service: 'Service',
+        severity: 'critical',
         hosts: ['gitlab.com'],
         payload: payload,
         started_at: started_at
       )
     end
 
+    context 'when severity given' do
+      let(:payload) { default_payload.merge(severity: 'low') }
+
+      it 'returns Alert compatible parameters' do
+        expect(subject[:severity]).to eq('low')
+      end
+    end
+
     context 'when there are no hosts in the payload' do
       let(:payload) { {} }
 

spec/lib/gitlab/alerting/notification_payload_parser_spec.rb

@@ -12,7 +12,8 @@ describe Gitlab::Alerting::NotificationPayloadParser do
         'description' => 'Description',
         'monitoring_tool' => 'Monitoring tool name',
         'service' => 'Service',
-        'hosts' => ['gitlab.com']
+        'hosts' => ['gitlab.com'],
+        'severity' => 'low'
       }
     end
 
@@ -26,7 +27,8 @@ describe Gitlab::Alerting::NotificationPayloadParser do
             'description' => 'Description',
             'monitoring_tool' => 'Monitoring tool name',
             'service' => 'Service',
-            'hosts' => ['gitlab.com']
+            'hosts' => ['gitlab.com'],
+            'severity' => 'low'
           },
           'startsAt' => starts_at.rfc3339
         }
@@ -67,11 +69,24 @@ describe Gitlab::Alerting::NotificationPayloadParser do
       let(:payload) { {} }
 
       it 'returns default parameters' do
-        is_expected.to eq(
-          'annotations' => { 'title' => 'New: Incident' },
+        is_expected.to match(
+          'annotations' => {
+            'title' => described_class::DEFAULT_TITLE,
+            'severity' => described_class::DEFAULT_SEVERITY
+          },
           'startsAt' => starts_at.rfc3339
         )
       end
+
+      context 'when severity is blank' do
+        before do
+          payload[:severity] = ''
+        end
+
+        it 'sets severity to the default ' do
+          expect(subject.dig('annotations', 'severity')).to eq(described_class::DEFAULT_SEVERITY)
+        end
+      end
     end
 
     context 'when payload attributes have blank lines' do
@@ -88,7 +103,10 @@ describe Gitlab::Alerting::NotificationPayloadParser do
 
       it 'returns default parameters' do
         is_expected.to eq(
-          'annotations' => { 'title' => 'New: Incident' },
+          'annotations' => {
+            'title' => 'New: Incident',
+            'severity' => described_class::DEFAULT_SEVERITY
+          },
           'startsAt' => starts_at.rfc3339
         )
       end
@@ -112,6 +130,7 @@ describe Gitlab::Alerting::NotificationPayloadParser do
         is_expected.to eq(
           'annotations' => {
             'title' => 'New: Incident',
+            'severity' => described_class::DEFAULT_SEVERITY,
             'description' => 'Description',
             'additional.params.1' => 'Some value 1',
             'additional.params.2' => 'Some value 2'

spec/services/projects/alerting/notify_service_spec.rb

@@ -76,9 +76,14 @@ describe Projects::Alerting::NotifyService do
     let(:service) { described_class.new(project, nil, payload) }
     let(:payload_raw) do
       {
-        'title' => 'alert title',
-        'start_time' => starts_at.rfc3339
-      }
+        title: 'alert title',
+        start_time: starts_at.rfc3339,
+        severity: 'low',
+        monitoring_tool: 'GitLab RSpec',
+        service: 'GitLab Test Suite',
+        description: 'Very detailed description',
+        hosts: ['1.1.1.1', '2.2.2.2']
+      }.with_indifferent_access
     end
     let(:payload) { ActionController::Parameters.new(payload_raw).permit! }
 
@@ -100,6 +105,12 @@ describe Projects::Alerting::NotifyService do
         end
 
         context 'with valid payload' do
+          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
@@ -107,26 +118,57 @@ describe Projects::Alerting::NotifyService do
           it 'created alert has all data properly assigned' do
             subject
 
-            alert = AlertManagement::Alert.last
-            alert_attributes = alert.attributes.except('id', 'iid', 'created_at', 'updated_at')
-
-            expect(alert_attributes).to eq(
-              'project_id' => project.id,
-              'issue_id' => nil,
-              'fingerprint' => nil,
-              'title' => 'alert title',
-              'description' => nil,
-              'monitoring_tool' => nil,
-              'service' => nil,
-              'hosts' => [],
-              'payload' => payload_raw,
-              'severity' => 'critical',
-              'status' => AlertManagement::Alert::STATUSES[:triggered],
-              'events' => 1,
-              'started_at' => alert.started_at,
-              'ended_at' => nil
+            expect(last_alert_attributes).to match(
+              project_id: project.id,
+              title: payload_raw.fetch(:title),
+              started_at: Time.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: nil,
+              ended_at: nil
             )
           end
+
+          context 'with a minimal payload' do
+            let(:payload_raw) do
+              {
+                title: 'alert title',
+                start_time: starts_at.rfc3339
+              }
+            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.parse(payload_raw.fetch(:start_time)),
+                severity: 'critical',
+                status: AlertManagement::Alert::STATUSES[:triggered],
+                events: 1,
+                hosts: [],
+                payload: payload_raw.with_indifferent_access,
+                issue_id: nil,
+                description: nil,
+                monitoring_tool: nil,
+                service: nil,
+                fingerprint: nil,
+                ended_at: nil
+              )
+            end
+          end
         end
 
         it_behaves_like 'does not process incident issues'