From c8003cdfe1597fa6aab21e91aab8cb77097e7eea Mon Sep 17 00:00:00 2001 From: GitLab Bot Date: Sat, 6 Jun 2020 15:08:10 +0000 Subject: [PATCH] Add latest changes from gitlab-org/gitlab@master --- app/models/alert_management/alert.rb | 11 +++ .../chat_message/alert_message.rb | 74 +++++++++++++++++++ app/models/project_services/slack_service.rb | 16 +++- app/models/service.rb | 6 +- .../process_prometheus_alert_service.rb | 5 +- .../projects/alerting/notify_service.rb | 15 +++- ...16326-send-alerts-to-slack-db-settings.yml | 5 ++ ...0526013844_add_alert_events_to_services.rb | 19 +++++ db/structure.sql | 4 +- lib/gitlab/data_builder/alert.rb | 27 +++++++ spec/lib/gitlab/data_builder/alert_spec.rb | 26 +++++++ .../import_export/safe_model_attributes.yml | 1 + .../chat_message/alert_message_spec.rb | 48 ++++++++++++ spec/models/service_spec.rb | 14 ++++ .../process_prometheus_alert_service_spec.rb | 30 ++++++++ .../projects/alerting/notify_service_spec.rb | 30 ++++++++ 16 files changed, 324 insertions(+), 7 deletions(-) create mode 100644 app/models/project_services/chat_message/alert_message.rb create mode 100644 changelogs/unreleased/216326-send-alerts-to-slack-db-settings.yml create mode 100644 db/migrate/20200526013844_add_alert_events_to_services.rb create mode 100644 lib/gitlab/data_builder/alert.rb create mode 100644 spec/lib/gitlab/data_builder/alert_spec.rb create mode 100644 spec/models/project_services/chat_message/alert_message_spec.rb diff --git a/app/models/alert_management/alert.rb b/app/models/alert_management/alert.rb index 35fae370e4a..564ef024c21 100644 --- a/app/models/alert_management/alert.rb +++ b/app/models/alert_management/alert.rb @@ -150,8 +150,19 @@ module AlertManagement '' end + def execute_services + return unless Feature.enabled?(:alert_slack_event, project) + return unless project.has_active_services?(:alert_hooks) + + project.execute_services(hook_data, :alert_hooks) + end + private + def hook_data + Gitlab::DataBuilder::Alert.build(self) + end + def hosts_length return unless hosts diff --git a/app/models/project_services/chat_message/alert_message.rb b/app/models/project_services/chat_message/alert_message.rb new file mode 100644 index 00000000000..bf36f1735dd --- /dev/null +++ b/app/models/project_services/chat_message/alert_message.rb @@ -0,0 +1,74 @@ +# frozen_string_literal: true + +module ChatMessage + class AlertMessage < BaseMessage + attr_reader :title + attr_reader :alert_url + attr_reader :severity + attr_reader :events + attr_reader :status + attr_reader :started_at + + def initialize(params) + @project_name = params[:project_name] || params.dig(:project, :path_with_namespace) + @project_url = params.dig(:project, :web_url) || params[:project_url] + @title = params.dig(:object_attributes, :title) + @alert_url = params.dig(:object_attributes, :url) + @severity = params.dig(:object_attributes, :severity) + @events = params.dig(:object_attributes, :events) + @status = params.dig(:object_attributes, :status) + @started_at = params.dig(:object_attributes, :started_at) + end + + def attachments + [{ + title: title, + title_link: alert_url, + color: attachment_color, + fields: attachment_fields + }] + end + + def message + "Alert firing in #{project_name}" + end + + private + + def attachment_color + "#C95823" + end + + def attachment_fields + [ + { + title: "Severity", + value: severity.to_s.humanize, + short: true + }, + { + title: "Events", + value: events, + short: true + }, + { + title: "Status", + value: status.to_s.humanize, + short: true + }, + { + title: "Start time", + value: format_time(started_at), + short: true + } + ] + end + + # This formats time into the following format + # April 23rd, 2020 1:06AM UTC + def format_time(time) + time = Time.zone.parse(time.to_s) + time.strftime("%B #{time.day.ordinalize}, %Y%l:%M%p %Z") + end + end +end diff --git a/app/models/project_services/slack_service.rb b/app/models/project_services/slack_service.rb index 6d567bb1383..1e7918016ea 100644 --- a/app/models/project_services/slack_service.rb +++ b/app/models/project_services/slack_service.rb @@ -1,6 +1,8 @@ # frozen_string_literal: true class SlackService < ChatNotificationService + prop_accessor EVENT_CHANNEL['alert'] + def title 'Slack notifications' end @@ -21,13 +23,25 @@ class SlackService < ChatNotificationService 'https://hooks.slack.com/services/…' end + def supported_events + additional = [] + additional << 'alert' if Feature.enabled?(:alert_slack_event, project) + + super + additional + end + + def get_message(object_kind, data) + return ChatMessage::AlertMessage.new(data) if object_kind == 'alert' + + super + end + module Notifier private def notify(message, opts) # See https://gitlab.com/gitlab-org/slack-notifier/#custom-http-client notifier = Slack::Messenger.new(webhook, opts.merge(http_client: HTTPClient)) - notifier.ping( message.pretext, attachments: message.attachments, diff --git a/app/models/service.rb b/app/models/service.rb index a2c23947932..6c4050f678e 100644 --- a/app/models/service.rb +++ b/app/models/service.rb @@ -22,6 +22,7 @@ class Service < ApplicationRecord serialize :properties, JSON # rubocop:disable Cop/ActiveRecordSerialize default_value_for :active, false + default_value_for :alert_events, true default_value_for :push_events, true default_value_for :issues_events, true default_value_for :confidential_issues_events, true @@ -72,6 +73,7 @@ class Service < ApplicationRecord scope :pipeline_hooks, -> { where(pipeline_events: true, active: true) } scope :wiki_page_hooks, -> { where(wiki_page_events: true, active: true) } scope :deployment_hooks, -> { where(deployment_events: true, active: true) } + scope :alert_hooks, -> { where(alert_events: true, active: true) } scope :external_issue_trackers, -> { issue_trackers.active.without_defaults } scope :deployment, -> { where(category: 'deployment') } @@ -172,7 +174,7 @@ class Service < ApplicationRecord end def configurable_events - events = self.class.supported_events + events = supported_events # No need to disable individual triggers when there is only one if events.count == 1 @@ -403,6 +405,8 @@ class Service < ApplicationRecord "Event will be triggered when a commit is created/updated" when "deployment" "Event will be triggered when a deployment finishes" + when "alert" + "Event will be triggered when a new, unique alert is recorded" end end diff --git a/app/services/alert_management/process_prometheus_alert_service.rb b/app/services/alert_management/process_prometheus_alert_service.rb index 157cfc995f7..90fcbd95e4b 100644 --- a/app/services/alert_management/process_prometheus_alert_service.rb +++ b/app/services/alert_management/process_prometheus_alert_service.rb @@ -48,7 +48,10 @@ module AlertManagement def create_alert_management_alert am_alert = AlertManagement::Alert.new(am_alert_params.merge(ended_at: nil)) - return if am_alert.save + if am_alert.save + am_alert.execute_services + return + end logger.warn( message: 'Unable to create AlertManagement::Alert', diff --git a/app/services/projects/alerting/notify_service.rb b/app/services/projects/alerting/notify_service.rb index 3598e723429..e01c1ab68aa 100644 --- a/app/services/projects/alerting/notify_service.rb +++ b/app/services/projects/alerting/notify_service.rb @@ -32,15 +32,24 @@ module Projects end def process_alert - if alert = find_alert_by_fingerprint(am_alert_params[:fingerprint]) - alert.register_new_event! + existing_alert = find_alert_by_fingerprint(am_alert_params[:fingerprint]) + + if existing_alert + process_existing_alert(existing_alert) else create_alert end end + def process_existing_alert(alert) + alert.register_new_event! + end + def create_alert - AlertManagement::Alert.create(am_alert_params) + alert = AlertManagement::Alert.create(am_alert_params) + alert.execute_services if alert.persisted? + + alert end def find_alert_by_fingerprint(fingerprint) diff --git a/changelogs/unreleased/216326-send-alerts-to-slack-db-settings.yml b/changelogs/unreleased/216326-send-alerts-to-slack-db-settings.yml new file mode 100644 index 00000000000..ab77ed18444 --- /dev/null +++ b/changelogs/unreleased/216326-send-alerts-to-slack-db-settings.yml @@ -0,0 +1,5 @@ +--- +title: Add column for alert slack notifications +merge_request: 33017 +author: +type: added diff --git a/db/migrate/20200526013844_add_alert_events_to_services.rb b/db/migrate/20200526013844_add_alert_events_to_services.rb new file mode 100644 index 00000000000..1dba7fcaad4 --- /dev/null +++ b/db/migrate/20200526013844_add_alert_events_to_services.rb @@ -0,0 +1,19 @@ +# frozen_string_literal: true + +class AddAlertEventsToServices < ActiveRecord::Migration[6.0] + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + + def up + with_lock_retries do + add_column :services, :alert_events, :boolean + end + end + + def down + with_lock_retries do + remove_column :services, :alert_events + end + end +end diff --git a/db/structure.sql b/db/structure.sql index 20cc636c343..7acc76ab99b 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -6147,7 +6147,8 @@ CREATE TABLE public.services ( template boolean DEFAULT false, instance boolean DEFAULT false NOT NULL, comment_detail smallint, - inherit_from_id bigint + inherit_from_id bigint, + alert_events boolean ); CREATE SEQUENCE public.services_id_seq @@ -13778,6 +13779,7 @@ COPY "schema_migrations" (version) FROM STDIN; 20200525114553 20200525121014 20200526000407 +20200526013844 20200526120714 20200526153844 20200526164946 diff --git a/lib/gitlab/data_builder/alert.rb b/lib/gitlab/data_builder/alert.rb new file mode 100644 index 00000000000..e34bdeea799 --- /dev/null +++ b/lib/gitlab/data_builder/alert.rb @@ -0,0 +1,27 @@ +# frozen_string_literal: true + +module Gitlab + module DataBuilder + module Alert + extend self + + def build(alert) + { + object_kind: 'alert', + object_attributes: hook_attrs(alert) + } + end + + def hook_attrs(alert) + { + title: alert.title, + url: Gitlab::Routing.url_helpers.details_project_alert_management_url(alert.project, alert.iid), + severity: alert.severity, + events: alert.events, + status: alert.status_name, + started_at: alert.started_at + } + end + end + end +end diff --git a/spec/lib/gitlab/data_builder/alert_spec.rb b/spec/lib/gitlab/data_builder/alert_spec.rb new file mode 100644 index 00000000000..b881fb8139b --- /dev/null +++ b/spec/lib/gitlab/data_builder/alert_spec.rb @@ -0,0 +1,26 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Gitlab::DataBuilder::Alert do + let_it_be(:project) { create(:project) } + let_it_be(:alert) { create(:alert_management_alert, project: project) } + + describe '.build' do + let_it_be(:data) { described_class.build(alert) } + + it { expect(data).to be_a(Hash) } + it { expect(data[:object_kind]).to eq('alert') } + + it 'contains the correct object attributes', :aggregate_failures do + object_attributes = data[:object_attributes] + + expect(object_attributes[:title]).to eq(alert.title) + expect(object_attributes[:url]).to eq(Gitlab::Routing.url_helpers.details_project_alert_management_url(project, alert.iid)) + expect(object_attributes[:severity]).to eq(alert.severity) + expect(object_attributes[:events]).to eq(alert.events) + expect(object_attributes[:status]).to eq(alert.status_name) + expect(object_attributes[:started_at]).to eq(alert.started_at) + end + end +end diff --git a/spec/lib/gitlab/import_export/safe_model_attributes.yml b/spec/lib/gitlab/import_export/safe_model_attributes.yml index aae4ba6e025..b6a60dfe759 100644 --- a/spec/lib/gitlab/import_export/safe_model_attributes.yml +++ b/spec/lib/gitlab/import_export/safe_model_attributes.yml @@ -472,6 +472,7 @@ Service: - properties - template - instance +- alert_events - push_events - issues_events - commit_events diff --git a/spec/models/project_services/chat_message/alert_message_spec.rb b/spec/models/project_services/chat_message/alert_message_spec.rb new file mode 100644 index 00000000000..27f8685f44e --- /dev/null +++ b/spec/models/project_services/chat_message/alert_message_spec.rb @@ -0,0 +1,48 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe ChatMessage::AlertMessage do + subject { described_class.new(args) } + + let(:alert) { create(:alert_management_alert) } + + let(:args) do + { + project_name: 'project_name', + project_url: 'http://example.com' + }.merge(Gitlab::DataBuilder::Alert.build(alert)) + end + + describe '#message' do + it 'returns the correct message' do + expect(subject.message).to eq("Alert firing in #{args[:project_name]}") + end + end + + describe '#attachments' do + it 'returns an array of one' do + expect(subject.attachments).to be_a(Array) + expect(subject.attachments.size).to eq(1) + end + + it 'contains the correct attributes' do + attachments_item = subject.attachments.first + expect(attachments_item).to have_key(:title) + expect(attachments_item).to have_key(:title_link) + expect(attachments_item).to have_key(:color) + expect(attachments_item).to have_key(:fields) + end + + it 'returns the correct color' do + expect(subject.attachments.first[:color]).to eq("#C95823") + end + + it 'returns the correct attachment fields' do + attachments_item = subject.attachments.first + fields = attachments_item[:fields].map { |h| h[:title] } + + expect(fields).to match_array(['Severity', 'Events', 'Status', 'Start time']) + end + end +end diff --git a/spec/models/service_spec.rb b/spec/models/service_spec.rb index 6d0fe905aed..5a145b5ea39 100644 --- a/spec/models/service_spec.rb +++ b/spec/models/service_spec.rb @@ -114,6 +114,20 @@ describe Service do expect(described_class.confidential_note_hooks.count).to eq 0 end end + + describe '.alert_hooks' do + it 'includes services where alert_events is true' do + create(:service, active: true, alert_events: true) + + expect(described_class.alert_hooks.count).to eq 1 + end + + it 'excludes services where alert_events is false' do + create(:service, active: true, alert_events: false) + + expect(described_class.alert_hooks.count).to eq 0 + end + end end describe "Test Button" do diff --git a/spec/services/alert_management/process_prometheus_alert_service_spec.rb b/spec/services/alert_management/process_prometheus_alert_service_spec.rb index 32fde5b3167..7fa6fc98d31 100644 --- a/spec/services/alert_management/process_prometheus_alert_service_spec.rb +++ b/spec/services/alert_management/process_prometheus_alert_service_spec.rb @@ -5,6 +5,10 @@ require 'spec_helper' RSpec.describe AlertManagement::ProcessPrometheusAlertService do let_it_be(:project) { create(:project) } + before do + allow(ProjectServiceWorker).to receive(:perform_async) + end + describe '#execute' do subject(:execute) { described_class.new(project, nil, payload).execute } @@ -47,6 +51,12 @@ RSpec.describe AlertManagement::ProcessPrometheusAlertService do end end + it 'does not executes the alert service hooks' do + expect(alert).not_to receive(:execute_services) + + subject + end + context 'when status change did not succeed' do before do allow(AlertManagement::Alert).to receive(:for_fingerprint).and_return([alert]) @@ -72,6 +82,26 @@ RSpec.describe AlertManagement::ProcessPrometheusAlertService do it 'creates a new alert' do 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 + + context 'feature flag disabled' do + before do + stub_feature_flags(alert_slack_event: false) + end + + it 'does not execute the alert service hooks' do + subject + + expect(ProjectServiceWorker).not_to have_received(:perform_async) + end + end end context 'when alert cannot be created' do diff --git a/spec/services/projects/alerting/notify_service_spec.rb b/spec/services/projects/alerting/notify_service_spec.rb index d0e911203f4..9d39585b839 100644 --- a/spec/services/projects/alerting/notify_service_spec.rb +++ b/spec/services/projects/alerting/notify_service_spec.rb @@ -8,12 +8,17 @@ describe Projects::Alerting::NotifyService do before do # We use `let_it_be(:project)` so we make sure to clear caches project.clear_memoization(:licensed_feature_available) + allow(ProjectServiceWorker).to receive(:perform_async) end shared_examples 'processes incident issues' do |amount| let(:create_incident_service) { spy } let(:new_alert) { instance_double(AlertManagement::Alert, id: 503, persisted?: true) } + before do + allow(new_alert).to receive(:execute_services) + end + it 'processes issues' do expect(AlertManagement::Alert) .to receive(:create) @@ -138,6 +143,25 @@ describe Projects::Alerting::NotifyService do ) 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 + + context 'feature flag disabled' do + before do + stub_feature_flags(alert_slack_event: false) + end + + it 'does not executes the alert service hooks' do + subject + + expect(ProjectServiceWorker).not_to have_received(:perform_async) + end + end + context 'existing alert with same fingerprint' do let!(:existing_alert) { create(:alert_management_alert, project: project, fingerprint: Digest::SHA1.hexdigest(fingerprint)) } @@ -148,6 +172,12 @@ describe Projects::Alerting::NotifyService do it 'increments the existing alert count' do expect { subject }.to change { existing_alert.reload.events }.from(1).to(2) end + + it 'does not executes the alert service hooks' do + subject + + expect(ProjectServiceWorker).not_to have_received(:perform_async) + end end context 'with a minimal payload' do