search

instana / ruby-sensor

💎 Ruby Distributed Tracing & Metrics Sensor for Instana
https://www.instana.com/
MIT License
26 stars 25 forks source link

[Bug]: All Net::HTTP requests fail if URI has no query #250

Closed fgimenezm closed 3 years ago

fgimenezm commented 3 years ago

Problem Description

After adding the Ruby instrumentation we noticed many outgoing API calls that use Net::HTTP started to fail. Doing some research I found the culprit to be this line:

https://github.com/instana/ruby-sensor/blob/master/lib/instana/instrumentation/net-http.rb#L39

If the URI has no query (uri.query is nil) then you get an ArgumentError exception: ArgumentError: bad argument (expected URI object or URI string)

I created a minimal sample app that shows the bug here: https://github.com/fgimenezm/instana_nethttp_bug_sample

The sample app is a very minimal sinatra app that makes an Net::HTTP.get(uri) call copied from the "GET by URI" example in Net::HTTP's docs but without query parameters.

If I start the app with:

INSTANA_DISABLE_AUTO_INSTR=true INSTANA_DISABLE=true bundle exec rackup

Then it works OK:

~ ❯❯❯ curl http://localhost:9292
<!doctype html>
<html>
<head>
...
...

but if I start the app normally with:

bundle exec rackup

then it throws the exception:

~ ❯❯❯ curl http://localhost:9292                                                                                                                                          ✘ 10
ArgumentError: bad argument (expected URI object or URI string)
    /Users/fgimen/.rbenv/versions/2.6.7/lib/ruby/2.6.0/uri/common.rb:739:in `URI'
    /Users/fgimen/.rbenv/versions/2.6.7/lib/ruby/gems/2.6.0/gems/instana-1.203.0/lib/instana/secrets.rb:17:in `remove_from_query'
...
...
...

and the log from the app:

2021-07-01 20:13:40 - ArgumentError - bad argument (expected URI object or URI string):
    /Users/fgimen/.rbenv/versions/2.6.7/lib/ruby/2.6.0/uri/common.rb:739:in `URI'
    /Users/fgimen/.rbenv/versions/2.6.7/lib/ruby/gems/2.6.0/gems/instana-1.203.0/lib/instana/secrets.rb:17:in `remove_from_query'
    /Users/fgimen/.rbenv/versions/2.6.7/lib/ruby/gems/2.6.0/gems/instana-1.203.0/lib/instana/instrumentation/net-http.rb:39:in `request'
    /Users/fgimen/.rbenv/versions/2.6.7/lib/ruby/2.6.0/net/http.rb:1380:in `request_get'
    /Users/fgimen/.rbenv/versions/2.6.7/lib/ruby/2.6.0/net/http.rb:483:in `block in get_response'
    /Users/fgimen/.rbenv/versions/2.6.7/lib/ruby/2.6.0/net/http.rb:920:in `start'
    /Users/fgimen/.rbenv/versions/2.6.7/lib/ruby/2.6.0/net/http.rb:605:in `start'
    /Users/fgimen/.rbenv/versions/2.6.7/lib/ruby/2.6.0/net/http.rb:481:in `get_response'
    /Users/fgimen/.rbenv/versions/2.6.7/lib/ruby/2.6.0/net/http.rb:458:in `get'
    /Users/fgimen/gitrepos/minimal_instana_bug_sample/hello.rb:8:in `block in <top (required)>'
    /Users/fgimen/.rbenv/versions/2.6.7/lib/ruby/gems/2.6.0/gems/sinatra-2.1.0/lib/sinatra/base.rb:1675:in `call'
    /Users/fgimen/.rbenv/versions/2.6.7/lib/ruby/gems/2.6.0/gems/sinatra-2.1.0/lib/sinatra/base.rb:1675:in `block in compile!'
    /Users/fgimen/.rbenv/versions/2.6.7/lib/ruby/gems/2.6.0/gems/sinatra-2.1.0/lib/sinatra/base.rb:1013:in `block (3 levels) in route!'
    /Users/fgimen/.rbenv/versions/2.6.7/lib/ruby/gems/2.6.0/gems/sinatra-2.1.0/lib/sinatra/base.rb:1032:in `route_eval'
    /Users/fgimen/.rbenv/versions/2.6.7/lib/ruby/gems/2.6.0/gems/sinatra-2.1.0/lib/sinatra/base.rb:1013:in `block (2 levels) in route!'
    /Users/fgimen/.rbenv/versions/2.6.7/lib/ruby/gems/2.6.0/gems/sinatra-2.1.0/lib/sinatra/base.rb:1061:in `block in process_route'
    /Users/fgimen/.rbenv/versions/2.6.7/lib/ruby/gems/2.6.0/gems/sinatra-2.1.0/lib/sinatra/base.rb:1059:in `catch'
    /Users/fgimen/.rbenv/versions/2.6.7/lib/ruby/gems/2.6.0/gems/sinatra-2.1.0/lib/sinatra/base.rb:1059:in `process_route'
    /Users/fgimen/.rbenv/versions/2.6.7/lib/ruby/gems/2.6.0/gems/sinatra-2.1.0/lib/sinatra/base.rb:1011:in `block in route!'
    /Users/fgimen/.rbenv/versions/2.6.7/lib/ruby/gems/2.6.0/gems/sinatra-2.1.0/lib/sinatra/base.rb:1008:in `each'
    /Users/fgimen/.rbenv/versions/2.6.7/lib/ruby/gems/2.6.0/gems/sinatra-2.1.0/lib/sinatra/base.rb:1008:in `route!'
    /Users/fgimen/.rbenv/versions/2.6.7/lib/ruby/gems/2.6.0/gems/sinatra-2.1.0/lib/sinatra/base.rb:1129:in `block in dispatch!'
    /Users/fgimen/.rbenv/versions/2.6.7/lib/ruby/gems/2.6.0/gems/sinatra-2.1.0/lib/sinatra/base.rb:1101:in `block in invoke'
    /Users/fgimen/.rbenv/versions/2.6.7/lib/ruby/gems/2.6.0/gems/sinatra-2.1.0/lib/sinatra/base.rb:1101:in `catch'
    /Users/fgimen/.rbenv/versions/2.6.7/lib/ruby/gems/2.6.0/gems/sinatra-2.1.0/lib/sinatra/base.rb:1101:in `invoke'
    /Users/fgimen/.rbenv/versions/2.6.7/lib/ruby/gems/2.6.0/gems/sinatra-2.1.0/lib/sinatra/base.rb:1124:in `dispatch!'
    /Users/fgimen/.rbenv/versions/2.6.7/lib/ruby/gems/2.6.0/gems/sinatra-2.1.0/lib/sinatra/base.rb:939:in `block in call!'
    /Users/fgimen/.rbenv/versions/2.6.7/lib/ruby/gems/2.6.0/gems/sinatra-2.1.0/lib/sinatra/base.rb:1101:in `block in invoke'
    /Users/fgimen/.rbenv/versions/2.6.7/lib/ruby/gems/2.6.0/gems/sinatra-2.1.0/lib/sinatra/base.rb:1101:in `catch'
    /Users/fgimen/.rbenv/versions/2.6.7/lib/ruby/gems/2.6.0/gems/sinatra-2.1.0/lib/sinatra/base.rb:1101:in `invoke'
    /Users/fgimen/.rbenv/versions/2.6.7/lib/ruby/gems/2.6.0/gems/sinatra-2.1.0/lib/sinatra/base.rb:939:in `call!'
    /Users/fgimen/.rbenv/versions/2.6.7/lib/ruby/gems/2.6.0/gems/sinatra-2.1.0/lib/sinatra/base.rb:929:in `call'
    /Users/fgimen/.rbenv/versions/2.6.7/lib/ruby/gems/2.6.0/gems/instana-1.203.0/lib/instana/instrumentation/rack.rb:22:in `call'
    /Users/fgimen/.rbenv/versions/2.6.7/lib/ruby/gems/2.6.0/gems/rack-protection-2.1.0/lib/rack/protection/xss_header.rb:18:in `call'
    /Users/fgimen/.rbenv/versions/2.6.7/lib/ruby/gems/2.6.0/gems/rack-protection-2.1.0/lib/rack/protection/path_traversal.rb:16:in `call'
    /Users/fgimen/.rbenv/versions/2.6.7/lib/ruby/gems/2.6.0/gems/rack-protection-2.1.0/lib/rack/protection/json_csrf.rb:26:in `call'
    /Users/fgimen/.rbenv/versions/2.6.7/lib/ruby/gems/2.6.0/gems/rack-protection-2.1.0/lib/rack/protection/base.rb:50:in `call'
    /Users/fgimen/.rbenv/versions/2.6.7/lib/ruby/gems/2.6.0/gems/rack-protection-2.1.0/lib/rack/protection/base.rb:50:in `call'
    /Users/fgimen/.rbenv/versions/2.6.7/lib/ruby/gems/2.6.0/gems/rack-protection-2.1.0/lib/rack/protection/frame_options.rb:31:in `call'
    /Users/fgimen/.rbenv/versions/2.6.7/lib/ruby/gems/2.6.0/gems/rack-2.2.3/lib/rack/logger.rb:17:in `call'
    /Users/fgimen/.rbenv/versions/2.6.7/lib/ruby/gems/2.6.0/gems/sinatra-2.1.0/lib/sinatra/base.rb:246:in `call'
    /Users/fgimen/.rbenv/versions/2.6.7/lib/ruby/gems/2.6.0/gems/rack-2.2.3/lib/rack/head.rb:12:in `call'
    /Users/fgimen/.rbenv/versions/2.6.7/lib/ruby/gems/2.6.0/gems/rack-2.2.3/lib/rack/method_override.rb:24:in `call'
    /Users/fgimen/.rbenv/versions/2.6.7/lib/ruby/gems/2.6.0/gems/sinatra-2.1.0/lib/sinatra/show_exceptions.rb:22:in `call'
    /Users/fgimen/.rbenv/versions/2.6.7/lib/ruby/gems/2.6.0/gems/sinatra-2.1.0/lib/sinatra/base.rb:216:in `call'
    /Users/fgimen/.rbenv/versions/2.6.7/lib/ruby/gems/2.6.0/gems/sinatra-2.1.0/lib/sinatra/base.rb:1991:in `call'
    /Users/fgimen/.rbenv/versions/2.6.7/lib/ruby/gems/2.6.0/gems/sinatra-2.1.0/lib/sinatra/base.rb:1542:in `block in call'
    /Users/fgimen/.rbenv/versions/2.6.7/lib/ruby/gems/2.6.0/gems/sinatra-2.1.0/lib/sinatra/base.rb:1769:in `synchronize'
    /Users/fgimen/.rbenv/versions/2.6.7/lib/ruby/gems/2.6.0/gems/sinatra-2.1.0/lib/sinatra/base.rb:1542:in `call'
    /Users/fgimen/.rbenv/versions/2.6.7/lib/ruby/gems/2.6.0/gems/rack-2.2.3/lib/rack/tempfile_reaper.rb:15:in `call'
    /Users/fgimen/.rbenv/versions/2.6.7/lib/ruby/gems/2.6.0/gems/rack-2.2.3/lib/rack/lint.rb:50:in `_call'
    /Users/fgimen/.rbenv/versions/2.6.7/lib/ruby/gems/2.6.0/gems/rack-2.2.3/lib/rack/lint.rb:38:in `call'
    /Users/fgimen/.rbenv/versions/2.6.7/lib/ruby/gems/2.6.0/gems/rack-2.2.3/lib/rack/show_exceptions.rb:23:in `call'
    /Users/fgimen/.rbenv/versions/2.6.7/lib/ruby/gems/2.6.0/gems/rack-2.2.3/lib/rack/common_logger.rb:38:in `call'
    /Users/fgimen/.rbenv/versions/2.6.7/lib/ruby/gems/2.6.0/gems/sinatra-2.1.0/lib/sinatra/base.rb:253:in `call'
    /Users/fgimen/.rbenv/versions/2.6.7/lib/ruby/gems/2.6.0/gems/rack-2.2.3/lib/rack/content_length.rb:17:in `call'
    /Users/fgimen/.rbenv/versions/2.6.7/lib/ruby/gems/2.6.0/gems/rack-2.2.3/lib/rack/handler/webrick.rb:95:in `service'
    /Users/fgimen/.rbenv/versions/2.6.7/lib/ruby/2.6.0/webrick/httpserver.rb:140:in `service'
    /Users/fgimen/.rbenv/versions/2.6.7/lib/ruby/2.6.0/webrick/httpserver.rb:96:in `run'
    /Users/fgimen/.rbenv/versions/2.6.7/lib/ruby/2.6.0/webrick/server.rb:307:in `block in start_thread'
::1 - - [01/Jul/2021:20:13:40 -0300] "GET / HTTP/1.1" 500 6793 0.1767

Minimal, Complete, Verifiable, Example

Minimal example code in Github: https://github.com/fgimenezm/instana_nethttp_bug_sample

Gemfile.lock

GEM
  remote: https://rubygems.org/
  specs:
    concurrent-ruby (1.1.9)
    ffi (1.15.3)
    instana (1.203.0)
      concurrent-ruby (>= 1.1)
      oj (>= 3.0.11)
      sys-proctable (>= 1.2.2)
    mustermann (1.1.1)
      ruby2_keywords (~> 0.0.1)
    oj (3.11.7)
    rack (2.2.3)
    rack-protection (2.1.0)
      rack
    ruby2_keywords (0.0.4)
    sinatra (2.1.0)
      mustermann (~> 1.0)
      rack (~> 2.2)
      rack-protection (= 2.1.0)
      tilt (~> 2.0)
    sys-proctable (1.2.6)
      ffi
    tilt (2.0.10)

PLATFORMS
  x86_64-darwin-20

DEPENDENCIES
  instana
  rack
  sinatra

BUNDLED WITH
   2.2.21

Ruby Version

2.6.7
fgimenezm commented 3 years ago

IMPORTANT: This only happens if Instana.agent.secret_values is not nil