summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorStan Hu <stanhu@gmail.com>2019-06-19 10:44:04 -0700
committerStan Hu <stanhu@gmail.com>2019-06-19 12:04:24 -0700
commit30f52b690f3dc373f1ec98d9def9d9d2cd37832a (patch)
treedb45c48023360ad8f9c294db8395d0c5b702efb0
parent0d2537bfa942f4bf198241bfbc5c4083929f3e46 (diff)
downloadgitlab-ce-sh-clean-up-bitbucket-import-errors.tar.gz
Avoid storing backtraces from Bitbucket Cloud imports in the databasesh-clean-up-bitbucket-import-errors
We noticed in https://gitlab.com/gitlab-com/gl-infra/production/issues/908 some Bitbucket imports took over a second to load their projects row because `import_error` was huge due to errors. To prevent this, we now: 1. Clean the backtraces 2. Log the details into importer.log 3. Omit the details from the database
-rw-r--r--changelogs/unreleased/sh-clean-up-bitbucket-import-errors.yml5
-rw-r--r--lib/gitlab/bitbucket_import/importer.rb28
-rw-r--r--spec/lib/gitlab/bitbucket_import/importer_spec.rb27
3 files changed, 53 insertions, 7 deletions
diff --git a/changelogs/unreleased/sh-clean-up-bitbucket-import-errors.yml b/changelogs/unreleased/sh-clean-up-bitbucket-import-errors.yml
new file mode 100644
index 00000000000..e4c9de74e6a
--- /dev/null
+++ b/changelogs/unreleased/sh-clean-up-bitbucket-import-errors.yml
@@ -0,0 +1,5 @@
+---
+title: Avoid storing backtraces from Bitbucket Cloud imports in the database
+merge_request: 29862
+author:
+type: performance
diff --git a/lib/gitlab/bitbucket_import/importer.rb b/lib/gitlab/bitbucket_import/importer.rb
index c9f0ed66a54..8047ef4fa15 100644
--- a/lib/gitlab/bitbucket_import/importer.rb
+++ b/lib/gitlab/bitbucket_import/importer.rb
@@ -11,6 +11,7 @@ module Gitlab
{ title: 'task', color: '#7F8C8D' }].freeze
attr_reader :project, :client, :errors, :users
+ attr_accessor :logger
def initialize(project)
@project = project
@@ -19,6 +20,7 @@ module Gitlab
@labels = {}
@errors = []
@users = {}
+ @logger = Gitlab::Import::Logger.build
end
def execute
@@ -41,6 +43,18 @@ module Gitlab
}.to_json)
end
+ def store_pull_request_error(pull_request, ex)
+ backtrace = Gitlab::Profiler.clean_backtrace(ex.backtrace)
+ error = { type: :pull_request, iid: pull_request.iid, errors: ex.message, trace: backtrace, raw_response: pull_request.raw }
+
+ log_error(error)
+ # Omit the details from the database to avoid blowing up usage in the error column
+ error.delete(:trace)
+ error.delete(:raw_response)
+
+ errors << error
+ end
+
def gitlab_user_id(project, username)
find_user_id(username) || project.creator_id
end
@@ -176,7 +190,7 @@ module Gitlab
import_pull_request_comments(pull_request, merge_request) if merge_request.persisted?
rescue StandardError => e
- errors << { type: :pull_request, iid: pull_request.iid, errors: e.message, trace: e.backtrace.join("\n"), raw_response: pull_request.raw }
+ store_pull_request_error(pull_request, e)
end
end
@@ -254,6 +268,18 @@ module Gitlab
updated_at: comment.updated_at
}
end
+
+ def log_error(details)
+ logger.error(log_base_data.merge(details))
+ end
+
+ def log_base_data
+ {
+ class: self.class.name,
+ project_id: project.id,
+ project_path: project.full_path
+ }
+ end
end
end
end
diff --git a/spec/lib/gitlab/bitbucket_import/importer_spec.rb b/spec/lib/gitlab/bitbucket_import/importer_spec.rb
index 2e90f6c7f71..35700e0b588 100644
--- a/spec/lib/gitlab/bitbucket_import/importer_spec.rb
+++ b/spec/lib/gitlab/bitbucket_import/importer_spec.rb
@@ -98,12 +98,8 @@ describe Gitlab::BitbucketImport::Importer do
describe '#import_pull_requests' do
let(:source_branch_sha) { sample.commits.last }
let(:target_branch_sha) { sample.commits.first }
-
- before do
- allow(subject).to receive(:import_wiki)
- allow(subject).to receive(:import_issues)
-
- pull_request = instance_double(
+ let(:pull_request) do
+ instance_double(
Bitbucket::Representation::PullRequest,
iid: 10,
source_branch_sha: source_branch_sha,
@@ -116,6 +112,11 @@ describe Gitlab::BitbucketImport::Importer do
author: 'other',
created_at: Time.now,
updated_at: Time.now)
+ end
+
+ before do
+ allow(subject).to receive(:import_wiki)
+ allow(subject).to receive(:import_issues)
# https://gitlab.com/gitlab-org/gitlab-test/compare/c1acaa58bbcbc3eafe538cb8274ba387047b69f8...5937ac0a7beb003549fc5fd26fc247ad
@inline_note = instance_double(
@@ -167,6 +168,20 @@ describe Gitlab::BitbucketImport::Importer do
expect(reply_note.note).to eq(@reply.note)
end
+ context 'when importing a pull request throws an exception' do
+ before do
+ allow(pull_request).to receive(:raw).and_return('hello world')
+ allow(subject.client).to receive(:pull_request_comments).and_raise(HTTParty::Error)
+ end
+
+ it 'logs an error without the backtrace' do
+ subject.execute
+
+ expect(subject.errors.count).to eq(1)
+ expect(subject.errors.first.keys).to match_array(%i(type iid errors))
+ end
+ end
+
context "when branches' sha is not found in the repository" do
let(:source_branch_sha) { 'a' * Commit::MIN_SHA_LENGTH }
let(:target_branch_sha) { 'b' * Commit::MIN_SHA_LENGTH }