Commit 7b77e067 authored by Doug Stull's avatar Doug Stull Committed by Vasilii Iakliushin

Track failure/success of GitHub Importer

parent 3239e63e
...@@ -16,6 +16,8 @@ module Projects ...@@ -16,6 +16,8 @@ module Projects
end end
def execute def execute
track_start_import
add_repository_to_project add_repository_to_project
download_lfs_objects download_lfs_objects
...@@ -25,16 +27,17 @@ module Projects ...@@ -25,16 +27,17 @@ module Projects
after_execute_hook after_execute_hook
success success
rescue Gitlab::UrlBlocker::BlockedUrlError => e rescue Gitlab::UrlBlocker::BlockedUrlError, StandardError => e
Gitlab::ErrorTracking.track_exception(e, project_path: project.full_path, importer: project.import_type) 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) message = Projects::ImportErrorFilter.filter_message(e.message)
error(s_("ImportProjects|Error importing repository %{project_safe_import_url} into %{project_full_path} - %{message}") %
Gitlab::ErrorTracking.track_exception(e, project_path: project.full_path, importer: project.import_type) { 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 end
protected protected
...@@ -54,6 +57,10 @@ module Projects ...@@ -54,6 +57,10 @@ module Projects
# Defined in EE::Projects::ImportService # Defined in EE::Projects::ImportService
end end
def track_start_import
has_importer? && importer_class.try(:track_start_import, project)
end
def add_repository_to_project def add_repository_to_project
if project.external_import? && !unknown_url? if project.external_import? && !unknown_url?
begin begin
......
...@@ -18,36 +18,28 @@ module Gitlab ...@@ -18,36 +18,28 @@ module Gitlab
# project - An instance of Project. # project - An instance of Project.
def import(_, project) def import(_, project)
@project = project
project.after_import project.after_import
report_import_time(project) report_import_time
end end
def report_import_time(project) private
duration = Time.zone.now - project.created_at
histogram.observe({ project: project.full_path }, duration) attr_reader :project
counter.increment
def report_import_time
metrics.track_finished_import
info( info(
project.id, project.id,
message: "GitHub project import finished", message: "GitHub project import finished",
duration_s: duration.round(2), duration_s: metrics.duration.round(2),
object_counts: ::Gitlab::GithubImport::ObjectCounter.summary(project) object_counts: ::Gitlab::GithubImport::ObjectCounter.summary(project)
) )
end end
def histogram def metrics
@histogram ||= Gitlab::Metrics.histogram( @metrics ||= Gitlab::Import::Metrics.new(:github_importer, project)
: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'
)
end end
end end
end end
......
...@@ -31,6 +31,22 @@ module Gitlab ...@@ -31,6 +31,22 @@ module Gitlab
project.import_state.refresh_jid_expiration project.import_state.refresh_jid_expiration
ImportPullRequestsWorker.perform_async(project.id) 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 end
end end
......
...@@ -27,6 +27,22 @@ module Gitlab ...@@ -27,6 +27,22 @@ module Gitlab
{ waiter.key => waiter.jobs_remaining }, { waiter.key => waiter.jobs_remaining },
:pull_requests_merged_by :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 end
end end
......
...@@ -33,6 +33,17 @@ module Gitlab ...@@ -33,6 +33,17 @@ module Gitlab
counter.increment counter.increment
ImportBaseDataWorker.perform_async(project.id) 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 end
def counter def counter
......
---
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
---
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
---
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
---
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
---
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
---
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
---
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
...@@ -83,8 +83,10 @@ RSpec.describe 'Every metric definition' do ...@@ -83,8 +83,10 @@ RSpec.describe 'Every metric definition' do
stub_usage_data_connections stub_usage_data_connections
end 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)
expect(metric_files_key_paths).to match_array(usage_ping_key_paths), msg
end end
context 'with value json schema' do context 'with value json schema' do
......
...@@ -15,6 +15,10 @@ module Gitlab ...@@ -15,6 +15,10 @@ module Gitlab
true true
end 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 # This is a workaround for a Ruby 2.3.7 bug. rspec-mocks cannot restore
# the visibility of prepended modules. See # the visibility of prepended modules. See
# https://github.com/rspec/rspec-mocks/issues/1231 for more details. # https://github.com/rspec/rspec-mocks/issues/1231 for more details.
......
...@@ -53,7 +53,8 @@ module Gitlab ...@@ -53,7 +53,8 @@ module Gitlab
project_id: project.id, project_id: project.id,
error_source: self.class.name, error_source: self.class.name,
exception: e, exception: e,
fail_import: abort_on_failure fail_import: abort_on_failure,
metrics: true
) )
raise(e) raise(e)
......
...@@ -33,18 +33,41 @@ module Gitlab ...@@ -33,18 +33,41 @@ module Gitlab
end end
def execute def execute
Importer::RepositoryImporter.new(project, client).execute metrics.track_start_import
SEQUENTIAL_IMPORTERS.each do |klass| begin
klass.new(project, client).execute 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 end
PARALLEL_IMPORTERS.each do |klass| PARALLEL_IMPORTERS.each do |klass|
klass.new(project, client, parallel: false).execute klass.new(project, client, parallel: false).execute
end end
metrics.track_finished_import
true true
end end
private
def metrics
@metrics ||= Gitlab::Import::Metrics.new(:github_importer, project)
end
end end
end end
end end
...@@ -8,14 +8,15 @@ module Gitlab ...@@ -8,14 +8,15 @@ module Gitlab
import_state: nil, import_state: nil,
project_id: nil, project_id: nil,
error_source: nil, error_source: nil,
fail_import: false fail_import: false,
metrics: false
) )
new( new(
exception: exception, exception: exception,
import_state: import_state, import_state: import_state,
project_id: project_id, project_id: project_id,
error_source: error_source error_source: error_source
).execute(fail_import: fail_import) ).execute(fail_import: fail_import, metrics: metrics)
end end
def initialize(exception:, import_state: nil, project_id: nil, error_source: nil) def initialize(exception:, import_state: nil, project_id: nil, error_source: nil)
...@@ -35,10 +36,11 @@ module Gitlab ...@@ -35,10 +36,11 @@ module Gitlab
@error_source = error_source @error_source = error_source
end end
def execute(fail_import:) def execute(fail_import:, metrics:)
track_exception track_exception
persist_failure persist_failure
track_metrics if metrics
import_state.mark_as_failed(exception.message) if fail_import import_state.mark_as_failed(exception.message) if fail_import
end end
...@@ -71,6 +73,10 @@ module Gitlab ...@@ -71,6 +73,10 @@ module Gitlab
correlation_id_value: Labkit::Correlation::CorrelationId.current_or_new_id correlation_id_value: Labkit::Correlation::CorrelationId.current_or_new_id
) )
end end
def track_metrics
Gitlab::Import::Metrics.new("#{project.import_type}_importer", project).track_failed_import
end
end end
end end
end end
...@@ -3,20 +3,35 @@ ...@@ -3,20 +3,35 @@
module Gitlab module Gitlab
module Import module Import
class Metrics class Metrics
include Gitlab::Utils::UsageData
IMPORT_DURATION_BUCKETS = [0.5, 1, 3, 5, 10, 60, 120, 240, 360, 720, 1440].freeze 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) def initialize(importer, project)
@importer = importer @importer = importer
@project = project @project = project
end end
def track_start_import
return unless project.github_import?
track_usage_event(:github_import_project_start, project.id)
end
def track_finished_import 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 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 end
def issues_counter def issues_counter
...@@ -35,6 +50,8 @@ module Gitlab ...@@ -35,6 +50,8 @@ module Gitlab
private private
attr_reader :project
def duration_histogram def duration_histogram
@duration_histogram ||= Gitlab::Metrics.histogram( @duration_histogram ||= Gitlab::Metrics.histogram(
:"#{importer}_total_duration_seconds", :"#{importer}_total_duration_seconds",
...@@ -50,6 +67,20 @@ module Gitlab ...@@ -50,6 +67,20 @@ module Gitlab
'The number of imported projects' 'The number of imported projects'
) )
end 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 end
end end
---
# 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
...@@ -9,6 +9,18 @@ RSpec.describe Gitlab::GithubImport::ParallelImporter do ...@@ -9,6 +9,18 @@ RSpec.describe Gitlab::GithubImport::ParallelImporter do
end end
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 describe '#execute', :clean_gitlab_redis_shared_state do
let(:project) { create(:project) } let(:project) { create(:project) }
let(:importer) { described_class.new(project) } let(:importer) { described_class.new(project) }
......
...@@ -130,7 +130,8 @@ RSpec.describe Gitlab::GithubImport::ParallelScheduling do ...@@ -130,7 +130,8 @@ RSpec.describe Gitlab::GithubImport::ParallelScheduling do
project_id: project.id, project_id: project.id,
exception: exception, exception: exception,
error_source: 'MyImporter', error_source: 'MyImporter',
fail_import: false fail_import: false,
metrics: true
).and_call_original ).and_call_original
expect { importer.execute } expect { importer.execute }
...@@ -195,7 +196,8 @@ RSpec.describe Gitlab::GithubImport::ParallelScheduling do ...@@ -195,7 +196,8 @@ RSpec.describe Gitlab::GithubImport::ParallelScheduling do
project_id: project.id, project_id: project.id,
exception: exception, exception: exception,
error_source: 'MyImporter', error_source: 'MyImporter',
fail_import: true fail_import: true,
metrics: true
).and_call_original ).and_call_original
expect { importer.execute } expect { importer.execute }
......
...@@ -4,10 +4,17 @@ require 'spec_helper' ...@@ -4,10 +4,17 @@ require 'spec_helper'
RSpec.describe Gitlab::GithubImport::SequentialImporter do RSpec.describe Gitlab::GithubImport::SequentialImporter do
describe '#execute' 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 it 'imports a project in sequence' do
repository = double(:repository) expect_next_instance_of(Gitlab::Import::Metrics) do |instance|
project = double(:project, id: 1, repository: repository, import_url: 'http://t0ken@github.another-domain.com/repo-org/repo.git', group: nil) expect(instance).to receive(:track_start_import)
importer = described_class.new(project, token: 'foo') expect(instance).to receive(:track_finished_import)
end
expect_next_instance_of(Gitlab::GithubImport::Importer::RepositoryImporter) do |instance| expect_next_instance_of(Gitlab::GithubImport::Importer::RepositoryImporter) do |instance|
expect(instance).to receive(:execute) expect(instance).to receive(:execute)
...@@ -35,5 +42,23 @@ RSpec.describe Gitlab::GithubImport::SequentialImporter do ...@@ -35,5 +42,23 @@ RSpec.describe Gitlab::GithubImport::SequentialImporter do
expect(importer.execute).to eq(true) expect(importer.execute).to eq(true)
end 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
end end
...@@ -2,135 +2,171 @@ ...@@ -2,135 +2,171 @@
require 'spec_helper' 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(: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(: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 let(:exe_arguments) do
it 'when the failure does not abort the import' do {
expect(Gitlab::ErrorTracking) fail_import: '_fail_import_',
.to receive(:track_exception) metrics: '_metrics_'
.with( }
exception, end
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
shared_examples 'logs the exception and does not fail the import' do it 'invokes a new instance and executes' do
it 'when the failure does not abort the import' do expect(described_class).to receive(:new).with(**instance_arguments).and_return(instance)
expect(Gitlab::ErrorTracking) expect(instance).to receive(:execute).with(**exe_arguments)
.to receive(:track_exception)
.with( described_class.track(**instance_arguments.merge(exe_arguments))
exception, end
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')
end end
end
context 'when using the project as reference' do context 'with only necessary arguments utilizing defaults' do
context 'when it fails the import' do let(:instance) { double(:failure_service) }
let(:arguments) do let(:instance_arguments) do
{ {
project_id: project.id,
exception: exception, exception: exception,
error_source: 'SomeImporter', import_state: nil,
fail_import: true project_id: nil,
error_source: nil
} }
end end
it_behaves_like 'logs the exception and fails the import' let(:exe_arguments) do
end
context 'when it does not fail the import' do
let(:arguments) do
{ {
project_id: project.id, fail_import: false,
exception: exception, metrics: false
error_source: 'SomeImporter',
fail_import: false
} }
end 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
end end
context 'when using the import_state as reference' do describe '#execute' do
context 'when it fails the import' do subject(:service) { described_class.new(**base_arguments) }
let(:arguments) do
{ shared_examples 'logs the exception and fails the import' do
import_state: import_state, it 'when the failure does not abort the import' do
exception: exception, expect(Gitlab::ErrorTracking)
error_source: 'SomeImporter', .to receive(:track_exception)
fail_import: true .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
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 end
context 'when it does not fail the import' do context 'when tracking metrics' do
let(:arguments) do let(:exe_arguments) { { fail_import: false, metrics: true } }
{
import_state: import_state, it 'tracks the failed import' do
exception: exception, metrics = double(:metrics)
error_source: 'SomeImporter',
fail_import: false 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
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 end
end end
...@@ -4,7 +4,7 @@ require 'spec_helper' ...@@ -4,7 +4,7 @@ require 'spec_helper'
RSpec.describe Gitlab::Import::Metrics, :aggregate_failures do RSpec.describe Gitlab::Import::Metrics, :aggregate_failures do
let(:importer) { :test_importer } 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(:histogram) { double(:histogram) }
let(:counter) { double(:counter) } let(:counter) { double(:counter) }
...@@ -13,6 +13,51 @@ RSpec.describe Gitlab::Import::Metrics, :aggregate_failures do ...@@ -13,6 +13,51 @@ RSpec.describe Gitlab::Import::Metrics, :aggregate_failures do
before do before do
allow(Gitlab::Metrics).to receive(:counter) { counter } allow(Gitlab::Metrics).to receive(:counter) { counter }
allow(counter).to receive(:increment) 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 end
describe '#track_finished_import' do describe '#track_finished_import' do
...@@ -34,9 +79,34 @@ RSpec.describe Gitlab::Import::Metrics, :aggregate_failures do ...@@ -34,9 +79,34 @@ RSpec.describe Gitlab::Import::Metrics, :aggregate_failures do
) )
expect(counter).to receive(:increment) expect(counter).to receive(:increment)
expect(histogram).to receive(:observe).with({ importer: :test_importer }, anything)
subject.track_finished_import 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
end end
......
...@@ -47,6 +47,7 @@ RSpec.describe Gitlab::UsageDataCounters::HLLRedisCounter, :clean_gitlab_redis_s ...@@ -47,6 +47,7 @@ RSpec.describe Gitlab::UsageDataCounters::HLLRedisCounter, :clean_gitlab_redis_s
'epics_usage', 'epics_usage',
'epic_boards_usage', 'epic_boards_usage',
'secure', 'secure',
'importer',
'network_policies' 'network_policies'
) )
end end
......
...@@ -86,6 +86,12 @@ RSpec.describe Projects::ImportService do ...@@ -86,6 +86,12 @@ RSpec.describe Projects::ImportService do
end end
context 'with a Github repository' do 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 it 'succeeds if repository import was scheduled' do
expect_any_instance_of(Gitlab::GithubImport::ParallelImporter) expect_any_instance_of(Gitlab::GithubImport::ParallelImporter)
.to receive(:execute) .to receive(:execute)
......
...@@ -7,39 +7,27 @@ RSpec.describe Gitlab::GithubImport::Stage::FinishImportWorker do ...@@ -7,39 +7,27 @@ RSpec.describe Gitlab::GithubImport::Stage::FinishImportWorker do
let(:worker) { described_class.new } let(:worker) { described_class.new }
describe '#perform' do 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(project).to receive(:after_import)
expect(worker).to receive(:report_import_time).with(project) expect_next_instance_of(Gitlab::Import::Metrics) do |instance|
expect(instance).to receive(:track_finished_import)
worker.import(double(:client), project) expect(instance).to receive(:duration).and_return(3.005)
end 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(Gitlab::GithubImport::Logger) expect(Gitlab::GithubImport::Logger)
.to receive(:info) .to receive(:info)
.with( .with(
message: 'GitHub project import finished', message: 'GitHub project import finished',
import_stage: 'Gitlab::GithubImport::Stage::FinishImportWorker', import_stage: 'Gitlab::GithubImport::Stage::FinishImportWorker',
object_counts: { object_counts: {
'fetched' => {}, 'fetched' => {},
'imported' => {} 'imported' => {}
}, },
project_id: project.id, project_id: project.id,
duration_s: a_kind_of(Numeric) duration_s: 3.01
) )
worker.report_import_time(project) worker.import(double(:client), project)
end end
end end
end end
...@@ -3,15 +3,15 @@ ...@@ -3,15 +3,15 @@
require 'spec_helper' require 'spec_helper'
RSpec.describe Gitlab::GithubImport::Stage::ImportBaseDataWorker do RSpec.describe Gitlab::GithubImport::Stage::ImportBaseDataWorker do
let(:project) { create(:project) } let_it_be(:project) { create(:project) }
let(:import_state) { create(:import_state, project: project) } let_it_be(:import_state) { create(:import_state, project: project) }
let(:worker) { described_class.new } let(:worker) { described_class.new }
let(:importer) { double(:importer) }
let(:client) { double(:client) }
describe '#import' do describe '#import' do
it 'imports the base data of a project' do it 'imports the base data of a project' do
importer = double(:importer)
client = double(:client)
described_class::IMPORTERS.each do |klass| described_class::IMPORTERS.each do |klass|
expect(klass) expect(klass)
.to receive(:new) .to receive(:new)
...@@ -29,5 +29,23 @@ RSpec.describe Gitlab::GithubImport::Stage::ImportBaseDataWorker do ...@@ -29,5 +29,23 @@ RSpec.describe Gitlab::GithubImport::Stage::ImportBaseDataWorker do
worker.import(client, project) worker.import(client, project)
end 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
end end
...@@ -3,14 +3,15 @@ ...@@ -3,14 +3,15 @@
require 'spec_helper' require 'spec_helper'
RSpec.describe Gitlab::GithubImport::Stage::ImportPullRequestsWorker do RSpec.describe Gitlab::GithubImport::Stage::ImportPullRequestsWorker do
let(:project) { create(:project) } let_it_be(:project) { create(:project) }
let(:import_state) { create(:import_state, project: project) } let_it_be(:import_state) { create(:import_state, project: project) }
let(:worker) { described_class.new } let(:worker) { described_class.new }
let(:importer) { double(:importer) }
let(:client) { double(:client) }
describe '#import' do describe '#import' do
it 'imports all the pull requests' do it 'imports all the pull requests' do
importer = double(:importer)
client = double(:client)
waiter = Gitlab::JobWaiter.new(2, '123') waiter = Gitlab::JobWaiter.new(2, '123')
expect(Gitlab::GithubImport::Importer::PullRequestsImporter) expect(Gitlab::GithubImport::Importer::PullRequestsImporter)
...@@ -32,4 +33,22 @@ RSpec.describe Gitlab::GithubImport::Stage::ImportPullRequestsWorker do ...@@ -32,4 +33,22 @@ RSpec.describe Gitlab::GithubImport::Stage::ImportPullRequestsWorker do
worker.import(client, project) worker.import(client, project)
end end
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 end
...@@ -3,7 +3,7 @@ ...@@ -3,7 +3,7 @@
require 'spec_helper' require 'spec_helper'
RSpec.describe Gitlab::GithubImport::Stage::ImportRepositoryWorker do 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 } let(:worker) { described_class.new }
...@@ -43,6 +43,15 @@ RSpec.describe Gitlab::GithubImport::Stage::ImportRepositoryWorker do ...@@ -43,6 +43,15 @@ RSpec.describe Gitlab::GithubImport::Stage::ImportRepositoryWorker do
expect(instance).to receive(:execute).and_raise(exception_class) expect(instance).to receive(:execute).and_raise(exception_class)
end 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) expect(Gitlab::GithubImport::Stage::ImportBaseDataWorker)
.not_to receive(:perform_async) .not_to receive(:perform_async)
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment