Welcome to mirror list, hosted at ThFree Co, Russian Federation.

gitlab.com/gitlab-org/gitlab-foss.git - Unnamed repository; edit this file 'description' to name the repository.
summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorShinya Maeda <shinya@gitlab.com>2018-11-29 08:34:14 +0300
committerShinya Maeda <shinya@gitlab.com>2018-11-29 08:35:00 +0300
commit3fbd48e127053517e9ee0f6307989758a4d59f9a (patch)
tree78a93bf5c8294368f52ea834116d0a5b83e658fc
parenta5f4627857bddc7e431d0fca6f17560e4b07e0b9 (diff)
Squashed commit of the following:
commit 10456b1e9240886432f565dd17689080bbb133b9 Merge: 312c1a9bdf8 a5f4627857b Author: Shinya Maeda <shinya@gitlab.com> Date: Thu Nov 29 14:33:21 2018 +0900 Merge branch 'master-ce' into add-counter-for-trace-chunks commit 312c1a9bdf8efc45c3fed5ff50f05cc589bbb4ed Author: Shinya Maeda <shinya@gitlab.com> Date: Wed Nov 28 20:06:18 2018 +0900 Fix coding offence commit e397cc2ccc1b2cf7f8b3558b8fa81fe2aa0ab366 Author: Shinya Maeda <shinya@gitlab.com> Date: Wed Nov 28 14:40:24 2018 +0900 Fix tracking archive failure
-rw-r--r--app/services/ci/archive_trace_service.rb25
-rw-r--r--app/workers/archive_trace_worker.rb2
-rw-r--r--app/workers/ci/archive_traces_cron_worker.rb14
-rw-r--r--spec/services/ci/archive_trace_service_spec.rb39
-rw-r--r--spec/workers/archive_trace_worker_spec.rb28
-rw-r--r--spec/workers/ci/archive_traces_cron_worker_spec.rb22
6 files changed, 112 insertions, 18 deletions
diff --git a/app/services/ci/archive_trace_service.rb b/app/services/ci/archive_trace_service.rb
new file mode 100644
index 00000000000..8504ceb2327
--- /dev/null
+++ b/app/services/ci/archive_trace_service.rb
@@ -0,0 +1,25 @@
+# frozen_string_literal: true
+
+module Ci
+ class ArchiveTraceService
+ def execute(job)
+ job.trace.archive!
+ rescue ::Gitlab::Ci::Trace::AlreadyArchivedError
+ # It's already archived, thus we can safely ignore this exception.
+ rescue => e
+ archive_error(e, job)
+ end
+
+ private
+
+ def failed_archive_counter
+ @failed_archive_counter ||= Gitlab::Metrics.counter(:job_trace_archive_failed_total, "Counter of failed attempts of trace archiving")
+ end
+
+ def archive_error(error, job)
+ failed_archive_counter.increment
+ Gitlab::Sentry.track_exception(error, issue_url: 'https://gitlab.com/gitlab-org/gitlab-ce/issues/51502', extra: { job_id: job.id })
+ Rails.logger.error "Failed to archive trace. id: #{job.id} message: #{error.message}"
+ end
+ end
+end
diff --git a/app/workers/archive_trace_worker.rb b/app/workers/archive_trace_worker.rb
index c1283e9b2fc..4a9becf0ca7 100644
--- a/app/workers/archive_trace_worker.rb
+++ b/app/workers/archive_trace_worker.rb
@@ -7,7 +7,7 @@ class ArchiveTraceWorker
# rubocop: disable CodeReuse/ActiveRecord
def perform(job_id)
Ci::Build.without_archived_trace.find_by(id: job_id).try do |job|
- job.trace.archive!
+ Ci::ArchiveTraceService.new.execute(job)
end
end
# rubocop: enable CodeReuse/ActiveRecord
diff --git a/app/workers/ci/archive_traces_cron_worker.rb b/app/workers/ci/archive_traces_cron_worker.rb
index 7443aad1380..f65ff239866 100644
--- a/app/workers/ci/archive_traces_cron_worker.rb
+++ b/app/workers/ci/archive_traces_cron_worker.rb
@@ -11,21 +11,9 @@ module Ci
# This could happen when ArchiveTraceWorker sidekiq jobs were lost by receiving SIGKILL
# More details in https://gitlab.com/gitlab-org/gitlab-ce/issues/36791
Ci::Build.finished.with_live_trace.find_each(batch_size: 100) do |build|
- begin
- build.trace.archive!
- rescue ::Gitlab::Ci::Trace::AlreadyArchivedError
- rescue => e
- failed_archive_counter.increment
- Rails.logger.error "Failed to archive stale live trace. id: #{build.id} message: #{e.message}"
- end
+ Ci::ArchiveTraceService.new.execute(build)
end
end
# rubocop: enable CodeReuse/ActiveRecord
-
- private
-
- def failed_archive_counter
- @failed_archive_counter ||= Gitlab::Metrics.counter(:job_trace_archive_failed_total, "Counter of failed attempts of traces archiving")
- end
end
end
diff --git a/spec/services/ci/archive_trace_service_spec.rb b/spec/services/ci/archive_trace_service_spec.rb
new file mode 100644
index 00000000000..6389d7a921f
--- /dev/null
+++ b/spec/services/ci/archive_trace_service_spec.rb
@@ -0,0 +1,39 @@
+require 'spec_helper'
+
+describe Ci::ArchiveTraceService, '#execute' do
+ subject { described_class.new.execute(job) }
+
+ context 'when job is finished' do
+ let(:job) { create(:ci_build, :success, :trace_live) }
+
+ it 'creates an archived trace' do
+ expect { subject }.not_to raise_error
+
+ expect(job.reload.job_artifacts_trace).to be_exist
+ end
+ end
+
+ context 'when job is running' do
+ let(:job) { create(:ci_build, :running, :trace_live) }
+
+ it 'increments Prometheus counter, sends crash report to Sentry and ignore an error for continuing to archive' do
+ expect(Gitlab::Sentry)
+ .to receive(:track_exception)
+ .with(::Gitlab::Ci::Trace::ArchiveError,
+ issue_url: 'https://gitlab.com/gitlab-org/gitlab-ce/issues/51502',
+ extra: { job_id: job.id } ).once
+
+ expect(Rails.logger)
+ .to receive(:error)
+ .with("Failed to archive trace. id: #{job.id} message: Job is not finished yet")
+ .and_call_original
+
+ expect(Gitlab::Metrics)
+ .to receive(:counter)
+ .with(:job_trace_archive_failed_total, "Counter of failed attempts of trace archiving")
+ .and_call_original
+
+ expect { subject }.not_to raise_error
+ end
+ end
+end
diff --git a/spec/workers/archive_trace_worker_spec.rb b/spec/workers/archive_trace_worker_spec.rb
index b768588c6e1..ce71468202f 100644
--- a/spec/workers/archive_trace_worker_spec.rb
+++ b/spec/workers/archive_trace_worker_spec.rb
@@ -23,5 +23,33 @@ describe ArchiveTraceWorker do
subject
end
end
+
+ context 'when an unexpected exception happened during archiving' do
+ let!(:job) { create(:ci_build, :success, :trace_live) }
+
+ before do
+ allow_any_instance_of(Gitlab::Ci::Trace).to receive(:archive_stream!).and_raise('Unexpected error')
+ end
+
+ it 'increments Prometheus counter, sends crash report to Sentry and ignore an error for continuing to archive' do
+ expect(Gitlab::Sentry)
+ .to receive(:track_exception)
+ .with(RuntimeError,
+ issue_url: 'https://gitlab.com/gitlab-org/gitlab-ce/issues/51502',
+ extra: { job_id: job.id } ).once
+
+ expect(Rails.logger)
+ .to receive(:error)
+ .with("Failed to archive trace. id: #{job.id} message: Unexpected error")
+ .and_call_original
+
+ expect(Gitlab::Metrics)
+ .to receive(:counter)
+ .with(:job_trace_archive_failed_total, "Counter of failed attempts of trace archiving")
+ .and_call_original
+
+ expect { subject }.not_to raise_error
+ end
+ end
end
end
diff --git a/spec/workers/ci/archive_traces_cron_worker_spec.rb b/spec/workers/ci/archive_traces_cron_worker_spec.rb
index 23f5dda298a..8cd1c7e3906 100644
--- a/spec/workers/ci/archive_traces_cron_worker_spec.rb
+++ b/spec/workers/ci/archive_traces_cron_worker_spec.rb
@@ -46,13 +46,27 @@ describe Ci::ArchiveTracesCronWorker do
let!(:build) { create(:ci_build, :success, :trace_live) }
before do
- allow_any_instance_of(Gitlab::Ci::Trace).to receive(:archive!).and_raise('Unexpected error')
+ allow_any_instance_of(Gitlab::Ci::Trace).to receive(:archive_stream!).and_raise('Unexpected error')
end
- it 'puts a log' do
- expect(Rails.logger).to receive(:error).with("Failed to archive stale live trace. id: #{build.id} message: Unexpected error")
+ it 'increments Prometheus counter, sends crash report to Sentry and ignore an error for continuing to archive' do
+ expect(Gitlab::Sentry)
+ .to receive(:track_exception)
+ .with(RuntimeError,
+ issue_url: 'https://gitlab.com/gitlab-org/gitlab-ce/issues/51502',
+ extra: { job_id: build.id } ).once
- subject
+ expect(Rails.logger)
+ .to receive(:error)
+ .with("Failed to archive trace. id: #{build.id} message: Unexpected error")
+ .and_call_original
+
+ expect(Gitlab::Metrics)
+ .to receive(:counter)
+ .with(:job_trace_archive_failed_total, "Counter of failed attempts of trace archiving")
+ .and_call_original
+
+ expect { subject }.not_to raise_error
end
end
end