Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Does not work with constraints in routes #88

Open
werleo opened this issue Nov 23, 2016 · 4 comments
Open

Does not work with constraints in routes #88

werleo opened this issue Nov 23, 2016 · 4 comments

Comments

@werleo
Copy link

werleo commented Nov 23, 2016

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:
heartcombo/devise#3747 (comment)
rails/rails#21477 (comment)

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

@jcarres-mdsol
Copy link
Collaborator

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.

@sullerandras
Copy link

@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.

@jcarres-mdsol
Copy link
Collaborator

so current version solves the problem for you? @sullerandras

@sullerandras
Copy link

@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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants