From e8e149fc945f6f78725f312254577fb03b7666ec Mon Sep 17 00:00:00 2001 From: GitLab Bot Date: Mon, 3 Jul 2023 18:08:30 +0000 Subject: [PATCH] Add latest changes from gitlab-org/gitlab@master --- lib/gitlab/exception_log_formatter.rb | 14 ++++++ lib/gitlab/git/base_error.rb | 12 ++++- lib/gitlab/gitaly_client/call.rb | 14 ++++++ .../gitlab/exception_log_formatter_spec.rb | 48 +++++++++++++++++++ spec/lib/gitlab/git/base_error_spec.rb | 29 ++++++++++- spec/lib/gitlab/gitaly_client/call_spec.rb | 47 ++++++++++++++++-- 6 files changed, 158 insertions(+), 6 deletions(-) diff --git a/lib/gitlab/exception_log_formatter.rb b/lib/gitlab/exception_log_formatter.rb index 52ad67d6f8b..a32f837eee9 100644 --- a/lib/gitlab/exception_log_formatter.rb +++ b/lib/gitlab/exception_log_formatter.rb @@ -21,6 +21,10 @@ module Gitlab payload['exception.cause_class'] = exception.cause.class.name end + if gitaly_metadata = find_gitaly_metadata(exception) + payload['exception.gitaly'] = gitaly_metadata.to_s + end + if sql = find_sql(exception) payload['exception.sql'] = sql end @@ -35,6 +39,16 @@ module Gitlab end end + def find_gitaly_metadata(exception) + if exception.is_a?(::Gitlab::Git::BaseError) + exception.metadata + elsif exception.is_a?(::GRPC::BadStatus) + exception.metadata[::Gitlab::Git::BaseError::METADATA_KEY] + elsif exception.cause.present? + find_gitaly_metadata(exception.cause) + end + end + private def normalize_query(sql) diff --git a/lib/gitlab/git/base_error.rb b/lib/gitlab/git/base_error.rb index 0b0fdef54cc..330e947844c 100644 --- a/lib/gitlab/git/base_error.rb +++ b/lib/gitlab/git/base_error.rb @@ -4,6 +4,7 @@ require 'grpc' module Gitlab module Git class BaseError < StandardError + METADATA_KEY = :gitaly_error_metadata DEBUG_ERROR_STRING_REGEX = /(.*?) debug_error_string:.*$/m.freeze GRPC_CODES = { '0' => 'ok', @@ -25,12 +26,15 @@ module Gitlab '16' => 'unauthenticated' }.freeze - attr_reader :status, :code, :service + attr_reader :status, :code, :service, :metadata def initialize(msg = nil) super && return if msg.nil? - set_grpc_error_code(msg) if msg.is_a?(::GRPC::BadStatus) + if msg.is_a?(::GRPC::BadStatus) + set_grpc_error_code(msg) + set_grpc_error_metadata(msg) + end super(build_raw_message(msg)) end @@ -46,6 +50,10 @@ module Gitlab @code = GRPC_CODES[@status.to_s] @service = 'git' end + + def set_grpc_error_metadata(grpc_error) + @metadata = grpc_error.metadata.fetch(METADATA_KEY, {}).clone + end end end end diff --git a/lib/gitlab/gitaly_client/call.rb b/lib/gitlab/gitaly_client/call.rb index 3fe3702cfe1..37d3921d6d5 100644 --- a/lib/gitlab/gitaly_client/call.rb +++ b/lib/gitlab/gitaly_client/call.rb @@ -32,6 +32,8 @@ module Gitlab end rescue StandardError => err store_timings + set_gitaly_error_metadata(err) if err.is_a?(::GRPC::BadStatus) + raise err end @@ -44,6 +46,9 @@ module Gitlab yielder.yield(value) end + rescue ::GRPC::BadStatus => err + set_gitaly_error_metadata(err) + raise err ensure store_timings end @@ -73,6 +78,15 @@ module Gitlab backtrace: Gitlab::BacktraceCleaner.clean_backtrace(caller) ) end + + def set_gitaly_error_metadata(err) + err.metadata[::Gitlab::Git::BaseError::METADATA_KEY] = { + storage: @storage, + address: ::Gitlab::GitalyClient.address(@storage), + service: @service, + rpc: @rpc + } + end end end end diff --git a/spec/lib/gitlab/exception_log_formatter_spec.rb b/spec/lib/gitlab/exception_log_formatter_spec.rb index 82166971603..179054e2d15 100644 --- a/spec/lib/gitlab/exception_log_formatter_spec.rb +++ b/spec/lib/gitlab/exception_log_formatter_spec.rb @@ -67,5 +67,53 @@ RSpec.describe Gitlab::ExceptionLogFormatter do expect(payload['exception.sql']).to eq('SELECT SELECT FROM SELECT') end end + + context 'when exception is a gRPC bad status' do + let(:unavailable_error) do + ::GRPC::Unavailable.new( + "unavailable", + gitaly_error_metadata: { + storage: 'default', + address: 'unix://gitaly.socket', + service: :ref_service, + rpc: :find_local_branches + } + ) + end + + context 'when the gRPC error is wrapped by ::Gitlab::Git::BaseError' do + let(:exception) { ::Gitlab::Git::CommandError.new(unavailable_error) } + + it 'adds gitaly metadata to payload' do + described_class.format!(exception, payload) + + expect(payload['exception.gitaly']).to eq('{:storage=>"default", :address=>"unix://gitaly.socket", :service=>:ref_service, :rpc=>:find_local_branches}') + end + end + + context 'when the gRPC error is wrapped by another error' do + before do + allow(exception).to receive(:cause).and_return(unavailable_error) + end + + it 'adds gitaly metadata to payload' do + described_class.format!(exception, payload) + + expect(payload['exception.cause_class']).to eq('GRPC::Unavailable') + expect(payload['exception.gitaly']).to eq('{:storage=>"default", :address=>"unix://gitaly.socket", :service=>:ref_service, :rpc=>:find_local_branches}') + end + end + + context 'when the gRPC error is not wrapped' do + let(:exception) { unavailable_error } + + it 'adds gitaly metadata to payload' do + described_class.format!(exception, payload) + + expect(payload['exception.cause_class']).to be_nil + expect(payload['exception.gitaly']).to eq('{:storage=>"default", :address=>"unix://gitaly.socket", :service=>:ref_service, :rpc=>:find_local_branches}') + end + end + end end end diff --git a/spec/lib/gitlab/git/base_error_spec.rb b/spec/lib/gitlab/git/base_error_spec.rb index d4db7cf2430..6efebd778b7 100644 --- a/spec/lib/gitlab/git/base_error_spec.rb +++ b/spec/lib/gitlab/git/base_error_spec.rb @@ -21,7 +21,7 @@ RSpec.describe Gitlab::Git::BaseError do it { is_expected.to eq(result) } end - describe "When initialized with GRPC errors" do + describe "when initialized with GRPC errors without metadata" do let(:grpc_error) { GRPC::DeadlineExceeded.new } let(:git_error) { described_class.new grpc_error } @@ -29,6 +29,33 @@ RSpec.describe Gitlab::Git::BaseError do expect(git_error.service).to eq('git') expect(git_error.status).to eq(4) expect(git_error.code).to eq('deadline_exceeded') + expect(git_error.metadata).to eq({}) + end + end + + describe "when initialized with GRPC errors with metadata" do + let(:grpc_error) do + GRPC::DeadlineExceeded.new( + "deadline exceeded", + gitaly_error_metadata: { + storage: "default", + address: "unix://gitaly.socket", + service: :ref_service, rpc: :find_local_branches + } + ) + end + + let(:git_error) { described_class.new grpc_error } + + it "has status, code, and metadata fields" do + expect(git_error.service).to eq('git') + expect(git_error.status).to eq(4) + expect(git_error.code).to eq('deadline_exceeded') + expect(git_error.metadata).to eq( + storage: "default", + address: "unix://gitaly.socket", + service: :ref_service, rpc: :find_local_branches + ) end end end diff --git a/spec/lib/gitlab/gitaly_client/call_spec.rb b/spec/lib/gitlab/gitaly_client/call_spec.rb index 099307fc4e1..c3c3c7bb2e8 100644 --- a/spec/lib/gitlab/gitaly_client/call_spec.rb +++ b/spec/lib/gitlab/gitaly_client/call_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Gitlab::GitalyClient::Call do +RSpec.describe Gitlab::GitalyClient::Call, feature_category: :gitaly do describe '#call', :request_store do let(:client) { Gitlab::GitalyClient } let(:storage) { 'default' } @@ -50,7 +50,7 @@ RSpec.describe Gitlab::GitalyClient::Call do expect_call_details_to_match end - context 'when err' do + context 'when the call raises an standard error' do before do allow(client).to receive(:execute).and_raise(StandardError) end @@ -62,6 +62,25 @@ RSpec.describe Gitlab::GitalyClient::Call do expect_call_details_to_match end end + + context 'when the call raises a BadStatus error' do + before do + allow(client).to receive(:execute).and_raise(GRPC::Unavailable) + end + + it 'attaches gitaly metadata' do + expect { subject }.to raise_error do |err| + expect(err.metadata).to eql( + gitaly_error_metadata: { + storage: storage, + address: client.address(storage), + service: service, + rpc: rpc + } + ) + end + end + end end context 'when the response is an enumerator' do @@ -103,7 +122,7 @@ RSpec.describe Gitlab::GitalyClient::Call do expect_call_details_to_match(duration_higher_than: 0.1) end - context 'when err' do + context 'when the call raises an standard error' do let(:response) do Enumerator.new do |yielder| sleep 0.2 @@ -119,6 +138,28 @@ RSpec.describe Gitlab::GitalyClient::Call do expect_call_details_to_match(duration_higher_than: 0.2) end end + + context 'when the call raises a BadStatus error' do + let(:response) do + Enumerator.new do |yielder| + yielder << 1 + raise GRPC::Unavailable + end + end + + it 'attaches gitaly metadata' do + expect { subject.to_a }.to raise_error do |err| + expect(err.metadata).to eql( + gitaly_error_metadata: { + storage: storage, + address: client.address(storage), + service: service, + rpc: rpc + } + ) + end + end + end end end end