Skip to content

Commit

Permalink
Make sure the client interceptor actually returns the response
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
KJ Tsanaktsidis committed Jun 23, 2023
1 parent b5eee29 commit 8d5ed99
Show file tree
Hide file tree
Showing 2 changed files with 12 additions and 4 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 eql(:returned_object)
end
end

describe '#client_streamer' do
Expand Down

0 comments on commit 8d5ed99

Please sign in to comment.