diff --git a/lib/diego/client.rb b/lib/diego/client.rb index c0348e5585a..4a3836dd62d 100644 --- a/lib/diego/client.rb +++ b/lib/diego/client.rb @@ -125,12 +125,14 @@ def actual_lrps_by_process_guid(process_guid) end def request_with_error_handling(req) - tries ||= 3 + attempt ||= 1 http_client.ipaddr = bbs_ip # tell the HTTP client which exact IP to target + logger.info("attempt #{attempt}: trying bbs endpoint #{req.path} on #{bbs_ip}") http_client.request(req) rescue => e - ips_remaining.shift - retry unless ips_remaining.empty? && (tries -= 1).zero? + eliminated_ip = ips_remaining.shift + logger.info("attempt #{attempt}: failed to reach bbs server on #{eliminated_ip}, removing from list") + retry unless ips_remaining.empty? && (attempt += 1) > 3 raise RequestError.new(e.message) end @@ -144,6 +146,10 @@ def bbs_ip attr_reader :http_client, :bbs_url attr_accessor :ips_remaining + def logger + @logger ||= Steno.logger('cc.diego.client') + end + def protobuf_encode!(hash, protobuf_message_class) # See below link to understand proto3 message encoding # https://developers.google.com/protocol-buffers/docs/reference/ruby-generated#message diff --git a/spec/diego/client_spec.rb b/spec/diego/client_spec.rb index 76e4e921dd6..6fa7f42679d 100644 --- a/spec/diego/client_spec.rb +++ b/spec/diego/client_spec.rb @@ -11,6 +11,7 @@ module Diego let(:client_key_file) { File.join(Paths::FIXTURES, 'certs/bbs_client.key') } let(:bbs_ip_1) { '1.2.3.4' } let(:bbs_ip_2) { '5.6.7.8' } + let(:logger) { instance_double(Steno::Logger) } subject(:client) do Client.new(url: bbs_uri, ca_cert_file: ca_cert_file, client_cert_file: client_cert_file, client_key_file: client_key_file, @@ -18,6 +19,8 @@ module Diego end before do allow(::Resolv).to receive(:getaddresses).with(bbs_domain).and_return([bbs_ip_1, bbs_ip_2]) + allow(Steno).to receive(:logger).and_return(logger) + allow(logger).to receive(:info) end describe 'configuration' do @@ -797,6 +800,21 @@ module Diego expect(http_client).to have_received(:ipaddr=).with(bbs_ip_2).twice end end + + context 'logging' do + before do + stub_request(:post, "https://#{bbs_domain}:#{bbs_port}/fake_path"). + to_raise(StandardError.new('error message')).then. + to_return(status: 200) + end + + it 'logs before each request and after each failed request' do + client.request_with_error_handling(Net::HTTP::Post.new('/fake_path')) + expect(logger).to have_received(:info).with(%r{attempt 1: trying bbs endpoint /fake_path on #{bbs_ip_1}}).once + expect(logger).to have_received(:info).with(/attempt 1: failed to reach bbs server on #{bbs_ip_1}, removing from list/).once + expect(logger).to have_received(:info).with(%r{attempt 1: trying bbs endpoint /fake_path on #{bbs_ip_2}}).once + end + end end end end