Merge branch...

Merge branch 'mwaw/212617-generate-panel-ids-based-on-panel-title-and-group-title-in-the-dashboard-yml-file' into 'master'

Generate panel Ids based on panel title and group title in the dashboard yml file

Closes #212617

See merge request gitlab-org/gitlab!28341

app/models/performance_monitoring/prometheus_panel.rb

@@ -4,7 +4,7 @@ module PerformanceMonitoring
   class PrometheusPanel
     include ActiveModel::Model
 
-    attr_accessor :type, :title, :y_label, :weight, :metrics
+    attr_accessor :type, :title, :y_label, :weight, :metrics, :y_axis
 
     validates :title, presence: true
     validates :metrics, presence: true
@@ -20,5 +20,9 @@ module PerformanceMonitoring
 
       panel.tap(&:validate!)
     end
+
+    def id(group_title)
+      Digest::SHA2.hexdigest([group_title, type, title].join)
+    end
   end
 end

app/services/metrics/dashboard/base_service.rb

@@ -11,6 +11,7 @@ module Metrics
       SEQUENCE = [
         STAGES::CommonMetricsInserter,
         STAGES::EndpointInserter,
+        STAGES::PanelIdsInserter,
         STAGES::Sorter
       ].freeze
 

app/services/metrics/dashboard/gitlab_alert_embed_service.rb

@@ -10,7 +10,10 @@ module Metrics
     class GitlabAlertEmbedService < ::Metrics::Dashboard::BaseEmbedService
       include Gitlab::Utils::StrongMemoize
 
-      SEQUENCE = [STAGES::EndpointInserter].freeze
+      SEQUENCE = [
+        STAGES::EndpointInserter,
+        STAGES::PanelIdsInserter
+      ].freeze
 
       class << self
         # Determines whether the provided params are sufficient

app/services/metrics/dashboard/grafana_metric_embed_service.rb

@@ -10,7 +10,8 @@ module Metrics
       include ReactiveCaching
 
       SEQUENCE = [
-        ::Gitlab::Metrics::Dashboard::Stages::GrafanaFormatter
+        ::Gitlab::Metrics::Dashboard::Stages::GrafanaFormatter,
+        ::Gitlab::Metrics::Dashboard::Stages::PanelIdsInserter
       ].freeze
 
       self.reactive_cache_key = ->(service) { service.cache_key }

app/services/metrics/dashboard/predefined_dashboard_service.rb

@@ -11,6 +11,7 @@ module Metrics
 
       SEQUENCE = [
         STAGES::EndpointInserter,
+        STAGES::PanelIdsInserter,
         STAGES::Sorter
       ].freeze
 

app/services/metrics/dashboard/self_monitoring_dashboard_service.rb

@@ -11,6 +11,7 @@ module Metrics
       SEQUENCE = [
         STAGES::CustomMetricsInserter,
         STAGES::EndpointInserter,
+        STAGES::PanelIdsInserter,
         STAGES::Sorter
       ].freeze
 

app/services/metrics/dashboard/system_dashboard_service.rb

@@ -13,6 +13,7 @@ module Metrics
         STAGES::CustomMetricsInserter,
         STAGES::CustomMetricsDetailsInserter,
         STAGES::EndpointInserter,
+        STAGES::PanelIdsInserter,
         STAGES::Sorter
       ].freeze
 

changelogs/unreleased/mwaw-212617-generate-panel-ids-based-on-panel-title-and-group-title-in-th.yml

+---
+title: Automatically assign id to each panel within dashboard to support panel scoped annotations
+merge_request: 28341
+author:
+type: added

ee/app/services/metrics/dashboard/cluster_dashboard_service.rb

@@ -10,6 +10,7 @@ module Metrics
 
       SEQUENCE = [
         STAGES::ClusterEndpointInserter,
+        STAGES::PanelIdsInserter,
         STAGES::Sorter
       ].freeze
 

ee/app/services/metrics/dashboard/cluster_metrics_embed_service.rb

@@ -28,7 +28,8 @@ module Metrics
 
       def sequence
         [
-          STAGES::ClusterEndpointInserter
+          STAGES::ClusterEndpointInserter,
+          STAGES::PanelIdsInserter
         ]
       end
     end

ee/spec/models/performance_monitoring/prometheus_panel_spec.rb

@@ -18,6 +18,16 @@ describe PerformanceMonitoring::PrometheusPanel do
     }
   end
 
+  describe '#new' do
+    it 'accepts old schema format' do
+      expect { described_class.new(json_content) }.not_to raise_error
+    end
+
+    it 'accepts new schema format' do
+      expect { described_class.new(json_content.merge("y_axis" => { "precision" => 0 })) }.not_to raise_error
+    end
+  end
+
   describe '.from_json' do
     subject { described_class.from_json(json_content) }
 
@@ -52,4 +62,15 @@ describe PerformanceMonitoring::PrometheusPanel do
       end
     end
   end
+
+  describe '.id' do
+    it 'returns hexdigest of group_title, type and title as the panel id' do
+      group_title = 'Business Group'
+      panel_type  = 'area-chart'
+      panel_title = 'New feature requests made'
+
+      expect(Digest::SHA2).to receive(:hexdigest).with("#{group_title}#{panel_type}#{panel_title}").and_return('hexdigest')
+      expect(described_class.new(title: panel_title, type: panel_type).id(group_title)).to eql 'hexdigest'
+    end
+  end
 end

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

@@ -37,10 +37,8 @@ module Gitlab
           def for_metrics
             missing_panel_groups! unless dashboard[:panel_groups].is_a?(Array)
 
-            dashboard[:panel_groups].each do |panel_group|
-              missing_panels! unless panel_group[:panels].is_a?(Array)
-
-              panel_group[:panels].each do |panel|
+            for_panel_groups do |panel_group|
+              for_panels_in(panel_group) do |panel|
                 missing_metrics! unless panel[:metrics].is_a?(Array)
 
                 panel[:metrics].each do |metric|
@@ -49,6 +47,20 @@ module Gitlab
               end
             end
           end
+
+          def for_panel_groups
+            dashboard[:panel_groups].each do |panel_group|
+              yield panel_group
+            end
+          end
+
+          def for_panels_in(panel_group)
+            missing_panels! unless panel_group[:panels].is_a?(Array)
+
+            panel_group[:panels].each do |panel|
+              yield panel
+            end
+          end
         end
       end
     end

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

+# frozen_string_literal: true
+
+module Gitlab
+  module Metrics
+    module Dashboard
+      module Stages
+        class PanelIdsInserter < BaseStage
+          # For each panel within given dashboard inserts panel_id unique in scope of the dashboard
+          def transform!
+            missing_panel_groups! unless dashboard[:panel_groups]
+
+            for_panels_group_with_panels do |panel_group, panel|
+              id = generate_panel_id(panel_group, panel)
+              remove_panel_ids! && break if duplicated_panel_id?(id)
+
+              insert_panel_id(id, panel)
+            end
+          end
+
+          private
+
+          def generate_panel_id(group, panel)
+            ::PerformanceMonitoring::PrometheusPanel.new(panel.with_indifferent_access).id(group[:group])
+          end
+
+          def insert_panel_id(id, panel)
+            track_inserted_panel_ids(id, panel)
+            panel[:id] = id
+          end
+
+          def track_inserted_panel_ids(id, panel)
+            panel_ids[id] = panel
+          end
+
+          def duplicated_panel_id?(id)
+            panel_ids.key?(id)
+          end
+
+          def remove_panel_ids!
+            panel_ids.each_value { |panel| panel.delete(:id) }
+          end
+
+          def panel_ids
+            @_panel_ids ||= {}
+          end
+
+          def for_panels_group_with_panels
+            for_panel_groups do |panel_group|
+              for_panels_in(panel_group) do |panel|
+                yield panel_group, panel
+              end
+            end
+          end
+        end
+      end
+    end
+  end
+end

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

@@ -7,6 +7,7 @@
   ],
   "properties": {
     "title": { "type": "string" },
+    "id": { "type": "string" },
     "type": { "type": "string" },
     "y_label": { "type": "string" },
     "y_axis": { "$ref": "axis.json" },

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

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe Gitlab::Metrics::Dashboard::Stages::PanelIdsInserter do
+  let(:project) { build_stubbed(:project) }
+
+  def fetch_panel_ids(dashboard_hash)
+    dashboard_hash[:panel_groups].flat_map { |group| group[:panels].flat_map { |panel| panel[:id] } }
+  end
+
+  describe '#transform!' do
+    subject(:transform!) { described_class.new(project, dashboard, nil).transform! }
+
+    let(:dashboard) { YAML.safe_load(fixture_file('lib/gitlab/metrics/dashboard/sample_dashboard.yml')).deep_symbolize_keys }
+
+    context 'when dashboard panels are present' do
+      it 'assigns unique ids to each panel using PerformanceMonitoring::PrometheusPanel', :aggregate_failures do
+        dashboard.fetch(:panel_groups).each do |group|
+          group.fetch(:panels).each do |panel|
+            panel_double = instance_double(::PerformanceMonitoring::PrometheusPanel)
+
+            expect(::PerformanceMonitoring::PrometheusPanel).to receive(:new).with(panel).and_return(panel_double)
+            expect(panel_double).to receive(:id).with(group[:group]).and_return(FFaker::Lorem.unique.characters(125))
+          end
+        end
+
+        transform!
+
+        expect(fetch_panel_ids(dashboard)).not_to include nil
+      end
+    end
+
+    context 'when dashboard panels has duplicated ids' do
+      it 'no panel has assigned id' do
+        panel_double = instance_double(::PerformanceMonitoring::PrometheusPanel)
+        allow(::PerformanceMonitoring::PrometheusPanel).to receive(:new).and_return(panel_double)
+        allow(panel_double).to receive(:id).and_return('duplicated id')
+
+        transform!
+
+        expect(fetch_panel_ids(dashboard)).to all be_nil
+        expect(fetch_panel_ids(dashboard)).not_to include 'duplicated id'
+      end
+    end
+
+    context 'when there are no panels in the dashboard' do
+      it 'raises a processing error' do
+        dashboard[:panel_groups][0].delete(:panels)
+
+        expect { transform! }.to(
+          raise_error(::Gitlab::Metrics::Dashboard::Errors::DashboardProcessingError)
+        )
+      end
+    end
+
+    context 'when there are no panel_groups in the dashboard' do
+      it 'raises a processing error' do
+        dashboard.delete(:panel_groups)
+
+        expect { transform! }.to(
+          raise_error(::Gitlab::Metrics::Dashboard::Errors::DashboardProcessingError)
+        )
+      end
+    end
+  end
+end