Merge branch 'kassio/github-importer-count-fetched-objects-globally' into 'master'

GithubImporter: Count fetched/import objects globally and by project

See merge request gitlab-org/gitlab!65700

app/workers/concerns/gitlab/github_import/object_importer.rb

@@ -36,25 +36,13 @@ module Gitlab
 
         importer_class.new(object, project, client).execute
 
-        increment_counters(project)
+        Gitlab::GithubImport::ObjectCounter.increment(project, object_type, :imported)
 
         info(project.id, message: 'importer finished')
       rescue StandardError => e
         error(project.id, e, hash)
       end
 
-      # Counters incremented:
-      # - global (prometheus): for metrics in Grafana
-      # - project (redis): used in FinishImportWorker to report number of objects imported
-      def increment_counters(project)
-        counter.increment
-        Gitlab::GithubImport::ObjectCounter.increment(project, object_type, :imported)
-      end
-
-      def counter
-        @counter ||= Gitlab::Metrics.counter(counter_name, counter_description)
-      end
-
       def object_type
         raise NotImplementedError
       end
@@ -70,16 +58,6 @@ module Gitlab
         raise NotImplementedError
       end
 
-      # Returns the name (as a Symbol) of the Prometheus counter.
-      def counter_name
-        raise NotImplementedError
-      end
-
-      # Returns the description (as a String) of the Prometheus counter.
-      def counter_description
-        raise NotImplementedError
-      end
-
       private
 
       attr_accessor :github_id

app/workers/gitlab/github_import/import_diff_note_worker.rb

@@ -16,14 +16,6 @@ module Gitlab
       def object_type
         :diff_note
       end
-
-      def counter_name
-        :github_importer_imported_diff_notes
-      end
-
-      def counter_description
-        'The number of imported GitHub pull request review comments'
-      end
     end
   end
 end

app/workers/gitlab/github_import/import_issue_worker.rb

@@ -16,14 +16,6 @@ module Gitlab
       def object_type
         :issue
       end
-
-      def counter_name
-        :github_importer_imported_issues
-      end
-
-      def counter_description
-        'The number of imported GitHub issues'
-      end
     end
   end
 end

app/workers/gitlab/github_import/import_lfs_object_worker.rb

@@ -16,14 +16,6 @@ module Gitlab
       def object_type
         :lfs_object
       end
-
-      def counter_name
-        :github_importer_imported_lfs_objects
-      end
-
-      def counter_description
-        'The number of imported GitHub Lfs Objects'
-      end
     end
   end
 end

app/workers/gitlab/github_import/import_note_worker.rb

@@ -16,14 +16,6 @@ module Gitlab
       def object_type
         :note
       end
-
-      def counter_name
-        :github_importer_imported_notes
-      end
-
-      def counter_description
-        'The number of imported GitHub comments'
-      end
     end
   end
 end

app/workers/gitlab/github_import/import_pull_request_merged_by_worker.rb

@@ -18,14 +18,6 @@ module Gitlab
       def object_type
         :pull_request_merged_by
       end
-
-      def counter_name
-        :github_importer_imported_pull_requests_merged_by
-      end
-
-      def counter_description
-        'The number of imported GitHub pull requests merged by'
-      end
     end
   end
 end

app/workers/gitlab/github_import/import_pull_request_review_worker.rb

@@ -18,14 +18,6 @@ module Gitlab
       def object_type
         :pull_request_review
       end
-
-      def counter_name
-        :github_importer_imported_pull_request_reviews
-      end
-
-      def counter_description
-        'The number of imported GitHub pull request reviews'
-      end
     end
   end
 end

app/workers/gitlab/github_import/import_pull_request_worker.rb

@@ -16,14 +16,6 @@ module Gitlab
       def object_type
         :pull_request
       end
-
-      def counter_name
-        :github_importer_imported_pull_requests
-      end
-
-      def counter_description
-        'The number of imported GitHub pull requests'
-      end
     end
   end
 end

lib/gitlab/github_import/object_counter.rb

 # frozen_string_literal: true
 
-# Count objects fetched or imported from Github in the context of the
-# project being imported.
+# Count objects fetched or imported from Github.
 module Gitlab
   module GithubImport
     class ObjectCounter
       OPERATIONS = %w[fetched imported].freeze
-      COUNTER_LIST_KEY = 'github-importer/object-counters-list/%{project}/%{operation}'
-      COUNTER_KEY = 'github-importer/object-counter/%{project}/%{operation}/%{object_type}'
+      PROJECT_COUNTER_LIST_KEY = 'github-importer/object-counters-list/%{project}/%{operation}'
+      PROJECT_COUNTER_KEY = 'github-importer/object-counter/%{project}/%{operation}/%{object_type}'
+
+      GLOBAL_COUNTER_KEY = 'github_importer_%{operation}_%{object_type}'
+      GLOBAL_COUNTER_DESCRIPTION = 'The number of %{operation} Github %{object_type}'
+
       CACHING = Gitlab::Cache::Import::Caching
 
       class << self
         def increment(project, object_type, operation)
           validate_operation!(operation)
 
-          counter_key = COUNTER_KEY % { project: project.id, operation: operation, object_type: object_type }
-
-          add_counter_to_list(project, operation, counter_key)
-
-          CACHING.increment(counter_key)
+          increment_project_counter(project, object_type, operation)
+          increment_global_counter(object_type, operation)
         end
 
         def summary(project)
@@ -37,12 +37,40 @@ module Gitlab
 
         private
 
+        # Global counters are long lived, in Prometheus,
+        # and it's used to report the health of the Github Importer
+        # in the Grafana Dashboard
+        # https://dashboards.gitlab.net/d/2zgM_rImz/github-importer?orgId=1
+        def increment_global_counter(object_type, operation)
+          key = GLOBAL_COUNTER_KEY % {
+            operation: operation,
+            object_type: object_type
+          }
+          description = GLOBAL_COUNTER_DESCRIPTION % {
+            operation: operation,
+            object_type: object_type.to_s.humanize
+          }
+
+          Gitlab::Metrics.counter(key.to_sym, description).increment
+        end
+
+        # Project counters are short lived, in Redis,
+        # and it's used to report how successful a project
+        # import was with the #summary method.
+        def increment_project_counter(project, object_type, operation)
+          counter_key = PROJECT_COUNTER_KEY % { project: project.id, operation: operation, object_type: object_type }
+
+          add_counter_to_list(project, operation, counter_key)
+
+          CACHING.increment(counter_key)
+        end
+
         def add_counter_to_list(project, operation, key)
           CACHING.set_add(counter_list_key(project, operation), key)
         end
 
         def counter_list_key(project, operation)
-          COUNTER_LIST_KEY % { project: project.id, operation: operation }
+          PROJECT_COUNTER_LIST_KEY % { project: project.id, operation: operation }
         end
 
         def validate_operation!(operation)

spec/lib/gitlab/github_import/object_counter_spec.rb

@@ -11,6 +11,18 @@ RSpec.describe Gitlab::GithubImport::ObjectCounter, :clean_gitlab_redis_cache do
   end
 
   it 'increments the counter and saves the key to be listed in the summary later' do
+    expect(Gitlab::Metrics)
+      .to receive(:counter)
+      .twice
+      .with(:github_importer_fetched_issue, 'The number of fetched Github Issue')
+      .and_return(double(increment: true))
+
+    expect(Gitlab::Metrics)
+      .to receive(:counter)
+      .twice
+      .with(:github_importer_imported_issue, 'The number of imported Github Issue')
+      .and_return(double(increment: true))
+
     described_class.increment(project, :issue, :fetched)
     described_class.increment(project, :issue, :fetched)
     described_class.increment(project, :issue, :imported)

spec/workers/concerns/gitlab/github_import/object_importer_spec.rb

@@ -11,14 +11,6 @@ RSpec.describe Gitlab::GithubImport::ObjectImporter do
 
       include(Gitlab::GithubImport::ObjectImporter)
 
-      def counter_name
-        :dummy_counter
-      end
-
-      def counter_description
-        'This is a counter'
-      end
-
       def object_type
         :dummy
       end
@@ -68,10 +60,6 @@ RSpec.describe Gitlab::GithubImport::ObjectImporter do
       expect(importer_instance)
         .to receive(:execute)
 
-      expect(worker.counter)
-        .to receive(:increment)
-        .and_call_original
-
       expect_next_instance_of(Gitlab::Import::Logger) do |logger|
         expect(logger)
           .to receive(:info)
@@ -185,18 +173,4 @@ RSpec.describe Gitlab::GithubImport::ObjectImporter do
         .to raise_error(KeyError, 'key not found: :github_id')
     end
   end
-
-  describe '#counter' do
-    it 'returns a Prometheus counter' do
-      expect(worker)
-        .to receive(:counter_name)
-        .and_call_original
-
-      expect(worker)
-        .to receive(:counter_description)
-        .and_call_original
-
-      worker.counter
-    end
-  end
 end

spec/workers/gitlab/github_import/import_diff_note_worker_spec.rb

@@ -34,7 +34,7 @@ RSpec.describe Gitlab::GithubImport::ImportDiffNoteWorker do
       expect(importer)
         .to receive(:execute)
 
-      expect(worker.counter)
+      expect(Gitlab::GithubImport::ObjectCounter)
         .to receive(:increment)
         .and_call_original
 

spec/workers/gitlab/github_import/import_issue_worker_spec.rb

@@ -37,7 +37,7 @@ RSpec.describe Gitlab::GithubImport::ImportIssueWorker do
       expect(importer)
         .to receive(:execute)
 
-      expect(worker.counter)
+      expect(Gitlab::GithubImport::ObjectCounter)
         .to receive(:increment)
         .and_call_original
 

spec/workers/gitlab/github_import/import_note_worker_spec.rb

@@ -32,7 +32,7 @@ RSpec.describe Gitlab::GithubImport::ImportNoteWorker do
       expect(importer)
         .to receive(:execute)
 
-      expect(worker.counter)
+      expect(Gitlab::GithubImport::ObjectCounter)
         .to receive(:increment)
         .and_call_original
 

spec/workers/gitlab/github_import/import_pull_request_merged_by_worker_spec.rb

@@ -12,12 +12,4 @@ RSpec.describe Gitlab::GithubImport::ImportPullRequestMergedByWorker do
   describe '#importer_class' do
     it { expect(subject.importer_class).to eq(Gitlab::GithubImport::Importer::PullRequestMergedByImporter) }
   end
-
-  describe '#counter_name' do
-    it { expect(subject.counter_name).to eq(:github_importer_imported_pull_requests_merged_by) }
-  end
-
-  describe '#counter_description' do
-    it { expect(subject.counter_description).to eq('The number of imported GitHub pull requests merged by') }
-  end
 end

spec/workers/gitlab/github_import/import_pull_request_review_worker_spec.rb

@@ -12,12 +12,4 @@ RSpec.describe Gitlab::GithubImport::ImportPullRequestReviewWorker do
   describe '#importer_class' do
     it { expect(subject.importer_class).to eq(Gitlab::GithubImport::Importer::PullRequestReviewImporter) }
   end
-
-  describe '#counter_name' do
-    it { expect(subject.counter_name).to eq(:github_importer_imported_pull_request_reviews) }
-  end
-
-  describe '#counter_description' do
-    it { expect(subject.counter_description).to eq('The number of imported GitHub pull request reviews') }
-  end
 end

spec/workers/gitlab/github_import/import_pull_request_worker_spec.rb

@@ -43,7 +43,7 @@ RSpec.describe Gitlab::GithubImport::ImportPullRequestWorker do
       expect(importer)
         .to receive(:execute)
 
-      expect(worker.counter)
+      expect(Gitlab::GithubImport::ObjectCounter)
         .to receive(:increment)
         .and_call_original