Commit cf200b64 authored by Kamil Trzciński's avatar Kamil Trzciński

Merge branch '51970-correct-ordering-of-metrics' into 'master'

Correct the ordering of metrics on performance dashboard

Closes #51970

See merge request gitlab-org/gitlab-ce!23630
parents 11f8a97f 2cd7b783
...@@ -18,6 +18,54 @@ class PrometheusMetric < ActiveRecord::Base ...@@ -18,6 +18,54 @@ class PrometheusMetric < ActiveRecord::Base
system: 2 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
validates :title, presence: true validates :title, presence: true
validates :query, presence: true validates :query, presence: true
validates :group, presence: true validates :group, presence: true
...@@ -29,36 +77,16 @@ class PrometheusMetric < ActiveRecord::Base ...@@ -29,36 +77,16 @@ class PrometheusMetric < ActiveRecord::Base
scope :common, -> { where(common: true) } scope :common, -> { where(common: true) }
GROUP_TITLES = { def priority
# built-in groups group_details(group).fetch(:priority)
nginx_ingress_vts: _('Response metrics (NGINX Ingress VTS)'), end
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)'),
# custom/user groups
business: _('Business metrics (Custom)'),
response: _('Response metrics (Custom)'),
system: _('System metrics (Custom)')
}.freeze
REQUIRED_METRICS = {
nginx_ingress_vts: %w(nginx_upstream_responses_total nginx_upstream_response_msecs_avg),
nginx_ingress: %w(nginx_ingress_controller_requests nginx_ingress_controller_ingress_upstream_latency_seconds_sum),
ha_proxy: %w(haproxy_frontend_http_requests_total haproxy_frontend_http_responses_total),
aws_elb: %w(aws_elb_request_count_sum aws_elb_latency_average aws_elb_httpcode_backend_5_xx_sum),
nginx: %w(nginx_server_requests nginx_server_requestMsec),
kubernetes: %w(container_memory_usage_bytes container_cpu_usage_seconds_total)
}.freeze
def group_title def group_title
GROUP_TITLES[group.to_sym] group_details(group).fetch(:group_title)
end end
def required_metrics def required_metrics
REQUIRED_METRICS[group.to_sym].to_a.map(&:to_s) group_details(group).fetch(:required_metrics, []).map(&:to_s)
end end
def to_query_metric def to_query_metric
...@@ -89,4 +117,10 @@ class PrometheusMetric < ActiveRecord::Base ...@@ -89,4 +117,10 @@ class PrometheusMetric < ActiveRecord::Base
}] }]
end end
end end
private
def group_details(group)
GROUP_DETAILS.fetch(group.to_sym)
end
end end
---
title: Correct the ordering of metrics on the performance dashboard
merge_request: 23630
author:
type: fixed
...@@ -10,9 +10,15 @@ module Gitlab ...@@ -10,9 +10,15 @@ module Gitlab
validates :name, :priority, :metrics, presence: true validates :name, :priority, :metrics, presence: true
def self.common_metrics def self.common_metrics
::PrometheusMetric.common.group_by(&:group_title).map do |name, metrics| all_groups = ::PrometheusMetric.common.group_by(&:group_title).map do |name, metrics|
MetricGroup.new(name: name, priority: 0, metrics: metrics.map(&:to_query_metric)) MetricGroup.new(
name: name,
priority: metrics.map(&:priority).max,
metrics: metrics.map(&:to_query_metric)
)
end end
all_groups.sort_by(&:priority).reverse
end end
# EE only # EE only
......
...@@ -4,12 +4,18 @@ require 'rails_helper' ...@@ -4,12 +4,18 @@ require 'rails_helper'
require Rails.root.join("db", "importers", "common_metrics_importer.rb") require Rails.root.join("db", "importers", "common_metrics_importer.rb")
describe Importers::PrometheusMetric do 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 it 'group enum equals ::PrometheusMetric' do
expect(described_class.groups).to eq(::PrometheusMetric.groups) expect(described_class.groups).to eq(::PrometheusMetric.groups)
end end
it 'GROUP_TITLES equals ::PrometheusMetric' do it 'GROUP_TITLES equals ::PrometheusMetric' do
expect(described_class::GROUP_TITLES).to eq(::PrometheusMetric::GROUP_TITLES) expect(described_class::GROUP_TITLES).to eq(existing_group_titles)
end end
end end
......
...@@ -21,6 +21,13 @@ describe Gitlab::Prometheus::MetricGroup do ...@@ -21,6 +21,13 @@ describe Gitlab::Prometheus::MetricGroup do
common_metric_group_a.id, common_metric_group_b_q1.id, common_metric_group_a.id, common_metric_group_b_q1.id,
common_metric_group_b_q2.id) common_metric_group_b_q2.id)
end end
it 'orders by priority' do
priorities = subject.map(&:priority)
names = subject.map(&:name)
expect(priorities).to eq([10, 5])
expect(names).to eq(['Response metrics (AWS ELB)', 'System metrics (Kubernetes)'])
end
end end
describe '.for_project' do describe '.for_project' do
......
...@@ -59,11 +59,65 @@ describe PrometheusMetric do ...@@ -59,11 +59,65 @@ describe PrometheusMetric do
end end
end end
it_behaves_like 'group_title', :nginx_ingress_vts, 'Response metrics (NGINX Ingress VTS)'
it_behaves_like 'group_title', :nginx_ingress, 'Response metrics (NGINX Ingress)'
it_behaves_like 'group_title', :ha_proxy, 'Response metrics (HA Proxy)'
it_behaves_like 'group_title', :aws_elb, 'Response metrics (AWS ELB)'
it_behaves_like 'group_title', :nginx, 'Response metrics (NGINX)'
it_behaves_like 'group_title', :kubernetes, 'System metrics (Kubernetes)'
it_behaves_like 'group_title', :business, 'Business metrics (Custom)' it_behaves_like 'group_title', :business, 'Business metrics (Custom)'
it_behaves_like 'group_title', :response, 'Response metrics (Custom)' it_behaves_like 'group_title', :response, 'Response metrics (Custom)'
it_behaves_like 'group_title', :system, 'System metrics (Custom)' it_behaves_like 'group_title', :system, 'System metrics (Custom)'
end end
describe '#priority' do
using RSpec::Parameterized::TableSyntax
where(:group, :priority) do
:nginx_ingress_vts | 10
:nginx_ingress | 10
:ha_proxy | 10
:aws_elb | 10
:nginx | 10
:kubernetes | 5
:business | 0
:response | -5
:system | -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
:nginx_ingress_vts | %w(nginx_upstream_responses_total nginx_upstream_response_msecs_avg)
:nginx_ingress | %w(nginx_ingress_controller_requests nginx_ingress_controller_ingress_upstream_latency_seconds_sum)
:ha_proxy | %w(haproxy_frontend_http_requests_total haproxy_frontend_http_responses_total)
:aws_elb | %w(aws_elb_request_count_sum aws_elb_latency_average aws_elb_httpcode_backend_5_xx_sum)
:nginx | %w(nginx_server_requests nginx_server_requestMsec)
:kubernetes | %w(container_memory_usage_bytes container_cpu_usage_seconds_total)
:business | %w()
:response | %w()
:system | %w()
end
with_them do
before do
subject.group = group
end
it { expect(subject.required_metrics).to eq(required_metrics) }
end
end
describe '#to_query_metric' do describe '#to_query_metric' do
it 'converts to queryable metric object' do it 'converts to queryable metric object' do
expect(subject.to_query_metric).to be_instance_of(Gitlab::Prometheus::Metric) expect(subject.to_query_metric).to be_instance_of(Gitlab::Prometheus::Metric)
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment