From 01d7a0e8e695ec74eec6577f29b8111352c5da2d Mon Sep 17 00:00:00 2001 From: KJ Tsanaktsidis Date: Fri, 23 Jun 2023 22:06:57 +1000 Subject: [PATCH] Make sure the client interceptor actually returns the response Datadog::Tracing::Contrib::GRPC::DatadogInterceptor::Client#trace is currently causing all GRPC client operations to return 0.0 instead of whatever they were supposed to return. This is a bit of a Ruby gotcha. This method returns :three ```ruby def broken_method begin :one rescue => e :two else :three end end ``` So when `#trace` is putting `yield` in a `begin` block with an `else`, it's not actually returning the result of yield; it's returning the result of the else block. Fix it by explicitly returning the result of yield. --- .../tracing/contrib/grpc/datadog_interceptor/client.rb | 6 +++--- .../contrib/grpc/datadog_interceptor/client_spec.rb | 10 +++++++++- 2 files changed, 12 insertions(+), 4 deletions(-) diff --git a/lib/datadog/tracing/contrib/grpc/datadog_interceptor/client.rb b/lib/datadog/tracing/contrib/grpc/datadog_interceptor/client.rb index 11c48e82a25..10ad034945a 100644 --- a/lib/datadog/tracing/contrib/grpc/datadog_interceptor/client.rb +++ b/lib/datadog/tracing/contrib/grpc/datadog_interceptor/client.rb @@ -29,15 +29,15 @@ def trace(keywords) annotate!(trace, span, keywords, formatter) begin - yield + result = yield rescue StandardError => e code = e.is_a?(::GRPC::BadStatus) ? e.code : ::GRPC::Core::StatusCodes::UNKNOWN span.set_tag(Contrib::Ext::RPC::GRPC::TAG_STATUS_CODE, code) raise - else - span.set_tag(Contrib::Ext::RPC::GRPC::TAG_STATUS_CODE, ::GRPC::Core::StatusCodes::OK) end + span.set_tag(Contrib::Ext::RPC::GRPC::TAG_STATUS_CODE, ::GRPC::Core::StatusCodes::OK) + result end end diff --git a/spec/datadog/tracing/contrib/grpc/datadog_interceptor/client_spec.rb b/spec/datadog/tracing/contrib/grpc/datadog_interceptor/client_spec.rb index f9d7a2c52b2..004ca14e4e9 100644 --- a/spec/datadog/tracing/contrib/grpc/datadog_interceptor/client_spec.rb +++ b/spec/datadog/tracing/contrib/grpc/datadog_interceptor/client_spec.rb @@ -125,13 +125,21 @@ let(:original_metadata) { { some: 'datum' } } + let(:request_response) do + subject.request_response(**keywords) { :returned_object } + end + before do - subject.request_response(**keywords) {} + request_response end it_behaves_like 'span data contents' it_behaves_like 'inject distributed tracing metadata' + + it 'actually returns the client response' do + expect(request_response).to be(:returned_object) + end end describe '#client_streamer' do