Merge branch '339392-track-failure-success-of-github-importer' into 'master'

Track failure/success of GitHub Importer

See merge request gitlab-org/gitlab!70012

app/services/projects/import_service.rb

@@ -16,6 +16,8 @@ module Projects
     end
 
     def execute
+      track_start_import
+
       add_repository_to_project
 
       download_lfs_objects
@@ -25,16 +27,17 @@ module Projects
       after_execute_hook
 
       success
-    rescue Gitlab::UrlBlocker::BlockedUrlError => e
-      Gitlab::ErrorTracking.track_exception(e, project_path: project.full_path, importer: project.import_type)
+    rescue Gitlab::UrlBlocker::BlockedUrlError, StandardError => e
+      Gitlab::Import::ImportFailureService.track(
+        project_id: project.id,
+        error_source: self.class.name,
+        exception: e,
+        metrics: true
+      )
 
-      error(s_("ImportProjects|Error importing repository %{project_safe_import_url} into %{project_full_path} - %{message}") % { project_safe_import_url: project.safe_import_url, project_full_path: project.full_path, message: e.message })
-    rescue StandardError => e
       message = Projects::ImportErrorFilter.filter_message(e.message)
-
-      Gitlab::ErrorTracking.track_exception(e, project_path: project.full_path, importer: project.import_type)
-
-      error(s_("ImportProjects|Error importing repository %{project_safe_import_url} into %{project_full_path} - %{message}") % { project_safe_import_url: project.safe_import_url, project_full_path: project.full_path, message: message })
+      error(s_("ImportProjects|Error importing repository %{project_safe_import_url} into %{project_full_path} - %{message}") %
+              { project_safe_import_url: project.safe_import_url, project_full_path: project.full_path, message: message })
     end
 
     protected
@@ -54,6 +57,10 @@ module Projects
       # Defined in EE::Projects::ImportService
     end
 
+    def track_start_import
+      has_importer? && importer_class.try(:track_start_import, project)
+    end
+
     def add_repository_to_project
       if project.external_import? && !unknown_url?
         begin

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

@@ -18,36 +18,28 @@ module Gitlab
 
         # project - An instance of Project.
         def import(_, project)
+          @project = project
           project.after_import
-          report_import_time(project)
+          report_import_time
         end
 
-        def report_import_time(project)
-          duration = Time.zone.now - project.created_at
+        private
 
-          histogram.observe({ project: project.full_path }, duration)
-          counter.increment
+        attr_reader :project
+
+        def report_import_time
+          metrics.track_finished_import
 
           info(
             project.id,
             message: "GitHub project import finished",
-            duration_s: duration.round(2),
+            duration_s: metrics.duration.round(2),
             object_counts: ::Gitlab::GithubImport::ObjectCounter.summary(project)
           )
         end
 
-        def histogram
-          @histogram ||= Gitlab::Metrics.histogram(
-            :github_importer_total_duration_seconds,
-            'Total time spent importing GitHub projects, in seconds'
-          )
-        end
-
-        def counter
-          @counter ||= Gitlab::Metrics.counter(
-            :github_importer_imported_projects,
-            'The number of imported GitHub projects'
-          )
+        def metrics
+          @metrics ||= Gitlab::Import::Metrics.new(:github_importer, project)
         end
       end
     end

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

@@ -31,6 +31,22 @@ module Gitlab
           project.import_state.refresh_jid_expiration
 
           ImportPullRequestsWorker.perform_async(project.id)
+        rescue StandardError => e
+          Gitlab::Import::ImportFailureService.track(
+            project_id: project.id,
+            error_source: self.class.name,
+            exception: e,
+            fail_import: abort_on_failure,
+            metrics: true
+          )
+
+          raise(e)
+        end
+
+        private
+
+        def abort_on_failure
+          true
         end
       end
     end

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

@@ -27,6 +27,22 @@ module Gitlab
             { waiter.key => waiter.jobs_remaining },
             :pull_requests_merged_by
           )
+        rescue StandardError => e
+          Gitlab::Import::ImportFailureService.track(
+            project_id: project.id,
+            error_source: self.class.name,
+            exception: e,
+            fail_import: abort_on_failure,
+            metrics: true
+          )
+
+          raise(e)
+        end
+
+        private
+
+        def abort_on_failure
+          true
         end
       end
     end

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

@@ -33,6 +33,17 @@ module Gitlab
           counter.increment
 
           ImportBaseDataWorker.perform_async(project.id)
+
+        rescue StandardError => e
+          Gitlab::Import::ImportFailureService.track(
+            project_id: project.id,
+            error_source: self.class.name,
+            exception: e,
+            fail_import: abort_on_failure,
+            metrics: true
+          )
+
+          raise(e)
         end
 
         def counter

config/feature_flags/development/track_importer_activity.yml

+---
+name: track_importer_activity
+introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/70012
+rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/339392
+milestone: '14.4'
+type: development
+group: group::import
+default_enabled: false

config/metrics/counts_28d/20210930125418_github_import_project_start_monthly.yml

+---
+key_path: redis_hll_counters.importer.github_import_project_start_monthly
+description: The number of github projects that were enqueued to start monthy
+product_section: dev
+product_stage: devops
+product_group: group::import
+product_category:
+value_type: number
+status: active
+milestone: "14.4"
+introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/70012
+time_frame: 28d
+data_source: redis_hll
+data_category: optional
+instrumentation_class: RedisHLLMetric
+options:
+  events:
+    - github_import_project_start
+performance_indicator_type: []
+distribution:
+- ce
+- ee
+tier:
+- free
+- premium
+- ultimate

config/metrics/counts_28d/20210930130531_github_import_project_success_monthly.yml

+---
+key_path: redis_hll_counters.importer.github_import_project_success_monthly
+description: The number of github projects that were successful monthly
+product_section: dev
+product_stage: devops
+product_group: group::import
+product_category:
+value_type: number
+status: active
+milestone: "14.4"
+introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/70012
+time_frame: 28d
+data_source: redis_hll
+data_category: optional
+instrumentation_class: RedisHLLMetric
+options:
+  events:
+    - github_import_project_success
+distribution:
+- ce
+- ee
+tier:
+- free
+- premium
+- ultimate

config/metrics/counts_28d/20210930163813_github_import_project_failure_monthly.yml

+---
+key_path: redis_hll_counters.importer.github_import_project_failure_monthly
+description: The number of github projects that failed monthly
+product_section: dev
+product_stage: devops
+product_group: group::import
+product_category:
+value_type: number
+status: active
+milestone: "14.4"
+introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/70012
+time_frame: 28d
+data_source: redis_hll
+data_category: optional
+instrumentation_class: RedisHLLMetric
+options:
+  events:
+    - github_import_project_failure
+distribution:
+- ce
+- ee
+tier:
+- free
+- premium
+- ultimate

config/metrics/counts_7d/20210930125411_github_import_project_start_weekly.yml

+---
+key_path: redis_hll_counters.importer.github_import_project_start_weekly
+description: The number of github projects that were enqueued to start weekly
+product_section: dev
+product_stage: devops
+product_group: group::import
+product_category:
+value_type: number
+status: active
+milestone: "14.4"
+introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/70012
+time_frame: 7d
+data_source: redis_hll
+data_category: optional
+instrumentation_class: RedisHLLMetric
+options:
+  events:
+    - github_import_project_start
+performance_indicator_type: []
+distribution:
+- ce
+- ee
+tier:
+- free
+- premium
+- ultimate

config/metrics/counts_7d/20210930130525_github_import_project_success_weekly.yml

+---
+key_path: redis_hll_counters.importer.github_import_project_success_weekly
+description: The number of github projects that were successful weekly
+product_section: dev
+product_stage: devops
+product_group: group::import
+product_category:
+value_type: number
+status: active
+milestone: "14.4"
+introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/70012
+time_frame: 7d
+data_source: redis_hll
+data_category: optional
+instrumentation_class: RedisHLLMetric
+options:
+  events:
+    - github_import_project_success
+performance_indicator_type: []
+distribution:
+- ce
+- ee
+tier:
+- free
+- premium
+- ultimate

config/metrics/counts_7d/20210930163807_github_import_project_failure_weekly.yml

+---
+key_path: redis_hll_counters.importer.github_import_project_failure_weekly
+description: The number of github projects that failed weekly
+product_section: dev
+product_stage: devops
+product_group: group::import
+product_category:
+value_type: number
+status: active
+milestone: "14.4"
+introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/70012
+time_frame: 7d
+data_source: redis_hll
+data_category: optional
+instrumentation_class: RedisHLLMetric
+options:
+  events:
+    - github_import_project_failure
+distribution:
+- ce
+- ee
+tier:
+- free
+- premium
+- ultimate

ee/spec/config/metrics/every_metric_definition_spec.rb

@@ -83,8 +83,10 @@ RSpec.describe 'Every metric definition' do
     stub_usage_data_connections
   end
 
-  it 'is included in the Usage Ping hash structure' do
+  it 'is included in the Usage Ping hash structure', :aggregate_failures do
+    msg = "see https://docs.gitlab.com/ee/development/service_ping/metrics_dictionary.html#metrics-added-dynamic-to-service-ping-payload"
     expect(metric_files_key_paths).to match_array(usage_ping_key_paths)
+    expect(metric_files_key_paths).to match_array(usage_ping_key_paths), msg
   end
 
   context 'with value json schema' do

lib/gitlab/github_import/parallel_importer.rb

@@ -15,6 +15,10 @@ module Gitlab
         true
       end
 
+      def self.track_start_import(project)
+        Gitlab::Import::Metrics.new(:github_importer, project).track_start_import
+      end
+
       # This is a workaround for a Ruby 2.3.7 bug. rspec-mocks cannot restore
       # the visibility of prepended modules. See
       # https://github.com/rspec/rspec-mocks/issues/1231 for more details.

lib/gitlab/github_import/parallel_scheduling.rb

@@ -53,7 +53,8 @@ module Gitlab
           project_id: project.id,
           error_source: self.class.name,
           exception: e,
-          fail_import: abort_on_failure
+          fail_import: abort_on_failure,
+          metrics: true
         )
 
         raise(e)

lib/gitlab/github_import/sequential_importer.rb

@@ -33,18 +33,41 @@ module Gitlab
       end
 
       def execute
-        Importer::RepositoryImporter.new(project, client).execute
+        metrics.track_start_import
 
-        SEQUENTIAL_IMPORTERS.each do |klass|
-          klass.new(project, client).execute
+        begin
+          Importer::RepositoryImporter.new(project, client).execute
+
+          SEQUENTIAL_IMPORTERS.each do |klass|
+            klass.new(project, client).execute
+          end
+
+        rescue StandardError => e
+          Gitlab::Import::ImportFailureService.track(
+            project_id: project.id,
+            error_source: self.class.name,
+            exception: e,
+            fail_import: true,
+            metrics: true
+          )
+
+          raise(e)
         end
 
         PARALLEL_IMPORTERS.each do |klass|
           klass.new(project, client, parallel: false).execute
         end
 
+        metrics.track_finished_import
+
         true
       end
+
+      private
+
+      def metrics
+        @metrics ||= Gitlab::Import::Metrics.new(:github_importer, project)
+      end
     end
   end
 end

lib/gitlab/import/import_failure_service.rb

@@ -8,14 +8,15 @@ module Gitlab
         import_state: nil,
         project_id: nil,
         error_source: nil,
-        fail_import: false
+        fail_import: false,
+        metrics: false
       )
         new(
           exception: exception,
           import_state: import_state,
           project_id: project_id,
           error_source: error_source
-        ).execute(fail_import: fail_import)
+        ).execute(fail_import: fail_import, metrics: metrics)
       end
 
       def initialize(exception:, import_state: nil, project_id: nil, error_source: nil)
@@ -35,10 +36,11 @@ module Gitlab
         @error_source = error_source
       end
 
-      def execute(fail_import:)
+      def execute(fail_import:, metrics:)
         track_exception
         persist_failure
 
+        track_metrics if metrics
         import_state.mark_as_failed(exception.message) if fail_import
       end
 
@@ -71,6 +73,10 @@ module Gitlab
           correlation_id_value: Labkit::Correlation::CorrelationId.current_or_new_id
         )
       end
+
+      def track_metrics
+        Gitlab::Import::Metrics.new("#{project.import_type}_importer", project).track_failed_import
+      end
     end
   end
 end

lib/gitlab/import/metrics.rb

@@ -3,20 +3,35 @@
 module Gitlab
   module Import
     class Metrics
+      include Gitlab::Utils::UsageData
+
       IMPORT_DURATION_BUCKETS = [0.5, 1, 3, 5, 10, 60, 120, 240, 360, 720, 1440].freeze
 
-      attr_reader :importer
+      attr_reader :importer, :duration
 
       def initialize(importer, project)
         @importer = importer
         @project = project
       end
 
+      def track_start_import
+        return unless project.github_import?
+
+        track_usage_event(:github_import_project_start, project.id)
+      end
+
       def track_finished_import
-        duration = Time.zone.now - @project.created_at
+        @duration = Time.zone.now - project.created_at
 
-        duration_histogram.observe({ importer: importer }, duration)
+        observe_histogram
         projects_counter.increment
+        track_finish_metric
+      end
+
+      def track_failed_import
+        return unless project.github_import?
+
+        track_usage_event(:github_import_project_failure, project.id)
       end
 
       def issues_counter
@@ -35,6 +50,8 @@ module Gitlab
 
       private
 
+      attr_reader :project
+
       def duration_histogram
         @duration_histogram ||= Gitlab::Metrics.histogram(
           :"#{importer}_total_duration_seconds",
@@ -50,6 +67,20 @@ module Gitlab
           'The number of imported projects'
         )
       end
+
+      def observe_histogram
+        if project.github_import?
+          duration_histogram.observe({ project: project.full_path }, duration)
+        else
+          duration_histogram.observe({ importer: importer }, duration)
+        end
+      end
+
+      def track_finish_metric
+        return unless project.github_import?
+
+        track_usage_event(:github_import_project_success, project.id)
+      end
     end
   end
 end

lib/gitlab/usage_data_counters/known_events/importer_events.yml

+---
+# Importer events
+- name: github_import_project_start
+  category: importer
+  redis_slot: import
+  aggregation: weekly
+  feature_flag: track_importer_activity
+- name: github_import_project_success
+  category: importer
+  redis_slot: import
+  aggregation: weekly
+  feature_flag: track_importer_activity
+- name: github_import_project_failure
+  category: importer
+  redis_slot: import
+  aggregation: weekly
+  feature_flag: track_importer_activity

spec/lib/gitlab/github_import/parallel_importer_spec.rb

@@ -9,6 +9,18 @@ RSpec.describe Gitlab::GithubImport::ParallelImporter do
     end
   end
 
+  describe '.track_start_import' do
+    it 'tracks the start of import' do
+      project = double(:project)
+      metrics = double(:metrics)
+
+      expect(Gitlab::Import::Metrics).to receive(:new).with(:github_importer, project).and_return(metrics)
+      expect(metrics).to receive(:track_start_import)
+
+      described_class.track_start_import(project)
+    end
+  end
+
   describe '#execute', :clean_gitlab_redis_shared_state do
     let(:project) { create(:project) }
     let(:importer) { described_class.new(project) }

spec/lib/gitlab/github_import/parallel_scheduling_spec.rb

@@ -130,7 +130,8 @@ RSpec.describe Gitlab::GithubImport::ParallelScheduling do
             project_id: project.id,
             exception: exception,
             error_source: 'MyImporter',
-            fail_import: false
+            fail_import: false,
+            metrics: true
           ).and_call_original
 
         expect { importer.execute }
@@ -195,7 +196,8 @@ RSpec.describe Gitlab::GithubImport::ParallelScheduling do
             project_id: project.id,
             exception: exception,
             error_source: 'MyImporter',
-            fail_import: true
+            fail_import: true,
+            metrics: true
           ).and_call_original
 
         expect { importer.execute }

spec/lib/gitlab/github_import/sequential_importer_spec.rb

@@ -4,10 +4,17 @@ require 'spec_helper'
 
 RSpec.describe Gitlab::GithubImport::SequentialImporter do
   describe '#execute' do
+    let_it_be(:project) do
+      create(:project, import_url: 'http://t0ken@github.another-domain.com/repo-org/repo.git', import_type: 'github')
+    end
+
+    subject(:importer) { described_class.new(project, token: 'foo') }
+
     it 'imports a project in sequence' do
-      repository = double(:repository)
-      project = double(:project, id: 1, repository: repository, import_url: 'http://t0ken@github.another-domain.com/repo-org/repo.git', group: nil)
-      importer = described_class.new(project, token: 'foo')
+      expect_next_instance_of(Gitlab::Import::Metrics) do |instance|
+        expect(instance).to receive(:track_start_import)
+        expect(instance).to receive(:track_finished_import)
+      end
 
       expect_next_instance_of(Gitlab::GithubImport::Importer::RepositoryImporter) do |instance|
         expect(instance).to receive(:execute)
@@ -35,5 +42,23 @@ RSpec.describe Gitlab::GithubImport::SequentialImporter do
 
       expect(importer.execute).to eq(true)
     end
+
+    it 'raises an error' do
+      exception = StandardError.new('_some_error_')
+
+      expect_next_instance_of(Gitlab::GithubImport::Importer::RepositoryImporter) do |importer|
+        expect(importer).to receive(:execute).and_raise(exception)
+      end
+      expect(Gitlab::Import::ImportFailureService).to receive(:track)
+                                                        .with(
+                                                          project_id: project.id,
+                                                          exception: exception,
+                                                          error_source: described_class.name,
+                                                          fail_import: true,
+                                                          metrics: true
+                                                        ).and_call_original
+
+      expect { importer.execute }.to raise_error(StandardError)
+    end
   end
 end

spec/lib/gitlab/import/import_failure_service_spec.rb

@@ -2,135 +2,171 @@
 
 require 'spec_helper'
 
-RSpec.describe Gitlab::Import::ImportFailureService do
+RSpec.describe Gitlab::Import::ImportFailureService, :aggregate_failures do
   let_it_be(:import_type) { 'import_type' }
+  let_it_be(:project) { create(:project, :import_started, import_type: import_type) }
 
-  let_it_be(:project) do
-    create(
-      :project,
-      :import_started,
-      import_type: import_type
-    )
-  end
-
-  let(:import_state) { project.import_state }
   let(:exception) { StandardError.new('some error') }
+  let(:arguments) { { project_id: project.id } }
+  let(:base_arguments) { { error_source: 'SomeImporter', exception: exception }.merge(arguments) }
+  let(:exe_arguments) { { fail_import: false, metrics: false } }
+
+  describe '.track' do
+    context 'with all arguments provided' do
+      let(:instance) { double(:failure_service) }
+      let(:instance_arguments) do
+        {
+          exception: exception,
+          import_state: '_import_state_',
+          project_id: '_project_id_',
+          error_source: '_error_source_'
+        }
+      end
 
-  shared_examples 'logs the exception and fails the import' do
-    it 'when the failure does not abort the import' do
-      expect(Gitlab::ErrorTracking)
-        .to receive(:track_exception)
-        .with(
-          exception,
-          project_id: project.id,
-          import_type: import_type,
-          source: 'SomeImporter'
-        )
-
-      expect(Gitlab::Import::Logger)
-        .to receive(:error)
-        .with(
-          message: 'importer failed',
-          'error.message': 'some error',
-          project_id: project.id,
-          import_type: import_type,
-          source: 'SomeImporter'
-        )
-
-      described_class.track(**arguments)
-
-      expect(project.import_state.reload.status).to eq('failed')
-
-      expect(project.import_failures).not_to be_empty
-      expect(project.import_failures.last.exception_class).to eq('StandardError')
-      expect(project.import_failures.last.exception_message).to eq('some error')
-    end
-  end
+      let(:exe_arguments) do
+        {
+          fail_import: '_fail_import_',
+          metrics: '_metrics_'
+        }
+      end
 
-  shared_examples 'logs the exception and does not fail the import' do
-    it 'when the failure does not abort the import' do
-      expect(Gitlab::ErrorTracking)
-        .to receive(:track_exception)
-        .with(
-          exception,
-          project_id: project.id,
-          import_type: import_type,
-          source: 'SomeImporter'
-        )
-
-      expect(Gitlab::Import::Logger)
-        .to receive(:error)
-        .with(
-          message: 'importer failed',
-          'error.message': 'some error',
-          project_id: project.id,
-          import_type: import_type,
-          source: 'SomeImporter'
-        )
-
-      described_class.track(**arguments)
-
-      expect(project.import_state.reload.status).to eq('started')
-
-      expect(project.import_failures).not_to be_empty
-      expect(project.import_failures.last.exception_class).to eq('StandardError')
-      expect(project.import_failures.last.exception_message).to eq('some error')
+      it 'invokes a new instance and executes' do
+        expect(described_class).to receive(:new).with(**instance_arguments).and_return(instance)
+        expect(instance).to receive(:execute).with(**exe_arguments)
+
+        described_class.track(**instance_arguments.merge(exe_arguments))
+      end
     end
-  end
 
-  context 'when using the project as reference' do
-    context 'when it fails the import' do
-      let(:arguments) do
+    context 'with only necessary arguments utilizing defaults' do
+      let(:instance) { double(:failure_service) }
+      let(:instance_arguments) do
         {
-          project_id: project.id,
           exception: exception,
-          error_source: 'SomeImporter',
-          fail_import: true
+          import_state: nil,
+          project_id: nil,
+          error_source: nil
         }
       end
 
-      it_behaves_like 'logs the exception and fails the import'
-    end
-
-    context 'when it does not fail the import' do
-      let(:arguments) do
+      let(:exe_arguments) do
         {
-          project_id: project.id,
-          exception: exception,
-          error_source: 'SomeImporter',
-          fail_import: false
+          fail_import: false,
+          metrics: false
         }
       end
 
-      it_behaves_like 'logs the exception and does not fail the import'
+      it 'invokes a new instance and executes' do
+        expect(described_class).to receive(:new).with(**instance_arguments).and_return(instance)
+        expect(instance).to receive(:execute).with(**exe_arguments)
+
+        described_class.track(exception: exception)
+      end
     end
   end
 
-  context 'when using the import_state as reference' do
-    context 'when it fails the import' do
-      let(:arguments) do
-        {
-          import_state: import_state,
-          exception: exception,
-          error_source: 'SomeImporter',
-          fail_import: true
-        }
+  describe '#execute' do
+    subject(:service) { described_class.new(**base_arguments) }
+
+    shared_examples 'logs the exception and fails the import' do
+      it 'when the failure does not abort the import' do
+        expect(Gitlab::ErrorTracking)
+          .to receive(:track_exception)
+          .with(
+            exception,
+            project_id: project.id,
+            import_type: import_type,
+            source: 'SomeImporter'
+          )
+
+        expect(Gitlab::Import::Logger)
+          .to receive(:error)
+          .with(
+            message: 'importer failed',
+            'error.message': 'some error',
+            project_id: project.id,
+            import_type: import_type,
+            source: 'SomeImporter'
+          )
+
+        service.execute(**exe_arguments)
+
+        expect(project.import_state.reload.status).to eq('failed')
+
+        expect(project.import_failures).not_to be_empty
+        expect(project.import_failures.last.exception_class).to eq('StandardError')
+        expect(project.import_failures.last.exception_message).to eq('some error')
       end
+    end
 
-      it_behaves_like 'logs the exception and fails the import'
+    shared_examples 'logs the exception and does not fail the import' do
+      it 'when the failure does not abort the import' do
+        expect(Gitlab::ErrorTracking)
+          .to receive(:track_exception)
+          .with(
+            exception,
+            project_id: project.id,
+            import_type: import_type,
+            source: 'SomeImporter'
+          )
+
+        expect(Gitlab::Import::Logger)
+          .to receive(:error)
+          .with(
+            message: 'importer failed',
+            'error.message': 'some error',
+            project_id: project.id,
+            import_type: import_type,
+            source: 'SomeImporter'
+          )
+
+        service.execute(**exe_arguments)
+
+        expect(project.import_state.reload.status).to eq('started')
+
+        expect(project.import_failures).not_to be_empty
+        expect(project.import_failures.last.exception_class).to eq('StandardError')
+        expect(project.import_failures.last.exception_message).to eq('some error')
+      end
     end
 
-    context 'when it does not fail the import' do
-      let(:arguments) do
-        {
-          import_state: import_state,
-          exception: exception,
-          error_source: 'SomeImporter',
-          fail_import: false
-        }
+    context 'when tracking metrics' do
+      let(:exe_arguments) { { fail_import: false, metrics: true } }
+
+      it 'tracks the failed import' do
+        metrics = double(:metrics)
+
+        expect(Gitlab::Import::Metrics).to receive(:new).with("#{project.import_type}_importer", project).and_return(metrics)
+        expect(metrics).to receive(:track_failed_import)
+
+        service.execute(**exe_arguments)
       end
+    end
+
+    context 'when using the project as reference' do
+      context 'when it fails the import' do
+        let(:exe_arguments) { { fail_import: true, metrics: false } }
 
-      it_behaves_like 'logs the exception and does not fail the import'
+        it_behaves_like 'logs the exception and fails the import'
+      end
+
+      context 'when it does not fail the import' do
+        it_behaves_like 'logs the exception and does not fail the import'
+      end
+    end
+
+    context 'when using the import_state as reference' do
+      let(:arguments) { { import_state: project.import_state } }
+
+      context 'when it fails the import' do
+        let(:exe_arguments) { { fail_import: true, metrics: false } }
+
+        it_behaves_like 'logs the exception and fails the import'
+      end
+
+      context 'when it does not fail the import' do
+        it_behaves_like 'logs the exception and does not fail the import'
+      end
     end
   end
 end

spec/lib/gitlab/import/metrics_spec.rb

@@ -4,7 +4,7 @@ require 'spec_helper'
 
 RSpec.describe Gitlab::Import::Metrics, :aggregate_failures do
   let(:importer) { :test_importer }
-  let(:project) { double(:project, created_at: Time.current) }
+  let(:project) { build(:project, id: non_existing_record_id, created_at: Time.current) }
   let(:histogram) { double(:histogram) }
   let(:counter) { double(:counter) }
 
@@ -13,6 +13,51 @@ RSpec.describe Gitlab::Import::Metrics, :aggregate_failures do
   before do
     allow(Gitlab::Metrics).to receive(:counter) { counter }
     allow(counter).to receive(:increment)
+    allow(histogram).to receive(:observe)
+  end
+
+  describe '#track_start_import' do
+    context 'when project is not a github import' do
+      it 'does not emit importer metrics' do
+        expect(subject).not_to receive(:track_usage_event)
+
+        subject.track_start_import
+      end
+    end
+
+    context 'when project is a github import' do
+      before do
+        project.import_type = 'github'
+      end
+
+      it 'emits importer metrics' do
+        expect(subject).to receive(:track_usage_event).with(:github_import_project_start, project.id)
+
+        subject.track_start_import
+      end
+    end
+  end
+
+  describe '#track_failed_import' do
+    context 'when project is not a github import' do
+      it 'does not emit importer metrics' do
+        expect(subject).not_to receive(:track_usage_event)
+
+        subject.track_failed_import
+      end
+    end
+
+    context 'when project is a github import' do
+      before do
+        project.import_type = 'github'
+      end
+
+      it 'emits importer metrics' do
+        expect(subject).to receive(:track_usage_event).with(:github_import_project_failure, project.id)
+
+        subject.track_failed_import
+      end
+    end
   end
 
   describe '#track_finished_import' do
@@ -34,9 +79,34 @@ RSpec.describe Gitlab::Import::Metrics, :aggregate_failures do
       )
 
       expect(counter).to receive(:increment)
-      expect(histogram).to receive(:observe).with({ importer: :test_importer }, anything)
 
       subject.track_finished_import
+
+      expect(subject.duration).not_to be_nil
+    end
+
+    context 'when project is not a github import' do
+      it 'does not emit importer metrics' do
+        expect(subject).not_to receive(:track_usage_event)
+
+        subject.track_finished_import
+
+        expect(histogram).to have_received(:observe).with({ importer: :test_importer }, anything)
+      end
+    end
+
+    context 'when project is a github import' do
+      before do
+        project.import_type = 'github'
+      end
+
+      it 'emits importer metrics' do
+        expect(subject).to receive(:track_usage_event).with(:github_import_project_success, project.id)
+
+        subject.track_finished_import
+
+        expect(histogram).to have_received(:observe).with({ project: project.full_path }, anything)
+      end
     end
   end
 

spec/lib/gitlab/usage_data_counters/hll_redis_counter_spec.rb

@@ -47,6 +47,7 @@ RSpec.describe Gitlab::UsageDataCounters::HLLRedisCounter, :clean_gitlab_redis_s
         'epics_usage',
         'epic_boards_usage',
         'secure',
+        'importer',
         'network_policies'
       )
     end

spec/services/projects/import_service_spec.rb

@@ -86,6 +86,12 @@ RSpec.describe Projects::ImportService do
       end
 
       context 'with a Github repository' do
+        it 'tracks the start of import' do
+          expect(Gitlab::GithubImport::ParallelImporter).to receive(:track_start_import)
+
+          subject.execute
+        end
+
         it 'succeeds if repository import was scheduled' do
           expect_any_instance_of(Gitlab::GithubImport::ParallelImporter)
             .to receive(:execute)

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

@@ -7,39 +7,27 @@ RSpec.describe Gitlab::GithubImport::Stage::FinishImportWorker do
   let(:worker) { described_class.new }
 
   describe '#perform' do
-    it 'marks the import as finished' do
+    it 'marks the import as finished and reports import statistics' do
       expect(project).to receive(:after_import)
-      expect(worker).to receive(:report_import_time).with(project)
-
-      worker.import(double(:client), project)
-    end
-  end
-
-  describe '#report_import_time' do
-    it 'reports the total import time' do
-      expect(worker.histogram)
-        .to receive(:observe)
-        .with({ project: project.path_with_namespace }, a_kind_of(Numeric))
-        .and_call_original
-
-      expect(worker.counter)
-        .to receive(:increment)
-        .and_call_original
+      expect_next_instance_of(Gitlab::Import::Metrics) do |instance|
+        expect(instance).to receive(:track_finished_import)
+        expect(instance).to receive(:duration).and_return(3.005)
+      end
 
       expect(Gitlab::GithubImport::Logger)
         .to receive(:info)
-        .with(
-          message: 'GitHub project import finished',
-          import_stage: 'Gitlab::GithubImport::Stage::FinishImportWorker',
-          object_counts: {
-            'fetched' => {},
-            'imported' => {}
-          },
-          project_id: project.id,
-          duration_s: a_kind_of(Numeric)
-        )
+              .with(
+                message: 'GitHub project import finished',
+                import_stage: 'Gitlab::GithubImport::Stage::FinishImportWorker',
+                object_counts: {
+                  'fetched' => {},
+                  'imported' => {}
+                },
+                project_id: project.id,
+                duration_s: 3.01
+              )
 
-      worker.report_import_time(project)
+      worker.import(double(:client), project)
     end
   end
 end

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

@@ -3,15 +3,15 @@
 require 'spec_helper'
 
 RSpec.describe Gitlab::GithubImport::Stage::ImportBaseDataWorker do
-  let(:project) { create(:project) }
-  let(:import_state) { create(:import_state, project: project) }
+  let_it_be(:project) { create(:project) }
+  let_it_be(:import_state) { create(:import_state, project: project) }
+
   let(:worker) { described_class.new }
+  let(:importer) { double(:importer) }
+  let(:client) { double(:client) }
 
   describe '#import' do
     it 'imports the base data of a project' do
-      importer = double(:importer)
-      client = double(:client)
-
       described_class::IMPORTERS.each do |klass|
         expect(klass)
           .to receive(:new)
@@ -29,5 +29,23 @@ RSpec.describe Gitlab::GithubImport::Stage::ImportBaseDataWorker do
 
       worker.import(client, project)
     end
+
+    it 'raises an error' do
+      exception = StandardError.new('_some_error_')
+
+      expect_next_instance_of(Gitlab::GithubImport::Importer::LabelsImporter) do |importer|
+        expect(importer).to receive(:execute).and_raise(exception)
+      end
+      expect(Gitlab::Import::ImportFailureService).to receive(:track)
+                                                        .with(
+                                                          project_id: project.id,
+                                                          exception: exception,
+                                                          error_source: described_class.name,
+                                                          fail_import: true,
+                                                          metrics: true
+                                                        ).and_call_original
+
+      expect { worker.import(client, project) }.to raise_error(StandardError)
+    end
   end
 end

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

@@ -3,14 +3,15 @@
 require 'spec_helper'
 
 RSpec.describe Gitlab::GithubImport::Stage::ImportPullRequestsWorker do
-  let(:project) { create(:project) }
-  let(:import_state) { create(:import_state, project: project) }
+  let_it_be(:project) { create(:project) }
+  let_it_be(:import_state) { create(:import_state, project: project) }
+
   let(:worker) { described_class.new }
+  let(:importer) { double(:importer) }
+  let(:client) { double(:client) }
 
   describe '#import' do
     it 'imports all the pull requests' do
-      importer = double(:importer)
-      client = double(:client)
       waiter = Gitlab::JobWaiter.new(2, '123')
 
       expect(Gitlab::GithubImport::Importer::PullRequestsImporter)
@@ -32,4 +33,22 @@ RSpec.describe Gitlab::GithubImport::Stage::ImportPullRequestsWorker do
       worker.import(client, project)
     end
   end
+
+  it 'raises an error' do
+    exception = StandardError.new('_some_error_')
+
+    expect_next_instance_of(Gitlab::GithubImport::Importer::PullRequestsImporter) do |importer|
+      expect(importer).to receive(:execute).and_raise(exception)
+    end
+    expect(Gitlab::Import::ImportFailureService).to receive(:track)
+                                                      .with(
+                                                        project_id: project.id,
+                                                        exception: exception,
+                                                        error_source: described_class.name,
+                                                        fail_import: true,
+                                                        metrics: true
+                                                      ).and_call_original
+
+    expect { worker.import(client, project) }.to raise_error(StandardError)
+  end
 end

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

@@ -3,7 +3,7 @@
 require 'spec_helper'
 
 RSpec.describe Gitlab::GithubImport::Stage::ImportRepositoryWorker do
-  let(:project) { double(:project, id: 4) }
+  let_it_be(:project) { create(:project, :import_started) }
 
   let(:worker) { described_class.new }
 
@@ -43,6 +43,15 @@ RSpec.describe Gitlab::GithubImport::Stage::ImportRepositoryWorker do
           expect(instance).to receive(:execute).and_raise(exception_class)
         end
 
+        expect(Gitlab::Import::ImportFailureService).to receive(:track)
+                                                          .with(
+                                                            project_id: project.id,
+                                                            exception: exception_class,
+                                                            error_source: described_class.name,
+                                                            fail_import: true,
+                                                            metrics: true
+                                                          ).and_call_original
+
         expect(Gitlab::GithubImport::Stage::ImportBaseDataWorker)
           .not_to receive(:perform_async)