elastic / elastic/apm-agent-ruby

Address Net::HTTP instrumentation shortcomings

Open
#1,304 0 comments 0 reactions 0 assignees View on GitHub
agent-ruby bug
Dominant language
Ruby
Stars
173
Forks
151
Avg merge
4d 8h
Merged PRs (30d)
2

Description

There are a few issues with our `Net::HTTP` instrumentation.

1) We set the host to `localhost` for the span context uri field if it's not otherwise specified in the `Net::HTTP` instrumentation [here](https://github.com/elastic/apm-agent-ruby/blob/v4.5.1/lib/elastic_apm/spies/net_http.rb#L61).
We should evaluate if this should continue being the case. As `Net::HTTP` doesn't default to `localhost`, it could be misleading to put `localhost` in the uri field.

2) As of the time of this issue creation, `Net::HTTP` allows a few different ways of sending a request. When a `nil` address is provided, none of them reach our instrumentation.

Case A
> Net::HTTP.start(nil) do...end

Doesn’t get to the instrumented code, because the `Net::HTTP` tries to connect to the address before it gets to the `Net::HTTP#request` method (the one we instrument).

Case B
> Net::HTTP.get(nil)

Doesn’t get to the instrumented code because this method first calls `Net::HTTP.start` and has the same problem as noted above.

Case C
> h = Net::HTTP.new(nil)
> h.get(‘/‘)

`Net::HTTP.new` returns a new class that doesn’t have the ElasticAPM::Spies::NetHTTPSpy::Ext module preprended, so any methods called using that object are not instrumented.

Case A and B should be considered edge cases, as users probably wouldn't provide a `nil` address to the http methods. Case C is a gap in our instrumentation that should be addressed.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.