Merge branch '63044-cluster-health-group-refactor' into 'master'

Refactor common_metrics importer into a more extensible pattern

Closes #63044

See merge request gitlab-org/gitlab-ce!30012

app/models/prometheus_metric.rb

@@ -3,68 +3,7 @@
 class PrometheusMetric < ApplicationRecord
   belongs_to :project, validate: true, inverse_of: :prometheus_metrics
 
-  enum group: {
-    # built-in groups
-    nginx_ingress_vts: -1,
-    ha_proxy: -2,
-    aws_elb: -3,
-    nginx: -4,
-    kubernetes: -5,
-    nginx_ingress: -6,
-
-    # custom/user groups
-    business: 0,
-    response: 1,
-    system: 2
-  }
-
-  GROUP_DETAILS = {
-    # built-in groups
-    nginx_ingress_vts: {
-      group_title: _('Response metrics (NGINX Ingress VTS)'),
-      required_metrics: %w(nginx_upstream_responses_total nginx_upstream_response_msecs_avg),
-      priority: 10
-    }.freeze,
-    nginx_ingress: {
-      group_title: _('Response metrics (NGINX Ingress)'),
-      required_metrics: %w(nginx_ingress_controller_requests nginx_ingress_controller_ingress_upstream_latency_seconds_sum),
-      priority: 10
-    }.freeze,
-    ha_proxy: {
-      group_title: _('Response metrics (HA Proxy)'),
-      required_metrics: %w(haproxy_frontend_http_requests_total haproxy_frontend_http_responses_total),
-      priority: 10
-    }.freeze,
-    aws_elb: {
-      group_title: _('Response metrics (AWS ELB)'),
-      required_metrics: %w(aws_elb_request_count_sum aws_elb_latency_average aws_elb_httpcode_backend_5_xx_sum),
-      priority: 10
-    }.freeze,
-    nginx: {
-      group_title: _('Response metrics (NGINX)'),
-      required_metrics: %w(nginx_server_requests nginx_server_requestMsec),
-      priority: 10
-    }.freeze,
-    kubernetes: {
-      group_title: _('System metrics (Kubernetes)'),
-      required_metrics: %w(container_memory_usage_bytes container_cpu_usage_seconds_total),
-      priority: 5
-    }.freeze,
-
-    # custom/user groups
-    business: {
-      group_title: _('Business metrics (Custom)'),
-      priority: 0
-    }.freeze,
-    response: {
-      group_title: _('Response metrics (Custom)'),
-      priority: -5
-    }.freeze,
-    system: {
-      group_title: _('System metrics (Custom)'),
-      priority: -10
-    }.freeze
-  }.freeze
+  enum group: PrometheusMetricEnums.groups
 
   validates :title, presence: true
   validates :query, presence: true
@@ -121,6 +60,6 @@ class PrometheusMetric < ApplicationRecord
   private
 
   def group_details(group)
-    GROUP_DETAILS.fetch(group.to_sym)
+    PrometheusMetricEnums.group_details.fetch(group.to_sym)
   end
 end

app/models/prometheus_metric_enums.rb

+# frozen_string_literal: true
+
+module PrometheusMetricEnums
+  def self.groups
+    {
+      # built-in groups
+      nginx_ingress_vts: -1,
+      ha_proxy: -2,
+      aws_elb: -3,
+      nginx: -4,
+      kubernetes: -5,
+      nginx_ingress: -6,
+
+      # custom/user groups
+      business: 0,
+      response: 1,
+      system: 2
+    }
+  end
+
+  def self.group_details
+    {
+      # built-in groups
+      nginx_ingress_vts: {
+        group_title: _('Response metrics (NGINX Ingress VTS)'),
+        required_metrics: %w(nginx_upstream_responses_total nginx_upstream_response_msecs_avg),
+        priority: 10
+      }.freeze,
+      nginx_ingress: {
+        group_title: _('Response metrics (NGINX Ingress)'),
+        required_metrics: %w(nginx_ingress_controller_requests nginx_ingress_controller_ingress_upstream_latency_seconds_sum),
+        priority: 10
+      }.freeze,
+      ha_proxy: {
+        group_title: _('Response metrics (HA Proxy)'),
+        required_metrics: %w(haproxy_frontend_http_requests_total haproxy_frontend_http_responses_total),
+        priority: 10
+      }.freeze,
+      aws_elb: {
+        group_title: _('Response metrics (AWS ELB)'),
+        required_metrics: %w(aws_elb_request_count_sum aws_elb_latency_average aws_elb_httpcode_backend_5_xx_sum),
+        priority: 10
+      }.freeze,
+      nginx: {
+        group_title: _('Response metrics (NGINX)'),
+        required_metrics: %w(nginx_server_requests nginx_server_requestMsec),
+        priority: 10
+      }.freeze,
+      kubernetes: {
+        group_title: _('System metrics (Kubernetes)'),
+        required_metrics: %w(container_memory_usage_bytes container_cpu_usage_seconds_total),
+        priority: 5
+      }.freeze,
+
+      # custom/user groups
+      business: {
+        group_title: _('Business metrics (Custom)'),
+        priority: 0
+      }.freeze,
+      response: {
+        group_title: _('Response metrics (Custom)'),
+      priority: -5
+      }.freeze,
+      system: {
+        group_title: _('System metrics (Custom)'),
+        priority: -10
+      }.freeze
+    }.freeze
+  end
+end

db/fixtures/development/99_common_metrics.rb

 # frozen_string_literal: true
 
-require Rails.root.join('db/importers/common_metrics_importer.rb')
-
-::Importers::CommonMetricsImporter.new.execute
+::Gitlab::DatabaseImporters::CommonMetrics::Importer.new.execute

db/fixtures/production/999_common_metrics.rb

 # frozen_string_literal: true
 
-require Rails.root.join('db/importers/common_metrics_importer.rb')
-
-::Importers::CommonMetricsImporter.new.execute
+::Gitlab::DatabaseImporters::CommonMetrics::Importer.new.execute

db/importers/common_metrics_importer.rb

-# frozen_string_literal: true
-
-module Importers
-  class PrometheusMetric < ActiveRecord::Base
-    enum group: {
-      # built-in groups
-      nginx_ingress_vts: -1,
-      ha_proxy: -2,
-      aws_elb: -3,
-      nginx: -4,
-      kubernetes: -5,
-      nginx_ingress: -6,
-
-      # custom groups
-      business: 0,
-      response: 1,
-      system: 2
-    }
-
-    scope :common, -> { where(common: true) }
-
-    GROUP_TITLES = {
-      business: _('Business metrics (Custom)'),
-      response: _('Response metrics (Custom)'),
-      system: _('System metrics (Custom)'),
-      nginx_ingress_vts: _('Response metrics (NGINX Ingress VTS)'),
-      nginx_ingress: _('Response metrics (NGINX Ingress)'),
-      ha_proxy: _('Response metrics (HA Proxy)'),
-      aws_elb: _('Response metrics (AWS ELB)'),
-      nginx: _('Response metrics (NGINX)'),
-      kubernetes: _('System metrics (Kubernetes)')
-    }.freeze
-  end
-
-  class CommonMetricsImporter
-    MissingQueryId = Class.new(StandardError)
-
-    attr_reader :content
-
-    def initialize(filename = 'common_metrics.yml')
-      @content = YAML.load_file(Rails.root.join('config', 'prometheus', filename))
-    end
-
-    def execute
-      PrometheusMetric.reset_column_information
-
-      process_content do |id, attributes|
-        find_or_build_metric!(id)
-          .update!(**attributes)
-      end
-    end
-
-    private
-
-    def process_content(&blk)
-      content['panel_groups'].map do |group|
-        process_group(group, &blk)
-      end
-    end
-
-    def process_group(group, &blk)
-      attributes = {
-        group: find_group_title_key(group['group'])
-      }
-
-      group['panels'].map do |panel|
-        process_panel(panel, attributes, &blk)
-      end
-    end
-
-    def process_panel(panel, attributes, &blk)
-      attributes = attributes.merge(
-        title: panel['title'],
-        y_label: panel['y_label'])
-
-      panel['metrics'].map do |metric_details|
-        process_metric_details(metric_details, attributes, &blk)
-      end
-    end
-
-    def process_metric_details(metric_details, attributes, &blk)
-      attributes = attributes.merge(
-        legend: metric_details['label'],
-        query: metric_details['query_range'],
-        unit: metric_details['unit'])
-
-      yield(metric_details['id'], attributes)
-    end
-
-    def find_or_build_metric!(id)
-      raise MissingQueryId unless id
-
-      PrometheusMetric.common.find_by(identifier: id) ||
-        PrometheusMetric.new(common: true, identifier: id)
-    end
-
-    def find_group_title_key(title)
-      PrometheusMetric.groups[find_group_title(title)]
-    end
-
-    def find_group_title(title)
-      PrometheusMetric::GROUP_TITLES.invert[title]
-    end
-  end
-end

db/migrate/20180831164910_import_common_metrics.rb

@@ -3,12 +3,10 @@
 class ImportCommonMetrics < ActiveRecord::Migration[4.2]
   include Gitlab::Database::MigrationHelpers
 
-  require Rails.root.join('db/importers/common_metrics_importer.rb')
-
   DOWNTIME = false
 
   def up
-    Importers::CommonMetricsImporter.new.execute
+    ::Gitlab::DatabaseImporters::CommonMetrics::Importer.new.execute
   end
 
   def down

db/migrate/20181006004100_import_common_metrics_nginx_vts.rb

 class ImportCommonMetricsNginxVts < ActiveRecord::Migration[5.0]
   include Gitlab::Database::MigrationHelpers
 
-  require Rails.root.join('db/importers/common_metrics_importer.rb')
-
   DOWNTIME = false
 
   def up
-    Importers::CommonMetricsImporter.new.execute
+    ::Gitlab::DatabaseImporters::CommonMetrics::Importer.new.execute
   end
 
   def down

db/migrate/20190326164045_import_common_metrics_knative.rb

@@ -3,12 +3,10 @@
 class ImportCommonMetricsKnative < ActiveRecord::Migration[5.0]
   include Gitlab::Database::MigrationHelpers
 
-  require Rails.root.join('db/importers/common_metrics_importer.rb')
-
   DOWNTIME = false
 
   def up
-    Importers::CommonMetricsImporter.new.execute
+    ::Gitlab::DatabaseImporters::CommonMetrics::Importer.new.execute
   end
 
   def down

db/migrate/20190408163745_prometheus_knative05_fix.rb

@@ -6,12 +6,10 @@
 class PrometheusKnative05Fix < ActiveRecord::Migration[5.0]
   include Gitlab::Database::MigrationHelpers
 
-  require Rails.root.join('db/importers/common_metrics_importer.rb')
-
   DOWNTIME = false
 
   def up
-    Importers::CommonMetricsImporter.new.execute
+    ::Gitlab::DatabaseImporters::CommonMetrics::Importer.new.execute
   end
 
   def down

doc/development/prometheus_metrics.md

@@ -33,12 +33,10 @@ For example: you might be interested in migrating all dependent data to a differ
 class ImportCommonMetrics < ActiveRecord::Migration[4.2]
   include Gitlab::Database::MigrationHelpers
 
-  require Rails.root.join('db/importers/common_metrics_importer.rb')
-
   DOWNTIME = false
 
   def up
-    Importers::CommonMetricsImporter.new.execute
+    ::Gitlab::DatabaseImporters::CommonMetrics::Importer.new.execute
   end
 
   def down

lib/gitlab/database_importers/common_metrics.rb

+# frozen_string_literal: true
+
+module Gitlab
+  module DatabaseImporters
+    module CommonMetrics
+    end
+  end
+end

lib/gitlab/database_importers/common_metrics/importer.rb

+# frozen_string_literal: true
+
+module Gitlab
+  module DatabaseImporters
+    module CommonMetrics
+      class Importer
+        MissingQueryId = Class.new(StandardError)
+
+        attr_reader :content
+
+        def initialize(filename = 'common_metrics.yml')
+          @content = YAML.load_file(Rails.root.join('config', 'prometheus', filename))
+        end
+
+        def execute
+          CommonMetrics::PrometheusMetric.reset_column_information
+
+          process_content do |id, attributes|
+            find_or_build_metric!(id)
+              .update!(**attributes)
+          end
+        end
+
+        private
+
+        def process_content(&blk)
+          content['panel_groups'].map do |group|
+            process_group(group, &blk)
+          end
+        end
+
+        def process_group(group, &blk)
+          attributes = {
+            group: find_group_title_key(group['group'])
+          }
+
+          group['panels'].map do |panel|
+            process_panel(panel, attributes, &blk)
+          end
+        end
+
+        def process_panel(panel, attributes, &blk)
+          attributes = attributes.merge(
+            title: panel['title'],
+            y_label: panel['y_label'])
+
+          panel['metrics'].map do |metric_details|
+            process_metric_details(metric_details, attributes, &blk)
+          end
+        end
+
+        def process_metric_details(metric_details, attributes, &blk)
+          attributes = attributes.merge(
+            legend: metric_details['label'],
+            query: metric_details['query_range'],
+            unit: metric_details['unit'])
+
+          yield(metric_details['id'], attributes)
+        end
+
+        def find_or_build_metric!(id)
+          raise MissingQueryId unless id
+
+          CommonMetrics::PrometheusMetric.common.find_by(identifier: id) ||
+            CommonMetrics::PrometheusMetric.new(common: true, identifier: id)
+        end
+
+        def find_group_title_key(title)
+          CommonMetrics::PrometheusMetricEnums.groups[find_group_title(title)]
+        end
+
+        def find_group_title(title)
+          CommonMetrics::PrometheusMetricEnums.group_titles.invert[title]
+        end
+      end
+    end
+  end
+end

lib/gitlab/database_importers/common_metrics/prometheus_metric.rb

+# frozen_string_literal: true
+
+module Gitlab
+  module DatabaseImporters
+    module CommonMetrics
+      class PrometheusMetric < ApplicationRecord
+        enum group: PrometheusMetricEnums.groups
+        scope :common, -> { where(common: true) }
+      end
+    end
+  end
+end

lib/gitlab/database_importers/common_metrics/prometheus_metric_enums.rb

+# frozen_string_literal: true
+
+module Gitlab
+  module DatabaseImporters
+    module CommonMetrics
+      module PrometheusMetricEnums
+        def self.groups
+          {
+            # built-in groups
+            nginx_ingress_vts: -1,
+            ha_proxy: -2,
+            aws_elb: -3,
+            nginx: -4,
+            kubernetes: -5,
+            nginx_ingress: -6,
+
+            # custom groups
+            business: 0,
+            response: 1,
+            system: 2
+          }
+        end
+
+        def self.group_titles
+          {
+            business: _('Business metrics (Custom)'),
+            response: _('Response metrics (Custom)'),
+            system: _('System metrics (Custom)'),
+            nginx_ingress_vts: _('Response metrics (NGINX Ingress VTS)'),
+            nginx_ingress: _('Response metrics (NGINX Ingress)'),
+            ha_proxy: _('Response metrics (HA Proxy)'),
+            aws_elb: _('Response metrics (AWS ELB)'),
+            nginx: _('Response metrics (NGINX)'),
+            kubernetes: _('System metrics (Kubernetes)')
+          }
+        end
+      end
+    end
+  end
+end

spec/db/importers/common_metrics_importer_spec.rb → spec/lib/gitlab/database_importers/common_metrics/importer_spec.rb

 # frozen_string_literal: true
 
 require 'rails_helper'
-require Rails.root.join("db", "importers", "common_metrics_importer.rb")
 
-describe Importers::PrometheusMetric do
-  let(:existing_group_titles) do
-    ::PrometheusMetric::GROUP_DETAILS.each_with_object({}) do |(key, value), memo|
-      memo[key] = value[:group_title]
-    end
-  end
-
-  it 'group enum equals ::PrometheusMetric' do
-    expect(described_class.groups).to eq(::PrometheusMetric.groups)
-  end
-
-  it 'GROUP_TITLES equals ::PrometheusMetric' do
-    expect(described_class::GROUP_TITLES).to eq(existing_group_titles)
-  end
-end
-
-describe Importers::CommonMetricsImporter do
+describe Gitlab::DatabaseImporters::CommonMetrics::Importer do
   subject { described_class.new }
 
   context "does import common_metrics.yml" do
@@ -104,7 +87,7 @@ describe Importers::CommonMetricsImporter do
       let(:query_identifier) { }
 
       it 'raises exception' do
-        expect { subject.execute }.to raise_error(described_class::MissingQueryId)
+        expect { subject.execute }.to raise_error(Gitlab::DatabaseImporters::CommonMetrics::Importer::MissingQueryId)
       end
     end
 

spec/lib/gitlab/database_importers/common_metrics/prometheus_metric_spec.rb

+# frozen_string_literal: true
+
+require 'rails_helper'
+
+describe Gitlab::DatabaseImporters::CommonMetrics::PrometheusMetric do
+  it 'group enum equals ::PrometheusMetric' do
+    expect(described_class.groups).to eq(::PrometheusMetric.groups)
+  end
+
+  it '.group_titles equals ::PrometheusMetric' do
+    existing_group_titles = ::PrometheusMetricEnums.group_details.each_with_object({}) do |(key, value), memo|
+      memo[key] = value[:group_title]
+    end
+    expect(Gitlab::DatabaseImporters::CommonMetrics::PrometheusMetricEnums.group_titles).to eq(existing_group_titles)
+  end
+end