Merge branch '214250-usage-ping-counts-for-all-search' into 'master'

Resolve "Usage Ping counts for all Search"

See merge request gitlab-org/gitlab!32111

app/controllers/search_controller.rb

@@ -33,7 +33,7 @@ class SearchController < ApplicationController
     render_commits if @scope == 'commits'
     eager_load_user_status if @scope == 'users'
 
-    increment_navbar_searches_counter
+    increment_search_counters
 
     check_single_commit_result
   end
@@ -98,9 +98,11 @@ class SearchController < ApplicationController
     end
   end
 
-  def increment_navbar_searches_counter
+  def increment_search_counters
+    Gitlab::UsageDataCounters::SearchCounter.count(:all_searches)
+
     return if params[:nav_source] != 'navbar'
 
-    Gitlab::UsageDataCounters::SearchCounter.increment_navbar_searches_count
+    Gitlab::UsageDataCounters::SearchCounter.count(:navbar_searches)
   end
 end

changelogs/unreleased/214250-usage-ping-counts-for-all-search.yml

+---
+title: Add Usage Ping count for all searches
+merge_request: 32111
+author:
+type: changed

lib/api/search.rb

@@ -36,6 +36,8 @@ module API
 
         results = SearchService.new(current_user, search_params).search_objects(preload_method)
 
+        Gitlab::UsageDataCounters::SearchCounter.count(:all_searches)
+
         paginate(results)
       end
 

lib/gitlab/usage_data_counters/base_counter.rb

@@ -8,7 +8,7 @@ module Gitlab::UsageDataCounters
 
     class << self
       def redis_key(event)
-        Gitlab::ErrorTracking.track_and_raise_for_dev_exception(UnknownEvent.new, event: event) unless known_events.include?(event.to_s)
+        require_known_event(event)
 
         "USAGE_#{prefix}_#{event}".upcase
       end
@@ -31,6 +31,10 @@ module Gitlab::UsageDataCounters
 
       private
 
+      def require_known_event(event)
+        Gitlab::ErrorTracking.track_and_raise_for_dev_exception(UnknownEvent.new, event: event) unless known_events.include?(event.to_s)
+      end
+
       def counter_key(event)
         "#{prefix}_#{event}".to_sym
       end

lib/gitlab/usage_data_counters/designs_counter.rb

@@ -4,7 +4,7 @@ module Gitlab::UsageDataCounters
   class DesignsCounter
     extend Gitlab::UsageDataCounters::RedisCounter
 
-    KNOWN_EVENTS = %w[create update delete].map(&:freeze).freeze
+    KNOWN_EVENTS = %w[create update delete].freeze
 
     UnknownEvent = Class.new(StandardError)
 

lib/gitlab/usage_data_counters/search_counter.rb

@@ -2,28 +2,20 @@
 
 module Gitlab
   module UsageDataCounters
-    class SearchCounter
-      extend RedisCounter
-
-      NAVBAR_SEARCHES_COUNT_KEY = 'NAVBAR_SEARCHES_COUNT'
+    class SearchCounter < BaseCounter
+      KNOWN_EVENTS = %w[all_searches navbar_searches].freeze
 
       class << self
-        def increment_navbar_searches_count
-          increment(NAVBAR_SEARCHES_COUNT_KEY)
-        end
+        def redis_key(event)
+          require_known_event(event)
 
-        def total_navbar_searches_count
-          total_count(NAVBAR_SEARCHES_COUNT_KEY)
+          "#{event}_COUNT".upcase
         end
 
-        def totals
-          {
-            navbar_searches: total_navbar_searches_count
-          }
-        end
+        private
 
-        def fallback_totals
-          { navbar_searches: -1 }
+        def counter_key(event)
+          "#{event}".to_sym
         end
       end
     end

spec/features/global_search_spec.rb

@@ -14,7 +14,8 @@ describe 'Global search' do
   end
 
   it 'increases usage ping searches counter' do
-    expect(Gitlab::UsageDataCounters::SearchCounter).to receive(:increment_navbar_searches_count)
+    expect(Gitlab::UsageDataCounters::SearchCounter).to receive(:count).with(:navbar_searches)
+    expect(Gitlab::UsageDataCounters::SearchCounter).to receive(:count).with(:all_searches)
 
     submit_search('foobar')
   end

spec/lib/gitlab/usage_data_counters/search_counter_spec.rb

@@ -3,11 +3,21 @@
 require 'spec_helper'
 
 describe Gitlab::UsageDataCounters::SearchCounter, :clean_gitlab_redis_shared_state do
-  it 'increments counter and return the total count' do
-    expect(described_class.total_navbar_searches_count).to eq(0)
+  shared_examples_for 'usage counter with totals' do |counter|
+    it 'increments counter and returns total count' do
+      expect(described_class.read(counter)).to eq(0)
 
-    2.times { described_class.increment_navbar_searches_count }
+      2.times { described_class.count(counter) }
 
-    expect(described_class.total_navbar_searches_count).to eq(2)
+      expect(described_class.read(counter)).to eq(2)
+    end
+  end
+
+  context 'all_searches counter' do
+    it_behaves_like 'usage counter with totals', :all_searches
+  end
+
+  context 'navbar_searches counter' do
+    it_behaves_like 'usage counter with totals', :navbar_searches
   end
 end

spec/requests/api/search_spec.rb

@@ -15,6 +15,14 @@ describe API::Search do
     it { expect(json_response.size).to eq(size) }
   end
 
+  shared_examples 'ping counters' do |scope:, search: ''|
+    it 'increases usage ping searches counter' do
+      expect(Gitlab::UsageDataCounters::SearchCounter).to receive(:count).with(:all_searches)
+
+      get api(endpoint, user), params: { scope: scope, search: search }
+    end
+  end
+
   shared_examples 'pagination' do |scope:, search: ''|
     it 'returns a different result for each page' do
       get api(endpoint, user), params: { scope: scope, search: search, page: 1, per_page: 1 }
@@ -75,6 +83,8 @@ describe API::Search do
         it_behaves_like 'response is correct', schema: 'public_api/v4/projects'
 
         it_behaves_like 'pagination', scope: :projects
+
+        it_behaves_like 'ping counters', scope: :projects
       end
 
       context 'for issues scope' do
@@ -86,6 +96,8 @@ describe API::Search do
 
         it_behaves_like 'response is correct', schema: 'public_api/v4/issues'
 
+        it_behaves_like 'ping counters', scope: :issues
+
         describe 'pagination' do
           before do
             create(:issue, project: project, title: 'another issue')
@@ -104,6 +116,8 @@ describe API::Search do
 
         it_behaves_like 'response is correct', schema: 'public_api/v4/merge_requests'
 
+        it_behaves_like 'ping counters', scope: :merge_requests
+
         describe 'pagination' do
           before do
             create(:merge_request, source_project: repo_project, title: 'another mr', target_branch: 'another_branch')
@@ -125,6 +139,8 @@ describe API::Search do
 
           it_behaves_like 'response is correct', schema: 'public_api/v4/milestones'
 
+          it_behaves_like 'ping counters', scope: :milestones
+
           describe 'pagination' do
             before do
               create(:milestone, project: project, title: 'another milestone')
@@ -161,6 +177,8 @@ describe API::Search do
 
         it_behaves_like 'pagination', scope: :users
 
+        it_behaves_like 'ping counters', scope: :users
+
         context 'when users search feature is disabled' do
           before do
             stub_feature_flags(users_search: false)
@@ -183,6 +201,8 @@ describe API::Search do
 
         it_behaves_like 'response is correct', schema: 'public_api/v4/snippets'
 
+        it_behaves_like 'ping counters', scope: :snippet_titles
+
         describe 'pagination' do
           before do
             create(:snippet, :public, title: 'another snippet', content: 'snippet content')
@@ -248,6 +268,8 @@ describe API::Search do
         it_behaves_like 'response is correct', schema: 'public_api/v4/projects'
 
         it_behaves_like 'pagination', scope: :projects
+
+        it_behaves_like 'ping counters', scope: :projects
       end
 
       context 'for issues scope' do
@@ -259,6 +281,8 @@ describe API::Search do
 
         it_behaves_like 'response is correct', schema: 'public_api/v4/issues'
 
+        it_behaves_like 'ping counters', scope: :issues
+
         describe 'pagination' do
           before do
             create(:issue, project: project, title: 'another issue')
@@ -277,6 +301,8 @@ describe API::Search do
 
         it_behaves_like 'response is correct', schema: 'public_api/v4/merge_requests'
 
+        it_behaves_like 'ping counters', scope: :merge_requests
+
         describe 'pagination' do
           before do
             create(:merge_request, source_project: repo_project, title: 'another mr', target_branch: 'another_branch')
@@ -295,6 +321,8 @@ describe API::Search do
 
         it_behaves_like 'response is correct', schema: 'public_api/v4/milestones'
 
+        it_behaves_like 'ping counters', scope: :milestones
+
         describe 'pagination' do
           before do
             create(:milestone, project: project, title: 'another milestone')
@@ -326,6 +354,8 @@ describe API::Search do
 
         it_behaves_like 'response is correct', schema: 'public_api/v4/user/basics'
 
+        it_behaves_like 'ping counters', scope: :users
+
         describe 'pagination' do
           before do
             create(:group_member, :developer, group: group)
@@ -395,7 +425,7 @@ describe API::Search do
       end
     end
 
-    context 'when user does can not see the project' do
+    context 'when user can not see the project' do
       it 'returns 404 error' do
         project.update!(visibility_level: Gitlab::VisibilityLevel::PRIVATE)
 
@@ -415,6 +445,8 @@ describe API::Search do
 
         it_behaves_like 'response is correct', schema: 'public_api/v4/issues'
 
+        it_behaves_like 'ping counters', scope: :issues
+
         describe 'pagination' do
           before do
             create(:issue, project: project, title: 'another issue')
@@ -435,6 +467,8 @@ describe API::Search do
 
         it_behaves_like 'response is correct', schema: 'public_api/v4/merge_requests'
 
+        it_behaves_like 'ping counters', scope: :merge_requests
+
         describe 'pagination' do
           before do
             create(:merge_request, source_project: repo_project, title: 'another mr', target_branch: 'another_branch')
@@ -456,6 +490,8 @@ describe API::Search do
 
           it_behaves_like 'response is correct', schema: 'public_api/v4/milestones'
 
+          it_behaves_like 'ping counters', scope: :milestones
+
           describe 'pagination' do
             before do
               create(:milestone, project: project, title: 'another milestone')
@@ -491,6 +527,8 @@ describe API::Search do
 
         it_behaves_like 'response is correct', schema: 'public_api/v4/user/basics'
 
+        it_behaves_like 'ping counters', scope: :users
+
         describe 'pagination' do
           before do
             create(:project_member, :developer, project: project)
@@ -521,6 +559,8 @@ describe API::Search do
 
         it_behaves_like 'response is correct', schema: 'public_api/v4/notes'
 
+        it_behaves_like 'ping counters', scope: :notes
+
         describe 'pagination' do
           before do
             mr = create(:merge_request, source_project: project, target_branch: 'another_branch')
@@ -542,6 +582,8 @@ describe API::Search do
 
         it_behaves_like 'response is correct', schema: 'public_api/v4/blobs'
 
+        it_behaves_like 'ping counters', scope: :wiki_blobs
+
         describe 'pagination' do
           before do
             create(:wiki_page, wiki: wiki, title: 'home 2', content: 'Another page')
@@ -561,6 +603,8 @@ describe API::Search do
         it_behaves_like 'response is correct', schema: 'public_api/v4/commits_details'
 
         it_behaves_like 'pagination', scope: :commits, search: 'merge'
+
+        it_behaves_like 'ping counters', scope: :commits
       end
 
       context 'for commits scope with project path as id' do
@@ -582,6 +626,8 @@ describe API::Search do
 
         it_behaves_like 'pagination', scope: :blobs, search: 'monitors'
 
+        it_behaves_like 'ping counters', scope: :blobs
+
         context 'filters' do
           it 'by filename' do
             get api(endpoint, user), params: { scope: 'blobs', search: 'mon filename:PROCESS.md' }