Skip to content

Commit

Permalink
Create a cleaner temporary request object (#129)
Browse files Browse the repository at this point in the history
* Create a cleaner temporary request object

Using `Rails.application.routes.router.recognize` is not safe as it updates the passed argument, which is request in our case. Specifically it updates `rails_req.script_name` from `""` to `"/"` which makes `request.path` to have double slash in the beginning. To solve this issue it has been created a temporary clean request object which will be disposed later.

See also: https://github.com/openzipkin/zipkin-ruby/pull/122/files (PR that has the same line before).

* Improve syntax. Bump version. Update changelog.
  • Loading branch information
yellowred authored and jcarres-mdsol committed Nov 15, 2018
1 parent 7c50494 commit d92d521
Show file tree
Hide file tree
Showing 3 changed files with 9 additions and 2 deletions.
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
# 0.29.1
* Patch abstract route to work with Grape

# 0.29.0
* Add abstract route to span name
* Fix not to raise "NoMethodError" for Rails:Module
Expand Down
6 changes: 5 additions & 1 deletion lib/zipkin-tracer/application.rb
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,11 @@ def self.routable_request?(path_info, http_method)

def self.get_route(env)
return nil unless defined?(Rails)
req = Rack::Request.new(env)
stub_env = {
"PATH_INFO" => env[ZipkinTracer::RackHandler::PATH_INFO],
"REQUEST_METHOD" => env[ZipkinTracer::RackHandler::REQUEST_METHOD]
}
req = Rack::Request.new(stub_env)
# Returns a string like /some/path/:id
Rails.application.routes.router.recognize(req) do |route|
return route.path.spec.to_s
Expand Down
2 changes: 1 addition & 1 deletion lib/zipkin-tracer/version.rb
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
module ZipkinTracer
VERSION = '0.29.0'.freeze
VERSION = '0.29.1'.freeze
end

0 comments on commit d92d521

Please sign in to comment.