Merge branch...

Merge branch '343145-follow-up-from-fix-handling-service-ping-response-with-only-usage_data_id' into 'master'

Add validations to DevOpsReport::Metric

See merge request gitlab-org/gitlab!75060

app/models/dev_ops_report/metric.rb

@@ -6,6 +6,20 @@ module DevOpsReport
 
     self.table_name = 'conversational_development_index_metrics'
 
+    METRICS = %w[leader_issues instance_issues percentage_issues leader_notes instance_notes
+                 percentage_notes leader_milestones instance_milestones percentage_milestones
+                 leader_boards instance_boards percentage_boards leader_merge_requests
+                 instance_merge_requests percentage_merge_requests leader_ci_pipelines
+                 instance_ci_pipelines percentage_ci_pipelines leader_environments instance_environments
+                 percentage_environments leader_deployments instance_deployments percentage_deployments
+                 leader_projects_prometheus_active instance_projects_prometheus_active
+                 percentage_projects_prometheus_active leader_service_desk_issues instance_service_desk_issues
+                 percentage_service_desk_issues].freeze
+
+    METRICS.each do |metric_name|
+      validates metric_name, presence: true, numericality: { greater_than_or_equal_to: 0 }
+    end
+
     def instance_score(feature)
       self["instance_#{feature}"]
     end

app/services/service_ping/devops_report_service.rb

@@ -2,28 +2,25 @@
 
 module ServicePing
   class DevopsReportService
-    METRICS = %w[leader_issues instance_issues percentage_issues leader_notes instance_notes
-                 percentage_notes leader_milestones instance_milestones percentage_milestones
-                 leader_boards instance_boards percentage_boards leader_merge_requests
-                 instance_merge_requests percentage_merge_requests leader_ci_pipelines
-                 instance_ci_pipelines percentage_ci_pipelines leader_environments instance_environments
-                 percentage_environments leader_deployments instance_deployments percentage_deployments
-                 leader_projects_prometheus_active instance_projects_prometheus_active
-                 percentage_projects_prometheus_active leader_service_desk_issues instance_service_desk_issues
-                 percentage_service_desk_issues].freeze
-
     def initialize(data)
       @data = data
     end
 
     def execute
-      metrics = @data['conv_index'] || @data['dev_ops_score'] # leaving dev_ops_score here, as the data comes from the gitlab-version-com
+      # `conv_index` was previously named `dev_ops_score` in
+      # version-gitlab-com, so we check both for backwards compatibility.
+      metrics = @data['conv_index'] || @data['dev_ops_score']
 
-      return unless metrics.except('usage_data_id').present?
+      # Do not attempt to save a report for the first Service Ping
+      # response for a given GitLab instance, which comes without
+      # metrics.
+      return if metrics.keys == ['usage_data_id']
 
-      DevOpsReport::Metric.create!(
-        metrics.slice(*METRICS)
+      report = DevOpsReport::Metric.create(
+        metrics.slice(*DevOpsReport::Metric::METRICS)
       )
+
+      Gitlab::ErrorTracking.track_and_raise_for_dev_exception(ActiveRecord::RecordInvalid.new(report)) unless report.persisted?
     end
   end
 end

spec/models/dev_ops_report/metric_spec.rb

@@ -5,6 +5,13 @@ require 'spec_helper'
 RSpec.describe DevOpsReport::Metric do
   let(:conv_dev_index) { create(:dev_ops_report_metric) }
 
+  describe 'validations' do
+    DevOpsReport::Metric::METRICS.each do |metric_name|
+      it { is_expected.to validate_presence_of(metric_name) }
+      it { is_expected.to validate_numericality_of(metric_name).is_greater_than_or_equal_to(0) }
+    end
+  end
+
   describe '#percentage_score' do
     it 'returns stored percentage score' do
       expect(conv_dev_index.percentage_score('issues')).to eq(13.331)