Merge branch 'add-user-secure-scan-pings' into 'master'

Add user secure scan pings

See merge request gitlab-org/gitlab!49444

ee/app/models/security/scan.rb

@@ -26,6 +26,7 @@ module Security
     }
 
     scope :by_scan_types, -> (scan_types) { where(scan_type: scan_types) }
+
     scope :has_dismissal_feedback, -> do
       # The `category` enum on `vulnerability_feedback` table starts from 0 but the `scan_type` enum
       # on `security_scans` from 1. For this reason, we have to decrease the value of `scan_type` by one
@@ -35,6 +36,8 @@ module Security
         .merge(Vulnerabilities::Feedback.for_dismissal)
     end
 
+    scope :latest_successful_by_build, -> { joins(:build).where(ci_builds: { status: 'success', retried: [nil, false] }) }
+
     delegate :project, to: :build
   end
 end

ee/changelogs/unreleased/add-user-secure-scan-pings.yml

+---
+title: Add usage ping for user secure scans
+merge_request: 49444
+author:
+type: added

ee/lib/ee/gitlab/usage_data.rb

@@ -363,8 +363,9 @@ module EE
                                                                           finish: user_maximum_id)
           end
 
+          results.merge!(count_secure_user_scans(time_period))
           results.merge!(count_secure_pipelines(time_period))
-          results.merge!(count_secure_jobs(time_period))
+          results.merge!(count_secure_scans(time_period))
 
           results[:"#{prefix}unique_users_all_secure_scanners"] = distinct_count(::Ci::Build.where(name: SECURE_PRODUCT_TYPES.keys).where(time_period), :user_id)
 
@@ -379,9 +380,34 @@ module EE
 
         private
 
+        # rubocop:disable UsageData/LargeTable
+        # rubocop:disable CodeReuse/ActiveRecord
+        def count_secure_user_scans(time_period)
+          return {} if time_period.blank?
+          return {} unless ::Feature.enabled?(:postgres_hll_batch_counting)
+
+          user_scans = {}
+          start_id, finish_id = min_max_security_scan_id(time_period)
+
+          ::Security::Scan.scan_types.each do |name, scan_type|
+            relation = ::Security::Scan
+                         .latest_successful_by_build
+                         .by_scan_types(scan_type)
+                         .where(security_scans: time_period)
+
+            if start_id && finish_id
+              user_scans["user_#{name}_scans".to_sym] = estimate_batch_distinct_count(relation, :user_id, batch_size: 1000, start: start_id, finish: finish_id)
+            end
+          end
+
+          user_scans
+        end
+        # rubocop:enable UsageData/LargeTable
+        # rubocop:enable CodeReuse/ActiveRecord
+
         # rubocop:disable CodeReuse/ActiveRecord
         # rubocop: disable UsageData/LargeTable
-        def count_secure_jobs(time_period)
+        def count_secure_scans(time_period)
           start = ::Security::Scan.minimum(:build_id)
           finish = ::Security::Scan.maximum(:build_id)
 
@@ -407,33 +433,12 @@ module EE
           # time outing batch queries, to avoid that
           # different join strategy is used for HLL counter
           if ::Feature.enabled?(:postgres_hll_batch_counting)
-            scans_table = ::Security::Scan.arel_table
-            inner_relation = ::Security::Scan.select(:id)
-                               .where(
-                                 to_date_arel_node(Arel.sql('date_range_source'))
-                                   .eq(to_date_arel_node(scans_table[time_period.keys[0]]))
-                               )
-
-            outer_relation = ::Security::Scan
-                               .from("generate_series(
-                                '#{time_period.values[0].first.to_time.to_s(:db)}'::timestamp,
-                                '#{time_period.values[0].last.to_time.to_s(:db)}'::timestamp,
-                                '1 day'::interval) date_range_source")
-
-            start_id = outer_relation
-                         .select("(#{inner_relation.order(id: :asc).limit(1).to_sql})")
-                         .order('1 ASC NULLS LAST')
-                         .first&.id
-
-            finish_id = outer_relation
-                          .select("(#{inner_relation.order(id: :desc).limit(1).to_sql})")
-                          .order('1 DESC NULLS LAST')
-                          .first&.id
+            start_id, finish_id = min_max_security_scan_id(time_period)
 
             ::Security::Scan.scan_types.each do |name, scan_type|
-              relation = ::Security::Scan.joins(:build)
-                           .where(ci_builds: { status: 'success', retried: [nil, false] })
-                           .where('security_scans.scan_type = ?', scan_type)
+              relation = ::Security::Scan
+                           .latest_successful_by_build
+                           .by_scan_types(scan_type)
                            .where(security_scans: time_period)
 
               metric_name = "#{name}_pipeline"
@@ -468,6 +473,33 @@ module EE
 
           pipelines_with_secure_jobs
         end
+
+        def min_max_security_scan_id(time_period)
+          scans_table = ::Security::Scan.arel_table
+          inner_relation = ::Security::Scan.select(:id)
+                             .where(
+                               to_date_arel_node(Arel.sql('date_range_source'))
+                                 .eq(to_date_arel_node(scans_table[time_period.keys[0]]))
+                             )
+
+          outer_relation = ::Security::Scan
+                             .from("generate_series(
+                                '#{time_period.values[0].first.to_time.to_s(:db)}'::timestamp,
+                                '#{time_period.values[0].last.to_time.to_s(:db)}'::timestamp,
+                                '1 day'::interval) date_range_source")
+
+          start_id = outer_relation
+                       .select("(#{inner_relation.order(id: :asc).limit(1).to_sql})")
+                       .order('1 ASC NULLS LAST')
+                       .first&.id
+
+          finish_id = outer_relation
+                        .select("(#{inner_relation.order(id: :desc).limit(1).to_sql})")
+                        .order('1 DESC NULLS LAST')
+                        .first&.id
+
+          [start_id, finish_id]
+        end
         # rubocop: enable UsageData/LargeTable
 
         def to_date_arel_node(column)

ee/spec/factories/ci/builds.rb

@@ -25,6 +25,12 @@ FactoryBot.define do
         after(:build) do |build|
           build.job_artifacts << build(:ee_ci_job_artifact, report_type, job: build)
         end
+
+        after(:create) do |build|
+          if Security::Scan.scan_types.include?(report_type)
+            build.security_scans << build(:security_scan, scan_type: report_type, build: build)
+          end
+        end
       end
     end
 

ee/spec/lib/ee/gitlab/usage_data_spec.rb

@@ -732,13 +732,64 @@ RSpec.describe Gitlab::UsageData do
       )
     end
 
+    it 'counts users who have run scans' do
+      for_defined_days_back do
+        create(:ee_ci_build, :api_fuzzing, :success, user: user3)
+        create(:ee_ci_build, :dast, :running, user: user2)
+        create(:ee_ci_build, :dast, :success, user: user3)
+        create(:ee_ci_build, :container_scanning, :success, user: user3)
+        create(:ee_ci_build, :coverage_fuzzing, :success, user: user)
+        create(:ee_ci_build, :dependency_scanning, :success, user: user)
+        create(:ee_ci_build, :dependency_scanning, :failed, user: user2)
+        create(:ee_ci_build, :sast, :success, user: user2)
+        create(:ee_ci_build, :sast, :success, user: user3)
+        create(:ee_ci_build, :secret_detection, :success, user: user)
+        create(:ee_ci_build, :secret_detection, :success, user: user)
+        create(:ee_ci_build, :secret_detection, :failed, user: user2)
+      end
+
+      expect(described_class.usage_activity_by_stage_secure(described_class.last_28_days_time_period)).to include(
+        user_api_fuzzing_scans: be_within(error_rate).percent_of(1),
+        user_container_scanning_scans: be_within(error_rate).percent_of(1),
+        user_coverage_fuzzing_scans: be_within(error_rate).percent_of(1),
+        user_dast_scans: be_within(error_rate).percent_of(1),
+        user_dependency_scanning_scans: be_within(error_rate).percent_of(1),
+        user_sast_scans: be_within(error_rate).percent_of(2),
+        user_secret_detection_scans: be_within(error_rate).percent_of(1)
+      )
+    end
+
     context 'with feature flag: postgres_hll_batch_counting is disabled' do
       before do
         stub_feature_flags(postgres_hll_batch_counting: false)
       end
 
+      it 'does not count users who have run scans' do
+        for_defined_days_back do
+          create(:ee_ci_build, :api_fuzzing, :success, user: user3)
+          create(:ee_ci_build, :dast, :success, user: user2)
+          create(:ee_ci_build, :container_scanning, :success, user: user3)
+          create(:ee_ci_build, :coverage_fuzzing, :success, user: user)
+          create(:ee_ci_build, :dependency_scanning, :success, user: user)
+          create(:ee_ci_build, :sast, :success, user: user2)
+          create(:ee_ci_build, :secret_detection, :success, user: user)
+          create(:ee_ci_build, :secret_detection, :running, user: user2)
+          create(:ee_ci_build, :secret_detection, :failed, user: user3)
+        end
+
+        expect(described_class.usage_activity_by_stage_secure(described_class.last_28_days_time_period)).not_to include(
+          :user_api_fuzzing_scans,
+          :user_container_scanning_scans,
+          :user_coverage_fuzzing_scans,
+          :user_dast_scans,
+          :user_dependency_scanning_scans,
+          :user_sast_scans,
+          :user_secret_detection_scans
+        )
+      end
+
       it 'includes accurate usage_activity_by_stage data' do
-        expect(described_class.usage_activity_by_stage_secure(described_class.last_28_days_time_period)).to eq(
+        expect(described_class.usage_activity_by_stage_secure(described_class.last_28_days_time_period)).to include(
           user_preferences_group_overview_security_dashboard: 3,
           user_container_scanning_jobs: 1,
           user_api_fuzzing_jobs: 1,
@@ -836,7 +887,7 @@ RSpec.describe Gitlab::UsageData do
           create(:ci_build, name: 'dast', user: user3)
         end
 
-        expect(described_class.usage_activity_by_stage_secure(described_class.last_28_days_time_period)).to eq(
+        expect(described_class.usage_activity_by_stage_secure(described_class.last_28_days_time_period)).to include(
           user_preferences_group_overview_security_dashboard: 3,
           user_api_fuzzing_jobs: 1,
           user_api_fuzzing_dnd_jobs: 1,
@@ -870,7 +921,7 @@ RSpec.describe Gitlab::UsageData do
           create(:ci_build, name: 'license_scanning', user: user)
         end
 
-        expect(described_class.usage_activity_by_stage_secure(described_class.last_28_days_time_period)).to eq(
+        expect(described_class.usage_activity_by_stage_secure(described_class.last_28_days_time_period)).to include(
           user_preferences_group_overview_security_dashboard: 3,
           user_api_fuzzing_jobs: 1,
           user_api_fuzzing_dnd_jobs: 1,

ee/spec/models/security/scan_spec.rb

@@ -28,6 +28,16 @@ RSpec.describe Security::Scan do
     it { is_expected.to match_array(expected_scans) }
   end
 
+  describe '.latest_successful_by_build' do
+    let!(:first_successful_scan) { create(:security_scan, build: create(:ci_build, :success, :retried)) }
+    let!(:second_successful_scan) { create(:security_scan, build: create(:ci_build, :success)) }
+    let!(:failed_scan) { create(:security_scan, build: create(:ci_build, :failed)) }
+
+    subject { described_class.latest_successful_by_build }
+
+    it { is_expected.to match_array([second_successful_scan]) }
+  end
+
   describe '.has_dismissal_feedback' do
     let(:scan_1) { create(:security_scan) }
     let(:scan_2) { create(:security_scan) }