diff options
author | Pawel Chojnacki <pawel@chojnacki.ws> | 2017-09-07 11:15:27 +0200 |
---|---|---|
committer | Pawel Chojnacki <pawel@chojnacki.ws> | 2017-11-02 18:11:44 +0100 |
commit | f64085e6932ff128facdc361c340cb951214c1c6 (patch) | |
tree | 04e4572e2d77e2372ef73ffdfd257ff920ea083d | |
parent | accc3a4517091f67ebaa61da2457e7165e7621d9 (diff) | |
download | gitlab-ce-f64085e6932ff128facdc361c340cb951214c1c6.tar.gz |
Move labels tests from Metrics rack spec to Transaction spec
-rw-r--r-- | lib/gitlab/metrics/rack_middleware.rb | 12 | ||||
-rw-r--r-- | lib/gitlab/metrics/transaction.rb | 26 | ||||
-rw-r--r-- | spec/lib/gitlab/metrics/rack_middleware_spec.rb | 74 | ||||
-rw-r--r-- | spec/lib/gitlab/metrics/samplers/gc_sampler_spec.rb | 0 | ||||
-rw-r--r-- | spec/lib/gitlab/metrics/transaction_spec.rb | 55 |
5 files changed, 77 insertions, 90 deletions
diff --git a/lib/gitlab/metrics/rack_middleware.rb b/lib/gitlab/metrics/rack_middleware.rb index 11af1fb6322..a5aa0c49551 100644 --- a/lib/gitlab/metrics/rack_middleware.rb +++ b/lib/gitlab/metrics/rack_middleware.rb @@ -41,18 +41,6 @@ module Gitlab def filtered_path(env) ActionDispatch::Request.new(env).filtered_path.presence || env['REQUEST_URI'] end - - def endpoint_paths_cache - @endpoint_paths_cache ||= Hash.new do |hash, http_method| - hash[http_method] = Hash.new do |inner_hash, raw_path| - inner_hash[raw_path] = endpoint_instrumentable_path(raw_path) - end - end - end - - def endpoint_instrumentable_path(raw_path) - raw_path.sub('(.:format)', '').sub('/:version', '') - end end end end diff --git a/lib/gitlab/metrics/transaction.rb b/lib/gitlab/metrics/transaction.rb index 91e9628606b..5bf8580d91f 100644 --- a/lib/gitlab/metrics/transaction.rb +++ b/lib/gitlab/metrics/transaction.rb @@ -55,7 +55,7 @@ module Gitlab end def action - "#{labels[:controller]}##{labels[:action]}" if labels + "#{labels[:controller]}##{labels[:action]}" if labels && !labels.empty? end def labels @@ -63,9 +63,9 @@ module Gitlab # memoize transaction labels only source env variables were present @labels = if @env[CONTROLLER_KEY] - labels_from_controller(@env) || {} + labels_from_controller || {} elsif @env[ENDPOINT_KEY] - labels_from_endpoint(@env) || {} + labels_from_endpoint || {} end @labels || {} @@ -199,8 +199,8 @@ module Gitlab ) end - def labels_from_controller(env) - controller = env[CONTROLLER_KEY] + def labels_from_controller + controller = @env[CONTROLLER_KEY] action = "#{controller.action_name}" suffix = CONTENT_TYPES[controller.content_type] @@ -212,8 +212,8 @@ module Gitlab { controller: controller.class.name, action: action } end - def labels_from_endpoint(env) - endpoint = env[ENDPOINT_KEY] + def labels_from_endpoint + endpoint = @env[ENDPOINT_KEY] begin route = endpoint.route @@ -228,6 +228,18 @@ module Gitlab { controller: 'Grape', action: "#{route.request_method} #{path}" } end end + + def endpoint_paths_cache + @endpoint_paths_cache ||= Hash.new do |hash, http_method| + hash[http_method] = Hash.new do |inner_hash, raw_path| + inner_hash[raw_path] = endpoint_instrumentable_path(raw_path) + end + end + end + + def endpoint_instrumentable_path(raw_path) + raw_path.sub('(.:format)', '').sub('/:version', '') + end end end end diff --git a/spec/lib/gitlab/metrics/rack_middleware_spec.rb b/spec/lib/gitlab/metrics/rack_middleware_spec.rb index ec415f2bd85..ee12882f4c0 100644 --- a/spec/lib/gitlab/metrics/rack_middleware_spec.rb +++ b/spec/lib/gitlab/metrics/rack_middleware_spec.rb @@ -18,9 +18,9 @@ describe Gitlab::Metrics::RackMiddleware do expect(middleware.call(env)).to eq('yay') end - it 'tags a transaction with the name and action of a controller' do - klass = double(:klass, name: 'TestController', content_type: 'text/html') - controller = double(:controller, class: klass, action_name: 'show') + xit 'tags a transaction with the name and action of a controller' do + klass = double(:klass, name: 'TestController') + controller = double(:controller, class: klass, action_name: 'show', content_type: 'text/html') env['action_controller.instance'] = controller @@ -32,20 +32,6 @@ describe Gitlab::Metrics::RackMiddleware do middleware.call(env) end - it 'tags a transaction with the method and path of the route in the grape endpoint' do - route = double(:route, request_method: "GET", path: "/:version/projects/:id/archive(.:format)") - endpoint = double(:endpoint, route: route) - - env['api.endpoint'] = endpoint - - allow(app).to receive(:call).with(env) - - expect(middleware).to receive(:tag_endpoint) - .with(an_instance_of(Gitlab::Metrics::Transaction), env) - - middleware.call(env) - end - it 'tracks any raised exceptions' do expect(app).to receive(:call).with(env).and_raise(RuntimeError) @@ -84,58 +70,4 @@ describe Gitlab::Metrics::RackMiddleware do end end end - - describe '#tag_controller' do - let(:transaction) { middleware.transaction_from_env(env) } - let(:content_type) { 'text/html' } - - before do - klass = double(:klass, name: 'TestController') - controller = double(:controller, class: klass, action_name: 'show', content_type: content_type) - - env['action_controller.instance'] = controller - end - - it 'tags a transaction with the name and action of a controller' do - middleware.tag_controller(transaction, env) - - expect(transaction.action).to eq('TestController#show') - end - - context 'when the response content type is not :html' do - let(:content_type) { 'application/json' } - - it 'appends the mime type to the transaction action' do - middleware.tag_controller(transaction, env) - - expect(transaction.action).to eq('TestController#show.json') - end - end - end - - describe '#tag_endpoint' do - let(:transaction) { middleware.transaction_from_env(env) } - - it 'tags a transaction with the method and path of the route in the grape endpount' do - route = double(:route, request_method: "GET", path: "/:version/projects/:id/archive(.:format)") - endpoint = double(:endpoint, route: route) - - env['api.endpoint'] = endpoint - - middleware.tag_endpoint(transaction, env) - - 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 diff --git a/spec/lib/gitlab/metrics/samplers/gc_sampler_spec.rb b/spec/lib/gitlab/metrics/samplers/gc_sampler_spec.rb deleted file mode 100644 index e69de29bb2d..00000000000 --- a/spec/lib/gitlab/metrics/samplers/gc_sampler_spec.rb +++ /dev/null diff --git a/spec/lib/gitlab/metrics/transaction_spec.rb b/spec/lib/gitlab/metrics/transaction_spec.rb index ad81554c9ff..19c2d0a2b0b 100644 --- a/spec/lib/gitlab/metrics/transaction_spec.rb +++ b/spec/lib/gitlab/metrics/transaction_spec.rb @@ -155,6 +155,61 @@ describe Gitlab::Metrics::Transaction do end end + describe '#labels' do + context 'when request goes to Grape endpoint' do + before do + route = double(:route, request_method: 'GET', path: '/:version/projects/:id/archive(.:format)') + endpoint = double(:endpoint, route: route) + + env['api.endpoint'] = endpoint + end + it 'provides labels with the method and path of the route in the grape endpoint' do + expect(transaction.labels).to eq({ controller: 'Grape', action: 'GET /projects/:id/archive' }) + expect(transaction.action).to eq('Grape#GET /projects/:id/archive') + end + + it 'does not provide labels if route infos are missing' do + endpoint = double(:endpoint) + allow(endpoint).to receive(:route).and_raise + + env['api.endpoint'] = endpoint + + expect(transaction.labels).to eq({}) + expect(transaction.action).to be_nil + end + end + + context 'when request goes to ActionController' do + let(:content_type) { 'text/html' } + + before do + klass = double(:klass, name: 'TestController') + controller = double(:controller, class: klass, action_name: 'show', content_type: content_type) + + env['action_controller.instance'] = controller + end + + it 'tags a transaction with the name and action of a controller' do + expect(transaction.labels).to eq({ controller: 'TestController', action: 'show' }) + expect(transaction.action).to eq('TestController#show') + end + + context 'when the response content type is not :html' do + let(:content_type) { 'application/json' } + + it 'appends the mime type to the transaction action' do + expect(transaction.labels).to eq({ controller: 'TestController', action: 'show.json' }) + expect(transaction.action).to eq('TestController#show.json') + end + end + end + + it 'returns no labels when no route information is present in env' do + expect(transaction.labels).to eq({}) + expect(transaction.action).to eq(nil) + end + end + describe '#add_event' do it 'adds a metric' do transaction.add_event(:meow) |