diff options
-rw-r--r-- | changelogs/unreleased/26587-metrics-middleware-endpoint-is-nil.yml | 4 | ||||
-rw-r--r-- | lib/gitlab/metrics/rack_middleware.rb | 15 | ||||
-rw-r--r-- | spec/lib/gitlab/metrics/rack_middleware_spec.rb | 11 |
3 files changed, 26 insertions, 4 deletions
diff --git a/changelogs/unreleased/26587-metrics-middleware-endpoint-is-nil.yml b/changelogs/unreleased/26587-metrics-middleware-endpoint-is-nil.yml new file mode 100644 index 00000000000..5891a5ef6e8 --- /dev/null +++ b/changelogs/unreleased/26587-metrics-middleware-endpoint-is-nil.yml @@ -0,0 +1,4 @@ +--- +title: Check for env[Grape::Env::GRAPE_ROUTING_ARGS] instead of endpoint.route +merge_request: 8544 +author: diff --git a/lib/gitlab/metrics/rack_middleware.rb b/lib/gitlab/metrics/rack_middleware.rb index d01d47a6a7a..47f88727fc8 100644 --- a/lib/gitlab/metrics/rack_middleware.rb +++ b/lib/gitlab/metrics/rack_middleware.rb @@ -71,10 +71,17 @@ module Gitlab def tag_endpoint(trans, env) endpoint = env[ENDPOINT_KEY] - # endpoint.route is nil in the case of a 405 response - if endpoint.route - path = endpoint_paths_cache[endpoint.route.request_method][endpoint.route.path] - trans.action = "Grape##{endpoint.route.request_method} #{path}" + begin + route = endpoint.route + rescue + # endpoint.route is calling env[Grape::Env::GRAPE_ROUTING_ARGS][:route_info] + # but env[Grape::Env::GRAPE_ROUTING_ARGS] is nil in the case of a 405 response + # so we're rescuing exceptions and bailing out + end + + if route + path = endpoint_paths_cache[route.request_method][route.path] + trans.action = "Grape##{route.request_method} #{path}" end end diff --git a/spec/lib/gitlab/metrics/rack_middleware_spec.rb b/spec/lib/gitlab/metrics/rack_middleware_spec.rb index 7371b578a48..fb470ea7568 100644 --- a/spec/lib/gitlab/metrics/rack_middleware_spec.rb +++ b/spec/lib/gitlab/metrics/rack_middleware_spec.rb @@ -126,5 +126,16 @@ describe Gitlab::Metrics::RackMiddleware do expect(transaction.action).to eq('Grape#GET /projects/:id/archive') end + + it 'does not tag a transaction if route infos are missing' do + endpoint = double(:endpoint) + allow(endpoint).to receive(:route).and_raise + + env['api.endpoint'] = endpoint + + middleware.tag_endpoint(transaction, env) + + expect(transaction.action).to be_nil + end end end |