Merge branch 'rp-measure-dashboard-processing-time' into 'master'

Measure custom dashboard processing time

See merge request gitlab-org/gitlab!27875

app/services/metrics/dashboard/base_service.rb

@@ -45,11 +45,32 @@ module Metrics
 
       # Returns a new dashboard Hash, supplemented with DB info
       def process_dashboard
-        ::Gitlab::Metrics::Dashboard::Processor
-          .new(project, raw_dashboard, sequence, process_params)
+        # Get the dashboard from cache/disk before beginning the benchmark.
+        dashboard = raw_dashboard
+        processed_dashboard = nil
+
+        benchmark_processing do
+          processed_dashboard = ::Gitlab::Metrics::Dashboard::Processor
+            .new(project, dashboard, sequence, process_params)
             .process
         end
 
+        processed_dashboard
+      end
+
+      def benchmark_processing
+        output = nil
+
+        processing_time_seconds = Benchmark.realtime { output = yield }
+
+        if output
+          processing_time_metric.observe(
+            processing_time_metric_labels,
+            processing_time_seconds * 1_000
+          )
+        end
+      end
+
       def process_params
         params
       end
@@ -72,6 +93,26 @@ module Metrics
       def sequence
         SEQUENCE
       end
+
+      def processing_time_metric
+        @processing_time_metric ||= ::Gitlab::Metrics.summary(
+          :gitlab_metrics_dashboard_processing_time_ms,
+          'Metrics dashboard processing time in milliseconds'
+        )
+      end
+
+      def processing_time_metric_labels
+        {
+          stages: sequence_string,
+          service: self.class.name
+        }
+      end
+
+      # If @sequence is [STAGES::CommonMetricsInserter, STAGES::CustomMetricsInserter],
+      # this function will output `CommonMetricsInserter-CustomMetricsInserter`.
+      def sequence_string
+        sequence.map { |stage_class| stage_class.to_s.split('::').last }.join('-')
+      end
     end
   end
 end

doc/administration/monitoring/prometheus/gitlab_metrics.md

@@ -92,6 +92,7 @@ The following metrics are available:
 | `failed_login_captcha_total`                                   | Gauge     |                   11.0 | Counter of failed CAPTCHA attempts during login                                                     |                                                     |
 | `successful_login_captcha_total`                               | Gauge     |                   11.0 | Counter of successful CAPTCHA attempts during login                                                 |                                                     |
 | `auto_devops_pipelines_completed_total`                        | Counter   |                   12.7 | Counter of completed Auto DevOps pipelines, labeled by status                                       |                                                     |
+| `gitlab_metrics_dashboard_processing_time_ms` | Summary | 12.10 | Metrics dashboard processing time in milliseconds | service, stages |
 
 ## Metrics controlled by a feature flag
 

spec/services/metrics/dashboard/custom_dashboard_service_spec.rb

@@ -16,10 +16,20 @@ describe Metrics::Dashboard::CustomDashboardService, :use_clean_rails_memory_sto
   describe '#get_dashboard' do
     let(:dashboard_path) { '.gitlab/dashboards/test.yml' }
     let(:service_params) { [project, user, { environment: environment, dashboard_path: dashboard_path }] }
-    let(:service_call) { described_class.new(*service_params).get_dashboard }
+    let(:service_call) { subject.get_dashboard }
+
+    subject { described_class.new(*service_params) }
 
     context 'when the dashboard does not exist' do
       it_behaves_like 'misconfigured dashboard service response', :not_found
+
+      it 'does not update gitlab_metrics_dashboard_processing_time_ms metric', :prometheus do
+        service_call
+        metric = subject.send(:processing_time_metric)
+        labels = subject.send(:processing_time_metric_labels)
+
+        expect(metric.get(labels)).to eq(0)
+      end
     end
 
     it_behaves_like 'raises error for users with insufficient permissions'
@@ -28,6 +38,7 @@ describe Metrics::Dashboard::CustomDashboardService, :use_clean_rails_memory_sto
       let(:project) { project_with_dashboard(dashboard_path) }
 
       it_behaves_like 'valid dashboard service response'
+      it_behaves_like 'updates gitlab_metrics_dashboard_processing_time_ms metric'
 
       it 'caches the unprocessed dashboard for subsequent calls' do
         expect_any_instance_of(described_class)

spec/services/metrics/dashboard/pod_dashboard_service_spec.rb

@@ -36,9 +36,12 @@ describe Metrics::Dashboard::PodDashboardService, :use_clean_rails_memory_store_
   describe '#get_dashboard' do
     let(:dashboard_path) { described_class::DASHBOARD_PATH }
     let(:service_params) { [project, user, { environment: environment, dashboard_path: dashboard_path }] }
-    let(:service_call) { described_class.new(*service_params).get_dashboard }
+    let(:service_call) { subject.get_dashboard }
+
+    subject { described_class.new(*service_params) }
 
     it_behaves_like 'valid dashboard service response'
     it_behaves_like 'caches the unprocessed dashboard for subsequent calls'
+    it_behaves_like 'updates gitlab_metrics_dashboard_processing_time_ms metric'
   end
 end

spec/services/metrics/dashboard/self_monitoring_dashboard_service_spec.rb

@@ -16,11 +16,14 @@ describe Metrics::Dashboard::SelfMonitoringDashboardService, :use_clean_rails_me
 
   describe '#get_dashboard' do
     let(:service_params) { [project, user, { environment: environment }] }
-    let(:service_call) { described_class.new(*service_params).get_dashboard }
+    let(:service_call) { subject.get_dashboard }
+
+    subject { described_class.new(*service_params) }
 
     it_behaves_like 'valid dashboard service response'
     it_behaves_like 'raises error for users with insufficient permissions'
     it_behaves_like 'caches the unprocessed dashboard for subsequent calls'
+    it_behaves_like 'updates gitlab_metrics_dashboard_processing_time_ms metric'
   end
 
   describe '.all_dashboard_paths' do

spec/services/metrics/dashboard/system_dashboard_service_spec.rb

@@ -16,11 +16,14 @@ describe Metrics::Dashboard::SystemDashboardService, :use_clean_rails_memory_sto
   describe '#get_dashboard' do
     let(:dashboard_path) { described_class::DASHBOARD_PATH }
     let(:service_params) { [project, user, { environment: environment, dashboard_path: dashboard_path }] }
-    let(:service_call) { described_class.new(*service_params).get_dashboard }
+    let(:service_call) { subject.get_dashboard }
+
+    subject { described_class.new(*service_params) }
 
     it_behaves_like 'valid dashboard service response'
     it_behaves_like 'raises error for users with insufficient permissions'
     it_behaves_like 'caches the unprocessed dashboard for subsequent calls'
+    it_behaves_like 'updates gitlab_metrics_dashboard_processing_time_ms metric'
 
     context 'when called with a non-system dashboard' do
       let(:dashboard_path) { 'garbage/dashboard/path' }

spec/support/shared_examples/services/metrics/dashboard_shared_examples.rb

@@ -118,3 +118,13 @@ RSpec.shared_examples 'misconfigured dashboard service response with stepable' d
     expect(result[:message]).to eq(message) if message
   end
 end
+
+RSpec.shared_examples 'updates gitlab_metrics_dashboard_processing_time_ms metric' do
+  specify :prometheus do
+    service_call
+    metric = subject.send(:processing_time_metric)
+    labels = subject.send(:processing_time_metric_labels)
+
+    expect(metric.get(labels)).to be > 0
+  end
+end