Commit a029a5a2 authored by Ash McKenzie's avatar Ash McKenzie

Merge branch 'feature-category-controller-metrics' into 'master'

Add feature categories to web metrics and logs

See merge request gitlab-org/gitlab!44918
parents e7655a30 70cc8857
......@@ -310,7 +310,7 @@ gem 'sentry-raven', '~> 3.0'
gem 'premailer-rails', '~> 1.10.3'
# LabKit: Tracing and Correlation
gem 'gitlab-labkit', '0.12.1'
gem 'gitlab-labkit', '0.12.2'
# I18n
gem 'ruby_parser', '~> 3.8', require: false
......
......@@ -428,7 +428,7 @@ GEM
fog-json (~> 1.2.0)
mime-types
ms_rest_azure (~> 0.12.0)
gitlab-labkit (0.12.1)
gitlab-labkit (0.12.2)
actionpack (>= 5.0.0, < 6.1.0)
activesupport (>= 5.0.0, < 6.1.0)
grpc (~> 1.19)
......@@ -1331,7 +1331,7 @@ DEPENDENCIES
github-markup (~> 1.7.0)
gitlab-chronic (~> 0.10.5)
gitlab-fog-azure-rm (~> 1.0)
gitlab-labkit (= 0.12.1)
gitlab-labkit (= 0.12.2)
gitlab-license (~> 1.0)
gitlab-mail_room (~> 0.0.7)
gitlab-markup (~> 1.7.1)
......
......@@ -271,6 +271,7 @@ class ApplicationController < ActionController::Base
headers['X-XSS-Protection'] = '1; mode=block'
headers['X-UA-Compatible'] = 'IE=edge'
headers['X-Content-Type-Options'] = 'nosniff'
headers[Gitlab::Metrics::RequestsRackMiddleware::FEATURE_CATEGORY_HEADER] = feature_category
end
def default_cache_headers
......@@ -465,7 +466,8 @@ class ApplicationController < ActionController::Base
user: -> { auth_user if strong_memoized?(:auth_user) },
project: -> { @project if @project&.persisted? },
namespace: -> { @group if @group&.persisted? },
caller_id: full_action_name) do
caller_id: caller_id,
feature_category: feature_category) do
yield
ensure
@current_context = Labkit::Context.current.to_h
......@@ -547,10 +549,14 @@ class ApplicationController < ActionController::Base
end
end
def full_action_name
def caller_id
"#{self.class.name}##{action_name}"
end
def feature_category
self.class.feature_category_for_action(action_name).to_s
end
def required_signup_info
return unless current_user
return unless current_user.role_required?
......
......@@ -29,6 +29,7 @@ DELETE /admin/sidekiq/queues/:queue_name
| `root_namespace` | string | no | The root namespace of the project |
| `subscription_plan` | string | no | The subscription plan of the root namespace (GitLab.com only) |
| `caller_id` | string | no | The endpoint or background job that schedule the job (for example: `ProjectsController#create`, `/api/:version/projects/:id`, `PostReceive`) |
| `feature_category` | string | no | The feature category of the background job (for example: `issue_tracking` or `code_review`) |
At least one attribute, other than `queue_name`, is required.
......
......@@ -114,6 +114,11 @@ input AdminSidekiqQueuesDeleteJobsInput {
"""
clientMutationId: String
"""
Delete jobs matching feature_category in the context metadata
"""
featureCategory: String
"""
Delete jobs matching project in the context metadata
"""
......
......@@ -381,6 +381,16 @@
},
"defaultValue": null
},
{
"name": "featureCategory",
"description": "Delete jobs matching feature_category in the context metadata",
"type": {
"kind": "SCALAR",
"name": "String",
"ofType": null
},
"defaultValue": null
},
{
"name": "queueName",
"description": "The name of the queue to delete jobs from",
......@@ -12,7 +12,8 @@ module Gitlab
Attribute.new(:namespace, Namespace),
Attribute.new(:user, User),
Attribute.new(:caller_id, String),
Attribute.new(:related_class, String)
Attribute.new(:related_class, String),
Attribute.new(:feature_category, String)
].freeze
def self.with_context(args, &block)
......@@ -45,6 +46,7 @@ module Gitlab
hash[:root_namespace] = -> { root_namespace_path } if include_namespace?
hash[:caller_id] = caller_id if set_values.include?(:caller_id)
hash[:related_class] = related_class if set_values.include?(:related_class)
hash[:feature_category] = feature_category if set_values.include?(:feature_category)
end
end
......
......@@ -15,6 +15,9 @@ module Gitlab
HEALTH_ENDPOINT = /^\/-\/(liveness|readiness|health|metrics)\/?$/.freeze
FEATURE_CATEGORY_HEADER = 'X-Gitlab-Feature-Category'
FEATURE_CATEGORY_DEFAULT = 'unknown'
def initialize(app)
@app = app
end
......@@ -50,11 +53,13 @@ module Gitlab
started = Time.now.to_f
health_endpoint = health_endpoint?(env['PATH_INFO'])
status = 'undefined'
feature_category = nil
begin
status, headers, body = @app.call(env)
elapsed = Time.now.to_f - started
feature_category = headers&.fetch(FEATURE_CATEGORY_HEADER, nil)
unless health_endpoint
RequestsRackMiddleware.http_request_duration_seconds.observe({ method: method, status: status.to_s }, elapsed)
......@@ -66,9 +71,9 @@ module Gitlab
raise
ensure
if health_endpoint
RequestsRackMiddleware.http_health_requests_total.increment(method: method, status: status)
RequestsRackMiddleware.http_health_requests_total.increment(status: status, method: method)
else
RequestsRackMiddleware.http_request_total.increment(method: method, status: status)
RequestsRackMiddleware.http_request_total.increment(status: status, method: method, feature_category: feature_category || FEATURE_CATEGORY_DEFAULT)
end
end
end
......
......@@ -12,8 +12,10 @@ module Gitlab
# This is not a worker we know about, perhaps from a gem
return yield unless worker_class.respond_to?(:get_worker_context)
# Use the context defined on the class level as a base context
wrap_in_optional_context(worker_class.get_worker_context, &block)
Gitlab::ApplicationContext.with_context(feature_category: worker_class.get_feature_category.to_s) do
# Use the context defined on the class level as the more specific context
wrap_in_optional_context(worker_class.get_worker_context, &block)
end
end
end
end
......
......@@ -844,6 +844,8 @@ RSpec.describe ApplicationController do
describe '#set_current_context' do
controller(described_class) do
feature_category :issue_tracking
def index
Labkit::Context.with_context do |context|
render json: context.to_h
......@@ -893,6 +895,12 @@ RSpec.describe ApplicationController do
expect(json_response['meta.caller_id']).to eq('AnonymousController#index')
end
it 'sets the feature_category as defined in the controller' do
get :index, format: :json
expect(json_response['meta.feature_category']).to eq('issue_tracking')
end
it 'assigns the context to a variable for logging' do
get :index, format: :json
......
......@@ -22,7 +22,7 @@ RSpec.describe Gitlab::Metrics::RequestsRackMiddleware do
end
it 'increments requests count' do
expect(described_class).to receive_message_chain(:http_request_total, :increment).with(method: 'get', status: 200)
expect(described_class).to receive_message_chain(:http_request_total, :increment).with(method: 'get', status: 200, feature_category: 'unknown')
subject.call(env)
end
......@@ -68,7 +68,7 @@ RSpec.describe Gitlab::Metrics::RequestsRackMiddleware do
end
it 'increments overall counter rather than health endpoint counter' do
expect(described_class).to receive_message_chain(:http_request_total, :increment).with(method: 'get', status: 200)
expect(described_class).to receive_message_chain(:http_request_total, :increment).with(method: 'get', status: 200, feature_category: 'unknown')
expect(described_class).not_to receive(:http_health_requests_total)
subject.call(env)
......@@ -101,7 +101,7 @@ RSpec.describe Gitlab::Metrics::RequestsRackMiddleware do
end
it 'increments requests count' do
expect(described_class).to receive_message_chain(:http_request_total, :increment).with(method: 'get', status: 'undefined')
expect(described_class).to receive_message_chain(:http_request_total, :increment).with(method: 'get', status: 'undefined', feature_category: 'unknown')
expect { subject.call(env) }.to raise_error(StandardError)
end
......@@ -113,6 +113,27 @@ RSpec.describe Gitlab::Metrics::RequestsRackMiddleware do
end
end
context 'when a feature category header is present' do
before do
allow(app).to receive(:call).and_return([200, { described_class::FEATURE_CATEGORY_HEADER => 'issue_tracking' }, nil])
end
it 'adds the feature category to the labels for http_request_total' do
expect(described_class).to receive_message_chain(:http_request_total, :increment).with(method: 'get', status: 200, feature_category: 'issue_tracking')
subject.call(env)
end
it 'does not record a feature category for health check endpoints' do
env['PATH_INFO'] = '/-/liveness'
expect(described_class).to receive_message_chain(:http_health_requests_total, :increment).with(method: 'get', status: 200)
expect(described_class).not_to receive(:http_request_total)
subject.call(env)
end
end
describe '.initialize_http_request_duration_seconds' do
it "sets labels" do
expected_labels = []
......
......@@ -14,6 +14,7 @@ RSpec.describe Gitlab::SidekiqMiddleware::WorkerContext::Server do
include ApplicationWorker
feature_category :foo
worker_context user: nil
def perform(identifier, *args)
......@@ -56,6 +57,12 @@ RSpec.describe Gitlab::SidekiqMiddleware::WorkerContext::Server do
expect(TestWorker.contexts['identifier'].keys).not_to include('meta.user')
end
it 'takes the feature category from the worker' do
TestWorker.perform_async('identifier', 1)
expect(TestWorker.contexts['identifier']).to include('meta.feature_category' => 'foo')
end
it "doesn't fail for unknown workers" do
expect { OtherWorker.perform_async }.not_to raise_error
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