Merge branch '209243-remove-priority-weight' into 'master'

Stop using weight and priority keys in metrics dashboards

See merge request gitlab-org/gitlab!38572

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

doc/operations/metrics/dashboards/yaml.md

@@ -43,16 +43,27 @@ Read the documentation on [links](index.md#add-related-links-to-custom-dashboard
 
 ## **Panel group (`panel_groups`) properties**
 
+Dashboards display panel groups in the order they are listed in the dashboard YAML file.
+
+NOTE: **Note:**
+In GitLab versions 13.3 and below, panel groups were ordered by a `priority` key, which
+is no longer used.
+
 | Property | Type | Required | Description |
 | ------ | ------ | ------ | ------ |
 | `group` | string | required | Heading for the panel group. |
-| `priority` | number | optional, defaults to order in file | Order to appear on the dashboard. Higher number means higher priority, which will be higher on the page. Numbers do not need to be consecutive. |
 | `panels` | array | required | The panels which should be in the panel group. |
 
 Panels in a panel group are laid out in rows consisting of two panels per row. An exception to this rule are single panels on a row: these panels will take the full width of their containing row.
 
 ## **Panel (`panels`) properties**
 
+Dashboards display panels in the order they are listed in the dashboard YAML file.
+
+NOTE: **Note:**
+In GitLab versions 13.3 and below, panels were ordered by a `weight` key, which
+is no longer used.
+
 | Property | Type | Required | Description |
 | ------ | ------ | ------ | ------- |
 | `type` | string | no, defaults to `area-chart` | Specifies the panel type to use, for example `area-chart`, `line-chart` or `anomaly-chart`. Only types listed among [all panel types](panel_types.md) are allowed. |
@@ -60,7 +71,6 @@ Panels in a panel group are laid out in rows consisting of two panels per row. A
 | `y_label` | string | no, but highly encouraged | Y-Axis label for the panel. |
 | `y_axis` | string | no | Y-Axis configuration for the panel. |
 | `max_value` | number | no | Denominator value used for calculating [percentile based results](panel_types.md#percentile-based-results) |
-| `weight` | number | no, defaults to order in file | Order to appear within the grouping. Lower number means higher priority, which will be higher on the page. Numbers do not need to be consecutive. |
 | `metrics` | array | yes | The metrics which should be displayed in the panel. Any number of metrics can be displayed when `type` is `area-chart` or `line-chart`, whereas only 3 can be displayed when `type` is `anomaly-chart`. |
 | `links` | array | no | Add links to display on the chart's [context menu](index.md#chart-context-menu). |
 

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
 

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',