From 9aad4174e052ba330fdaf4abc0276d8497c7de03 Mon Sep 17 00:00:00 2001 From: Ryan Cobb Date: Thu, 27 Jun 2019 12:11:55 -0700 Subject: [PATCH] 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 | 65 +---------- app/models/prometheus_metric_enums.rb | 70 ++++++++++++ db/importers/common_metrics.rb | 17 +++ db/importers/common_metrics/importer.rb | 76 ++++++++++++ .../common_metrics/prometheus_metric.rb | 8 ++ .../common_metrics/prometheus_metric_enums.rb | 36 ++++++ db/importers/common_metrics_importer.rb | 108 +----------------- .../importers/common_metrics_importer_spec.rb | 8 +- 8 files changed, 216 insertions(+), 172 deletions(-) create mode 100644 app/models/prometheus_metric_enums.rb create mode 100644 db/importers/common_metrics.rb create mode 100644 db/importers/common_metrics/importer.rb create mode 100644 db/importers/common_metrics/prometheus_metric.rb create mode 100644 db/importers/common_metrics/prometheus_metric_enums.rb diff --git a/app/models/prometheus_metric.rb b/app/models/prometheus_metric.rb index 62090444f79..b8e7673dcf5 100644 --- a/app/models/prometheus_metric.rb +++ b/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 diff --git a/app/models/prometheus_metric_enums.rb b/app/models/prometheus_metric_enums.rb new file mode 100644 index 00000000000..6cb22cc69cd --- /dev/null +++ b/app/models/prometheus_metric_enums.rb @@ -0,0 +1,70 @@ +# 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 diff --git a/db/importers/common_metrics.rb b/db/importers/common_metrics.rb new file mode 100644 index 00000000000..411d366ef83 --- /dev/null +++ b/db/importers/common_metrics.rb @@ -0,0 +1,17 @@ +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 diff --git a/db/importers/common_metrics/importer.rb b/db/importers/common_metrics/importer.rb new file mode 100644 index 00000000000..24149fbbea6 --- /dev/null +++ b/db/importers/common_metrics/importer.rb @@ -0,0 +1,76 @@ +# 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 diff --git a/db/importers/common_metrics/prometheus_metric.rb b/db/importers/common_metrics/prometheus_metric.rb new file mode 100644 index 00000000000..9149549c750 --- /dev/null +++ b/db/importers/common_metrics/prometheus_metric.rb @@ -0,0 +1,8 @@ +module Importers + module CommonMetrics + class PrometheusMetric < ActiveRecord::Base + enum group: PrometheusMetricEnums.groups + scope :common, -> { where(common: true) } + end + end +end diff --git a/db/importers/common_metrics/prometheus_metric_enums.rb b/db/importers/common_metrics/prometheus_metric_enums.rb new file mode 100644 index 00000000000..50a1081a629 --- /dev/null +++ b/db/importers/common_metrics/prometheus_metric_enums.rb @@ -0,0 +1,36 @@ +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 diff --git a/db/importers/common_metrics_importer.rb b/db/importers/common_metrics_importer.rb index 195bde8f34a..cf5f5e181de 100644 --- a/db/importers/common_metrics_importer.rb +++ b/db/importers/common_metrics_importer.rb @@ -1,105 +1,3 @@ -# 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' diff --git a/spec/db/importers/common_metrics_importer_spec.rb b/spec/db/importers/common_metrics_importer_spec.rb index a717c8cd04d..c47effa6803 100644 --- a/spec/db/importers/common_metrics_importer_spec.rb +++ b/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