diff --git a/app/controllers/projects/prometheus/metrics_controller.rb b/app/controllers/projects/prometheus/metrics_controller.rb
index e61d357ce4eba1ec2bd45ec6c766e0fe842c7165..c5778ba15f23260e3394d95ae8509240f71bda64 100644
--- a/app/controllers/projects/prometheus/metrics_controller.rb
+++ b/app/controllers/projects/prometheus/metrics_controller.rb
@@ -74,9 +74,9 @@ def create
end
def update
- @metric = update_metrics_service(prometheus_metric).execute
+ @metric = prometheus_metric
- if @metric.persisted?
+ if @metric.update(metrics_params)
redirect_to edit_project_integration_path(project, ::Integrations::Prometheus),
notice: _('Metric was successfully updated.')
else
diff --git a/app/models/integrations/prometheus.rb b/app/models/integrations/prometheus.rb
index 5a30f55595b4c91275e94750698daf22a1ccddc7..427034edb791ad59822d585c7ffea68995688aa6 100644
--- a/app/models/integrations/prometheus.rb
+++ b/app/models/integrations/prometheus.rb
@@ -27,8 +27,6 @@ class Prometheus < BaseMonitoring
after_commit :track_events
- after_create_commit :create_default_alerts
-
scope :preload_project, -> { preload(:project) }
def show_active_box?
@@ -168,12 +166,6 @@ def disabled_manual_prometheus?
manual_configuration_changed? && !manual_configuration?
end
- def create_default_alerts
- return unless project_id
-
- ::Prometheus::CreateDefaultAlertsWorker.perform_async(project_id)
- end
-
def behind_iap?
manual_configuration? && google_iap_audience_client_id.present? && google_iap_service_account_json.present?
end
diff --git a/app/services/projects/prometheus/metrics/base_service.rb b/app/services/projects/prometheus/metrics/base_service.rb
index be1783dde705719e2a5c4ffef190f41ed99ed33c..15247d4577611dad739b8b95e0655dfcc55413c5 100644
--- a/app/services/projects/prometheus/metrics/base_service.rb
+++ b/app/services/projects/prometheus/metrics/base_service.rb
@@ -8,40 +8,12 @@ class BaseService
def initialize(metric, params = {})
@metric = metric
- @project = metric.project
@params = params.dup
end
protected
- attr_reader :metric, :project, :params
-
- def application
- alert.environment.cluster_prometheus_adapter
- end
-
- def schedule_alert_update
- return unless alert
- return unless alert.environment
-
- ::Clusters::Applications::ScheduleUpdateService.new(
- alert.environment.cluster_prometheus_adapter, project).execute
- end
-
- def alert
- strong_memoize(:alert) { find_alert(metric) }
- end
-
- def find_alert(metric)
- Projects::Prometheus::AlertsFinder
- .new(project: project, metric: metric)
- .execute
- .first
- end
-
- def has_alert?
- alert.present?
- end
+ attr_reader :metric, :params
end
end
end
diff --git a/app/services/projects/prometheus/metrics/destroy_service.rb b/app/services/projects/prometheus/metrics/destroy_service.rb
index 6a46eb5516ca7bfe1f16be6efd779090ffa79a4c..d85499dc4ae20ed698ca176adc2321ce831c211e 100644
--- a/app/services/projects/prometheus/metrics/destroy_service.rb
+++ b/app/services/projects/prometheus/metrics/destroy_service.rb
@@ -5,7 +5,6 @@ module Prometheus
module Metrics
class DestroyService < Metrics::BaseService
def execute
- schedule_alert_update if has_alert?
metric.destroy
end
end
diff --git a/app/services/projects/prometheus/metrics/update_service.rb b/app/services/projects/prometheus/metrics/update_service.rb
deleted file mode 100644
index 9b51f4ab47db94e0861b49bfadee1d70b1b7945f..0000000000000000000000000000000000000000
--- a/app/services/projects/prometheus/metrics/update_service.rb
+++ /dev/null
@@ -1,29 +0,0 @@
-# frozen_string_literal: true
-
-module Projects
- module Prometheus
- module Metrics
- class UpdateService < Metrics::BaseService
- def execute
- metric.update!(params)
- schedule_alert_update if requires_alert_update?
- metric
- end
-
- private
-
- def requires_alert_update?
- has_alert? && (changing_title? || changing_query?)
- end
-
- def changing_title?
- metric.previous_changes.include?(:title)
- end
-
- def changing_query?
- metric.previous_changes.include?(:query)
- end
- end
- end
- end
-end
diff --git a/app/services/prometheus/create_default_alerts_service.rb b/app/services/prometheus/create_default_alerts_service.rb
deleted file mode 100644
index eb8a9d456589e6ecfdbcd3612eee8839daa57b30..0000000000000000000000000000000000000000
--- a/app/services/prometheus/create_default_alerts_service.rb
+++ /dev/null
@@ -1,105 +0,0 @@
-# frozen_string_literal: true
-
-# DEPRECATED: To be removed as part of https://gitlab.com/groups/gitlab-org/-/epics/5877
-module Prometheus
- class CreateDefaultAlertsService < BaseService
- include Gitlab::Utils::StrongMemoize
-
- attr_reader :project
-
- DEFAULT_ALERTS = [
- {
- identifier: 'response_metrics_nginx_ingress_16_http_error_rate',
- operator: 'gt',
- threshold: 0.1
- },
- {
- identifier: 'response_metrics_nginx_ingress_http_error_rate',
- operator: 'gt',
- threshold: 0.1
- },
- {
- identifier: 'response_metrics_nginx_http_error_percentage',
- operator: 'gt',
- threshold: 0.1
- }
- ].freeze
-
- def initialize(project:)
- @project = project
- end
-
- def execute
- return ServiceResponse.error(message: 'Invalid project') unless project
- return ServiceResponse.error(message: 'Invalid environment') unless environment
-
- create_alerts
- schedule_prometheus_update
-
- ServiceResponse.success
- end
-
- private
-
- def create_alerts
- DEFAULT_ALERTS.each do |alert_hash|
- identifier = alert_hash[:identifier]
- next if alerts_by_identifier(environment).key?(identifier)
-
- metric = metrics_by_identifier[identifier]
- next unless metric
-
- create_alert(alert: alert_hash, metric: metric)
- end
- end
-
- def schedule_prometheus_update
- return unless prometheus_adapter
-
- ::Clusters::Applications::ScheduleUpdateService.new(prometheus_adapter, project).execute
- end
-
- def prometheus_adapter
- environment.cluster_prometheus_adapter
- end
-
- def metrics_by_identifier
- strong_memoize(:metrics_by_identifier) do
- metric_identifiers = DEFAULT_ALERTS.map { |alert| alert[:identifier] }
-
- PrometheusMetricsFinder
- .new(identifier: metric_identifiers, common: true)
- .execute
- .index_by(&:identifier)
- end
- end
-
- def alerts_by_identifier(environment)
- strong_memoize(:alerts_by_identifier) do
- Projects::Prometheus::AlertsFinder
- .new(project: project, metric: metrics_by_identifier.values, environment: environment)
- .execute
- .index_by { |alert| alert.prometheus_metric.identifier }
- end
- end
-
- def environment
- strong_memoize(:environment) do
- Environments::EnvironmentsFinder.new(project, nil, name: 'production').execute.first ||
- project.environments.first
- end
- end
-
- def create_alert(alert:, metric:)
- PrometheusAlert.create!(
- project: project,
- prometheus_metric: metric,
- environment: environment,
- threshold: alert[:threshold],
- operator: alert[:operator]
- )
- rescue ActiveRecord::RecordNotUnique
- # Ignore duplicate creations although it unlikely to happen
- end
- end
-end
diff --git a/app/workers/prometheus/create_default_alerts_worker.rb b/app/workers/prometheus/create_default_alerts_worker.rb
index 94ac02c4c044ef76ca7a32aeb21887b7076f6853..1a0fe7e8d561431e3097e9248d6acdb16e836d72 100644
--- a/app/workers/prometheus/create_default_alerts_worker.rb
+++ b/app/workers/prometheus/create_default_alerts_worker.rb
@@ -13,19 +13,7 @@ class CreateDefaultAlertsWorker
idempotent!
def perform(project_id)
- project = Project.find_by_id(project_id)
-
- return unless project
-
- result = ::Prometheus::CreateDefaultAlertsService.new(project: project).execute
-
- log_info(result.message) if result.error?
- end
-
- private
-
- def log_info(message)
- logger.info(structured_payload(message: message))
+ # No-op Will be removed in https://gitlab.com/gitlab-org/gitlab/-/issues/360756
end
end
end
diff --git a/lib/gitlab/metrics/dashboard/importers/prometheus_metrics.rb b/lib/gitlab/metrics/dashboard/importers/prometheus_metrics.rb
index 8a176be30a2f834b274a300bab5daba782b69017..e2b43798b22d928c10be80c9ee5b65d9e87d02a0 100644
--- a/lib/gitlab/metrics/dashboard/importers/prometheus_metrics.rb
+++ b/lib/gitlab/metrics/dashboard/importers/prometheus_metrics.rb
@@ -33,7 +33,6 @@ def execute!
def import
delete_stale_metrics
create_or_update_metrics
- update_prometheus_environments
end
# rubocop: disable CodeReuse/ActiveRecord
@@ -47,8 +46,6 @@ def create_or_update_metrics
affected_metric_ids << prometheus_metric.id
end
-
- @affected_environment_ids += find_alerts(affected_metric_ids).get_environment_id
end
# rubocop: enable CodeReuse/ActiveRecord
@@ -62,24 +59,9 @@ def delete_stale_metrics
return unless stale_metrics.exists?
- delete_stale_alerts(stale_metrics)
stale_metrics.each_batch { |batch| batch.delete_all }
end
- def delete_stale_alerts(stale_metrics)
- stale_alerts = find_alerts(stale_metrics)
-
- affected_environment_ids = stale_alerts.get_environment_id
- return unless affected_environment_ids.present?
-
- @affected_environment_ids += affected_environment_ids
- stale_alerts.each_batch { |batch| batch.delete_all }
- end
-
- def find_alerts(metrics)
- Projects::Prometheus::AlertsFinder.new(project: project, metric: metrics).execute
- end
-
def prometheus_metrics_attributes
@prometheus_metrics_attributes ||= begin
Dashboard::Transformers::Yml::V1::PrometheusMetrics.new(
@@ -89,19 +71,6 @@ def prometheus_metrics_attributes
).execute
end
end
-
- def update_prometheus_environments
- affected_environments = ::Environment.for_id(@affected_environment_ids.flatten.uniq).for_project(project)
-
- return unless affected_environments.exists?
-
- affected_environments.each do |affected_environment|
- ::Clusters::Applications::ScheduleUpdateService.new(
- affected_environment.cluster_prometheus_adapter,
- project
- ).execute
- end
- end
end
end
end
diff --git a/spec/lib/gitlab/metrics/dashboard/importers/prometheus_metrics_spec.rb b/spec/lib/gitlab/metrics/dashboard/importers/prometheus_metrics_spec.rb
index ff8f5797f9da18c7bd4edc3e411f17057e877872..c15e717b126ee6ce996ea25e2e95189e721b173e 100644
--- a/spec/lib/gitlab/metrics/dashboard/importers/prometheus_metrics_spec.rb
+++ b/spec/lib/gitlab/metrics/dashboard/importers/prometheus_metrics_spec.rb
@@ -12,12 +12,6 @@
subject { described_class.new(dashboard_hash, project: project, dashboard_path: dashboard_path) }
- before do
- allow_next_instance_of(::Clusters::Applications::ScheduleUpdateService) do |update_service|
- allow(update_service).to receive(:execute)
- end
- end
-
context 'valid dashboard' do
let(:dashboard_hash) { load_sample_dashboard }
@@ -45,13 +39,6 @@
create(:prometheus_metric, existing_metric_attributes)
end
- let!(:existing_alert) do
- alert = create(:prometheus_alert, project: project, prometheus_metric: existing_metric)
- existing_metric.prometheus_alerts << alert
-
- alert
- end
-
it 'updates existing PrometheusMetrics' do
subject.execute
@@ -68,15 +55,6 @@
expect { subject.execute }.to change { PrometheusMetric.count }.by(2)
end
- it 'updates affected environments' do
- expect(::Clusters::Applications::ScheduleUpdateService).to receive(:new).with(
- existing_alert.environment.cluster_prometheus_adapter,
- project
- ).and_return(double('ScheduleUpdateService', execute: true))
-
- subject.execute
- end
-
context 'with stale metrics' do
let!(:stale_metric) do
create(:prometheus_metric,
@@ -87,13 +65,6 @@
)
end
- let!(:stale_alert) do
- alert = create(:prometheus_alert, project: project, prometheus_metric: stale_metric)
- stale_metric.prometheus_alerts << alert
-
- alert
- end
-
it 'updates existing PrometheusMetrics' do
subject.execute
@@ -111,21 +82,6 @@
expect { stale_metric.reload }.to raise_error(ActiveRecord::RecordNotFound)
end
-
- it 'deletes stale alert' do
- subject.execute
-
- expect { stale_alert.reload }.to raise_error(ActiveRecord::RecordNotFound)
- end
-
- it 'updates affected environments' do
- expect(::Clusters::Applications::ScheduleUpdateService).to receive(:new).with(
- existing_alert.environment.cluster_prometheus_adapter,
- project
- ).and_return(double('ScheduleUpdateService', execute: true))
-
- subject.execute
- end
end
end
end
diff --git a/spec/models/integrations/prometheus_spec.rb b/spec/models/integrations/prometheus_spec.rb
index 76e20f20a00d19b76d2f71343e2c081e8ed5577c..a7495cb957458fd64e1a813101a3092104186d5b 100644
--- a/spec/models/integrations/prometheus_spec.rb
+++ b/spec/models/integrations/prometheus_spec.rb
@@ -122,34 +122,6 @@
end
end
- describe 'callbacks' do
- context 'after_create' do
- let(:project) { create(:project) }
- let(:integration) { build(:prometheus_integration, project: project) }
-
- subject(:create_integration) { integration.save! }
-
- it 'creates default alerts' do
- expect(Prometheus::CreateDefaultAlertsWorker)
- .to receive(:perform_async)
- .with(project.id)
-
- create_integration
- end
-
- context 'no project exists' do
- let(:integration) { build(:prometheus_integration, :instance) }
-
- it 'does not create default alerts' do
- expect(Prometheus::CreateDefaultAlertsWorker)
- .not_to receive(:perform_async)
-
- create_integration
- end
- end
- end
- end
-
describe '#test' do
before do
integration.manual_configuration = true
diff --git a/spec/services/projects/prometheus/metrics/destroy_service_spec.rb b/spec/services/projects/prometheus/metrics/destroy_service_spec.rb
index 17cc88b27b604dc04b18d7274b40d140ff4b98f2..b4af81f2c873bfca6d3b70a13f609faa9afcb5f7 100644
--- a/spec/services/projects/prometheus/metrics/destroy_service_spec.rb
+++ b/spec/services/projects/prometheus/metrics/destroy_service_spec.rb
@@ -12,17 +12,4 @@
expect(PrometheusMetric.find_by(id: metric.id)).to be_nil
end
-
- context 'when metric has a prometheus alert associated' do
- it 'schedules a prometheus alert update' do
- create(:prometheus_alert, project: metric.project, prometheus_metric: metric)
-
- schedule_update_service = spy
- allow(::Clusters::Applications::ScheduleUpdateService).to receive(:new).and_return(schedule_update_service)
-
- subject.execute
-
- expect(schedule_update_service).to have_received(:execute)
- end
- end
end
diff --git a/spec/services/projects/prometheus/metrics/update_service_spec.rb b/spec/services/projects/prometheus/metrics/update_service_spec.rb
deleted file mode 100644
index bf87093150c2419da11d26d9abb4cfa0579db721..0000000000000000000000000000000000000000
--- a/spec/services/projects/prometheus/metrics/update_service_spec.rb
+++ /dev/null
@@ -1,44 +0,0 @@
-# frozen_string_literal: true
-
-require 'spec_helper'
-
-RSpec.describe Projects::Prometheus::Metrics::UpdateService do
- let(:metric) { create(:prometheus_metric) }
-
- it 'updates the prometheus metric' do
- expect do
- described_class.new(metric, { title: "bar" }).execute
- end.to change { metric.reload.title }.to("bar")
- end
-
- context 'when metric has a prometheus alert associated' do
- let(:schedule_update_service) { spy }
-
- before do
- create(:prometheus_alert, project: metric.project, prometheus_metric: metric)
- allow(::Clusters::Applications::ScheduleUpdateService).to receive(:new).and_return(schedule_update_service)
- end
-
- context 'when updating title' do
- it 'schedules a prometheus alert update' do
- described_class.new(metric, { title: "bar" }).execute
-
- expect(schedule_update_service).to have_received(:execute)
- end
- end
-
- context 'when updating query' do
- it 'schedules a prometheus alert update' do
- described_class.new(metric, { query: "sum(bar)" }).execute
-
- expect(schedule_update_service).to have_received(:execute)
- end
- end
-
- it 'does not schedule a prometheus alert update without title nor query being changed' do
- described_class.new(metric, { y_label: "bar" }).execute
-
- expect(schedule_update_service).not_to have_received(:execute)
- end
- end
-end
diff --git a/spec/services/prometheus/create_default_alerts_service_spec.rb b/spec/services/prometheus/create_default_alerts_service_spec.rb
deleted file mode 100644
index 0880799b5893b9acfaffe17ae4745cf673451727..0000000000000000000000000000000000000000
--- a/spec/services/prometheus/create_default_alerts_service_spec.rb
+++ /dev/null
@@ -1,92 +0,0 @@
-# frozen_string_literal: true
-
-require 'spec_helper'
-
-RSpec.describe Prometheus::CreateDefaultAlertsService do
- let_it_be(:project) { create(:project, :repository) }
-
- let(:instance) { described_class.new(project: project) }
- let(:expected_alerts) { described_class::DEFAULT_ALERTS }
-
- describe '#execute' do
- subject(:execute) { instance.execute }
-
- shared_examples 'no alerts created' do
- it 'does not create alerts' do
- expect { execute }.not_to change { project.reload.prometheus_alerts.count }
- end
- end
-
- context 'no environment' do
- it_behaves_like 'no alerts created'
- end
-
- context 'environment exists' do
- let_it_be(:environment) { create(:environment, project: project) }
-
- context 'no found metric' do
- it_behaves_like 'no alerts created'
- end
-
- context 'metric exists' do
- before do
- create_expected_metrics!
- end
-
- context 'alert exists already' do
- before do
- create_pre_existing_alerts!(environment)
- end
-
- it_behaves_like 'no alerts created'
- end
-
- it 'creates alerts' do
- expect { execute }.to change { project.reload.prometheus_alerts.count }
- .by(expected_alerts.size)
- end
-
- it 'does not schedule an update to prometheus' do
- expect(::Clusters::Applications::ScheduleUpdateService).not_to receive(:new)
- execute
- end
-
- context 'cluster with prometheus exists' do
- let!(:cluster) { create(:cluster, :with_installed_prometheus, :provided_by_user, projects: [project]) }
-
- it 'schedules an update to prometheus' do
- expect_next_instance_of(::Clusters::Applications::ScheduleUpdateService) do |instance|
- expect(instance).to receive(:execute)
- end
-
- execute
- end
- end
-
- context 'multiple environments' do
- let!(:production) { create(:environment, project: project, name: 'production') }
-
- it 'uses the production environment' do
- expect { execute }.to change { production.reload.prometheus_alerts.count }
- .by(expected_alerts.size)
- end
- end
- end
- end
- end
-
- private
-
- def create_expected_metrics!
- expected_alerts.each do |alert_hash|
- create(:prometheus_metric, :common, identifier: alert_hash.fetch(:identifier))
- end
- end
-
- def create_pre_existing_alerts!(environment)
- expected_alerts.each do |alert_hash|
- metric = PrometheusMetric.for_identifier(alert_hash[:identifier]).first!
- create(:prometheus_alert, prometheus_metric: metric, project: project, environment: environment)
- end
- end
-end
diff --git a/spec/workers/prometheus/create_default_alerts_worker_spec.rb b/spec/workers/prometheus/create_default_alerts_worker_spec.rb
index 887d677c95f2b40d237ae84132dd7c310e1cdf14..d935bb20a29e6746d2a071a4df1367bc261bb9b3 100644
--- a/spec/workers/prometheus/create_default_alerts_worker_spec.rb
+++ b/spec/workers/prometheus/create_default_alerts_worker_spec.rb
@@ -5,63 +5,9 @@
RSpec.describe Prometheus::CreateDefaultAlertsWorker do
let_it_be(:project) { create(:project) }
- let(:worker) { described_class.new }
- let(:logger) { worker.send(:logger) }
- let(:service) { instance_double(Prometheus::CreateDefaultAlertsService) }
- let(:service_result) { ServiceResponse.success }
-
subject { described_class.new.perform(project.id) }
- before do
- allow(Prometheus::CreateDefaultAlertsService)
- .to receive(:new).with(project: project)
- .and_return(service)
- allow(service).to receive(:execute)
- .and_return(service_result)
- end
-
- it_behaves_like 'an idempotent worker' do
- let(:job_args) { [project.id] }
-
- it 'calls the service' do
- expect(service).to receive(:execute)
-
- subject
- end
-
- context 'project is nil' do
- let(:job_args) { [nil] }
-
- it 'does not call the service' do
- expect(service).not_to receive(:execute)
-
- subject
- end
- end
-
- context 'when service returns an error' do
- let(:error_message) { 'some message' }
- let(:service_result) { ServiceResponse.error(message: error_message) }
-
- it 'succeeds and logs the error' do
- expect(logger)
- .to receive(:info)
- .with(a_hash_including('message' => error_message))
- .exactly(worker_exec_times).times
-
- subject
- end
- end
- end
-
- context 'when service raises an exception' do
- let(:error_message) { 'some exception' }
- let(:exception) { StandardError.new(error_message) }
-
- it 're-raises exception' do
- allow(service).to receive(:execute).and_raise(exception)
-
- expect { subject }.to raise_error(exception)
- end
+ it 'does nothing' do
+ expect { subject }.not_to change { PrometheusAlert.count }
end
end