Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
52 changes: 5 additions & 47 deletions lib/prometheus/middleware/collector.rb
Original file line number Diff line number Diff line change
Expand Up @@ -72,12 +72,12 @@ def record(env, code, duration)
counter_labels = {
code: code,
method: env['REQUEST_METHOD'].downcase,
path: strip_ids_from_path(path),
path: path,
}

duration_labels = {
method: env['REQUEST_METHOD'].downcase,
path: strip_ids_from_path(path),
path: path,
}

@requests.increment(labels: counter_labels)
Expand All @@ -87,52 +87,10 @@ def record(env, code, duration)
nil
end

# While `PATH_INFO` is framework agnostic, and works for any Rack app, some Ruby web
# frameworks pass a more useful piece of information into the request env - the
# route that the request matched.
#
# This means that rather than using our generic `:id` and `:uuid` replacements in
# the `path` label for any path segments that look like dynamic IDs, we can put the
# actual route that matched in there, with correctly named parameters. For example,
# if a Sinatra app defined a route like:
#
# get "/foo/:bar" do
# ...
# end
#
# instead of containing `/foo/:id`, the `path` label would contain `/foo/:bar`.
#
# Sadly, Rails is a notable exception, and (as far as I can tell at the time of
# writing) doesn't provide this info in the request env.
def generate_path(env)
if env['sinatra.route']
route = env['sinatra.route'].partition(' ').last
elsif env['grape.routing_args']
# We are deep in the weeds of an object that Grape passes into the request env,
# but don't document any explicit guarantees about. Let's have a fallback in
# case they change it down the line.
#
# This code would be neater with the safe navigation operator (`&.`) here rather
# than the much more verbose `respond_to?` calls, but unlike Rails' `try`
# method, it still raises an error if the object is non-nil, but doesn't respond
# to the method being called on it.
route = nil

route_info = env.dig('grape.routing_args', :route_info)
if route_info.respond_to?(:pattern)
pattern = route_info.pattern
if pattern.respond_to?(:origin)
route = pattern.origin
end
end

# Fall back to PATH_INFO if Grape change the structure of `grape.routing_args`
route ||= env['PATH_INFO']
else
route = env['PATH_INFO']
end

[env['SCRIPT_NAME'], route].join
full_path = [env['SCRIPT_NAME'], env['PATH_INFO']].join

strip_ids_from_path(full_path)
end

def strip_ids_from_path(path)
Expand Down
48 changes: 0 additions & 48 deletions spec/prometheus/middleware/collector_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -123,54 +123,6 @@
expect(registry.get(metric).get(labels: labels)).to include("0.1" => 0, "0.5" => 1)
end

it 'prefers sinatra.route to PATH_INFO' do
metric = :http_server_requests_total

env('sinatra.route', 'GET /foo/:named_param')
get '/foo/7'
env('sinatra.route', nil)
expect(registry.get(metric).values.keys.last[:path]).to eql("/foo/:named_param")
end

it 'prefers grape.routing_args to PATH_INFO' do
metric = :http_server_requests_total

# request.env["grape.routing_args"][:route_info].pattern.origin
#
# Yes, this is the object you have to traverse to get the path.
#
# No, I'm not happy about it either.
grape_routing_args = {
route_info: double(:route_info,
pattern: double(:pattern,
origin: '/foo/:named_param'
)
)
}

env('grape.routing_args', grape_routing_args)
get '/foo/7'
env('grape.routing_args', nil)
expect(registry.get(metric).values.keys.last[:path]).to eql("/foo/:named_param")
end

it "falls back to PATH_INFO if the structure of grape.routing_args changes" do
metric = :http_server_requests_total

grape_routing_args = {
route_info: double(:route_info,
pattern: double(:pattern,
origin_but_different: '/foo/:named_param'
)
)
}

env('grape.routing_args', grape_routing_args)
get '/foo/7'
env('grape.routing_args', nil)
expect(registry.get(metric).values.keys.last[:path]).to eql("/foo/:id")
end

context 'when the app raises an exception' do
let(:original_app) do
lambda do |env|
Expand Down