Refactor common metrics importer

This refactors common metrics importer for two reasons. 1. To create a
new pattern that can be followed by future importers that will minimize
dependency collision and 2. To allow EE to more easily extend
dependencies.

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,7 +60,7 @@ 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
+
+PrometheusMetricEnums.prepend(EE::PrometheusMetricEnums)

db/importers/common_metrics.rb

+require_relative './common_metrics/importer'
+require_relative './common_metrics/prometheus_metric'
+require_relative './common_metrics/prometheus_metric_enums'
+
+require Rails.root.join('ee', 'db', 'importers', 'common_metrics') if Gitlab.ee?
+
+module Importers
+  module CommonMetrics
+  end
+
+  # Patch to preserve old CommonMetricsImporter api
+  module CommonMetricsImporter
+    def self.new(*args)
+      Importers::CommonMetrics::Importer.new(*args)
+    end
+  end
+end
+
+Importers::CommonMetrics.prepend(EE::Importers::CommonMetrics)

db/importers/common_metrics/importer.rb

+# frozen_string_literal: true
+
+module Importers
+  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
+        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)
+        PrometheusMetricEnums.groups[find_group_title(title)]
+      end
+
+      def find_group_title(title)
+        PrometheusMetricEnums.group_titles.invert[title]
+      end
+    end
+  end
+end

db/importers/common_metrics/prometheus_metric.rb

+module Importers
+  module CommonMetrics
+    class PrometheusMetric < ActiveRecord::Base
+      enum group: PrometheusMetricEnums.groups
+      scope :common, -> { where(common: true) }
+    end
+  end
+end

db/importers/common_metrics/prometheus_metric_enums.rb

+module Importers
+  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

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
+# This functionality has been moved to the common_metrics module.
+# This is here only to preserve existing ::Importers::CommonMetricsImporter api
+require_relative './common_metrics'

ee/app/models/ee/prometheus_metric_enums.rb

+# frozen_string_literal: true
+
+module EE
+  module PrometheusMetricEnums
+    extend ActiveSupport::Concern
+
+    class_methods do
+      extend ::Gitlab::Utils::Override
+
+      override :groups
+      def groups
+        super.merge(
+          # Start at 100 to avoid collisions with CE values
+          cluster_health: -100
+        )
+      end
+
+      override :group_details
+      def group_details
+        super.merge(
+          # keys can collide with CE values! please ensure you are not redefining a key that already exists in app/models/prometheus_metric_enums.rb#group_details
+          cluster_health: {
+            group_title: _('Cluster Health'),
+            required_metrics: %w(container_memory_usage_bytes container_cpu_usage_seconds_total),
+            priority: 10
+          }
+        )
+      end
+    end
+  end
+end

ee/db/importers/common_metrics.rb

+require_relative './common_metrics/prometheus_metric_enums'
+
+module EE
+  module Importers
+    module CommonMetrics
+      # rubocop: disable Cop/InjectEnterpriseEditionModule
+      def self.prepended(base)
+        ::Importers::CommonMetrics::PrometheusMetricEnums.prepend EE::Importers::CommonMetrics::PrometheusMetricEnums
+      end
+    end
+  end
+end

ee/db/importers/common_metrics/prometheus_metric_enums.rb

+module EE
+  module Importers
+    module CommonMetrics
+      module PrometheusMetricEnums
+        extend ActiveSupport::Concern
+
+        class_methods do
+          extend ::Gitlab::Utils::Override
+
+          override :groups
+          def groups
+            super.merge(
+              # Start at 100 to avoid collisions with CE values
+              cluster_healths: -100
+            )
+          end
+
+          override :group_titles
+          def group_titles
+            super.merge(
+              # keys can collide with CE values! please ensure you are not redefining a key that already exists
+              cluster_health: _('Cluster Health')
+            )
+          end
+        end
+      end
+    end
+  end
+end

ee/spec/models/prometheus_metric_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe PrometheusMetric do
+  subject { build(:prometheus_metric) }
+
+  describe '#group_title' do
+    shared_examples 'group_title' do |group, title|
+      subject { build(:prometheus_metric, group: group).group_title }
+
+      it "returns text #{title} for group #{group}" do
+        expect(subject).to eq(title)
+      end
+    end
+
+    it_behaves_like 'group_title', :cluster_health, 'Cluster Health'
+  end
+
+  describe '#priority' do
+    using RSpec::Parameterized::TableSyntax
+
+    where(:group, :priority) do
+      :cluster_health | 10
+    end
+
+    with_them do
+      before do
+        subject.group = group
+      end
+
+      it { expect(subject.priority).to eq(priority) }
+    end
+  end
+
+  describe '#required_metrics' do
+    using RSpec::Parameterized::TableSyntax
+
+    where(:group, :required_metrics) do
+      :cluster_health | %w(container_memory_usage_bytes container_cpu_usage_seconds_total)
+    end
+
+    with_them do
+      before do
+        subject.group = group
+      end
+
+      it { expect(subject.required_metrics).to eq(required_metrics) }
+    end
+  end
+end

locale/gitlab.pot

@@ -2873,6 +2873,9 @@ msgstr ""
 msgid "Closed issues"
 msgstr ""
 
+msgid "Cluster Health"
+msgstr ""
+
 msgid "ClusterIntegration| %{custom_domain_start}More information%{custom_domain_end}."
 msgstr ""
 

spec/db/importers/common_metrics_importer_spec.rb

@@ -3,9 +3,9 @@
 require 'rails_helper'
 require Rails.root.join("db", "importers", "common_metrics_importer.rb")
 
-describe Importers::PrometheusMetric do
+describe Importers::CommonMetrics::PrometheusMetric do
   let(:existing_group_titles) do
-    ::PrometheusMetric::GROUP_DETAILS.each_with_object({}) do |(key, value), memo|
+    ::PrometheusMetricEnums.group_details.each_with_object({}) do |(key, value), memo|
       memo[key] = value[:group_title]
     end
   end
@@ -15,7 +15,7 @@ describe Importers::PrometheusMetric do
   end
 
   it 'GROUP_TITLES equals ::PrometheusMetric' do
-    expect(described_class::GROUP_TITLES).to eq(existing_group_titles)
+    expect(Importers::CommonMetrics::PrometheusMetricEnums.group_titles).to eq(existing_group_titles)
   end
 end
 
@@ -104,7 +104,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(Importers::CommonMetrics::Importer::MissingQueryId)
       end
     end