Commit 949faa28 authored by Dmytro Zaporozhets's avatar Dmytro Zaporozhets

Merge branch '214624-save-generic-alert-in-database' into 'master'

Persist a Generic Alert information in the DB

Closes #214624

See merge request gitlab-org/gitlab!29941
parents 9f953fea 2b7d4b32
...@@ -10,7 +10,10 @@ module Projects ...@@ -10,7 +10,10 @@ module Projects
return forbidden unless alerts_service_activated? return forbidden unless alerts_service_activated?
return unauthorized unless valid_token?(token) return unauthorized unless valid_token?(token)
process_incident_issues if process_issues? alert = create_alert
return bad_request unless alert.persisted?
process_incident_issues(alert) if process_issues?
send_alert_email if send_email? send_alert_email if send_email?
ServiceResponse.success ServiceResponse.success
...@@ -22,13 +25,21 @@ module Projects ...@@ -22,13 +25,21 @@ module Projects
delegate :alerts_service, :alerts_service_activated?, to: :project delegate :alerts_service, :alerts_service_activated?, to: :project
def am_alert_params
Gitlab::AlertManagement::AlertParams.from_generic_alert(project: project, payload: params.to_h)
end
def create_alert
AlertManagement::Alert.create(am_alert_params)
end
def send_email? def send_email?
incident_management_setting.send_email? incident_management_setting.send_email?
end end
def process_incident_issues def process_incident_issues(alert)
IncidentManagement::ProcessAlertWorker IncidentManagement::ProcessAlertWorker
.perform_async(project.id, parsed_payload) .perform_async(project.id, parsed_payload, alert.id)
end end
def send_alert_email def send_alert_email
......
...@@ -7,11 +7,14 @@ module IncidentManagement ...@@ -7,11 +7,14 @@ module IncidentManagement
queue_namespace :incident_management queue_namespace :incident_management
feature_category :incident_management feature_category :incident_management
def perform(project_id, alert) def perform(project_id, alert_payload, am_alert_id = nil)
project = find_project(project_id) project = find_project(project_id)
return unless project return unless project
create_issue(project, alert) new_issue = create_issue(project, alert_payload)
return unless am_alert_id && new_issue.persisted?
link_issue_with_alert(am_alert_id, new_issue.id)
end end
private private
...@@ -20,10 +23,24 @@ module IncidentManagement ...@@ -20,10 +23,24 @@ module IncidentManagement
Project.find_by_id(project_id) Project.find_by_id(project_id)
end end
def create_issue(project, alert) def create_issue(project, alert_payload)
IncidentManagement::CreateIssueService IncidentManagement::CreateIssueService
.new(project, alert) .new(project, alert_payload)
.execute .execute
end end
def link_issue_with_alert(alert_id, issue_id)
alert = AlertManagement::Alert.find_by_id(alert_id)
return unless alert
return if alert.update(issue_id: issue_id)
Gitlab::GitLogger.warn(
message: 'Cannot link an Issue with Alert',
issue_id: issue_id,
alert_id: alert_id,
alert_errors: alert.errors.messages
)
end
end end
end end
# frozen_string_literal: true
module Gitlab
module AlertManagement
class AlertParams
def self.from_generic_alert(project:, payload:)
parsed_payload = Gitlab::Alerting::NotificationPayloadParser.call(payload).with_indifferent_access
annotations = parsed_payload[:annotations]
{
project_id: project.id,
title: annotations[:title],
description: annotations[:description],
monitoring_tool: annotations[:monitoring_tool],
service: annotations[:service],
hosts: Array(annotations[:hosts]),
payload: payload,
started_at: parsed_payload['startsAt']
}
end
end
end
end
# frozen_string_literal: true
require 'spec_helper'
describe Gitlab::AlertManagement::AlertParams do
let_it_be(:project) { create(:project, :repository, :private) }
describe '.from_generic_alert' do
let(:started_at) { Time.current.change(usec: 0).rfc3339 }
let(:payload) do
{
'title' => 'Alert title',
'description' => 'Description',
'monitoring_tool' => 'Monitoring tool name',
'service' => 'Service',
'hosts' => ['gitlab.com'],
'start_time' => started_at,
'some' => { 'extra' => { 'payload' => 'here' } }
}
end
subject { described_class.from_generic_alert(project: project, payload: payload) }
it 'returns Alert compatible parameters' do
is_expected.to eq(
project_id: project.id,
title: 'Alert title',
description: 'Description',
monitoring_tool: 'Monitoring tool name',
service: 'Service',
hosts: ['gitlab.com'],
payload: payload,
started_at: started_at
)
end
context 'when there are no hosts in the payload' do
let(:payload) { {} }
it 'hosts param is an empty array' do
expect(subject[:hosts]).to be_empty
end
end
end
end
...@@ -12,11 +12,16 @@ describe Projects::Alerting::NotifyService do ...@@ -12,11 +12,16 @@ describe Projects::Alerting::NotifyService do
shared_examples 'processes incident issues' do |amount| shared_examples 'processes incident issues' do |amount|
let(:create_incident_service) { spy } let(:create_incident_service) { spy }
let(:new_alert) { instance_double(AlertManagement::Alert, id: 503, persisted?: true) }
it 'processes issues' do it 'processes issues' do
expect(AlertManagement::Alert)
.to receive(:create)
.and_return(new_alert)
expect(IncidentManagement::ProcessAlertWorker) expect(IncidentManagement::ProcessAlertWorker)
.to receive(:perform_async) .to receive(:perform_async)
.with(project.id, kind_of(Hash)) .with(project.id, kind_of(Hash), new_alert.id)
.exactly(amount).times .exactly(amount).times
Sidekiq::Testing.inline! do Sidekiq::Testing.inline! do
...@@ -59,6 +64,12 @@ describe Projects::Alerting::NotifyService do ...@@ -59,6 +64,12 @@ describe Projects::Alerting::NotifyService do
end end
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 describe '#execute' do
let(:token) { 'invalid-token' } let(:token) { 'invalid-token' }
let(:starts_at) { Time.now.change(usec: 0) } let(:starts_at) { Time.now.change(usec: 0) }
...@@ -88,6 +99,36 @@ describe Projects::Alerting::NotifyService do ...@@ -88,6 +99,36 @@ describe Projects::Alerting::NotifyService do
.and_return(incident_management_setting) .and_return(incident_management_setting)
end end
context 'with valid payload' do
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
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' => 'triggered',
'events' => 1,
'started_at' => alert.started_at,
'ended_at' => nil
)
end
end
it_behaves_like 'does not process incident issues' it_behaves_like 'does not process incident issues'
context 'issue enabled' do context 'issue enabled' do
...@@ -103,6 +144,7 @@ describe Projects::Alerting::NotifyService do ...@@ -103,6 +144,7 @@ describe Projects::Alerting::NotifyService do
end end
it_behaves_like 'does not process incident issues due to error', http_status: :bad_request it_behaves_like 'does not process incident issues due to error', http_status: :bad_request
it_behaves_like 'NotifyService does not create alert'
end end
end end
...@@ -115,12 +157,14 @@ describe Projects::Alerting::NotifyService do ...@@ -115,12 +157,14 @@ describe Projects::Alerting::NotifyService do
context 'with invalid token' do context 'with invalid token' do
it_behaves_like 'does not process incident issues due to error', http_status: :unauthorized it_behaves_like 'does not process incident issues due to error', http_status: :unauthorized
it_behaves_like 'NotifyService does not create alert'
end end
context 'with deactivated Alerts Service' do context 'with deactivated Alerts Service' do
let!(:alerts_service) { create(:alerts_service, :inactive, project: project) } 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 'does not process incident issues due to error', http_status: :forbidden
it_behaves_like 'NotifyService does not create alert'
end end
end end
end end
......
...@@ -6,16 +6,24 @@ describe IncidentManagement::ProcessAlertWorker do ...@@ -6,16 +6,24 @@ describe IncidentManagement::ProcessAlertWorker do
let_it_be(:project) { create(:project) } let_it_be(:project) { create(:project) }
describe '#perform' do describe '#perform' do
let(:alert) { :alert } let(:alert_management_alert_id) { nil }
let(:create_issue_service) { spy(:create_issue_service) } let(:alert_payload) { { alert: 'payload' } }
let(:new_issue) { create(:issue, project: project) }
let(:create_issue_service) { instance_double(IncidentManagement::CreateIssueService, execute: new_issue) }
subject { described_class.new.perform(project.id, alert) } subject { described_class.new.perform(project.id, alert_payload, alert_management_alert_id) }
before do
allow(IncidentManagement::CreateIssueService)
.to receive(:new).with(project, alert_payload)
.and_return(create_issue_service)
end
it 'calls create issue service' do it 'calls create issue service' do
expect(Project).to receive(:find_by_id).and_call_original expect(Project).to receive(:find_by_id).and_call_original
expect(IncidentManagement::CreateIssueService) expect(IncidentManagement::CreateIssueService)
.to receive(:new).with(project, :alert) .to receive(:new).with(project, alert_payload)
.and_return(create_issue_service) .and_return(create_issue_service)
expect(create_issue_service).to receive(:execute) expect(create_issue_service).to receive(:execute)
...@@ -26,7 +34,7 @@ describe IncidentManagement::ProcessAlertWorker do ...@@ -26,7 +34,7 @@ describe IncidentManagement::ProcessAlertWorker do
context 'with invalid project' do context 'with invalid project' do
let(:invalid_project_id) { 0 } let(:invalid_project_id) { 0 }
subject { described_class.new.perform(invalid_project_id, alert) } subject { described_class.new.perform(invalid_project_id, alert_payload) }
it 'does not create issues' do it 'does not create issues' do
expect(Project).to receive(:find_by_id).and_call_original expect(Project).to receive(:find_by_id).and_call_original
...@@ -35,5 +43,54 @@ describe IncidentManagement::ProcessAlertWorker do ...@@ -35,5 +43,54 @@ describe IncidentManagement::ProcessAlertWorker do
subject subject
end end
end end
context 'when alert_management_alert_id is present' do
let!(:alert) { create(:alert_management_alert, project: project) }
let(:alert_management_alert_id) { alert.id }
before do
allow(AlertManagement::Alert)
.to receive(:find_by_id)
.with(alert_management_alert_id)
.and_return(alert)
allow(Gitlab::GitLogger).to receive(:warn).and_call_original
end
context 'when alert can be updated' do
it 'updates AlertManagement::Alert#issue_id' do
expect { subject }.to change { alert.reload.issue_id }.to(new_issue.id)
end
it 'does not write a warning to log' do
subject
expect(Gitlab::GitLogger).not_to have_received(:warn)
end
end
context 'when alert cannot be updated' do
before do
# invalidate alert
too_many_hosts = Array.new(AlertManagement::Alert::HOSTS_MAX_LENGTH + 1) { |_| 'host' }
alert.update_columns(hosts: too_many_hosts)
end
it 'updates AlertManagement::Alert#issue_id' do
expect { subject }.not_to change { alert.reload.issue_id }
end
it 'writes a worning to log' do
subject
expect(Gitlab::GitLogger).to have_received(:warn).with(
message: 'Cannot link an Issue with Alert',
issue_id: new_issue.id,
alert_id: alert_management_alert_id,
alert_errors: { hosts: ['hosts array is over 255 chars'] }
)
end
end
end
end end
end end
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment