Merge branch 'nd/fix-project-approval-usage-data-count' into 'master'

Fix project approval usage data count

See merge request gitlab-org/gitlab!81823

ee/config/metrics/counts_all/20210216183352_approval_project_rules_with_more_approvers_than_required.yml

@@ -10,6 +10,9 @@ value_type: number
 status: active
 time_frame: all
 data_source: database
+instrumentation_class: ApprovalProjectRulesWithUserMetric
+options:
+  count_type: more_approvers_than_required
 distribution:
 - ee
 tier:

ee/config/metrics/counts_all/20210216183354_approval_project_rules_with_less_approvers_than_required.yml

@@ -10,6 +10,9 @@ value_type: number
 status: active
 time_frame: all
 data_source: database
+instrumentation_class: ApprovalProjectRulesWithUserMetric
+options:
+  count_type: less_approvers_than_required
 distribution:
 - ee
 tier:

ee/config/metrics/counts_all/20210216183355_approval_project_rules_with_exact_required_approvers.yml

@@ -10,6 +10,9 @@ value_type: number
 status: active
 time_frame: all
 data_source: database
+instrumentation_class: ApprovalProjectRulesWithUserMetric
+options:
+  count_type: exact_required_approvers
 distribution:
 - ee
 tier:

ee/lib/ee/gitlab/usage_data.rb

@@ -132,24 +132,14 @@ module EE
 
         # rubocop:disable CodeReuse/ActiveRecord, UsageData/LargeTable
         def approval_rules_counts
-          approval_project_rules_with_users =
-            ApprovalProjectRule
-              .regular
-              .joins('INNER JOIN approval_project_rules_users ON approval_project_rules_users.approval_project_rule_id = approval_project_rules.id')
-              .group(:id)
-
           {
             approval_project_rules: count(ApprovalProjectRule),
             approval_project_rules_with_target_branch: count(ApprovalProjectRulesProtectedBranch, :approval_project_rule_id),
-            approval_project_rules_with_more_approvers_than_required: count_approval_rules_with_users(approval_project_rules_with_users.having('COUNT(approval_project_rules_users) > approvals_required')),
-            approval_project_rules_with_less_approvers_than_required: count_approval_rules_with_users(approval_project_rules_with_users.having('COUNT(approval_project_rules_users) < approvals_required')),
-            approval_project_rules_with_exact_required_approvers: count_approval_rules_with_users(approval_project_rules_with_users.having('COUNT(approval_project_rules_users) = approvals_required'))
+            approval_project_rules_with_more_approvers_than_required: add_metric('ApprovalProjectRulesWithUserMetric', options: { count_type: 'more_approvers_than_required' }),
+            approval_project_rules_with_less_approvers_than_required: add_metric('ApprovalProjectRulesWithUserMetric', options: { count_type: 'less_approvers_than_required' }),
+            approval_project_rules_with_exact_required_approvers: add_metric('ApprovalProjectRulesWithUserMetric', options: { count_type: 'exact_required_approvers' })
           }
         end
-
-        def count_approval_rules_with_users(relation)
-          count(relation, batch_size: 10_000, start: minimum_id(ApprovalProjectRule.regular), finish: maximum_id(ApprovalProjectRule.regular)).size
-        end
         # rubocop:enable CodeReuse/ActiveRecord, UsageData/LargeTable
 
         def security_products_usage

ee/lib/gitlab/usage/metrics/instrumentations/approval_project_rules_with_user_metric.rb

+# frozen_string_literal: true
+
+module Gitlab
+  module Usage
+    module Metrics
+      module Instrumentations
+        class ApprovalProjectRulesWithUserMetric < DatabaseMetric
+          operation :count
+
+          metric_options do
+            {
+              batch_size: 10_000
+            }
+          end
+
+          start { ApprovalProjectRule.regular.minimum(:id) }
+          finish { ApprovalProjectRule.regular.maximum(:id) }
+
+          def to_sql
+            # rubocop: disable CodeReuse/ActiveRecord
+            ApplicationRecord.select(Arel.star.count).from("(#{super}) subquery").to_sql
+            # rubocop: enable CodeReuse/ActiveRecord
+          end
+
+          def value
+            super.size
+          end
+
+          private
+
+          def relation
+            # rubocop: disable CodeReuse/ActiveRecord
+            ApprovalProjectRule
+              .regular
+              .joins('INNER JOIN approval_project_rules_users ON approval_project_rules_users.approval_project_rule_id = approval_project_rules.id')
+              .group(:id)
+              .having(having_clause)
+            # rubocop: enable CodeReuse/ActiveRecord
+          end
+
+          def having_clause
+            case options[:count_type]
+            when 'more_approvers_than_required'
+              'COUNT(approval_project_rules_users) > approvals_required'
+            when 'less_approvers_than_required'
+              'COUNT(approval_project_rules_users) < approvals_required'
+            when 'exact_required_approvers'
+              'COUNT(approval_project_rules_users) = approvals_required'
+            end
+          end
+        end
+      end
+    end
+  end
+end

ee/spec/lib/gitlab/usage/metrics/instrumentations/approval_project_rules_with_user_metric_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Gitlab::Usage::Metrics::Instrumentations::ApprovalProjectRulesWithUserMetric do
+  let_it_be(:user) { create(:user) }
+  let_it_be(:project) { create(:project) }
+
+  before do
+    allow(ApprovalProjectRule.connection).to receive(:transaction_open?).and_return(false)
+  end
+
+  let(:expected_value) { 2 }
+  let(:expected_query) do
+    "SELECT COUNT(*) FROM (SELECT COUNT(\"approval_project_rules\".\"id\") FROM \"approval_project_rules\" INNER JOIN approval_project_rules_users ON approval_project_rules_users.approval_project_rule_id = approval_project_rules.id WHERE \"approval_project_rules\".\"rule_type\" = 0 GROUP BY \"approval_project_rules\".\"id\" HAVING (#{having_clause})) subquery"
+  end
+
+  context 'for more approvers than required' do
+    let(:having_clause) { 'COUNT(approval_project_rules_users) > approvals_required'}
+
+    before do
+      create_list(:approval_project_rule, 2, project: project, users: create_list(:user, 2), approvals_required: 1)
+    end
+
+    it_behaves_like 'a correct instrumented metric value and query', { data_source: 'database', options: { count_type: 'more_approvers_than_required' } }
+  end
+
+  context 'for more approvers than required' do
+    let(:having_clause) { 'COUNT(approval_project_rules_users) < approvals_required'}
+
+    before do
+      create_list(:approval_project_rule, 2, project: project, users: [user], approvals_required: 2)
+    end
+
+    it_behaves_like 'a correct instrumented metric value and query', { data_source: 'database', options: { count_type: 'less_approvers_than_required' } }
+  end
+
+  context 'for more approvers than required' do
+    let(:having_clause) { "COUNT(approval_project_rules_users) = approvals_required"}
+
+    before do
+      create_list(:approval_project_rule, 2, project: project, users: [user], approvals_required: 1)
+    end
+
+    it_behaves_like 'a correct instrumented metric value and query', { data_source: 'database', options: { count_type: 'exact_required_approvers' } }
+  end
+end