Handle update and destroy behaviour of metrics to cleanly erase the associated alerts

db/schema.rb

@@ -2188,8 +2188,6 @@ ActiveRecord::Schema.define(version: 20180704204006) do
     t.integer "environment_id", null: false
     t.integer "project_id", null: false
     t.integer "prometheus_metric_id", null: false
-    t.text "name", null: false
-    t.string "query", null: false
   end
 
   add_index "prometheus_alerts", ["environment_id"], name: "index_prometheus_alerts_on_environment_id", using: :btree

ee/app/controllers/ee/projects/prometheus/metrics_controller.rb

@@ -47,7 +47,7 @@ module EE
 
         def update
           @metric = project.prometheus_metrics.find(params[:id]) # rubocop:disable Gitlab/ModuleWithInstanceVariables
-          @metric.update(metrics_params) # rubocop:disable Gitlab/ModuleWithInstanceVariables
+          @metric = update_metrics_service(@metric).execute # rubocop:disable Gitlab/ModuleWithInstanceVariables
 
           if @metric.persisted? # rubocop:disable Gitlab/ModuleWithInstanceVariables
             redirect_to edit_project_service_path(project, ::PrometheusService),
@@ -63,7 +63,7 @@ module EE
 
         def destroy
           metric = project.prometheus_metrics.find(params[:id])
-          metric.destroy
+          destroy_metrics_service(metric).execute
 
           respond_to do |format|
             format.html do
@@ -77,6 +77,14 @@ module EE
 
         private
 
+        def update_metrics_service(metric)
+          ::Projects::Prometheus::Metrics::UpdateService.new(metric, metrics_params)
+        end
+
+        def destroy_metrics_service(metric)
+          ::Projects::Prometheus::Metrics::DestroyService.new(metric)
+        end
+
         def metrics_params
           params.require(:prometheus_metric).permit(:title, :query, :y_label, :unit, :legend, :group)
         end

ee/app/controllers/projects/prometheus/alerts_controller.rb

@@ -60,7 +60,7 @@ module Projects
       private
 
       def alerts_params
-        alerts_params = params.permit(:query, :operator, :threshold, :name, :environment_id, :prometheus_metric_id)
+        alerts_params = params.permit(:operator, :threshold, :environment_id, :prometheus_metric_id)
 
         if alerts_params[:operator].present?
           alerts_params[:operator] = PrometheusAlert.operator_to_enum(alerts_params[:operator])

ee/app/models/prometheus_alert.rb

 class PrometheusAlert < ActiveRecord::Base
-  include AtomicInternalId
-
   OPERATORS_MAP = {
     lt: "<",
     eq: "=",
@@ -11,13 +9,13 @@ class PrometheusAlert < ActiveRecord::Base
   belongs_to :project, required: true, validate: true, inverse_of: :prometheus_alerts
   belongs_to :prometheus_metric, required: true, validate: true
 
-  validates :name, presence: true
-
   after_save :clear_prometheus_adapter_cache!
   after_destroy :clear_prometheus_adapter_cache!
 
   enum operator: [:lt, :eq, :gt]
 
+  delegate :title, :query, to: :prometheus_metric
+
   def self.operator_to_enum(op)
     OPERATORS_MAP.invert.fetch(op)
   end
@@ -32,7 +30,7 @@ class PrometheusAlert < ActiveRecord::Base
 
   def to_param
     {
-      "alert" => name,
+      "alert" => title,
       "expr" => full_query,
       "for" => "5m",
       "labels" => {

ee/app/serializers/prometheus_alert_entity.rb

@@ -2,7 +2,7 @@ class PrometheusAlertEntity < Grape::Entity
   include RequestAwareEntity
 
   expose :id
-  expose :name
+  expose :title
   expose :query
   expose :threshold
 

ee/app/services/projects/prometheus/metrics/base_service.rb

+module Projects
+  module Prometheus
+    module Metrics
+      class BaseService
+        def initialize(metric, params = {})
+          @metric = metric
+          @project = metric.project
+          @params = params.dup
+        end
+
+        protected
+
+        attr_reader :metric, :project, :params
+
+        def application
+          metric.prometheus_alert.environment.cluster_prometheus_adapter
+        end
+
+        def schedule_alert_update
+          ::Clusters::Applications::ScheduleUpdateService.new(application, project).execute
+        end
+
+        def has_alert?
+          metric.prometheus_alert.present?
+        end
+      end
+    end
+  end
+end

ee/app/services/projects/prometheus/metrics/destroy_service.rb

+module Projects
+  module Prometheus
+    module Metrics
+      class DestroyService < BaseService
+        def execute
+          schedule_alert_update if has_alert?
+          metric.destroy
+        end
+      end
+    end
+  end
+end

ee/app/services/projects/prometheus/metrics/update_service.rb

+module Projects
+  module Prometheus
+    module Metrics
+      class UpdateService < 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

ee/db/migrate/20180320142552_create_prometheus_alerts.rb

@@ -10,8 +10,6 @@ class CreatePrometheusAlerts < ActiveRecord::Migration
       t.references :environment, index: true, null: false, foreign_key: { on_delete: :cascade }
       t.references :project, null: false, foreign_key: { on_delete: :cascade }
       t.references :prometheus_metric, null: false, index: { unique: true }, foreign_key: { on_delete: :cascade }
-      t.text :name, null: false
-      t.string :query, null: false
     end
   end
 

ee/spec/controllers/projects/prometheus/alerts_controller_spec.rb

@@ -111,10 +111,8 @@ describe Projects::Prometheus::AlertsController do
       stub_licensed_features(prometheus_alerts: false)
 
       post :create, project_params(
-        query: "foo",
         operator: ">",
         threshold: "1",
-        name: "bar",
         environment_id: environment.id,
         prometheus_metric_id: metric.id
       )
@@ -125,8 +123,8 @@ describe Projects::Prometheus::AlertsController do
     it 'creates a new prometheus alert' do
       schedule_update_service = spy
       alert_params = {
-        "name" => "bar",
-        "query" => "foo",
+        "name" => metric.title,
+        "query" => metric.query,
         "operator" => ">",
         "threshold" => 1.0
       }
@@ -134,10 +132,8 @@ describe Projects::Prometheus::AlertsController do
       allow(::Clusters::Applications::ScheduleUpdateService).to receive(:new).and_return(schedule_update_service)
 
       post :create, project_params(
-        query: "foo",
         operator: ">",
         threshold: "1",
-        name: "bar",
         environment_id: environment.id,
         prometheus_metric_id: metric.id
       )
@@ -175,8 +171,8 @@ describe Projects::Prometheus::AlertsController do
       }
 
       expect do
-        put :update, project_params(id: alert.prometheus_metric_id, name: "bar")
-      end.to change { alert.reload.name }.to("bar")
+        put :update, project_params(id: alert.prometheus_metric_id, operator: "<")
+      end.to change { alert.reload.operator }.to("lt")
 
       expect(schedule_update_service).to have_received(:execute)
       expect(response).to have_gitlab_http_status(200)