Commit b4570697 authored by Heinrich Lee Yu's avatar Heinrich Lee Yu

Merge branch 'sy-adhere-issue-creation-to-setting' into 'master'

Only create issues if supposed to for Prometheus alerts

See merge request gitlab-org/gitlab!41468
parents 2ed92862 39cf8376
...@@ -31,7 +31,7 @@ module AlertManagement ...@@ -31,7 +31,7 @@ module AlertManagement
create_alert_management_alert create_alert_management_alert
end end
process_incident_alert process_incident_issues if process_issues?
end end
def reset_alert_management_alert_status def reset_alert_management_alert_status
...@@ -84,7 +84,7 @@ module AlertManagement ...@@ -84,7 +84,7 @@ module AlertManagement
SystemNoteService.auto_resolve_prometheus_alert(issue, project, User.alert_bot) if issue.reset.closed? SystemNoteService.auto_resolve_prometheus_alert(issue, project, User.alert_bot) if issue.reset.closed?
end end
def process_incident_alert def process_incident_issues
return unless alert.persisted? return unless alert.persisted?
return if alert.issue return if alert.issue
......
---
title: Only create issues if supposed to for Prometheus alerts
merge_request: 41468
author:
type: fixed
...@@ -10,7 +10,18 @@ RSpec.describe AlertManagement::ProcessPrometheusAlertService do ...@@ -10,7 +10,18 @@ RSpec.describe AlertManagement::ProcessPrometheusAlertService do
end end
describe '#execute' do describe '#execute' do
subject(:execute) { described_class.new(project, nil, payload).execute } let(:service) { described_class.new(project, nil, payload) }
let(:incident_management_setting) { double(auto_close_incident?: auto_close_incident, create_issue?: create_issue) }
let(:auto_close_incident) { true }
let(:create_issue) { true }
before do
allow(service)
.to receive(:incident_management_setting)
.and_return(incident_management_setting)
end
subject(:execute) { service.execute }
context 'when alert payload is valid' do 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: 'Prometheus') }
...@@ -43,6 +54,7 @@ RSpec.describe AlertManagement::ProcessPrometheusAlertService do ...@@ -43,6 +54,7 @@ RSpec.describe AlertManagement::ProcessPrometheusAlertService do
let!(:alert) { create(:alert_management_alert, project: project, fingerprint: fingerprint) } let!(:alert) { create(:alert_management_alert, project: project, fingerprint: fingerprint) }
it_behaves_like 'adds an alert management alert event' it_behaves_like 'adds an alert management alert event'
it_behaves_like 'processes incident issues'
context 'existing alert is resolved' do context 'existing alert is resolved' do
let!(:alert) { create(:alert_management_alert, :resolved, project: project, fingerprint: fingerprint) } let!(:alert) { create(:alert_management_alert, :resolved, project: project, fingerprint: fingerprint) }
...@@ -79,6 +91,12 @@ RSpec.describe AlertManagement::ProcessPrometheusAlertService do ...@@ -79,6 +91,12 @@ RSpec.describe AlertManagement::ProcessPrometheusAlertService do
execute execute
end end
end end
context 'when auto-alert creation is disabled' do
let(:create_issue) { false }
it_behaves_like 'does not process incident issues'
end
end end
context 'when alert does not exist' do context 'when alert does not exist' do
...@@ -89,13 +107,12 @@ RSpec.describe AlertManagement::ProcessPrometheusAlertService do ...@@ -89,13 +107,12 @@ RSpec.describe AlertManagement::ProcessPrometheusAlertService do
expect { subject }.to change(Note, :count).by(1) expect { subject }.to change(Note, :count).by(1)
end end
it 'processes the incident alert' do it_behaves_like 'processes incident issues'
expect(IncidentManagement::ProcessAlertWorker)
.to receive(:perform_async)
.with(nil, nil, kind_of(Integer))
.once
expect(subject).to be_success context 'when auto-alert creation is disabled' do
let(:create_issue) { false }
it_behaves_like 'does not process incident issues'
end end
end end
...@@ -114,12 +131,7 @@ RSpec.describe AlertManagement::ProcessPrometheusAlertService do ...@@ -114,12 +131,7 @@ RSpec.describe AlertManagement::ProcessPrometheusAlertService do
execute execute
end end
it 'does not create incident issue' do it_behaves_like 'does not process incident issues'
expect(IncidentManagement::ProcessAlertWorker)
.not_to receive(:perform_async)
expect(subject).to be_success
end
end end
it { is_expected.to be_success } it { is_expected.to be_success }
...@@ -131,8 +143,6 @@ RSpec.describe AlertManagement::ProcessPrometheusAlertService do ...@@ -131,8 +143,6 @@ RSpec.describe AlertManagement::ProcessPrometheusAlertService do
let!(:alert) { create(:alert_management_alert, project: project, fingerprint: fingerprint) } let!(:alert) { create(:alert_management_alert, project: project, fingerprint: fingerprint) }
context 'when auto_resolve_incident set to true' do 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) }
context 'when status can be changed' do context 'when status can be changed' do
it 'resolves an existing alert' do it 'resolves an existing alert' do
expect { execute }.to change { alert.reload.resolved? }.to(true) expect { execute }.to change { alert.reload.resolved? }.to(true)
...@@ -185,7 +195,7 @@ RSpec.describe AlertManagement::ProcessPrometheusAlertService do ...@@ -185,7 +195,7 @@ RSpec.describe AlertManagement::ProcessPrometheusAlertService do
end end
context 'when auto_resolve_incident set to false' do context 'when auto_resolve_incident set to false' do
let_it_be(:operations_settings) { create(:project_incident_management_setting, project: project, auto_close_incident: false) } let(:auto_close_incident) { false }
it 'does not resolve an existing alert' do it 'does not resolve an existing alert' do
expect { execute }.not_to change { alert.reload.resolved? } expect { execute }.not_to change { alert.reload.resolved? }
......
...@@ -9,44 +9,6 @@ RSpec.describe Projects::Alerting::NotifyService do ...@@ -9,44 +9,6 @@ RSpec.describe Projects::Alerting::NotifyService do
allow(ProjectServiceWorker).to receive(:perform_async) allow(ProjectServiceWorker).to receive(:perform_async)
end end
shared_examples 'processes incident issues' do
let(:create_incident_service) { spy }
before do
allow_any_instance_of(AlertManagement::Alert).to receive(:execute_services)
end
it 'processes issues' do
expect(IncidentManagement::ProcessAlertWorker)
.to receive(:perform_async)
.with(nil, nil, kind_of(Integer))
.once
Sidekiq::Testing.inline! do
expect(subject).to be_success
end
end
end
shared_examples 'does not process incident issues' do
it 'does not process issues' do
expect(IncidentManagement::ProcessAlertWorker)
.not_to receive(:perform_async)
expect(subject).to be_success
end
end
shared_examples 'does not process incident issues due to error' do |http_status:|
it 'does not process issues' do
expect(IncidentManagement::ProcessAlertWorker)
.not_to receive(:perform_async)
expect(subject).to be_error
expect(subject.http_status).to eq(http_status)
end
end
describe '#execute' do describe '#execute' do
let(:token) { 'invalid-token' } let(:token) { 'invalid-token' }
let(:starts_at) { Time.current.change(usec: 0) } let(:starts_at) { Time.current.change(usec: 0) }
......
...@@ -39,3 +39,41 @@ RSpec.shared_examples 'adds an alert management alert event' do ...@@ -39,3 +39,41 @@ RSpec.shared_examples 'adds an alert management alert event' do
subject subject
end end
end end
RSpec.shared_examples 'processes incident issues' do
let(:create_incident_service) { spy }
before do
allow_any_instance_of(AlertManagement::Alert).to receive(:execute_services)
end
it 'processes issues' do
expect(IncidentManagement::ProcessAlertWorker)
.to receive(:perform_async)
.with(nil, nil, kind_of(Integer))
.once
Sidekiq::Testing.inline! do
expect(subject).to be_success
end
end
end
RSpec.shared_examples 'does not process incident issues' do
it 'does not process issues' do
expect(IncidentManagement::ProcessAlertWorker)
.not_to receive(:perform_async)
expect(subject).to be_success
end
end
RSpec.shared_examples 'does not process incident issues due to error' do |http_status:|
it 'does not process issues' do
expect(IncidentManagement::ProcessAlertWorker)
.not_to receive(:perform_async)
expect(subject).to be_error
expect(subject.http_status).to eq(http_status)
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