zipkin-ruby icon indicating copy to clipboard operation
zipkin-ruby copied to clipboard

Does not work with constraints in routes

Open werleo opened this issue 9 years ago • 4 comments

We have constraints in routes which code depends on env passed from Rack. Rails guide: http://edgeguides.rubyonrails.org/routing.html#advanced-constraints

Example code to reproduce problem but ours is much more complicated:

# config/routes.rb
  resources :samples, :constraints => SampleConstraint.new

# lib/sample_constraint.rb
class SampleConstraint
  def matches?(request)
    request.host == 'myhost.com'
  end
end

In that case when sampling you call Rails.application.routes.recognize_path https://github.com/openzipkin/zipkin-ruby/blob/master/lib/zipkin-tracer/application.rb#L8 but recogize_path will use Rack::Mock https://github.com/rails/rails/blob/master/actionpack/lib/action_dispatch/routing/route_set.rb#L734 Rack::Mock will return 'example.org' as host. And we will never reach our endpoint in same trace.

recognize_path is private rails method which shouldn't be used. Related issues: https://github.com/plataformatec/devise/issues/3747#issuecomment-143448852 https://github.com/rails/rails/issues/21477#issuecomment-171873537

I don't know how to solve it yet. Any help?

werleo avatar Nov 23 '16 18:11 werleo

Very interesting, I am not sure when Rails checks those constrains to know if it should proceed with the routing. I am afraid it does something crazy like inserting a middleware at runtime or something.

But yeah, I did not even know you could do that so definitely the code is not ready for this.

jcarres-mdsol avatar Nov 26 '16 00:11 jcarres-mdsol

@jcarres-mdsol I had similar issues and using env['REQUEST_URI'] instead of env['PATH_INFO'] zipkin-tracer.rb#L50 seems to solve the problem.

sullerandras avatar Nov 28 '16 04:11 sullerandras

so current version solves the problem for you? @sullerandras

jcarres-mdsol avatar Nov 28 '16 06:11 jcarres-mdsol

@jcarres-mdsol No, the current version uses PATH_INFO. This monkey patch solved the issue for me:

module ZipkinTracer
  class RackHandler
    def routable_request?(env)
      Application.routable_request?(env['REQUEST_URI'], env['REQUEST_METHOD'])
    end
  end
end

sullerandras avatar Nov 28 '16 09:11 sullerandras