Merge branch 'track-compliance-page-vists' into 'master'

Track unique visitors in Compliance related pages

See merge request gitlab-org/gitlab!38851

ee/app/controllers/admin/audit_logs_controller.rb

@@ -4,9 +4,12 @@ class Admin::AuditLogsController < Admin::ApplicationController
   include AuditEvents::EnforcesValidDateParams
   include AuditEvents::AuditLogsParams
   include AuditEvents::Sortable
+  include Analytics::UniqueVisitsHelper
 
   before_action :check_license_admin_audit_log_available!
 
+  track_unique_visits :index, target_id: 'i_compliance_audit_events'
+
   PER_PAGE = 25
 
   def index

ee/app/controllers/admin/credentials_controller.rb

@@ -3,11 +3,14 @@
 class Admin::CredentialsController < Admin::ApplicationController
   extend ::Gitlab::Utils::Override
   include CredentialsInventoryActions
+  include Analytics::UniqueVisitsHelper
 
   helper_method :credentials_inventory_path, :user_detail_path
 
   before_action :check_license_credentials_inventory_available!, only: [:index]
 
+  track_unique_visits :index, target_id: 'i_compliance_credential_inventory'
+
   private
 
   def check_license_credentials_inventory_available!

ee/app/controllers/concerns/credentials_inventory_actions.rb

@@ -7,7 +7,11 @@ module CredentialsInventoryActions
   def index
     @credentials = filter_credentials.page(params[:page]).preload_users.without_count # rubocop:disable Gitlab/ModuleWithInstanceVariables
 
-    render 'shared/credentials_inventory/index'
+    respond_to do |format|
+      format.html do
+        render 'shared/credentials_inventory/index'
+      end
+    end
   end
 
   private

ee/app/controllers/groups/audit_events_controller.rb

@@ -4,10 +4,13 @@ class Groups::AuditEventsController < Groups::ApplicationController
   include AuditEvents::EnforcesValidDateParams
   include AuditEvents::AuditLogsParams
   include AuditEvents::Sortable
+  include Analytics::UniqueVisitsHelper
 
   before_action :authorize_admin_group!
   before_action :check_audit_events_available!
 
+  track_unique_visits :index, target_id: 'g_compliance_audit_events'
+
   layout 'group_settings'
 
   def index

ee/app/controllers/groups/security/compliance_dashboards_controller.rb

 # frozen_string_literal: true
 class Groups::Security::ComplianceDashboardsController < Groups::ApplicationController
   include Groups::SecurityFeaturesHelper
+  include Analytics::UniqueVisitsHelper
 
   layout 'group'
 
   before_action :authorize_compliance_dashboard!
 
+  track_unique_visits :show, target_id: 'g_compliance_dashboard'
+
   def show
     @last_page = paginated_merge_requests.last_page?
     @merge_requests = serialize(paginated_merge_requests)

ee/changelogs/unreleased/track-compliance-page-vists.yml

+---
+title: Track unique visitors in Compliance related pages
+merge_request: 38851
+author:
+type: added

ee/spec/controllers/admin/audit_logs_controller_spec.rb

@@ -24,6 +24,11 @@ RSpec.describe Admin::AuditLogsController do
           expect(assigns(:events)).to be_kind_of(Kaminari::PaginatableWithoutCount)
         end
       end
+
+      it_behaves_like 'tracking unique visits', :index do
+        let(:request_params) { { 'entity_type': 'User' } }
+        let(:target_id) { 'i_compliance_audit_events' }
+      end
     end
   end
 end

ee/spec/controllers/admin/credentials_controller_spec.rb

@@ -20,6 +20,11 @@ RSpec.describe Admin::CredentialsController do
           expect(response).to have_gitlab_http_status(:ok)
         end
 
+        it_behaves_like 'tracking unique visits', :index do
+          let(:request_params) { {} }
+          let(:target_id) { 'i_compliance_credential_inventory' }
+        end
+
         describe 'filtering by type of credential' do
           let_it_be(:personal_access_tokens) { create_list(:personal_access_token, 2) }
 

ee/spec/controllers/groups/audit_events_controller_spec.rb

@@ -12,113 +12,124 @@ RSpec.describe Groups::AuditEventsController do
     let(:entity_type) { nil }
     let(:entity_id) { nil }
 
-    let(:request) do
-      get :index, params: { group_id: group.to_param, sort: sort, entity_type: entity_type, entity_id: entity_id }
-    end
-
     context 'authorized' do
       before do
         group.add_owner(owner)
         sign_in(owner)
       end
 
-      context 'when audit_events feature is available' do
-        let(:level) { Gitlab::Audit::Levels::Group.new(group: group) }
-        let(:audit_logs_params) { ActionController::Parameters.new(sort: '', entity_type: '', entity_id: '').permit! }
-
-        before do
-          stub_licensed_features(audit_events: true)
-
-          allow(Gitlab::Audit::Levels::Group).to receive(:new).and_return(level)
-          allow(AuditLogFinder).to receive(:new).and_call_original
+      context do
+        let(:request) do
+          get :index, params: { group_id: group.to_param, sort: sort, entity_type: entity_type, entity_id: entity_id }
         end
 
-        shared_examples 'AuditLogFinder params' do
-          it 'has the correct params' do
-            request
+        context 'when audit_events feature is available' do
+          let(:level) { Gitlab::Audit::Levels::Group.new(group: group) }
+          let(:audit_logs_params) { ActionController::Parameters.new(sort: '', entity_type: '', entity_id: '').permit! }
 
-            expect(AuditLogFinder).to have_received(:new).with(
-              level: level, params: audit_logs_params
-            )
+          before do
+            stub_licensed_features(audit_events: true)
+
+            allow(Gitlab::Audit::Levels::Group).to receive(:new).and_return(level)
+            allow(AuditLogFinder).to receive(:new).and_call_original
           end
-        end
 
-        it 'renders index with 200 status code' do
-          request
+          shared_examples 'AuditLogFinder params' do
+            it 'has the correct params' do
+              request
 
-          expect(response).to have_gitlab_http_status(:ok)
-          expect(response).to render_template(:index)
-        end
+              expect(AuditLogFinder).to have_received(:new).with(
+                level: level, params: audit_logs_params
+              )
+            end
+          end
 
-        context 'invokes AuditLogFinder with correct arguments' do
-          it_behaves_like 'AuditLogFinder params'
-        end
+          it 'renders index with 200 status code' do
+            request
 
-        context 'author' do
-          context 'when no author entity type is specified' do
-            it_behaves_like 'AuditLogFinder params'
+            expect(response).to have_gitlab_http_status(:ok)
+            expect(response).to render_template(:index)
           end
 
-          context 'when the author entity type is specified' do
-            let(:entity_type) { 'Author' }
-            let(:entity_id) { 1 }
-            let(:audit_logs_params) { ActionController::Parameters.new(sort: '', author_id: '1').permit! }
-
+          context 'invokes AuditLogFinder with correct arguments' do
             it_behaves_like 'AuditLogFinder params'
           end
-        end
 
-        context 'ordering' do
-          shared_examples 'orders by id descending' do
-            it 'orders by id descending' do
-              request
+          context 'author' do
+            context 'when no author entity type is specified' do
+              it_behaves_like 'AuditLogFinder params'
+            end
 
-              expect(assigns(:events)).to eq(group.audit_events.order(id: :desc))
+            context 'when the author entity type is specified' do
+              let(:entity_type) { 'Author' }
+              let(:entity_id) { 1 }
+              let(:audit_logs_params) { ActionController::Parameters.new(sort: '', author_id: '1').permit! }
+
+              it_behaves_like 'AuditLogFinder params'
             end
           end
 
-          before do
-            create_list(:group_audit_event, 5, entity_id: group.id)
-          end
+          context 'ordering' do
+            shared_examples 'orders by id descending' do
+              it 'orders by id descending' do
+                request
 
-          context 'when no sort order is specified' do
-            it_behaves_like 'orders by id descending'
-          end
+                expect(assigns(:events)).to eq(group.audit_events.order(id: :desc))
+              end
+            end
 
-          context 'when sorting by latest events first' do
-            let(:sort) { 'created_desc' }
+            before do
+              create_list(:group_audit_event, 5, entity_id: group.id)
+            end
 
-            it_behaves_like 'orders by id descending'
-          end
+            context 'when no sort order is specified' do
+              it_behaves_like 'orders by id descending'
+            end
 
-          context 'when sorting by oldest events first' do
-            let(:sort) { 'created_asc' }
+            context 'when sorting by latest events first' do
+              let(:sort) { 'created_desc' }
 
-            it 'orders by id ascending' do
-              request
+              it_behaves_like 'orders by id descending'
+            end
+
+            context 'when sorting by oldest events first' do
+              let(:sort) { 'created_asc' }
+
+              it 'orders by id ascending' do
+                request
 
-              expect(assigns(:events)).to eq(group.audit_events.order(id: :asc))
+                expect(assigns(:events)).to eq(group.audit_events.order(id: :asc))
+              end
             end
-          end
 
-          context 'when sorting by an unsupported sort order' do
-            let(:sort) { 'FOO' }
+            context 'when sorting by an unsupported sort order' do
+              let(:sort) { 'FOO' }
 
-            it_behaves_like 'orders by id descending'
+              it_behaves_like 'orders by id descending'
+            end
           end
-        end
 
-        context 'pagination' do
-          it 'paginates audit events, without casting a count query' do
-            request
+          context 'pagination' do
+            it 'paginates audit events, without casting a count query' do
+              request
 
-            expect(assigns(:events)).to be_kind_of(Kaminari::PaginatableWithoutCount)
+              expect(assigns(:events)).to be_kind_of(Kaminari::PaginatableWithoutCount)
+            end
           end
         end
       end
+
+      it_behaves_like 'tracking unique visits', :index do
+        let(:request_params) { { group_id: group.to_param, sort: sort, entity_type: entity_type, entity_id: entity_id } }
+        let(:target_id) { 'g_compliance_audit_events' }
+      end
     end
 
     context 'unauthorized' do
+      let(:request) do
+        get :index, params: { group_id: group.to_param, sort: sort, entity_type: entity_type, entity_id: entity_id }
+      end
+
       before do
         stub_licensed_features(audit_events: true)
         sign_in(user)

ee/spec/controllers/groups/security/compliance_dashboards_controller_spec.rb

@@ -47,6 +47,11 @@ RSpec.describe Groups::Security::ComplianceDashboardsController do
             expect(assigns(:merge_requests)).not_to be_empty
           end
         end
+
+        it_behaves_like 'tracking unique visits', :show do
+          let(:request_params) { { group_id: group.to_param } }
+          let(:target_id) { 'g_compliance_dashboard' }
+        end
       end
 
       context 'when user is not allowed to access group compliance dashboard' do

lib/gitlab/analytics/unique_visits.rb

@@ -3,7 +3,7 @@
 module Gitlab
   module Analytics
     class UniqueVisits
-      TARGET_IDS = Set[
+      ANALYTICS_IDS = Set[
         'g_analytics_contribution',
         'g_analytics_insights',
         'g_analytics_issues',
@@ -17,6 +17,13 @@ module Gitlab
         'p_analytics_repo',
         'i_analytics_cohorts',
         'i_analytics_dev_ops_score'
+      ]
+
+      COMPLIANCE_IDS = Set[
+        'g_compliance_dashboard',
+        'g_compliance_audit_events',
+        'i_compliance_credential_inventory',
+        'i_compliance_audit_events'
       ].freeze
 
       KEY_EXPIRY_LENGTH = 12.weeks
@@ -34,8 +41,10 @@ module Gitlab
       # @param [Integer] weeks time frame length in weeks
       # @return [Integer] number of unique visitors
       def unique_visits_for(targets:, start_week: 7.days.ago, weeks: 1)
-        target_ids = if targets == :any
-                       TARGET_IDS
+        target_ids = if targets == :analytics
+                       ANALYTICS_IDS
+                     elsif targets == :compliance
+                       COMPLIANCE_IDS
                      else
                        Array(targets)
                      end
@@ -50,9 +59,12 @@ module Gitlab
       private
 
       def key(target_id, time)
-        raise "Invalid target id #{target_id}" unless TARGET_IDS.include?(target_id.to_s)
+        target_ids = ANALYTICS_IDS + COMPLIANCE_IDS
+
+        raise "Invalid target id #{target_id}" unless target_ids.include?(target_id.to_s)
+
+        target_key = target_id.to_s.gsub('analytics', '{analytics}').gsub('compliance', '{compliance}')
 
-        target_key = target_id.to_s.gsub('analytics', '{analytics}')
         year_week = time.strftime('%G-%V')
 
         "#{target_key}-#{year_week}"

lib/gitlab/usage_data.rb

@@ -37,6 +37,7 @@ module Gitlab
             .merge(usage_activity_by_stage)
             .merge(usage_activity_by_stage(:usage_activity_by_stage_monthly, last_28_days_time_period))
             .merge(analytics_unique_visits_data)
+            .merge(compliance_unique_visits_data)
         end
       end
 
@@ -581,15 +582,25 @@ module Gitlab
       end
 
       def analytics_unique_visits_data
-        results = ::Gitlab::Analytics::UniqueVisits::TARGET_IDS.each_with_object({}) do |target_id, hash|
+        results = ::Gitlab::Analytics::UniqueVisits::ANALYTICS_IDS.each_with_object({}) do |target_id, hash|
           hash[target_id] = redis_usage_data { unique_visit_service.unique_visits_for(targets: target_id) }
         end
-        results['analytics_unique_visits_for_any_target'] = redis_usage_data { unique_visit_service.unique_visits_for(targets: :any) }
-        results['analytics_unique_visits_for_any_target_monthly'] = redis_usage_data { unique_visit_service.unique_visits_for(targets: :any, weeks: 4) }
+        results['analytics_unique_visits_for_any_target'] = redis_usage_data { unique_visit_service.unique_visits_for(targets: :analytics) }
+        results['analytics_unique_visits_for_any_target_monthly'] = redis_usage_data { unique_visit_service.unique_visits_for(targets: :analytics, weeks: 4) }
 
         { analytics_unique_visits: results }
       end
 
+      def compliance_unique_visits_data
+        results = ::Gitlab::Analytics::UniqueVisits::COMPLIANCE_IDS.each_with_object({}) do |target_id, hash|
+          hash[target_id] = redis_usage_data { unique_visit_service.unique_visits_for(targets: target_id) }
+        end
+        results['compliance_unique_visits_for_any_target'] = redis_usage_data { unique_visit_service.unique_visits_for(targets: :compliance) }
+        results['compliance_unique_visits_for_any_target_monthly'] = redis_usage_data { unique_visit_service.unique_visits_for(targets: :compliance, weeks: 4) }
+
+        { compliance_unique_visits: results }
+      end
+
       def action_monthly_active_users(time_period)
         return {} unless Feature.enabled?(Gitlab::UsageDataCounters::TrackUniqueActions::FEATURE_FLAG)
 

spec/lib/gitlab/analytics/unique_visits_spec.rb

@@ -7,8 +7,11 @@ RSpec.describe Gitlab::Analytics::UniqueVisits, :clean_gitlab_redis_shared_state
   let(:target1_id) { 'g_analytics_contribution' }
   let(:target2_id) { 'g_analytics_insights' }
   let(:target3_id) { 'g_analytics_issues' }
+  let(:target4_id) { 'g_compliance_dashboard' }
+  let(:target5_id) { 'i_compliance_credential_inventory' }
   let(:visitor1_id) { 'dfb9d2d2-f56c-4c77-8aeb-6cddc4a1f857' }
   let(:visitor2_id) { '1dd9afb2-a3ee-4de1-8ae3-a405579c8584' }
+  let(:visitor3_id) { '34rfjuuy-ce56-sa35-ds34-dfer567dfrf2' }
 
   around do |example|
     # We need to freeze to a reference time
@@ -29,18 +32,32 @@ RSpec.describe Gitlab::Analytics::UniqueVisits, :clean_gitlab_redis_shared_state
       unique_visits.track_visit(visitor1_id, target2_id, 8.days.ago)
       unique_visits.track_visit(visitor1_id, target2_id, 15.days.ago)
 
+      unique_visits.track_visit(visitor3_id, target4_id, 7.days.ago)
+
+      unique_visits.track_visit(visitor3_id, target5_id, 15.days.ago)
+      unique_visits.track_visit(visitor2_id, target5_id, 15.days.ago)
+
       expect(unique_visits.unique_visits_for(targets: target1_id)).to eq(2)
       expect(unique_visits.unique_visits_for(targets: target2_id)).to eq(1)
+      expect(unique_visits.unique_visits_for(targets: target4_id)).to eq(1)
 
       expect(unique_visits.unique_visits_for(targets: target2_id, start_week: 15.days.ago)).to eq(1)
 
       expect(unique_visits.unique_visits_for(targets: target3_id)).to eq(0)
 
-      expect(unique_visits.unique_visits_for(targets: :any)).to eq(2)
-      expect(unique_visits.unique_visits_for(targets: :any, start_week: 15.days.ago)).to eq(1)
-      expect(unique_visits.unique_visits_for(targets: :any, start_week: 30.days.ago)).to eq(0)
+      expect(unique_visits.unique_visits_for(targets: target5_id, start_week: 15.days.ago)).to eq(2)
+
+      expect(unique_visits.unique_visits_for(targets: :analytics)).to eq(2)
+      expect(unique_visits.unique_visits_for(targets: :analytics, start_week: 15.days.ago)).to eq(1)
+      expect(unique_visits.unique_visits_for(targets: :analytics, start_week: 30.days.ago)).to eq(0)
+
+      expect(unique_visits.unique_visits_for(targets: :analytics, weeks: 4)).to eq(2)
+
+      expect(unique_visits.unique_visits_for(targets: :compliance)).to eq(1)
+      expect(unique_visits.unique_visits_for(targets: :compliance, start_week: 15.days.ago)).to eq(2)
+      expect(unique_visits.unique_visits_for(targets: :compliance, start_week: 30.days.ago)).to eq(0)
 
-      expect(unique_visits.unique_visits_for(targets: :any, weeks: 4)).to eq(2)
+      expect(unique_visits.unique_visits_for(targets: :compliance, weeks: 4)).to eq(2)
     end
 
     it 'sets the keys in Redis to expire automatically after 12 weeks' do

spec/lib/gitlab/usage_data_spec.rb

@@ -958,12 +958,12 @@ RSpec.describe Gitlab::UsageData, :aggregate_failures do
     subject { described_class.analytics_unique_visits_data }
 
     it 'returns the number of unique visits to pages with analytics features' do
-      ::Gitlab::Analytics::UniqueVisits::TARGET_IDS.each do |target_id|
+      ::Gitlab::Analytics::UniqueVisits::ANALYTICS_IDS.each do |target_id|
         expect_any_instance_of(::Gitlab::Analytics::UniqueVisits).to receive(:unique_visits_for).with(targets: target_id).and_return(123)
       end
 
-      expect_any_instance_of(::Gitlab::Analytics::UniqueVisits).to receive(:unique_visits_for).with(targets: :any).and_return(543)
-      expect_any_instance_of(::Gitlab::Analytics::UniqueVisits).to receive(:unique_visits_for).with(targets: :any, weeks: 4).and_return(987)
+      expect_any_instance_of(::Gitlab::Analytics::UniqueVisits).to receive(:unique_visits_for).with(targets: :analytics).and_return(543)
+      expect_any_instance_of(::Gitlab::Analytics::UniqueVisits).to receive(:unique_visits_for).with(targets: :analytics, weeks: 4).and_return(987)
 
       expect(subject).to eq({
         analytics_unique_visits: {
@@ -987,6 +987,37 @@ RSpec.describe Gitlab::UsageData, :aggregate_failures do
     end
   end
 
+  describe '.compliance_unique_visits_data' do
+    subject { described_class.compliance_unique_visits_data }
+
+    before do
+      described_class.clear_memoization(:unique_visit_service)
+
+      allow_next_instance_of(::Gitlab::Analytics::UniqueVisits) do |instance|
+        ::Gitlab::Analytics::UniqueVisits::COMPLIANCE_IDS.each do |target_id|
+          allow(instance).to receive(:unique_visits_for).with(targets: target_id).and_return(123)
+        end
+
+        allow(instance).to receive(:unique_visits_for).with(targets: :compliance).and_return(543)
+
+        allow(instance).to receive(:unique_visits_for).with(targets: :compliance, weeks: 4).and_return(987)
+      end
+    end
+
+    it 'returns the number of unique visits to pages with compliance features' do
+      expect(subject).to eq({
+        compliance_unique_visits: {
+          'g_compliance_dashboard' => 123,
+          'g_compliance_audit_events' => 123,
+          'i_compliance_credential_inventory' => 123,
+          'i_compliance_audit_events' => 123,
+          'compliance_unique_visits_for_any_target' => 543,
+          'compliance_unique_visits_for_any_target_monthly' => 987
+        }
+      })
+    end
+  end
+
   describe '.service_desk_counts' do
     subject { described_class.send(:service_desk_counts) }