Merge branch 'kassio/github-importer-log-imported-objects-counters' into 'master'

GithubImporter: Count and log each object imported

See merge request gitlab-org/gitlab!64256

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

@@ -36,16 +36,29 @@ module Gitlab
 
         importer_class.new(object, project, client).execute
 
-        counter.increment
+        increment_counters(project)
+
         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
+
       # Returns the representation class to use for the object. This class must
       # define the class method `from_json_hash`.
       def representation_class

app/workers/gitlab/github_import/import_diff_note_worker.rb

@@ -13,6 +13,10 @@ module Gitlab
         Importer::DiffNoteImporter
       end
 
+      def object_type
+        :diff_note
+      end
+
       def counter_name
         :github_importer_imported_diff_notes
       end

app/workers/gitlab/github_import/import_issue_worker.rb

@@ -13,6 +13,10 @@ module Gitlab
         Importer::IssueAndLabelLinksImporter
       end
 
+      def object_type
+        :issue
+      end
+
       def counter_name
         :github_importer_imported_issues
       end

app/workers/gitlab/github_import/import_lfs_object_worker.rb

@@ -13,6 +13,10 @@ module Gitlab
         Importer::LfsObjectImporter
       end
 
+      def object_type
+        :lfs_object
+      end
+
       def counter_name
         :github_importer_imported_lfs_objects
       end

app/workers/gitlab/github_import/import_note_worker.rb

@@ -13,6 +13,10 @@ module Gitlab
         Importer::NoteImporter
       end
 
+      def object_type
+        :note
+      end
+
       def counter_name
         :github_importer_imported_notes
       end

app/workers/gitlab/github_import/import_pull_request_merged_by_worker.rb

@@ -15,6 +15,10 @@ module Gitlab
         Importer::PullRequestMergedByImporter
       end
 
+      def object_type
+        :pull_request_merged_by
+      end
+
       def counter_name
         :github_importer_imported_pull_requests_merged_by
       end

app/workers/gitlab/github_import/import_pull_request_review_worker.rb

@@ -15,6 +15,10 @@ module Gitlab
         Importer::PullRequestReviewImporter
       end
 
+      def object_type
+        :pull_request_review
+      end
+
       def counter_name
         :github_importer_imported_pull_request_reviews
       end

app/workers/gitlab/github_import/import_pull_request_worker.rb

@@ -13,6 +13,10 @@ module Gitlab
         Importer::PullRequestImporter
       end
 
+      def object_type
+        :pull_request
+      end
+
       def counter_name
         :github_importer_imported_pull_requests
       end

app/workers/gitlab/github_import/stage/finish_import_worker.rb

@@ -29,7 +29,8 @@ module Gitlab
           info(
             project.id,
             message: "GitHub project import finished",
-            duration_s: duration.round(2)
+            duration_s: duration.round(2),
+            object_counts: ::Gitlab::GithubImport::ObjectCounter.summary(project)
           )
         end
 

lib/gitlab/github_import/importer/diff_notes_importer.rb

@@ -22,6 +22,10 @@ module Gitlab
           :pull_requests_comments
         end
 
+        def object_type
+          :diff_note
+        end
+
         def id_for_already_imported_cache(note)
           note.id
         end

lib/gitlab/github_import/importer/issues_importer.rb

@@ -18,6 +18,10 @@ module Gitlab
           ImportIssueWorker
         end
 
+        def object_type
+          :issue
+        end
+
         def collection_method
           :issues
         end

lib/gitlab/github_import/importer/lfs_objects_importer.rb

@@ -18,6 +18,10 @@ module Gitlab
           ImportLfsObjectWorker
         end
 
+        def object_type
+          :lfs_object
+        end
+
         def collection_method
           :lfs_objects
         end
@@ -26,6 +30,8 @@ module Gitlab
           lfs_objects = Projects::LfsPointers::LfsObjectDownloadListService.new(project).execute
 
           lfs_objects.each do |object|
+            Gitlab::GithubImport::ObjectCounter.increment(project, object_type, :fetched)
+
             yield object
           end
         rescue StandardError => e

lib/gitlab/github_import/importer/notes_importer.rb

@@ -18,6 +18,10 @@ module Gitlab
           ImportNoteWorker
         end
 
+        def object_type
+          :note
+        end
+
         def collection_method
           :issues_comments
         end

lib/gitlab/github_import/importer/pull_requests_importer.rb

@@ -22,6 +22,10 @@ module Gitlab
           pr.number
         end
 
+        def object_type
+          :pull_request
+        end
+
         def each_object_to_import
           super do |pr|
             update_repository if update_repository?(pr)

lib/gitlab/github_import/importer/pull_requests_merged_by_importer.rb

@@ -22,6 +22,10 @@ module Gitlab
           :pull_requests_merged_by
         end
 
+        def object_type
+          :pull_request_merged_by
+        end
+
         def id_for_already_imported_cache(merge_request)
           merge_request.id
         end
@@ -30,6 +34,8 @@ module Gitlab
           project.merge_requests.with_state(:merged).find_each do |merge_request|
             next if already_imported?(merge_request)
 
+            Gitlab::GithubImport::ObjectCounter.increment(project, object_type, :fetched)
+
             pull_request = client.pull_request(project.import_source, merge_request.iid)
             yield(pull_request)
 

lib/gitlab/github_import/importer/pull_requests_reviews_importer.rb

@@ -29,6 +29,10 @@ module Gitlab
           :pull_request_reviews
         end
 
+        def object_type
+          :pull_request_review
+        end
+
         def id_for_already_imported_cache(review)
           review.id
         end
@@ -57,6 +61,8 @@ module Gitlab
           project.merge_requests.find_each do |merge_request|
             next if already_imported?(merge_request)
 
+            Gitlab::GithubImport::ObjectCounter.increment(project, object_type, :fetched)
+
             client
               .pull_request_reviews(project.import_source, merge_request.iid)
               .each do |review|
@@ -81,6 +87,8 @@ module Gitlab
             page.objects.each do |review|
               next if already_imported?(review)
 
+              Gitlab::GithubImport::ObjectCounter.increment(project, object_type, :fetched)
+
               review.merge_request_id = merge_request.id
               yield(review)
 

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.
+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}'
+      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)
+        end
+
+        def summary(project)
+          OPERATIONS.each_with_object({}) do |operation, result|
+            result[operation] = {}
+
+            CACHING
+              .values_from_set(counter_list_key(project, operation))
+              .sort
+              .each do |counter|
+                object_type = counter.split('/').last
+                result[operation][object_type] = CACHING.read_integer(counter)
+              end
+          end
+        end
+
+        private
+
+        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 }
+        end
+
+        def validate_operation!(operation)
+          unless operation.to_s.presence_in(OPERATIONS)
+            raise ArgumentError, "Operation must be #{OPERATIONS.join(' or ')}"
+          end
+        end
+      end
+    end
+  end
+end

lib/gitlab/github_import/parallel_scheduling.rb

@@ -103,6 +103,8 @@ module Gitlab
           page.objects.each do |object|
             next if already_imported?(object)
 
+            Gitlab::GithubImport::ObjectCounter.increment(project, object_type, :fetched)
+
             yield object
 
             # We mark the object as imported immediately so we don't end up
@@ -129,6 +131,10 @@ module Gitlab
         Gitlab::Cache::Import::Caching.set_add(already_imported_cache_key, id)
       end
 
+      def object_type
+        raise NotImplementedError
+      end
+
       # Returns the ID to use for the cache used for checking if an object has
       # already been imported or not.
       #

spec/lib/gitlab/github_import/object_counter_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Gitlab::GithubImport::ObjectCounter, :clean_gitlab_redis_cache do
+  let_it_be(:project) { create(:project) }
+
+  it 'validates the operation being incremented' do
+    expect { described_class.increment(project, :issue, :unknown) }
+      .to raise_error(ArgumentError, 'Operation must be fetched or imported')
+  end
+
+  it 'increments the counter and saves the key to be listed in the summary later' do
+    described_class.increment(project, :issue, :fetched)
+    described_class.increment(project, :issue, :fetched)
+    described_class.increment(project, :issue, :imported)
+    described_class.increment(project, :issue, :imported)
+
+    expect(described_class.summary(project)).to eq({
+      'fetched' => { 'issue' => 2 },
+      'imported' => { 'issue' => 2 }
+    })
+  end
+end

spec/lib/gitlab/github_import/parallel_scheduling_spec.rb

@@ -11,6 +11,10 @@ RSpec.describe Gitlab::GithubImport::ParallelScheduling do
         Class
       end
 
+      def object_type
+        :dummy
+      end
+
       def collection_method
         :issues
       end

spec/lib/gitlab/github_import_spec.rb

@@ -4,7 +4,7 @@ require 'spec_helper'
 
 RSpec.describe Gitlab::GithubImport do
   context 'github.com' do
-    let(:project) { double(:project, import_url: 'http://t0ken@github.com/user/repo.git') }
+    let(:project) { double(:project, import_url: 'http://t0ken@github.com/user/repo.git', id: 1) }
 
     it 'returns a new Client with a custom token' do
       expect(described_class::Client)

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

@@ -19,6 +19,10 @@ RSpec.describe Gitlab::GithubImport::ObjectImporter do
         'This is a counter'
       end
 
+      def object_type
+        :dummy
+      end
+
       def representation_class
         MockRepresantation
       end
@@ -42,7 +46,7 @@ RSpec.describe Gitlab::GithubImport::ObjectImporter do
     end)
   end
 
-  describe '#import' do
+  describe '#import', :clean_gitlab_redis_shared_state do
     let(:importer_class) { double(:importer_class, name: 'klass_name') }
     let(:importer_instance) { double(:importer_instance) }
     let(:project) { double(:project, full_path: 'foo/bar', id: 1) }
@@ -90,6 +94,11 @@ RSpec.describe Gitlab::GithubImport::ObjectImporter do
       end
 
       worker.import(project, client, { 'number' => 10, 'github_id' => 1 })
+
+      expect(Gitlab::GithubImport::ObjectCounter.summary(project)).to eq({
+        'fetched' => {},
+        'imported' => { 'dummy' => 1 }
+      })
     end
 
     it 'logs error when the import fails' do

spec/workers/gitlab/github_import/stage/finish_import_worker_spec.rb

@@ -33,6 +33,10 @@ RSpec.describe Gitlab::GithubImport::Stage::FinishImportWorker do
             message: 'GitHub project import finished',
             import_stage: 'Gitlab::GithubImport::Stage::FinishImportWorker',
             import_source: :github,
+            object_counts: {
+              'fetched' => {},
+              'imported' => {}
+            },
             project_id: project.id,
             duration_s: a_kind_of(Numeric)
           )