Stop using weight and priority keys in metrics dashboards

Stop using weight and priority keys in metrics dashboards. Panels and
groups will now appear on dashboards in the order they are defined.
This should make the order of panels in a dashboard easier to predict
and understand.

app/services/metrics/dashboard/base_service.rb

@@ -13,7 +13,6 @@ module Metrics
         STAGES::MetricEndpointInserter,
         STAGES::VariableEndpointInserter,
         STAGES::PanelIdsInserter,
-        STAGES::Sorter,
         STAGES::AlertsInserter,
         STAGES::UrlValidator
       ].freeze

app/services/metrics/dashboard/clone_dashboard_service.rb

@@ -13,8 +13,7 @@ module Metrics
       SEQUENCES = {
         ::Metrics::Dashboard::SystemDashboardService::DASHBOARD_PATH => [
           ::Gitlab::Metrics::Dashboard::Stages::CommonMetricsInserter,
-          ::Gitlab::Metrics::Dashboard::Stages::CustomMetricsInserter,
-          ::Gitlab::Metrics::Dashboard::Stages::Sorter
+          ::Gitlab::Metrics::Dashboard::Stages::CustomMetricsInserter
         ].freeze,
 
         ::Metrics::Dashboard::SelfMonitoringDashboardService::DASHBOARD_PATH => [
@@ -22,8 +21,7 @@ module Metrics
         ].freeze,
 
         ::Metrics::Dashboard::ClusterDashboardService::DASHBOARD_PATH => [
-          ::Gitlab::Metrics::Dashboard::Stages::CommonMetricsInserter,
-          ::Gitlab::Metrics::Dashboard::Stages::Sorter
+          ::Gitlab::Metrics::Dashboard::Stages::CommonMetricsInserter
         ].freeze
       }.freeze
 

app/services/metrics/dashboard/cluster_dashboard_service.rb

@@ -13,8 +13,7 @@ module Metrics
 
       SEQUENCE = [
         STAGES::ClusterEndpointInserter,
-        STAGES::PanelIdsInserter,
-        STAGES::Sorter
+        STAGES::PanelIdsInserter
       ].freeze
 
       class << self

app/services/metrics/dashboard/custom_metric_embed_service.rb

@@ -75,7 +75,6 @@ module Metrics
       def panels
         [{
           type: DEFAULT_PANEL_TYPE,
-          weight: DEFAULT_PANEL_WEIGHT,
           title: title,
           y_label: y_label,
           metrics: metrics.map(&:to_metric_hash)

app/services/metrics/dashboard/pod_dashboard_service.rb

@@ -12,8 +12,7 @@ module Metrics
       SEQUENCE = [
         STAGES::MetricEndpointInserter,
         STAGES::VariableEndpointInserter,
-        STAGES::PanelIdsInserter,
-        STAGES::Sorter
+        STAGES::PanelIdsInserter
       ].freeze
 
       class << self

app/services/metrics/dashboard/predefined_dashboard_service.rb

@@ -12,8 +12,7 @@ module Metrics
       SEQUENCE = [
         STAGES::MetricEndpointInserter,
         STAGES::VariableEndpointInserter,
-        STAGES::PanelIdsInserter,
-        STAGES::Sorter
+        STAGES::PanelIdsInserter
       ].freeze
 
       class << self

app/services/metrics/dashboard/self_monitoring_dashboard_service.rb

@@ -15,8 +15,7 @@ module Metrics
         STAGES::CustomMetricsInserter,
         STAGES::MetricEndpointInserter,
         STAGES::VariableEndpointInserter,
-        STAGES::PanelIdsInserter,
-        STAGES::Sorter
+        STAGES::PanelIdsInserter
       ].freeze
 
       class << self

app/services/metrics/dashboard/system_dashboard_service.rb

@@ -18,7 +18,6 @@ module Metrics
         STAGES::MetricEndpointInserter,
         STAGES::VariableEndpointInserter,
         STAGES::PanelIdsInserter,
-        STAGES::Sorter,
         STAGES::AlertsInserter
       ].freeze
 

changelogs/unreleased/209243-remove-priority-weight.yml

+---
+title: Stop using priority and weight keys in metrics dashboards
+merge_request: 38572
+author:
+type: changed

lib/gitlab/metrics/dashboard/defaults.rb

@@ -7,7 +7,6 @@ module Gitlab
     module Dashboard
       module Defaults
         DEFAULT_PANEL_TYPE = 'area-chart'
-        DEFAULT_PANEL_WEIGHT = 0
       end
     end
   end

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

@@ -9,7 +9,10 @@ module Gitlab
           # config. If there are no project-specific metrics,
           # this will have no effect.
           def transform!
-            PrometheusMetricsFinder.new(project: project).execute.each do |project_metric|
+            custom_metrics = PrometheusMetricsFinder.new(project: project, ordered: true).execute
+            custom_metrics = Gitlab::Utils.stable_sort_by(custom_metrics) { |metric| -metric.priority }
+
+            custom_metrics.each do |project_metric|
               group = find_or_create_panel_group(dashboard[:panel_groups], project_metric)
               panel = find_or_create_panel(group[:panels], project_metric)
               find_or_create_metric(panel[:metrics], project_metric)
@@ -83,7 +86,6 @@ module Gitlab
           def new_panel_group(metric)
             {
               group: metric.group_title,
-              priority: metric.priority,
               panels: []
             }
           end

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

-# frozen_string_literal: true
-
-module Gitlab
-  module Metrics
-    module Dashboard
-      module Stages
-        class Sorter < BaseStage
-          def transform!
-            missing_panel_groups! unless dashboard[:panel_groups].is_a? Array
-
-            sort_groups!
-            sort_panels!
-          end
-
-          private
-
-          # Sorts the groups in the dashboard by the :priority key
-          def sort_groups!
-            dashboard[:panel_groups] = Gitlab::Utils.stable_sort_by(dashboard[:panel_groups]) { |group| -group[:priority].to_i }
-          end
-
-          # Sorts the panels in the dashboard by the :weight key
-          def sort_panels!
-            dashboard[:panel_groups].each do |group|
-              missing_panels! unless group[:panels].is_a? Array
-
-              group[:panels] = Gitlab::Utils.stable_sort_by(group[:panels]) { |panel| -panel[:weight].to_i }
-            end
-          end
-        end
-      end
-    end
-  end
-end

spec/fixtures/lib/gitlab/metrics/dashboard/sample_dashboard.yml

@@ -31,13 +31,29 @@ templating:
         series_selector: 'backend:haproxy_backend_availability:ratio{env="{{env}}"}'
         label: 'backend'
 panel_groups:
+- group: Group B
+  panels:
+  - title: "Super Chart B"
+    type: "area-chart"
+    y_label: "y_label"
+    metrics:
+    - id: metric_b
+      query_range: 'query'
+      unit: unit
+      label: Legend Label
 - group: Group A
-  priority: 1
   panels:
+  - title: "Super Chart A2"
+    type: "area-chart"
+    y_label: "y_label"
+    metrics:
+    - id: metric_a2
+      query_range: 'query'
+      label: Legend Label
+      unit: unit
   - title: "Super Chart A1"
     type: "area-chart"
     y_label: "y_label"
-    weight: 1
     max_value: 1
     metrics:
     - id: metric_a1
@@ -54,24 +70,3 @@ panel_groups:
         /1024/1024/1024
       unit: unit
       label: Legend Label
-  - title: "Super Chart A2"
-    type: "area-chart"
-    y_label: "y_label"
-    weight: 2
-    metrics:
-    - id: metric_a2
-      query_range: 'query'
-      label: Legend Label
-      unit: unit
-- group: Group B
-  priority: 10
-  panels:
-  - title: "Super Chart B"
-    type: "area-chart"
-    y_label: "y_label"
-    weight: 1
-    metrics:
-    - id: metric_b
-      query_range: 'query'
-      unit: unit
-      label: Legend Label

spec/fixtures/lib/gitlab/metrics/dashboard/schemas/panel_groups.json

@@ -6,7 +6,6 @@
   ],
   "properties": {
     "group": { "type": "string" },
-    "priority": { "type": "number" },
     "panels": {
       "type": "array",
       "items": { "$ref": "panels.json" }

spec/fixtures/lib/gitlab/metrics/dashboard/schemas/panels.json

@@ -12,7 +12,6 @@
     "y_label": { "type": "string" },
     "y_axis": { "$ref": "axis.json" },
     "max_value": { "type": "number" },
-    "weight": { "type": "number" },
     "metrics": {
       "type": "array",
       "items": { "$ref": "metrics.json" }

spec/lib/gitlab/metrics/dashboard/defaults_spec.rb

@@ -4,5 +4,4 @@ require 'spec_helper'
 
 RSpec.describe Gitlab::Metrics::Dashboard::Defaults do
   it { is_expected.to be_const_defined(:DEFAULT_PANEL_TYPE) }
-  it { is_expected.to be_const_defined(:DEFAULT_PANEL_WEIGHT) }
 end

spec/lib/gitlab/metrics/dashboard/processor_spec.rb

@@ -16,7 +16,6 @@ RSpec.describe Gitlab::Metrics::Dashboard::Processor do
         Gitlab::Metrics::Dashboard::Stages::CustomMetricsInserter,
         Gitlab::Metrics::Dashboard::Stages::CustomMetricsDetailsInserter,
         Gitlab::Metrics::Dashboard::Stages::MetricEndpointInserter,
-        Gitlab::Metrics::Dashboard::Stages::Sorter,
         Gitlab::Metrics::Dashboard::Stages::AlertsInserter,
         Gitlab::Metrics::Dashboard::Stages::PanelIdsInserter,
         Gitlab::Metrics::Dashboard::Stages::UrlValidator
@@ -66,14 +65,14 @@ RSpec.describe Gitlab::Metrics::Dashboard::Processor do
         expect(all_metrics).to include get_metric_details(project_business_metric)
       end
 
-      it 'orders groups by priority and panels by weight' do
+      it 'display groups and panels in the order they are defined' do
         expected_metrics_order = [
-          'metric_b', # group priority 10, panel weight 1
-          'metric_a2', # group priority 1, panel weight 2
-          'metric_a1', # group priority 1, panel weight 1
-          project_business_metric.id, # group priority 0, panel weight nil (0)
-          project_response_metric.id, # group priority -5, panel weight nil (0)
-          project_system_metric.id # group priority -10, panel weight nil (0)
+          'metric_b',
+          'metric_a2',
+          'metric_a1',
+          project_business_metric.id,
+          project_response_metric.id,
+          project_system_metric.id
         ]
         actual_metrics_order = all_metrics.map { |m| m[:id] || m[:metric_id] }
 
@@ -94,8 +93,7 @@ RSpec.describe Gitlab::Metrics::Dashboard::Processor do
         let(:sequence) do
           [
             Gitlab::Metrics::Dashboard::Stages::CommonMetricsInserter,
-            Gitlab::Metrics::Dashboard::Stages::MetricEndpointInserter,
-            Gitlab::Metrics::Dashboard::Stages::Sorter
+            Gitlab::Metrics::Dashboard::Stages::MetricEndpointInserter
           ]
         end
         let(:dashboard) { described_class.new(*process_params).process }

spec/lib/gitlab/metrics/dashboard/stages/metric_endpoint_inserter_spec.rb

@@ -20,7 +20,7 @@ RSpec.describe Gitlab::Metrics::Dashboard::Stages::MetricEndpointInserter do
 
       transform!
 
-      expect(all_metrics[0][:prometheus_endpoint_path]).to eq(prometheus_path(query))
+      expect(all_metrics[2][:prometheus_endpoint_path]).to eq(prometheus_path(query))
     end
 
     it 'includes a path for the prometheus endpoint with each metric' do

spec/models/prometheus_metric_spec.rb

@@ -138,10 +138,6 @@ RSpec.describe PrometheusMetric do
       expect(subject.to_query_metric.required_metrics).to eq([])
     end
 
-    it 'queryable metric has weight 0' do
-      expect(subject.to_query_metric.weight).to eq(0)
-    end
-
     it 'queryable metrics has query description' do
       queries = [
         {

spec/services/metrics/dashboard/clone_dashboard_service_spec.rb

@@ -84,14 +84,12 @@ RSpec.describe Metrics::Dashboard::CloneDashboardService, :use_clean_rails_memor
         it_behaves_like 'valid dashboard cloning process', ::Metrics::Dashboard::SystemDashboardService::DASHBOARD_PATH,
                         [
                           ::Gitlab::Metrics::Dashboard::Stages::CommonMetricsInserter,
-                          ::Gitlab::Metrics::Dashboard::Stages::CustomMetricsInserter,
-                          ::Gitlab::Metrics::Dashboard::Stages::Sorter
+                          ::Gitlab::Metrics::Dashboard::Stages::CustomMetricsInserter
                         ]
 
         it_behaves_like 'valid dashboard cloning process', ::Metrics::Dashboard::ClusterDashboardService::DASHBOARD_PATH,
                         [
-                          ::Gitlab::Metrics::Dashboard::Stages::CommonMetricsInserter,
-                          ::Gitlab::Metrics::Dashboard::Stages::Sorter
+                          ::Gitlab::Metrics::Dashboard::Stages::CommonMetricsInserter
                         ]
 
         it_behaves_like 'valid dashboard cloning process',