Commit 43071784 authored by Bob Van Landuyt's avatar Bob Van Landuyt

Include feature_category label in metrics

This adds the feature_category as a label to the requests for rails
controllers.

It also makes sure that all metrics include the feature_category label
so the labels stay consistent across the different kinds of
transactions.
parent 204020ab
...@@ -9,7 +9,7 @@ module Gitlab ...@@ -9,7 +9,7 @@ module Gitlab
end end
def labels def labels
{ controller: @worker_class.name, action: 'perform' } { controller: @worker_class.name, action: 'perform', feature_category: @worker_class.try(:get_feature_category).to_s }
end end
end end
end end
......
...@@ -7,7 +7,7 @@ module Gitlab ...@@ -7,7 +7,7 @@ module Gitlab
include Gitlab::Metrics::Methods include Gitlab::Metrics::Methods
# base labels shared among all transactions # base labels shared among all transactions
BASE_LABELS = { controller: nil, action: nil }.freeze BASE_LABELS = { controller: nil, action: nil, feature_category: nil }.freeze
# labels that potentially contain sensitive information and will be filtered # labels that potentially contain sensitive information and will be filtered
FILTERED_LABELS = [:branch, :path].freeze FILTERED_LABELS = [:branch, :path].freeze
......
...@@ -32,6 +32,10 @@ module Gitlab ...@@ -32,6 +32,10 @@ module Gitlab
action = "#{controller.action_name}" action = "#{controller.action_name}"
# Try to get the feature category, but don't fail when the controller is
# not an ApplicationController.
feature_category = controller.class.try(:feature_category_for_action, action).to_s
# Devise exposes a method called "request_format" that does the below. # Devise exposes a method called "request_format" that does the below.
# However, this method is not available to all controllers (e.g. certain # However, this method is not available to all controllers (e.g. certain
# Doorkeeper controllers). As such we use the underlying code directly. # Doorkeeper controllers). As such we use the underlying code directly.
...@@ -45,7 +49,7 @@ module Gitlab ...@@ -45,7 +49,7 @@ module Gitlab
action = "#{action}.#{suffix}" action = "#{action}.#{suffix}"
end end
{ controller: controller.class.name, action: action } { controller: controller.class.name, action: action, feature_category: feature_category }
end end
def labels_from_endpoint def labels_from_endpoint
...@@ -61,7 +65,10 @@ module Gitlab ...@@ -61,7 +65,10 @@ module Gitlab
if route if route
path = endpoint_paths_cache[route.request_method][route.path] path = endpoint_paths_cache[route.request_method][route.path]
{ controller: 'Grape', action: "#{route.request_method} #{path}" }
# Feature categories will be added for grape endpoints in
# https://gitlab.com/gitlab-com/gl-infra/scalability/-/issues/462
{ controller: 'Grape', action: "#{route.request_method} #{path}", feature_category: '' }
end end
end end
......
...@@ -9,7 +9,16 @@ RSpec.describe Gitlab::Metrics::BackgroundTransaction do ...@@ -9,7 +9,16 @@ RSpec.describe Gitlab::Metrics::BackgroundTransaction do
describe '#label' do describe '#label' do
it 'returns labels based on class name' do it 'returns labels based on class name' do
expect(subject.labels).to eq(controller: 'TestWorker', action: 'perform') expect(subject.labels).to eq(controller: 'TestWorker', action: 'perform', feature_category: '')
end
it 'contains only the labels defined for metrics' do
expect(subject.labels.keys).to contain_exactly(*described_class.superclass::BASE_LABELS.keys)
end
it 'includes the feature category if there is one' do
expect(test_worker_class).to receive(:get_feature_category).and_return('source_code_management')
expect(subject.labels).to include(feature_category: 'source_code_management')
end end
end end
end end
...@@ -70,6 +70,9 @@ RSpec.describe Gitlab::Metrics::WebTransaction do ...@@ -70,6 +70,9 @@ RSpec.describe Gitlab::Metrics::WebTransaction do
end end
describe '#labels' do describe '#labels' do
let(:request) { double(:request, format: double(:format, ref: :html)) }
let(:controller_class) { double(:controller_class, name: 'TestController') }
context 'when request goes to Grape endpoint' do context 'when request goes to Grape endpoint' do
before do before do
route = double(:route, request_method: 'GET', path: '/:version/projects/:id/archive(.:format)') route = double(:route, request_method: 'GET', path: '/:version/projects/:id/archive(.:format)')
...@@ -77,8 +80,13 @@ RSpec.describe Gitlab::Metrics::WebTransaction do ...@@ -77,8 +80,13 @@ RSpec.describe Gitlab::Metrics::WebTransaction do
env['api.endpoint'] = endpoint env['api.endpoint'] = endpoint
end end
it 'provides labels with the method and path of the route in the grape endpoint' do 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.labels).to eq({ controller: 'Grape', action: 'GET /projects/:id/archive', feature_category: '' })
end
it 'contains only the labels defined for transactions' do
expect(transaction.labels.keys).to contain_exactly(*described_class.superclass::BASE_LABELS.keys)
end end
it 'does not provide labels if route infos are missing' do it 'does not provide labels if route infos are missing' do
...@@ -92,24 +100,25 @@ RSpec.describe Gitlab::Metrics::WebTransaction do ...@@ -92,24 +100,25 @@ RSpec.describe Gitlab::Metrics::WebTransaction do
end end
context 'when request goes to ActionController' do context 'when request goes to ActionController' do
let(:request) { double(:request, format: double(:format, ref: :html)) }
before do before do
klass = double(:klass, name: 'TestController') controller = double(:controller, class: controller_class, action_name: 'show', request: request)
controller = double(:controller, class: klass, action_name: 'show', request: request)
env['action_controller.instance'] = controller env['action_controller.instance'] = controller
end end
it 'tags a transaction with the name and action of a controller' do it 'tags a transaction with the name and action of a controller' do
expect(transaction.labels).to eq({ controller: 'TestController', action: 'show' }) expect(transaction.labels).to eq({ controller: 'TestController', action: 'show', feature_category: '' })
end
it 'contains only the labels defined for transactions' do
expect(transaction.labels.keys).to contain_exactly(*described_class.superclass::BASE_LABELS.keys)
end end
context 'when the request content type is not :html' do context 'when the request content type is not :html' do
let(:request) { double(:request, format: double(:format, ref: :json)) } let(:request) { double(:request, format: double(:format, ref: :json)) }
it 'appends the mime type to the transaction action' do it 'appends the mime type to the transaction action' do
expect(transaction.labels).to eq({ controller: 'TestController', action: 'show.json' }) expect(transaction.labels).to eq({ controller: 'TestController', action: 'show.json', feature_category: '' })
end end
end end
...@@ -117,7 +126,14 @@ RSpec.describe Gitlab::Metrics::WebTransaction do ...@@ -117,7 +126,14 @@ RSpec.describe Gitlab::Metrics::WebTransaction do
let(:request) { double(:request, format: double(:format, ref: 'http://example.com')) } let(:request) { double(:request, format: double(:format, ref: 'http://example.com')) }
it 'does not append the MIME type to the transaction action' do it 'does not append the MIME type to the transaction action' do
expect(transaction.labels).to eq({ controller: 'TestController', action: 'show' }) expect(transaction.labels).to eq({ controller: 'TestController', action: 'show', feature_category: '' })
end
end
context 'when the feature category is known' do
it 'includes it in the feature category label' do
expect(controller_class).to receive(:feature_category_for_action).with('show').and_return(:source_code_management)
expect(transaction.labels).to eq({ controller: 'TestController', action: 'show', feature_category: "source_code_management" })
end end
end end
end end
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment