diff options
author | Sean McGivern <sean@gitlab.com> | 2019-04-04 12:16:37 +0300 |
---|---|---|
committer | Sean McGivern <sean@gitlab.com> | 2019-04-04 12:16:37 +0300 |
commit | 42f6b0250d9c82546e7d911a058ff3edb3f5068c (patch) | |
tree | ade12f83ce00ea5b193ab75c7b802224290b4634 | |
parent | a6e9175fdd7790cc433ba49a85eaadbf75a3c8e9 (diff) | |
parent | a9c855f12c408651e1db41aa5e42aac33aac4084 (diff) |
Merge branch 'add_backtrace_to_kubernetes_log' into 'master'
Show backtrace when logging to kubernetes.log
Closes #56768
See merge request gitlab-org/gitlab-ce!25726
7 files changed, 94 insertions, 189 deletions
diff --git a/app/services/clusters/applications/base_helm_service.rb b/app/services/clusters/applications/base_helm_service.rb index c38b2656260..adaa68b1efb 100644 --- a/app/services/clusters/applications/base_helm_service.rb +++ b/app/services/clusters/applications/base_helm_service.rb @@ -13,16 +13,20 @@ module Clusters def log_error(error) meta = { - exception: error.class.name, error_code: error.respond_to?(:error_code) ? error.error_code : nil, service: self.class.name, app_id: app.id, project_ids: app.cluster.project_ids, - group_ids: app.cluster.group_ids, - message: error.message + group_ids: app.cluster.group_ids } - logger.error(meta) + logger_meta = meta.merge( + exception: error.class.name, + message: error.message, + backtrace: Gitlab::Profiler.clean_backtrace(error.backtrace) + ) + + logger.error(logger_meta) Gitlab::Sentry.track_acceptable_exception(error, extra: meta) end diff --git a/changelogs/unreleased/add_backtrace_to_kubernetes_log.yml b/changelogs/unreleased/add_backtrace_to_kubernetes_log.yml new file mode 100644 index 00000000000..26b8ac4b1ef --- /dev/null +++ b/changelogs/unreleased/add_backtrace_to_kubernetes_log.yml @@ -0,0 +1,5 @@ +--- +title: Show error backtrace when logging errors to kubernetes.log +merge_request: 25726 +author: +type: other diff --git a/spec/services/clusters/applications/check_installation_progress_service_spec.rb b/spec/services/clusters/applications/check_installation_progress_service_spec.rb index 19446ce1cf8..1cca89d31d7 100644 --- a/spec/services/clusters/applications/check_installation_progress_service_spec.rb +++ b/spec/services/clusters/applications/check_installation_progress_service_spec.rb @@ -33,14 +33,22 @@ describe Clusters::Applications::CheckInstallationProgressService, '#execute' do end end - shared_examples 'error logging' do + shared_examples 'error handling' do context 'when installation raises a Kubeclient::HttpError' do let(:cluster) { create(:cluster, :provided_by_user, :project) } + let(:logger) { service.send(:logger) } + let(:error) { Kubeclient::HttpError.new(401, 'Unauthorized', nil) } before do application.update!(cluster: cluster) - expect(service).to receive(:installation_phase).and_raise(Kubeclient::HttpError.new(401, 'Unauthorized', nil)) + expect(service).to receive(:installation_phase).and_raise(error) + end + + include_examples 'logs kubernetes errors' do + let(:error_name) { 'Kubeclient::HttpError' } + let(:error_message) { 'Unauthorized' } + let(:error_code) { 401 } end it 'shows the response code from the error' do @@ -49,12 +57,6 @@ describe Clusters::Applications::CheckInstallationProgressService, '#execute' do expect(application).to be_errored.or(be_update_errored) expect(application.status_reason).to eq('Kubernetes error: 401') end - - it 'should log error' do - expect(service.send(:logger)).to receive(:error) - - service.execute - end end end @@ -66,7 +68,7 @@ describe Clusters::Applications::CheckInstallationProgressService, '#execute' do context 'when application is updating' do let(:application) { create(:clusters_applications_helm, :updating) } - include_examples 'error logging' + include_examples 'error handling' RESCHEDULE_PHASES.each { |phase| it_behaves_like 'a not yet terminated installation', phase } @@ -127,7 +129,7 @@ describe Clusters::Applications::CheckInstallationProgressService, '#execute' do end context 'when application is installing' do - include_examples 'error logging' + include_examples 'error handling' RESCHEDULE_PHASES.each { |phase| it_behaves_like 'a not yet terminated installation', phase } diff --git a/spec/services/clusters/applications/install_service_spec.rb b/spec/services/clusters/applications/install_service_spec.rb index 018d9822d3e..db0c33a95b2 100644 --- a/spec/services/clusters/applications/install_service_spec.rb +++ b/spec/services/clusters/applications/install_service_spec.rb @@ -39,51 +39,34 @@ describe Clusters::Applications::InstallService do expect(helm_client).to receive(:install).with(install_command).and_raise(error) end + include_examples 'logs kubernetes errors' do + let(:error_name) { 'Kubeclient::HttpError' } + let(:error_message) { 'system failure' } + let(:error_code) { 500 } + end + it 'make the application errored' do service.execute expect(application).to be_errored expect(application.status_reason).to match('Kubernetes error: 500') end - - it 'logs errors' do - expect(service.send(:logger)).to receive(:error).with( - { - exception: 'Kubeclient::HttpError', - message: 'system failure', - service: 'Clusters::Applications::InstallService', - app_id: application.id, - project_ids: application.cluster.project_ids, - group_ids: [], - error_code: 500 - } - ) - - expect(Gitlab::Sentry).to receive(:track_acceptable_exception).with( - error, - extra: { - exception: 'Kubeclient::HttpError', - message: 'system failure', - service: 'Clusters::Applications::InstallService', - app_id: application.id, - project_ids: application.cluster.project_ids, - group_ids: [], - error_code: 500 - } - ) - - service.execute - end end context 'a non kubernetes error happens' do let(:application) { create(:clusters_applications_helm, :scheduled) } - let(:error) { StandardError.new("something bad happened") } + let(:error) { StandardError.new('something bad happened') } before do expect(application).to receive(:make_installing!).once.and_raise(error) end + include_examples 'logs kubernetes errors' do + let(:error_name) { 'StandardError' } + let(:error_message) { 'something bad happened' } + let(:error_code) { nil } + end + it 'make the application errored' do expect(helm_client).not_to receive(:install) @@ -92,35 +75,6 @@ describe Clusters::Applications::InstallService do expect(application).to be_errored expect(application.status_reason).to eq("Can't start installation process.") end - - it 'logs errors' do - expect(service.send(:logger)).to receive(:error).with( - { - exception: 'StandardError', - error_code: nil, - message: 'something bad happened', - service: 'Clusters::Applications::InstallService', - app_id: application.id, - project_ids: application.cluster.projects.pluck(:id), - group_ids: [] - } - ) - - expect(Gitlab::Sentry).to receive(:track_acceptable_exception).with( - error, - extra: { - exception: 'StandardError', - error_code: nil, - message: 'something bad happened', - service: 'Clusters::Applications::InstallService', - app_id: application.id, - project_ids: application.cluster.projects.pluck(:id), - group_ids: [] - } - ) - - service.execute - end end end end diff --git a/spec/services/clusters/applications/patch_service_spec.rb b/spec/services/clusters/applications/patch_service_spec.rb index d4ee3243b84..10b1379a127 100644 --- a/spec/services/clusters/applications/patch_service_spec.rb +++ b/spec/services/clusters/applications/patch_service_spec.rb @@ -41,47 +41,30 @@ describe Clusters::Applications::PatchService do expect(helm_client).to receive(:update).with(update_command).and_raise(error) end + include_examples 'logs kubernetes errors' do + let(:error_name) { 'Kubeclient::HttpError' } + let(:error_message) { 'system failure' } + let(:error_code) { 500 } + end + it 'make the application errored' do service.execute expect(application).to be_update_errored expect(application.status_reason).to match('Kubernetes error: 500') end - - it 'logs errors' do - expect(service.send(:logger)).to receive(:error).with( - { - exception: 'Kubeclient::HttpError', - message: 'system failure', - service: 'Clusters::Applications::PatchService', - app_id: application.id, - project_ids: application.cluster.project_ids, - group_ids: [], - error_code: 500 - } - ) - - expect(Gitlab::Sentry).to receive(:track_acceptable_exception).with( - error, - extra: { - exception: 'Kubeclient::HttpError', - message: 'system failure', - service: 'Clusters::Applications::PatchService', - app_id: application.id, - project_ids: application.cluster.project_ids, - group_ids: [], - error_code: 500 - } - ) - - service.execute - end end context 'a non kubernetes error happens' do let(:application) { create(:clusters_applications_knative, :scheduled) } let(:error) { StandardError.new('something bad happened') } + include_examples 'logs kubernetes errors' do + let(:error_name) { 'StandardError' } + let(:error_message) { 'something bad happened' } + let(:error_code) { nil } + end + before do expect(application).to receive(:make_updating!).once.and_raise(error) end @@ -94,35 +77,6 @@ describe Clusters::Applications::PatchService do expect(application).to be_update_errored expect(application.status_reason).to eq("Can't start update process.") end - - it 'logs errors' do - expect(service.send(:logger)).to receive(:error).with( - { - exception: 'StandardError', - error_code: nil, - message: 'something bad happened', - service: 'Clusters::Applications::PatchService', - app_id: application.id, - project_ids: application.cluster.projects.pluck(:id), - group_ids: [] - } - ) - - expect(Gitlab::Sentry).to receive(:track_acceptable_exception).with( - error, - extra: { - exception: 'StandardError', - error_code: nil, - message: 'something bad happened', - service: 'Clusters::Applications::PatchService', - app_id: application.id, - project_ids: application.cluster.projects.pluck(:id), - group_ids: [] - } - ) - - service.execute - end end end end diff --git a/spec/services/clusters/applications/upgrade_service_spec.rb b/spec/services/clusters/applications/upgrade_service_spec.rb index 1822fc38dbd..dd2e6e94e4f 100644 --- a/spec/services/clusters/applications/upgrade_service_spec.rb +++ b/spec/services/clusters/applications/upgrade_service_spec.rb @@ -41,41 +41,18 @@ describe Clusters::Applications::UpgradeService do expect(helm_client).to receive(:update).with(install_command).and_raise(error) end + include_examples 'logs kubernetes errors' do + let(:error_name) { 'Kubeclient::HttpError' } + let(:error_message) { 'system failure' } + let(:error_code) { 500 } + end + it 'make the application errored' do service.execute expect(application).to be_update_errored expect(application.status_reason).to match('Kubernetes error: 500') end - - it 'logs errors' do - expect(service.send(:logger)).to receive(:error).with( - { - exception: 'Kubeclient::HttpError', - message: 'system failure', - service: 'Clusters::Applications::UpgradeService', - app_id: application.id, - project_ids: application.cluster.project_ids, - group_ids: [], - error_code: 500 - } - ) - - expect(Gitlab::Sentry).to receive(:track_acceptable_exception).with( - error, - extra: { - exception: 'Kubeclient::HttpError', - message: 'system failure', - service: 'Clusters::Applications::UpgradeService', - app_id: application.id, - project_ids: application.cluster.project_ids, - group_ids: [], - error_code: 500 - } - ) - - service.execute - end end context 'a non kubernetes error happens' do @@ -86,6 +63,12 @@ describe Clusters::Applications::UpgradeService do expect(application).to receive(:make_updating!).once.and_raise(error) end + include_examples 'logs kubernetes errors' do + let(:error_name) { 'StandardError' } + let(:error_message) { 'something bad happened' } + let(:error_code) { nil } + end + it 'make the application errored' do expect(helm_client).not_to receive(:update) @@ -94,35 +77,6 @@ describe Clusters::Applications::UpgradeService do expect(application).to be_update_errored expect(application.status_reason).to eq("Can't start upgrade process.") end - - it 'logs errors' do - expect(service.send(:logger)).to receive(:error).with( - { - exception: 'StandardError', - error_code: nil, - message: 'something bad happened', - service: 'Clusters::Applications::UpgradeService', - app_id: application.id, - project_ids: application.cluster.projects.pluck(:id), - group_ids: [] - } - ) - - expect(Gitlab::Sentry).to receive(:track_acceptable_exception).with( - error, - extra: { - exception: 'StandardError', - error_code: nil, - message: 'something bad happened', - service: 'Clusters::Applications::UpgradeService', - app_id: application.id, - project_ids: application.cluster.projects.pluck(:id), - group_ids: [] - } - ) - - service.execute - end end end end diff --git a/spec/support/shared_examples/services/base_helm_service_shared_examples.rb b/spec/support/shared_examples/services/base_helm_service_shared_examples.rb new file mode 100644 index 00000000000..78a8e49fd76 --- /dev/null +++ b/spec/support/shared_examples/services/base_helm_service_shared_examples.rb @@ -0,0 +1,32 @@ +# frozen_string_literal: true + +shared_examples 'logs kubernetes errors' do + let(:error_hash) do + { + service: service.class.name, + app_id: application.id, + project_ids: application.cluster.project_ids, + group_ids: [], + error_code: error_code + } + end + + let(:logger_hash) do + error_hash.merge( + exception: error_name, + message: error_message, + backtrace: instance_of(Array) + ) + end + + it 'logs into kubernetes.log and Sentry' do + expect(service.send(:logger)).to receive(:error).with(logger_hash) + + expect(Gitlab::Sentry).to receive(:track_acceptable_exception).with( + error, + extra: hash_including(error_hash) + ) + + service.execute + end +end |