Merge branch 'rc/hook_branch_update' into 'master'

Import dashboard metrics on default branch update

See merge request gitlab-org/gitlab!39658

app/models/environment.rb

@@ -86,6 +86,7 @@ class Environment < ApplicationRecord
   scope :with_rank, -> do
     select('environments.*, rank() OVER (PARTITION BY project_id ORDER BY id DESC)')
   end
+  scope :for_id, -> (id) { where(id: id) }
 
   state_machine :state, initial: :available do
     event :start do

app/models/prometheus_alert.rb

@@ -4,6 +4,7 @@ class PrometheusAlert < ApplicationRecord
   include Sortable
   include UsageStatistics
   include Presentable
+  include EachBatch
 
   OPERATORS_MAP = {
     lt: "<",
@@ -35,6 +36,7 @@ class PrometheusAlert < ApplicationRecord
   scope :for_metric, -> (metric) { where(prometheus_metric: metric) }
   scope :for_project, -> (project) { where(project_id: project) }
   scope :for_environment, -> (environment) { where(environment_id: environment) }
+  scope :get_environment_id, -> { select(:environment_id).pluck(:environment_id) }
 
   def self.distinct_projects
     sub_query = self.group(:project_id).select(1)

app/models/prometheus_metric.rb

 # frozen_string_literal: true
 
 class PrometheusMetric < ApplicationRecord
+  include EachBatch
+
   belongs_to :project, validate: true, inverse_of: :prometheus_metrics
   has_many :prometheus_alerts, inverse_of: :prometheus_metric
 

app/services/git/branch_hooks_service.rb

@@ -76,12 +76,20 @@ module Git
     def branch_change_hooks
       enqueue_process_commit_messages
       enqueue_jira_connect_sync_messages
+      enqueue_metrics_dashboard_sync
     end
 
     def branch_remove_hooks
       project.repository.after_remove_branch(expire_cache: false)
     end
 
+    def enqueue_metrics_dashboard_sync
+      return unless Feature.enabled?(:sync_metrics_dashboards, project)
+      return unless default_branch?
+
+      ::Metrics::Dashboard::SyncDashboardsWorker.perform_async(project.id)
+    end
+
     # Schedules processing of commit messages
     def enqueue_process_commit_messages
       referencing_commits = limited_commits.select(&:matches_cross_reference_regex?)

app/services/metrics/dashboard/custom_dashboard_service.rb

@@ -42,6 +42,12 @@ module Metrics
       def cache_key
         "project_#{project.id}_metrics_dashboard_#{dashboard_path}"
       end
+
+      def sequence
+        [
+          ::Gitlab::Metrics::Dashboard::Stages::CustomDashboardMetricsInserter
+        ] + super
+      end
     end
   end
 end

app/workers/all_queues.yml

@@ -1556,6 +1556,14 @@
   :weight: 1
   :idempotent: true
   :tags: []
+- :name: metrics_dashboard_sync_dashboards
+  :feature_category: :metrics
+  :has_external_dependencies:
+  :urgency: :low
+  :resource_boundary: :unknown
+  :weight: 1
+  :idempotent: true
+  :tags: []
 - :name: migrate_external_diffs
   :feature_category: :source_code_management
   :has_external_dependencies:

app/workers/metrics/dashboard/sync_dashboards_worker.rb

+# frozen_string_literal: true
+
+module Metrics
+  module Dashboard
+    class SyncDashboardsWorker
+      include ApplicationWorker
+
+      feature_category :metrics
+
+      idempotent!
+
+      def perform(project_id)
+        project = Project.find(project_id)
+        dashboard_paths = ::Gitlab::Metrics::Dashboard::RepoDashboardFinder.list_dashboards(project)
+
+        dashboard_paths.each do |dashboard_path|
+          ::Gitlab::Metrics::Dashboard::Importer.new(dashboard_path, project).execute!
+        end
+      end
+    end
+  end
+end

config/feature_flags/development/sync_metrics_dashboards.yml

+---
+name: sync_metrics_dashboards
+introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/39658
+rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/241793
+group: group::apm
+type: development
+default_enabled: false

config/sidekiq_queues.yml

@@ -168,6 +168,8 @@
   - 1
 - - metrics_dashboard_prune_old_annotations
   - 1
+- - metrics_dashboard_sync_dashboards
+  - 1
 - - migrate_external_diffs
   - 1
 - - namespaceless_project_destroy

lib/gitlab/metrics/dashboard/importers/prometheus_metrics.rb

@@ -13,11 +13,12 @@ module Gitlab
             @dashboard_hash = dashboard_hash
             @project = project
             @dashboard_path = dashboard_path
+            @affected_environment_ids = []
           end
 
           def execute
             import
-          rescue ActiveRecord::RecordInvalid, ::Gitlab::Metrics::Dashboard::Transformers::TransformerError
+          rescue ActiveRecord::RecordInvalid, Dashboard::Transformers::Errors::BaseError
             false
           end
 
@@ -32,28 +33,51 @@ module Gitlab
           def import
             delete_stale_metrics
             create_or_update_metrics
+            update_prometheus_environments
           end
 
           # rubocop: disable CodeReuse/ActiveRecord
           def create_or_update_metrics
             # TODO: use upsert and worker for callbacks?
+
+            affected_metric_ids = []
             prometheus_metrics_attributes.each do |attributes|
-              prometheus_metric = PrometheusMetric.find_or_initialize_by(attributes.slice(:identifier, :project))
+              prometheus_metric = PrometheusMetric.find_or_initialize_by(attributes.slice(:dashboard_path, :identifier, :project))
               prometheus_metric.update!(attributes.slice(*ALLOWED_ATTRIBUTES))
+
+              affected_metric_ids << prometheus_metric.id
             end
+
+            @affected_environment_ids += find_alerts(affected_metric_ids).get_environment_id
           end
           # rubocop: enable CodeReuse/ActiveRecord
 
           def delete_stale_metrics
-            identifiers = prometheus_metrics_attributes.map { |metric_attributes| metric_attributes[:identifier] }
+            identifiers_from_yml = prometheus_metrics_attributes.map { |metric_attributes| metric_attributes[:identifier] }
 
             stale_metrics = PrometheusMetric.for_project(project)
               .for_dashboard_path(dashboard_path)
               .for_group(Enums::PrometheusMetric.groups[:custom])
-              .not_identifier(identifiers)
+              .not_identifier(identifiers_from_yml)
+
+            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?
 
-            # TODO: use destroy_all and worker for callbacks?
-            stale_metrics.each(&:destroy)
+            @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
@@ -65,6 +89,19 @@ module Gitlab
               ).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

lib/gitlab/metrics/dashboard/stages/custom_dashboard_metrics_inserter.rb

+# frozen_string_literal: true
+
+module Gitlab
+  module Metrics
+    module Dashboard
+      module Stages
+        # Acts on metrics which have been ingested from source controlled dashboards
+        class CustomDashboardMetricsInserter < BaseStage
+          # For each metric in the dashboard config, attempts to
+          # find a corresponding database record. If found, includes
+          # the record's id in the dashboard config.
+          def transform!
+            database_metrics = ::PrometheusMetricsFinder.new(common: false, group: :custom, project: project).execute
+
+            for_metrics do |metric|
+              metric_record = database_metrics.find { |m| m.identifier == metric[:id] }
+              metric[:metric_id] = metric_record.id if metric_record
+            end
+          end
+        end
+      end
+    end
+  end
+end

lib/gitlab/metrics/dashboard/transformers/errors.rb

@@ -4,10 +4,10 @@ module Gitlab
   module Metrics
     module Dashboard
       module Transformers
-        TransformerError = Class.new(StandardError)
-
         module Errors
-          class MissingAttribute < TransformerError
+          BaseError = Class.new(StandardError)
+
+          class MissingAttribute < BaseError
             def initialize(attribute_name)
               super("Missing attribute: '#{attribute_name}'")
             end

spec/factories/prometheus_metrics.rb

@@ -9,6 +9,7 @@ FactoryBot.define do
     group { :business }
     project
     legend { 'legend' }
+    dashboard_path { '.gitlab/dashboards/dashboard_path.yml'}
 
     trait :common do
       common { true }

spec/lib/gitlab/metrics/dashboard/importers/prometheus_metrics_spec.rb

@@ -8,9 +8,16 @@ RSpec.describe Gitlab::Metrics::Dashboard::Importers::PrometheusMetrics do
   describe '#execute' do
     let(:project) { create(:project) }
     let(:dashboard_path) { 'path/to/dashboard.yml' }
+    let(:prometheus_adapter) { double('adapter', clear_prometheus_reactive_cache!: nil) }
 
     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 }
 
@@ -21,20 +28,32 @@ RSpec.describe Gitlab::Metrics::Dashboard::Importers::PrometheusMetrics do
       end
 
       context 'with existing metrics' do
+        let(:existing_metric_attributes) do
+          {
+            project:        project,
+            identifier:     'metric_b',
+            title:          'overwrite',
+            y_label:        'overwrite',
+            query:          'overwrite',
+            unit:           'overwrite',
+            legend:         'overwrite',
+            dashboard_path: dashboard_path
+          }
+        end
+
         let!(:existing_metric) do
-          create(:prometheus_metric, {
-            project:    project,
-            identifier: 'metric_b',
-            title:      'overwrite',
-            y_label:    'overwrite',
-            query:      'overwrite',
-            unit:       'overwrite',
-            legend:     'overwrite'
-          })
+          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
-          described_class.new(dashboard_hash, project: project, dashboard_path: dashboard_path).execute
+          subject.execute
 
           expect(existing_metric.reload.attributes.with_indifferent_access).to include({
             title:   'Super Chart B',
@@ -49,6 +68,15 @@ RSpec.describe Gitlab::Metrics::Dashboard::Importers::PrometheusMetrics do
           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,
@@ -59,11 +87,45 @@ RSpec.describe Gitlab::Metrics::Dashboard::Importers::PrometheusMetrics do
             )
           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
+
+            expect(existing_metric.reload.attributes.with_indifferent_access).to include({
+              title:   'Super Chart B',
+              y_label: 'y_label',
+              query:   'query',
+              unit:    'unit',
+              legend:  'Legend Label'
+            })
+          end
+
           it 'deletes stale metrics' do
             subject.execute
 
             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

spec/services/git/branch_hooks_service_spec.rb

@@ -429,4 +429,26 @@ RSpec.describe Git::BranchHooksService do
       end
     end
   end
+
+  describe 'Metrics dashboard sync' do
+    context 'with feature flag enabled' do
+      before do
+        Feature.enable(:metrics_dashboards_sync)
+      end
+
+      it 'imports metrics to database' do
+        expect(Metrics::Dashboard::SyncDashboardsWorker).to receive(:perform_async)
+
+        service.execute
+      end
+    end
+
+    context 'with feature flag disabled' do
+      it 'imports metrics to database' do
+        expect(Metrics::Dashboard::SyncDashboardsWorker).to receive(:perform_async)
+
+        service.execute
+      end
+    end
+  end
 end

spec/services/metrics/dashboard/custom_dashboard_service_spec.rb

@@ -67,6 +67,23 @@ RSpec.describe Metrics::Dashboard::CustomDashboardService, :use_clean_rails_memo
           .at_least(:once)
       end
 
+      context 'with metric in database' do
+        let!(:prometheus_metric) do
+          create(:prometheus_metric, project: project, identifier: 'metric_a1', group: 'custom')
+        end
+
+        it 'includes metric_id' do
+          dashboard = described_class.new(*service_params).get_dashboard
+
+          metric_id = dashboard[:dashboard][:panel_groups].find { |panel_group| panel_group[:group] == 'Group A' }
+            .fetch(:panels).find { |panel| panel[:title] == 'Super Chart A1' }
+            .fetch(:metrics).find { |metric| metric[:id] == 'metric_a1' }
+            .fetch(:metric_id)
+
+          expect(metric_id).to eq(prometheus_metric.id)
+        end
+      end
+
       context 'and the dashboard is then deleted' do
         it 'does not return the previously cached dashboard' do
           described_class.new(*service_params).get_dashboard

spec/workers/metrics/dashboard/sync_dashboards_worker_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Metrics::Dashboard::SyncDashboardsWorker do
+  include MetricsDashboardHelpers
+  subject(:worker) { described_class.new }
+
+  let(:project) { project_with_dashboard(dashboard_path) }
+  let(:dashboard_path) { '.gitlab/dashboards/test.yml' }
+
+  describe ".perform" do
+    it 'imports metrics' do
+      expect { worker.perform(project.id) }.to change(PrometheusMetric, :count).by(3)
+    end
+
+    it 'is idempotent' do
+      2.times do
+        worker.perform(project.id)
+      end
+
+      expect(PrometheusMetric.count).to eq(3)
+    end
+  end
+end