Merge branch '7565-prometheus-alerts-show-for-multiple-environments' into 'master'

Resolve "Prometheus alerts show for multiple environments"

Closes #7565

See merge request gitlab-org/gitlab-ee!7361

db/schema.rb

@@ -2472,7 +2472,7 @@ ActiveRecord::Schema.define(version: 20190222110418) do
     t.integer "project_id", null: false
     t.integer "prometheus_metric_id", null: false
     t.index ["environment_id"], name: "index_prometheus_alerts_on_environment_id", using: :btree
-    t.index ["project_id", "prometheus_metric_id"], name: "index_prometheus_alerts_on_project_id_and_prometheus_metric_id", unique: true, using: :btree
+    t.index ["project_id", "prometheus_metric_id", "environment_id"], name: "index_prometheus_alerts_metric_environment", unique: true, using: :btree
     t.index ["prometheus_metric_id"], name: "index_prometheus_alerts_on_prometheus_metric_id", using: :btree
   end
 

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

@@ -15,13 +15,11 @@ module Projects
       before_action :authorize_admin_project!, except: [:notify]
       before_action :alert, only: [:update, :show, :destroy]
 
-      # rubocop: disable CodeReuse/ActiveRecord
       def index
-        alerts = project.prometheus_alerts.reorder(id: :asc)
+        alerts = prometheus_alerts.order_by('id_asc')
 
         render json: serialize_as_json(alerts)
       end
-      # rubocop: enable CodeReuse/ActiveRecord
 
       def show
         render json: serialize_as_json(alert)
@@ -40,7 +38,7 @@ module Projects
       end
 
       def create
-        @alert = project.prometheus_alerts.create(alerts_params)
+        @alert = prometheus_alerts.create(alerts_params)
 
         if @alert.persisted?
           schedule_prometheus_update!
@@ -96,7 +94,7 @@ module Projects
       end
 
       def alert
-        @alert ||= project.prometheus_alerts.for_metric(params[:id]).first || render_404
+        @alert ||= prometheus_alerts.for_metric(params[:id]).first || render_404
       end
 
       def application
@@ -115,6 +113,10 @@ module Projects
 
         @project = Project.find_by_full_path("#{namespace}/#{id}")
       end
+
+      def prometheus_alerts
+        project.prometheus_alerts.for_environment(params[:environment_id])
+      end
     end
   end
 end

ee/app/models/prometheus_alert.rb

 # frozen_string_literal: true
 
 class PrometheusAlert < ActiveRecord::Base
+  include Sortable
+
   OPERATORS_MAP = {
     lt: "<",
     eq: "=",
@@ -24,6 +26,7 @@ class PrometheusAlert < ActiveRecord::Base
   delegate :title, :query, to: :prometheus_metric
 
   scope :for_metric, -> (metric) { where(prometheus_metric: metric) }
+  scope :for_environment, -> (environment) { where(environment_id: environment) }
 
   def self.distinct_projects
     sub_query = self.group(:project_id).select(1)

ee/changelogs/unreleased/7565-prometheus-alerts-for-multiple-environments.yml

+---
+title: Allow assigning Prometheus alerts to multiple environments
+merge_request: 7361
+author:
+type: fixed

ee/db/migrate/20180912113336_allow_prometheus_alerts_per_environment.rb

+# frozen_string_literal: true
+
+class AllowPrometheusAlertsPerEnvironment < ActiveRecord::Migration
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+  INDEX_METRIC_ENVIRONMENT_NAME = 'index_prometheus_alerts_metric_environment'
+
+  disable_ddl_transaction!
+
+  def up
+    # Before we create the new index we need to remove it to deal with possible
+    # failures from previous migration.
+    #
+    # See also https://gitlab.com/gitlab-org/gitlab-ce/issues/58164
+    remove_concurrent_index :prometheus_alerts, INDEX_METRIC_ENVIRONMENT_NAME
+
+    add_concurrent_index :prometheus_alerts, new_columns,
+      name: INDEX_METRIC_ENVIRONMENT_NAME, unique: true
+
+    remove_concurrent_index :prometheus_alerts, old_columns
+  end
+
+  def down
+    delete_duplicate_alerts!
+
+    # Before we create the new index we need to remove it to deal with possible
+    # failures from previous migration.
+    #
+    # See also https://gitlab.com/gitlab-org/gitlab-ce/issues/58164
+    remove_concurrent_index :prometheus_alerts, old_columns
+
+    add_concurrent_index :prometheus_alerts, old_columns, unique: true
+
+    remove_concurrent_index :prometheus_alerts, new_columns,
+      name: INDEX_METRIC_ENVIRONMENT_NAME
+  end
+
+  private
+
+  class PrometheusAlert < ActiveRecord::Base
+    include ::EachBatch
+  end
+
+  # Before adding a more narrow index again we need to make sure to delete
+  # newest "duplicate" alerts and keep only the oldest alert per project and metric.
+  def delete_duplicate_alerts!
+    duplicate_alerts = PrometheusAlert
+      .select('MIN(id) AS min, COUNT(id), project_id')
+      .group(:project_id)
+      .having('COUNT(id) > 1')
+
+    duplicate_alerts.each do |alert|
+      PrometheusAlert
+        .where(project_id: alert['project_id'])
+        .where('id <> ?', alert['min'])
+        .each_batch { |batch| batch.delete_all }
+    end
+  end
+
+  def new_columns
+    [:project_id, :prometheus_metric_id, :environment_id]
+  end
+
+  def old_columns
+    [:project_id, :prometheus_metric_id]
+  end
+end

ee/lib/ee/gitlab/prometheus/queries/query_additional_metrics.rb

@@ -13,7 +13,7 @@ module EE
 
           def query_with_alert(project, environment)
             alerts_map =
-              project.prometheus_alerts.each_with_object({}) do |alert, hsh|
+              environment.prometheus_alerts.each_with_object({}) do |alert, hsh|
                 hsh[alert[:prometheus_metric_id]] = alert.prometheus_metric_id
               end
 

ee/spec/migrations/allow_prometheus_alerts_per_environment_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+require Rails.root.join('ee', 'db', 'migrate', '20180912113336_allow_prometheus_alerts_per_environment.rb')
+
+describe AllowPrometheusAlertsPerEnvironment, :migration do
+  let(:migration) { described_class.new }
+  let(:namespaces) { table(:namespaces) }
+  let(:projects) { table(:projects) }
+  let(:environments) { table(:environments) }
+  let(:prometheus_metrics) { table(:prometheus_metrics) }
+  let(:prometheus_alerts) { table(:prometheus_alerts) }
+  let(:now) { Time.now }
+
+  let(:old_index_columns) { %i[project_id prometheus_metric_id] }
+  let(:new_index_columns) { %i[project_id prometheus_metric_id environment_id] }
+  let(:new_index_name) { described_class::INDEX_METRIC_ENVIRONMENT_NAME }
+
+  describe '#up' do
+    it 'creates a wider index dropping the narrower one' do
+      migration.up
+
+      expect(unique_index?(new_index_columns, name: new_index_name))
+        .to eq(true)
+
+      expect(unique_index?(old_index_columns)).to eq(false)
+    end
+  end
+
+  describe '#down' do
+    let(:ns) { namespace(id: 1, name: 'ns') }
+    let(:a) { project(id: 1, name: 'a') }
+    let(:b) { project(id: 2, name: 'b') }
+    let(:c) { project(id: 3, name: 'c') }
+    let(:a_prd) { environment(id: 11, name: 'a_prd', project: a) }
+    let(:a_stg) { environment(id: 12, name: 'a_stg', project: a) }
+    let(:a_can) { environment(id: 13, name: 'a_can', project: a) }
+    let(:b_prd) { environment(id: 14, name: 'b_prd', project: b) }
+    let(:b_stg) { environment(id: 15, name: 'b_stg', project: b) }
+    let(:c_prd) { environment(id: 16, name: 'c_prd', project: c) }
+    let(:metric_a) { metric(id: 21, project: a) }
+    let(:metric_b) { metric(id: 22, project: b) }
+    let(:metric_c) { metric(id: 23, project: c) }
+
+    let(:alert_a_prd) { alert(id: 31, metric: metric_a, environment: a_prd) }
+    let(:alert_a_stg) { alert(id: 32, metric: metric_a, environment: a_stg) }
+    let(:alert_a_can) { alert(id: 33, metric: metric_a, environment: a_can) }
+    let(:alert_b_stg) { alert(id: 34, metric: metric_b, environment: b_stg) }
+    let(:alert_b_prd) { alert(id: 35, metric: metric_b, environment: b_prd) }
+    let(:alert_c_prd) { alert(id: 36, metric: metric_c, environment: c_prd) }
+
+    before do
+      # Migration up to allow multiple alerts per environment
+      schema_migrate_up!
+    end
+
+    it 'deletes duplicate alerts before narrowing the index' do
+      # create
+      alert_a_prd
+      alert_a_stg
+      alert_a_can
+      alert_b_prd
+      alert_b_stg
+      alert_c_prd
+
+      migration.down
+
+      expect(unique_index?(old_index_columns, unique: true)).to eq(true)
+
+      expect(unique_index?(new_index_columns, name: new_index_name))
+        .to eq(false)
+
+      expect(prometheus_alerts.all.to_a)
+        .to contain_exactly(alert_a_prd, alert_b_stg, alert_c_prd)
+    end
+  end
+
+  private
+
+  def unique_index?(columns, opts = {})
+    migration.index_exists?(:prometheus_alerts,
+                            columns, opts.merge(unique: true))
+  end
+
+  def namespace(id:, name:)
+    namespaces.create!(id: id, name: name, path: name)
+  end
+
+  def project(id:, name:)
+    projects.create!(id: id, name: name, path: name, namespace_id: ns.id)
+  end
+
+  def environment(id:, name:, project:)
+    environments.create!(id: id, name: name, slug: name,
+                         project_id: project.id)
+  end
+
+  def metric(id:, project:)
+    prometheus_metrics.create!(
+      id: id,
+      project_id: project.id,
+      title: 'title',
+      query: 'query',
+      group: 1,
+      created_at: now,
+      updated_at: now,
+      common: false
+    )
+  end
+
+  def alert(id:, metric:, environment:)
+    prometheus_alerts.create!(
+      id: id,
+      project_id: metric.project_id,
+      environment_id: environment.id,
+      prometheus_metric_id: metric.id,
+      threshold: 1.0,
+      operator: '=',
+      created_at: now,
+      updated_at: now
+    )
+  end
+end