Skip to content

Commit

Permalink
Add abstract route to span name #96 (#122)
Browse files Browse the repository at this point in the history
  • Loading branch information
jmhon08 authored and adriancole committed Jun 23, 2018
1 parent e2f4299 commit 50640d9
Show file tree
Hide file tree
Showing 4 changed files with 86 additions and 12 deletions.
11 changes: 11 additions & 0 deletions lib/zipkin-tracer/application.rb
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,17 @@ def self.routable_request?(path_info, http_method)
false
end

def self.get_route(path_info, http_method)
return nil unless defined?(Rails)
req = Rack::Request.new("PATH_INFO" => path_info, "REQUEST_METHOD" => http_method)
# Returns a string like /some/path/:id
Rails.application.routes.router.recognize(req) { |route|
return route.path.spec.to_s
}
rescue
nil
end

def self.logger
if defined?(Rails) # If we happen to be inside a Rails app, use its logger
Rails.logger
Expand Down
8 changes: 6 additions & 2 deletions lib/zipkin-tracer/rack/zipkin-tracer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ def call(env)
if !trace_id.sampled? || !routable_request?(env)
@app.call(env)
else
@tracer.with_new_span(trace_id, env[REQUEST_METHOD].to_s.downcase) do |span|
@tracer.with_new_span(trace_id, "#{env[REQUEST_METHOD].to_s.downcase} #{route(env)}".strip) do |span|
trace!(span, zipkin_env) { @app.call(env) }
end
end
Expand All @@ -40,7 +40,11 @@ def call(env)
private

def routable_request?(env)
Application.routable_request?(env[PATH_INFO], env[REQUEST_METHOD])
Application.routable_request?(env[PATH_INFO], env[REQUEST_METHOD])
end

def route(env)
Application.get_route(env[PATH_INFO], env[REQUEST_METHOD])
end

def annotate_plugin(span, env, status, response_headers, response_body)
Expand Down
37 changes: 37 additions & 0 deletions spec/lib/application_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,43 @@ module ZipkinTracer
end
end

describe '.get_route' do
subject { Application.get_route("path", "METHOD") }

context 'Rails available' do
before do
stub_const('Rails', Class.new)
end

context 'route for /api/v1/messages/123 is found' do
before do
route = "/api/v1/messages/:id"
allow(Rails).to receive_message_chain('application.routes.router.recognize') { route }
end

it 'returns route' do
expect(subject).to eq("/api/v1/messages/:id")
end
end

context 'route is not found' do
before do
allow(Rails).to receive_message_chain('application.routes.router.recognize') { nil }
end

it 'returns nil' do
expect(subject).to eq(nil)
end
end
end

context 'Rails not available' do
it 'returns nil' do
expect(subject).to eq(nil)
end
end
end

describe '.logger' do
subject { Application.logger }

Expand Down
42 changes: 32 additions & 10 deletions spec/lib/rack/zipkin-tracer_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,10 @@ def mock_env(path = '/', params = {})
Rack::MockRequest.env_for(path, params)
end

def mock_env_route(path = '/thing/123', params = {})
Rack::MockRequest.env_for(path, params)
end

def expect_host(host)
expect(host).to be_a_kind_of(Trace::Endpoint)
expect(host.ipv4).to eq(host_ip)
Expand Down Expand Up @@ -75,30 +79,48 @@ def expect_host(host)
context 'Using Rails' do
subject { middleware(app) }

context 'accessing a valid URL of our service' do
context 'accessing a valid URL "/" of our service' do
before do
allow(middleware(app)).to receive(:routable_request?).and_return(true)
allow(ZipkinTracer::Application).to receive(:routable_request?).and_return(true)
allow(ZipkinTracer::Application).to receive(:get_route).and_return(nil)
end

it_should_behave_like 'traces the request'
end

context 'accessing an invalid URL our our service' do
context 'accessing a valid URL "/thing/123" of our service' do
before do
allow(middleware(app)).to receive(:routable_request?).and_return(false)
allow(ZipkinTracer::Application).to receive(:routable_request?).and_return(true)
allow(ZipkinTracer::Application).to receive(:get_route).and_return("/thing/:id")
end

it 'calls the app' do
it 'traces the request' do
expect(ZipkinTracer::TraceContainer).to receive(:with_trace_id).and_call_original
expect(tracer).to receive(:with_new_span).ordered.with(anything, 'get /thing/:id').and_call_original
expect_any_instance_of(Trace::Span).to receive(:record_tag).with('http.path', '/thing/123')
expect_any_instance_of(Trace::Span).to receive(:record).with(Trace::Annotation::SERVER_RECV)
expect_any_instance_of(Trace::Span).to receive(:record).with(Trace::Annotation::SERVER_SEND)

status, headers, body = subject.call(mock_env_route)
expect(status).to eq(app_status)
expect(headers).to eq(app_headers)
expect { |b| body.each &b }.to yield_with_args("/thing/123")
end
end

context 'accessing an invalid URL of our service' do
before do
allow(ZipkinTracer::Application).to receive(:routable_request?).and_return(false)
end

it 'calls the app and does not trace the request' do
expect_any_instance_of(Trace::Span).not_to receive(:record)

status, _, body = subject.call(mock_env)
# return expected status
expect(status).to eq(200)
expect { |b| body.each &b }.to yield_with_args(app_body)
end

it 'does not trace the request' do
expect(::Trace).not_to receive(:push)
expect(::Trace).not_to receive(:record)
end
end
end

Expand Down

0 comments on commit 50640d9

Please sign in to comment.