elastic / elastic/apm-agent-ruby
Address Net::HTTP instrumentation shortcomings
- 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
Assessment
This issue has not been assessed yet.