Skip to content

Commit

Permalink
Pass X-Vcap-Request-Id header in calls to BBS (#3305)
Browse files Browse the repository at this point in the history 
To support distributed tracing the request id received from the Gorouter
is passed to Diego BBS so that the same request can be traced along
multiple components.

Signed-off-by: Maria Shaldybin <[email protected]>
Co-authored-by: Marc Paquette <[email protected]>
  • Loading branch information
@mariash @MarcPaquette
mariash and MarcPaquette authored Jun 8, 2023
1 parent fb56f00 commit 820671d
Show file tree
Hide file tree
Showing 2 changed files with 57 additions and 28 deletions.
30 changes: 16 additions & 14 deletions lib/diego/client.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,6 @@

module Diego
class Client
PROTOBUF_HEADER = { 'Content-Type'.freeze => 'application/x-protobuf'.freeze }.freeze

def initialize(url:, ca_cert_file:, client_cert_file:, client_key_file:,
connect_timeout:, send_timeout:, receive_timeout:)
ENV['PB_IGNORE_DEPRECATIONS'] ||= 'true'
Expand All @@ -33,7 +31,7 @@ def upsert_domain(domain:, ttl:)
request = protobuf_encode!({ domain: domain, ttl: ttl.to_i }, Bbs::Models::UpsertDomainRequest)

response = with_request_error_handling do
client.post(Routes::UPSERT_DOMAIN, request, PROTOBUF_HEADER)
client.post(Routes::UPSERT_DOMAIN, request, headers)
end

validate_status_200!(response)
Expand All @@ -44,7 +42,7 @@ def desire_task(task_definition:, domain:, task_guid:)
request = protobuf_encode!({ task_definition: task_definition, domain: domain, task_guid: task_guid }, Bbs::Models::DesireTaskRequest)

response = with_request_error_handling do
client.post(Routes::DESIRE_TASK, request, PROTOBUF_HEADER)
client.post(Routes::DESIRE_TASK, request, headers)
end

validate_status_200!(response)
Expand All @@ -55,7 +53,7 @@ def task_by_guid(task_guid)
request = protobuf_encode!({ task_guid: task_guid }, Bbs::Models::TaskByGuidRequest)

response = with_request_error_handling do
client.post(Routes::TASK_BY_GUID, request, PROTOBUF_HEADER)
client.post(Routes::TASK_BY_GUID, request, headers)
end

validate_status_200!(response)
Expand All @@ -66,7 +64,7 @@ def tasks(domain: '', cell_id: '')
request = protobuf_encode!({ domain: domain, cell_id: cell_id }, Bbs::Models::TasksRequest)

response = with_request_error_handling do
client.post(Routes::LIST_TASKS, request, PROTOBUF_HEADER)
client.post(Routes::LIST_TASKS, request, headers)
end

validate_status_200!(response)
Expand All @@ -77,7 +75,7 @@ def cancel_task(task_guid)
request = protobuf_encode!({ task_guid: task_guid }, Bbs::Models::TaskGuidRequest)

response = with_request_error_handling do
client.post(Routes::CANCEL_TASK, request, PROTOBUF_HEADER)
client.post(Routes::CANCEL_TASK, request, headers)
end

validate_status_200!(response)
Expand All @@ -88,7 +86,7 @@ def desire_lrp(lrp)
request = protobuf_encode!({ desired_lrp: lrp }, Bbs::Models::DesireLRPRequest)

response = with_request_error_handling do
client.post(Routes::DESIRE_LRP, request, PROTOBUF_HEADER)
client.post(Routes::DESIRE_LRP, request, headers)
end

validate_status_200!(response)
Expand All @@ -99,7 +97,7 @@ def desired_lrp_by_process_guid(process_guid)
request = protobuf_encode!({ process_guid: process_guid }, Bbs::Models::DesiredLRPByProcessGuidRequest)

response = with_request_error_handling do
client.post(Routes::DESIRED_LRP_BY_PROCESS_GUID, request, PROTOBUF_HEADER)
client.post(Routes::DESIRED_LRP_BY_PROCESS_GUID, request, headers)
end

validate_status_200!(response)
Expand All @@ -110,7 +108,7 @@ def update_desired_lrp(process_guid, lrp_update)
request = protobuf_encode!({ process_guid: process_guid, update: lrp_update }, Bbs::Models::UpdateDesiredLRPRequest)

response = with_request_error_handling do
client.post(Routes::UPDATE_DESIRED_LRP, request, PROTOBUF_HEADER)
client.post(Routes::UPDATE_DESIRED_LRP, request, headers)
end

validate_status_200!(response)
Expand All @@ -121,7 +119,7 @@ def remove_desired_lrp(process_guid)
request = protobuf_encode!({ process_guid: process_guid }, Bbs::Models::RemoveDesiredLRPRequest)

response = with_request_error_handling do
client.post(Routes::REMOVE_DESIRED_LRP, request, PROTOBUF_HEADER)
client.post(Routes::REMOVE_DESIRED_LRP, request, headers)
end

validate_status_200!(response)
Expand All @@ -132,7 +130,7 @@ def retire_actual_lrp(actual_lrp_key)
request = protobuf_encode!({ actual_lrp_key: actual_lrp_key }, Bbs::Models::RetireActualLRPRequest)

response = with_request_error_handling do
client.post(Routes::RETIRE_ACTUAL_LRP, request, PROTOBUF_HEADER)
client.post(Routes::RETIRE_ACTUAL_LRP, request, headers)
end

validate_status_200!(response)
Expand All @@ -143,7 +141,7 @@ def desired_lrp_scheduling_infos(domain)
request = protobuf_encode!({ domain: domain }, Bbs::Models::DesiredLRPsRequest)

response = with_request_error_handling do
client.post(Routes::DESIRED_LRP_SCHEDULING_INFOS, request, PROTOBUF_HEADER)
client.post(Routes::DESIRED_LRP_SCHEDULING_INFOS, request, headers)
end

validate_status_200!(response)
Expand All @@ -154,7 +152,7 @@ def actual_lrps_by_process_guid(process_guid)
request = protobuf_encode!({ process_guid: process_guid }, Bbs::Models::ActualLRPsRequest)

response = with_request_error_handling do
client.post(Routes::ACTUAL_LRPS, request, PROTOBUF_HEADER)
client.post(Routes::ACTUAL_LRPS, request, headers)
end

validate_status_200!(response)
Expand Down Expand Up @@ -202,5 +200,9 @@ def build_client(url, ca_cert_file, client_cert_file, client_key_file,
client.ssl_config.set_trust_ca(ca_cert_file)
client
end

def headers
{ 'Content-Type' => 'application/x-protobuf', 'X-Vcap-Request-Id' => ::VCAP::Request.current_id.to_s.split(':')[0].to_s }
end
end
end
55 changes: 41 additions & 14 deletions spec/diego/client_spec.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,18 @@ module Diego
let(:client_cert_file) { File.join(Paths::FIXTURES, 'certs/bbs_client.crt') }
let(:client_key_file) { File.join(Paths::FIXTURES, 'certs/bbs_client.key') }
let(:timeout) { 10 }
let(:request_id) { '522960b781af4039b8b91a20ff6c0394' }

subject(:client) do
Client.new(url: bbs_url, ca_cert_file: ca_cert_file, client_cert_file: client_cert_file, client_key_file: client_key_file,
connect_timeout: timeout, send_timeout: timeout, receive_timeout: timeout)
end

before do
# from middleware/vcap_request_id.rb
::VCAP::Request.current_id = "#{request_id}::b62be6c2-0f2c-4199-94d3-41a69e00f67d"
end

describe 'configuration' do
it "should set ENV['PB_IGNORE_DEPRECATIONS'] to true" do
# to supress warnings in stderr when BBS sends deprecated keys in responses
Expand Down Expand Up @@ -84,7 +90,7 @@ module Diego
expect(response.error).to be_nil
expect(a_request(:post, "#{bbs_url}/v1/domains/upsert").with(
body: Bbs::Models::UpsertDomainRequest.encode(expected_domain_request).to_s,
headers: { 'Content-Type' => 'application/x-protobuf' }
headers: { 'Content-Type' => 'application/x-protobuf', 'X-Vcap-Request-Id' => request_id }
)).to have_been_made.once
end

Expand Down Expand Up @@ -140,7 +146,7 @@ module Diego
expect(response.error).to be_nil
expect(a_request(:post, "#{bbs_url}/v1/tasks/desire.r2").with(
body: Bbs::Models::DesireTaskRequest.encode(expected_task_request).to_s,
headers: { 'Content-Type' => 'application/x-protobuf' }
headers: { 'Content-Type' => 'application/x-protobuf', 'X-Vcap-Request-Id' => request_id }
)).to have_been_made.once
end

Expand Down Expand Up @@ -197,7 +203,7 @@ module Diego
expect(response.error).to be_nil
expect(a_request(:post, "#{bbs_url}/v1/tasks/list.r2").with(
body: Bbs::Models::TasksRequest.encode(expected_request).to_s,
headers: { 'Content-Type' => 'application/x-protobuf' }
headers: { 'Content-Type' => 'application/x-protobuf', 'X-Vcap-Request-Id' => request_id }
)).to have_been_made.once
end

Expand All @@ -210,7 +216,7 @@ module Diego
expect(response.error).to be_nil
expect(a_request(:post, "#{bbs_url}/v1/tasks/list.r2").with(
body: Bbs::Models::TasksRequest.encode(expected_request).to_s,
headers: { 'Content-Type' => 'application/x-protobuf' }
headers: { 'Content-Type' => 'application/x-protobuf', 'X-Vcap-Request-Id' => request_id }
)).to have_been_made.once
end

Expand All @@ -222,7 +228,7 @@ module Diego
expect(response.error).to be_nil
expect(a_request(:post, "#{bbs_url}/v1/tasks/list.r2").with(
body: Bbs::Models::TasksRequest.encode(expected_request).to_s,
headers: { 'Content-Type' => 'application/x-protobuf' }
headers: { 'Content-Type' => 'application/x-protobuf', 'X-Vcap-Request-Id' => request_id }
)).to have_been_made.once
end
end
Expand Down Expand Up @@ -278,7 +284,7 @@ module Diego
expect(response.error).to be_nil
expect(a_request(:post, "#{bbs_url}/v1/tasks/get_by_task_guid.r2").with(
body: Bbs::Models::TaskByGuidRequest.encode(expected_request).to_s,
headers: { 'Content-Type' => 'application/x-protobuf' }
headers: { 'Content-Type' => 'application/x-protobuf', 'X-Vcap-Request-Id' => request_id }
)).to have_been_made.once
end

Expand Down Expand Up @@ -333,7 +339,7 @@ module Diego
expect(response.error).to be_nil
expect(a_request(:post, "#{bbs_url}/v1/tasks/cancel").with(
body: Bbs::Models::TaskGuidRequest.encode(expected_cancel_request).to_s,
headers: { 'Content-Type' => 'application/x-protobuf' }
headers: { 'Content-Type' => 'application/x-protobuf', 'X-Vcap-Request-Id' => request_id }
)).to have_been_made.once
end

Expand Down Expand Up @@ -389,7 +395,7 @@ module Diego
expect(response.error).to be_nil
expect(a_request(:post, "#{bbs_url}/v1/desired_lrp/desire.r2").with(
body: Bbs::Models::DesireLRPRequest.encode(expected_desire_lrp_request).to_s,
headers: { 'Content-Type' => 'application/x-protobuf' }
headers: { 'Content-Type' => 'application/x-protobuf', 'X-Vcap-Request-Id' => request_id }
)).to have_been_made.once
end

Expand Down Expand Up @@ -448,7 +454,7 @@ module Diego
expect(response.desired_lrp).to eq(lrp)
expect(a_request(:post, "#{bbs_url}/v1/desired_lrps/get_by_process_guid.r2").with(
body: Bbs::Models::DesiredLRPByProcessGuidRequest.encode(expected_request).to_s,
headers: { 'Content-Type' => 'application/x-protobuf' }
headers: { 'Content-Type' => 'application/x-protobuf', 'X-Vcap-Request-Id' => request_id }
)).to have_been_made.once
end

Expand Down Expand Up @@ -506,7 +512,7 @@ module Diego
expect(response.error).to be_nil
expect(a_request(:post, "#{bbs_url}/v1/desired_lrp/remove").with(
body: Bbs::Models::RemoveDesiredLRPRequest.encode(expected_request).to_s,
headers: { 'Content-Type' => 'application/x-protobuf' }
headers: { 'Content-Type' => 'application/x-protobuf', 'X-Vcap-Request-Id' => request_id }
)).to have_been_made.once
end

Expand Down Expand Up @@ -562,7 +568,7 @@ module Diego
expect(response.error).to be_nil
expect(a_request(:post, "#{bbs_url}/v1/actual_lrps/retire").with(
body: Bbs::Models::RetireActualLRPRequest.encode(expected_request).to_s,
headers: { 'Content-Type' => 'application/x-protobuf' }
headers: { 'Content-Type' => 'application/x-protobuf', 'X-Vcap-Request-Id' => request_id }
)).to have_been_made.once
end

Expand Down Expand Up @@ -622,7 +628,7 @@ module Diego
expect(response.error).to be_nil
expect(a_request(:post, "#{bbs_url}/v1/desired_lrp/update").with(
body: Bbs::Models::UpdateDesiredLRPRequest.encode(expected_request).to_s,
headers: { 'Content-Type' => 'application/x-protobuf' }
headers: { 'Content-Type' => 'application/x-protobuf', 'X-Vcap-Request-Id' => request_id }
)).to have_been_made.once
end

Expand Down Expand Up @@ -684,7 +690,7 @@ module Diego
expect(response.actual_lrps).to eq(actual_lrps)
expect(a_request(:post, "#{bbs_url}/v1/actual_lrps/list").with(
body: Bbs::Models::ActualLRPsRequest.encode(expected_request).to_s,
headers: { 'Content-Type' => 'application/x-protobuf' }
headers: { 'Content-Type' => 'application/x-protobuf', 'X-Vcap-Request-Id' => request_id }
)).to have_been_made.once
end
end
Expand All @@ -711,7 +717,7 @@ module Diego
expect(response.desired_lrp_scheduling_infos).to eq(scheduling_infos)
expect(a_request(:post, "#{bbs_url}/v1/desired_lrp_scheduling_infos/list").with(
body: Bbs::Models::DesiredLRPsRequest.encode(expected_request).to_s,
headers: { 'Content-Type' => 'application/x-protobuf' }
headers: { 'Content-Type' => 'application/x-protobuf', 'X-Vcap-Request-Id' => request_id }
)).to have_been_made.once
end

Expand Down Expand Up @@ -784,5 +790,26 @@ module Diego
expect(TCPSocket).to have_received(:new).with(bbs_host, bbs_port, { connect_timeout: timeout / 2 }).exactly(3).times
end
end

describe '#headers' do
let(:response_body) { Bbs::Models::UpsertDomainResponse.encode(Bbs::Models::UpsertDomainResponse.new(error: nil)).to_s }
let(:response_status) { 200 }
let(:domain) { 'domain' }
let(:ttl) { VCAP::CloudController::Diego::APP_LRP_DOMAIN_TTL }

before do
::VCAP::Request.current_id = nil
stub_request(:post, "#{bbs_url}/v1/domains/upsert").to_return(status: response_status, body: response_body)
end

it 'returns an empty X-Vcap-Request-Id header when a nil current_id is provided' do
response = client.upsert_domain(domain: domain, ttl: ttl)

expect(response.error).to be_nil
expect(a_request(:post, "#{bbs_url}/v1/domains/upsert").with(
headers: { 'Content-Type' => 'application/x-protobuf', 'X-Vcap-Request-Id' => '' }
)).to have_been_made.once
end
end
end
end

0 comments on commit 820671d

Please sign in to comment.