Commit c95a757b authored by Sean Arnold's avatar Sean Arnold Committed by Amy Qualls

Open new alert when existing alert is resolved

- Add specs to cover Prometheus and generic alerts
parent 31a1c997
...@@ -57,7 +57,7 @@ module AlertManagement ...@@ -57,7 +57,7 @@ module AlertManagement
validates :started_at, presence: true validates :started_at, presence: true
validates :fingerprint, allow_blank: true, uniqueness: { validates :fingerprint, allow_blank: true, uniqueness: {
scope: :project, scope: :project,
conditions: -> { where.not(status: STATUSES[:resolved]) }, conditions: -> { not_resolved },
message: -> (object, data) { _('Cannot have multiple unresolved alerts') } message: -> (object, data) { _('Cannot have multiple unresolved alerts') }
}, unless: :resolved? }, unless: :resolved?
validate :hosts_length validate :hosts_length
...@@ -120,6 +120,7 @@ module AlertManagement ...@@ -120,6 +120,7 @@ module AlertManagement
scope :for_environment, -> (environment) { where(environment: environment) } scope :for_environment, -> (environment) { where(environment: environment) }
scope :search, -> (query) { fuzzy_search(query, [:title, :description, :monitoring_tool, :service]) } scope :search, -> (query) { fuzzy_search(query, [:title, :description, :monitoring_tool, :service]) }
scope :open, -> { with_status(:triggered, :acknowledged) } scope :open, -> { with_status(:triggered, :acknowledged) }
scope :not_resolved, -> { where.not(status: STATUSES[:resolved]) }
scope :with_prometheus_alert, -> { includes(:prometheus_alert) } scope :with_prometheus_alert, -> { includes(:prometheus_alert) }
scope :order_start_time, -> (sort_order) { order(started_at: sort_order) } scope :order_start_time, -> (sort_order) { order(started_at: sort_order) }
......
...@@ -94,7 +94,7 @@ module AlertManagement ...@@ -94,7 +94,7 @@ module AlertManagement
end end
def am_alert def am_alert
@am_alert ||= AlertManagement::Alert.for_fingerprint(project, gitlab_fingerprint).first @am_alert ||= AlertManagement::Alert.not_resolved.for_fingerprint(project, gitlab_fingerprint).first
end end
def bad_request def bad_request
......
...@@ -55,7 +55,7 @@ module Projects ...@@ -55,7 +55,7 @@ module Projects
def find_alert_by_fingerprint(fingerprint) def find_alert_by_fingerprint(fingerprint)
return unless fingerprint return unless fingerprint
AlertManagement::Alert.for_fingerprint(project, fingerprint).first AlertManagement::Alert.not_resolved.for_fingerprint(project, fingerprint).first
end end
def send_email? def send_email?
......
---
title: Open new alert when existing alert is resolved
merge_request: 36261
author:
type: added
...@@ -101,4 +101,6 @@ displays a counter on the ...@@ -101,4 +101,6 @@ displays a counter on the
[Alert Management List](../operations/alert_management.md#alert-management-list) [Alert Management List](../operations/alert_management.md#alert-management-list)
and details pages. and details pages.
If the existing alert is already `resolved`, then a new alert will be created instead.
![Alert Management List](../operations/img/alert_list_v13_1.png) ![Alert Management List](../operations/img/alert_list_v13_1.png)
...@@ -235,6 +235,14 @@ RSpec.describe AlertManagement::Alert do ...@@ -235,6 +235,14 @@ RSpec.describe AlertManagement::Alert do
it { is_expected.to contain_exactly(acknowledged_alert, triggered_alert) } it { is_expected.to contain_exactly(acknowledged_alert, triggered_alert) }
end end
describe '.not_resolved' do
subject { described_class.not_resolved }
let!(:acknowledged_alert) { create(:alert_management_alert, :acknowledged, project: project) }
it { is_expected.to contain_exactly(acknowledged_alert, triggered_alert, ignored_alert) }
end
end end
describe '.last_prometheus_alert_by_project_id' do describe '.last_prometheus_alert_by_project_id' do
......
...@@ -39,22 +39,27 @@ RSpec.describe AlertManagement::ProcessPrometheusAlertService do ...@@ -39,22 +39,27 @@ RSpec.describe AlertManagement::ProcessPrometheusAlertService do
context 'when Prometheus alert status is firing' do context 'when Prometheus alert status is firing' do
context 'when alert with the same fingerprint already exists' do context 'when alert with the same fingerprint already exists' do
let!(:alert) { create(:alert_management_alert, project: project, fingerprint: parsed_alert.gitlab_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: parsed_alert.gitlab_fingerprint) }
it 'increases alert events count' do it_behaves_like 'creates an alert management alert'
expect { execute }.to change { alert.reload.events }.by(1)
end end
context 'when status can be changed' do context 'existing alert is ignored' do
it 'changes status to triggered' do let!(:alert) { create(:alert_management_alert, :ignored, project: project, fingerprint: parsed_alert.gitlab_fingerprint) }
expect { execute }.to change { alert.reload.triggered? }.to(true)
end it_behaves_like 'adds an alert management alert event'
end end
it 'does not executes the alert service hooks' do context 'two existing alerts, one resolved one open' do
expect(alert).not_to receive(:execute_services) 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) }
subject it_behaves_like 'adds an alert management alert event'
end end
context 'when status change did not succeed' do context 'when status change did not succeed' do
...@@ -73,23 +78,11 @@ RSpec.describe AlertManagement::ProcessPrometheusAlertService do ...@@ -73,23 +78,11 @@ RSpec.describe AlertManagement::ProcessPrometheusAlertService do
execute execute
end end
end end
it { is_expected.to be_success }
end end
context 'when alert does not exist' do context 'when alert does not exist' do
context 'when alert can be created' do context 'when alert can be created' do
it 'creates a new alert' do it_behaves_like 'creates an alert management alert'
expect { execute }.to change { AlertManagement::Alert.where(project: project).count }.by(1)
end
it 'executes the alert service hooks' do
slack_service = create(:service, type: 'SlackService', project: project, alert_events: true, active: true)
subject
expect(ProjectServiceWorker).to have_received(:perform_async).with(slack_service.id, an_instance_of(Hash))
end
end end
context 'when alert cannot be created' do context 'when alert cannot be created' do
......
...@@ -64,12 +64,6 @@ RSpec.describe Projects::Alerting::NotifyService do ...@@ -64,12 +64,6 @@ RSpec.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.current.change(usec: 0) } let(:starts_at) { Time.current.change(usec: 0) }
...@@ -107,17 +101,8 @@ RSpec.describe Projects::Alerting::NotifyService do ...@@ -107,17 +101,8 @@ RSpec.describe Projects::Alerting::NotifyService do
end end
context 'with valid payload' do context 'with valid payload' do
let(:last_alert_attributes) do shared_examples 'assigns the alert properties' do
AlertManagement::Alert.last.attributes it 'ensure that created alert has all data properly assigned' do
.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
it 'created alert has all data properly assigned' do
subject subject
expect(last_alert_attributes).to match( expect(last_alert_attributes).to match(
...@@ -139,30 +124,41 @@ RSpec.describe Projects::Alerting::NotifyService do ...@@ -139,30 +124,41 @@ RSpec.describe Projects::Alerting::NotifyService do
environment_id: nil environment_id: nil
) )
end end
end
it 'executes the alert service hooks' do let(:last_alert_attributes) do
slack_service = create(:service, type: 'SlackService', project: project, alert_events: true, active: true) AlertManagement::Alert.last.attributes
subject .except('id', 'iid', 'created_at', 'updated_at')
.with_indifferent_access
expect(ProjectServiceWorker).to have_received(:perform_async).with(slack_service.id, an_instance_of(Hash))
end end
it_behaves_like 'creates an alert management alert'
it_behaves_like 'assigns the alert properties'
context 'existing alert with same fingerprint' do context 'existing alert with same fingerprint' do
let(:fingerprint_sha) { Digest::SHA1.hexdigest(fingerprint) } let(:fingerprint_sha) { Digest::SHA1.hexdigest(fingerprint) }
let!(:existing_alert) { create(:alert_management_alert, project: project, fingerprint: fingerprint_sha) } let!(:alert) { create(:alert_management_alert, project: project, fingerprint: fingerprint_sha) }
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: fingerprint_sha) }
it 'does not create AlertManagement::Alert' do it_behaves_like 'creates an alert management alert'
expect { subject }.not_to change(AlertManagement::Alert, :count) it_behaves_like 'assigns the alert properties'
end end
it 'increments the existing alert count' do context 'existing alert is ignored' do
expect { subject }.to change { existing_alert.reload.events }.from(1).to(2) let!(:alert) { create(:alert_management_alert, :ignored, project: project, fingerprint: fingerprint_sha) }
it_behaves_like 'adds an alert management alert event'
end end
it 'does not executes the alert service hooks' do context 'two existing alerts, one resolved one open' do
subject let!(:resolved_existing_alert) { create(:alert_management_alert, :resolved, project: project, fingerprint: fingerprint_sha) }
let!(:alert) { create(:alert_management_alert, project: project, fingerprint: fingerprint_sha) }
expect(ProjectServiceWorker).not_to have_received(:perform_async) it_behaves_like 'adds an alert management alert event'
end end
end end
...@@ -174,9 +170,7 @@ RSpec.describe Projects::Alerting::NotifyService do ...@@ -174,9 +170,7 @@ RSpec.describe Projects::Alerting::NotifyService do
} }
end end
it 'creates AlertManagement::Alert' do it_behaves_like 'creates an alert management alert'
expect { subject }.to change(AlertManagement::Alert, :count).by(1)
end
it 'created alert has all data properly assigned' do it 'created alert has all data properly assigned' do
subject subject
...@@ -218,19 +212,19 @@ RSpec.describe Projects::Alerting::NotifyService do ...@@ -218,19 +212,19 @@ RSpec.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' it_behaves_like 'does not an create alert management alert'
end end
context 'when alert already exists' do context 'when alert already exists' do
let(:fingerprint_sha) { Digest::SHA1.hexdigest(fingerprint) } let(:fingerprint_sha) { Digest::SHA1.hexdigest(fingerprint) }
let!(:existing_alert) { create(:alert_management_alert, project: project, fingerprint: fingerprint_sha) } let!(:alert) { create(:alert_management_alert, project: project, fingerprint: fingerprint_sha) }
context 'when existing alert does not have an associated issue' do context 'when existing alert does not have an associated issue' do
it_behaves_like 'processes incident issues' it_behaves_like 'processes incident issues'
end end
context 'when existing alert has an associated issue' do context 'when existing alert has an associated issue' do
let!(:existing_alert) { create(:alert_management_alert, :with_issue, project: project, fingerprint: fingerprint_sha) } let!(:alert) { create(:alert_management_alert, :with_issue, project: project, fingerprint: fingerprint_sha) }
it_behaves_like 'does not process incident issues' it_behaves_like 'does not process incident issues'
end end
...@@ -246,14 +240,14 @@ RSpec.describe Projects::Alerting::NotifyService do ...@@ -246,14 +240,14 @@ RSpec.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' it_behaves_like 'does not an create alert management 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' it_behaves_like 'does not an create alert management alert'
end end
end end
end end
......
# frozen_string_literal: true
RSpec.shared_examples 'creates an alert management alert' do
it { is_expected.to be_success }
it 'creates AlertManagement::Alert' do
expect { subject }.to change(AlertManagement::Alert, :count).by(1)
end
it 'executes the alert service hooks' do
slack_service = create(:service, type: 'SlackService', project: project, alert_events: true, active: true)
subject
expect(ProjectServiceWorker).to have_received(:perform_async).with(slack_service.id, an_instance_of(Hash))
end
end
RSpec.shared_examples 'does not an create alert management alert' do
it 'does not create alert' do
expect { subject }.not_to change(AlertManagement::Alert, :count)
end
end
RSpec.shared_examples 'adds an alert management alert event' do
it { is_expected.to be_success }
it 'does not create an alert' do
expect { subject }.not_to change(AlertManagement::Alert, :count)
end
it 'increases alert events count' do
expect { subject }.to change { alert.reload.events }.by(1)
end
it 'does not executes the alert service hooks' do
expect(alert).not_to receive(:execute_services)
subject
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