Commit 314a36ab authored by Heinrich Lee Yu's avatar Heinrich Lee Yu

Merge branch...

Merge branch 'qmnguyen0711/1223-make-it-possible-to-define-custom-request-duration-thresholds-in-the-rails-application' into 'master'

Make it possible to define custom request target duration

See merge request gitlab-org/gitlab!69877
parents bffc89f3 5c52eddb
...@@ -21,7 +21,7 @@ class ApplicationController < ActionController::Base ...@@ -21,7 +21,7 @@ class ApplicationController < ActionController::Base
include Impersonation include Impersonation
include Gitlab::Logging::CloudflareHelper include Gitlab::Logging::CloudflareHelper
include Gitlab::Utils::StrongMemoize include Gitlab::Utils::StrongMemoize
include ::Gitlab::WithFeatureCategory include ::Gitlab::EndpointAttributes
include FlocOptOut include FlocOptOut
before_action :authenticate_user!, except: [:route_not_found] before_action :authenticate_user!, except: [:route_not_found]
...@@ -135,6 +135,14 @@ class ApplicationController < ActionController::Base ...@@ -135,6 +135,14 @@ class ApplicationController < ActionController::Base
end end
end end
def feature_category
self.class.feature_category_for_action(action_name).to_s
end
def target_duration
self.class.target_duration_for_action(action_name)
end
protected protected
def workhorse_excluded_content_types def workhorse_excluded_content_types
...@@ -547,10 +555,6 @@ class ApplicationController < ActionController::Base ...@@ -547,10 +555,6 @@ class ApplicationController < ActionController::Base
auth_user if strong_memoized?(:auth_user) auth_user if strong_memoized?(:auth_user)
end end
def feature_category
self.class.feature_category_for_action(action_name).to_s
end
def required_signup_info def required_signup_info
return unless current_user return unless current_user
return unless current_user.role_required? return unless current_user.role_required?
......
...@@ -5,7 +5,7 @@ module Projects ...@@ -5,7 +5,7 @@ module Projects
class ConfigurationController < Projects::ApplicationController class ConfigurationController < Projects::ApplicationController
include SecurityAndCompliancePermissions include SecurityAndCompliancePermissions
feature_category :static_application_security_testing feature_category :static_application_security_testing, [:show]
def show def show
render_403 unless can?(current_user, :read_security_configuration, project) render_403 unless can?(current_user, :read_security_configuration, project)
......
...@@ -23,6 +23,7 @@ class SearchController < ApplicationController ...@@ -23,6 +23,7 @@ class SearchController < ApplicationController
layout 'search' layout 'search'
feature_category :global_search feature_category :global_search
target_duration :very_fast, [:opensearch]
def show def show
@project = search_service.project @project = search_service.project
......
...@@ -2,13 +2,17 @@ ...@@ -2,13 +2,17 @@
module API module API
class Base < Grape::API::Instance # rubocop:disable API/Base class Base < Grape::API::Instance # rubocop:disable API/Base
include ::Gitlab::WithFeatureCategory include ::Gitlab::EndpointAttributes
class << self class << self
def feature_category_for_app(app) def feature_category_for_app(app)
feature_category_for_action(path_for_app(app)) feature_category_for_action(path_for_app(app))
end end
def target_duration_for_app(app)
target_duration_for_action(path_for_app(app))
end
def path_for_app(app) def path_for_app(app)
normalize_path(app.namespace, app.options[:path].first) normalize_path(app.namespace, app.options[:path].first)
end end
...@@ -18,8 +22,13 @@ module API ...@@ -18,8 +22,13 @@ module API
end end
def route(methods, paths = ['/'], route_options = {}, &block) def route(methods, paths = ['/'], route_options = {}, &block)
actions = Array(paths).map { |path| normalize_path(namespace, path) }
if category = route_options.delete(:feature_category) if category = route_options.delete(:feature_category)
feature_category(category, Array(paths).map { |path| normalize_path(namespace, path) }) feature_category(category, actions)
end
if target = route_options.delete(:target_duration)
target_duration(target, actions)
end end
super super
......
...@@ -164,7 +164,7 @@ module API ...@@ -164,7 +164,7 @@ module API
# #
# Check whether an SSH key is known to GitLab # Check whether an SSH key is known to GitLab
# #
get '/authorized_keys', feature_category: :source_code_management do get '/authorized_keys', feature_category: :source_code_management, target_duration: :very_fast do
fingerprint = Gitlab::InsecureKeyFingerprint.new(params.fetch(:key)).fingerprint_sha256 fingerprint = Gitlab::InsecureKeyFingerprint.new(params.fetch(:key)).fingerprint_sha256
key = Key.find_by_fingerprint_sha256(fingerprint) key = Key.find_by_fingerprint_sha256(fingerprint)
......
...@@ -30,7 +30,7 @@ module API ...@@ -30,7 +30,7 @@ module API
end end
desc 'Get a list of features' desc 'Get a list of features'
get 'client/features' do get 'client/features', target_duration: :fast do
present :version, 1 present :version, 1
present :features, feature_flags, with: ::API::Entities::UnleashFeature present :features, feature_flags, with: ::API::Entities::UnleashFeature
end end
......
...@@ -787,7 +787,7 @@ module API ...@@ -787,7 +787,7 @@ module API
use :pagination use :pagination
optional :state, type: String, default: 'all', values: %w[all active inactive], desc: 'Filters (all|active|inactive) impersonation_tokens' optional :state, type: String, default: 'all', values: %w[all active inactive], desc: 'Filters (all|active|inactive) impersonation_tokens'
end end
get feature_category :authentication_and_authorization do get feature_category: :authentication_and_authorization do
present paginate(finder(declared_params(include_missing: false)).execute), with: Entities::ImpersonationToken present paginate(finder(declared_params(include_missing: false)).execute), with: Entities::ImpersonationToken
end end
......
# frozen_string_literal: true # frozen_string_literal: true
module Gitlab module Gitlab
module WithFeatureCategory module EndpointAttributes
extend ActiveSupport::Concern extend ActiveSupport::Concern
include Gitlab::ClassAttributes include Gitlab::ClassAttributes
DEFAULT_TARGET_DURATION = Config::TARGET_DURATIONS.fetch(:medium)
class_methods do class_methods do
def feature_category(category, actions = []) def feature_category(category, actions = [])
feature_category_configuration[category] ||= [] endpoint_attributes.set(actions, feature_category: category)
feature_category_configuration[category] += actions.map(&:to_s)
validate_config!(feature_category_configuration)
end end
def feature_category_for_action(action) def feature_category_for_action(action)
category_config = feature_category_configuration.find do |_, actions| category = endpoint_attributes.attribute_for_action(action, :feature_category)
actions.empty? || actions.include?(action) category || superclass_feature_category_for_action(action)
end
category_config&.first || superclass_feature_category_for_action(action)
end end
private def target_duration(duration, actions = [])
endpoint_attributes.set(actions, target_duration: duration)
def validate_config!(config) end
empty = config.find { |_, actions| actions.empty? }
duplicate_actions = config.values.map(&:uniq).flatten.group_by(&:itself).select { |_, v| v.count > 1 }.keys
if config.length > 1 && empty
raise ArgumentError, "#{empty.first} is defined for all actions, but other categories are set"
end
if duplicate_actions.any? def target_duration_for_action(action)
raise ArgumentError, "Actions have multiple feature categories: #{duplicate_actions.join(', ')}" duration = endpoint_attributes.attribute_for_action(action, :target_duration)
end duration || superclass_target_duration_for_action(action) || DEFAULT_TARGET_DURATION
end end
def feature_category_configuration private
class_attributes[:feature_category_config] ||= {}
def endpoint_attributes
class_attributes[:endpoint_attributes_config] ||= Config.new
end end
def superclass_feature_category_for_action(action) def superclass_feature_category_for_action(action)
...@@ -45,6 +37,12 @@ module Gitlab ...@@ -45,6 +37,12 @@ module Gitlab
superclass.feature_category_for_action(action) superclass.feature_category_for_action(action)
end end
def superclass_target_duration_for_action(action)
return unless superclass.respond_to?(:target_duration_for_action)
superclass.target_duration_for_action(action)
end
end end
end end
end end
# frozen_string_literal: true
module Gitlab
module EndpointAttributes
class Config
Duration = Struct.new(:name, :duration)
TARGET_DURATIONS = [
Duration.new(:very_fast, 0.25),
Duration.new(:fast, 0.5),
Duration.new(:medium, 1),
Duration.new(:slow, 5)
].index_by(&:name).freeze
SUPPORTED_ATTRIBUTES = %i[feature_category target_duration].freeze
def initialize
@default_attributes = {}
@action_attributes = {}
end
def defined_actions
@action_attributes.keys
end
def set(actions, attributes)
sanitize_attributes!(attributes)
if actions.empty?
conflicted = conflicted_attributes(attributes, @default_attributes)
raise ArgumentError, "Attributes already defined: #{conflicted.join(", ")}" if conflicted.present?
@default_attributes.merge!(attributes)
else
set_attributes_for_actions(actions, attributes)
end
nil
end
def attribute_for_action(action, attribute_name)
value = @action_attributes.dig(action.to_s, attribute_name) || @default_attributes[attribute_name]
# Translate target duration to a representative struct
value = TARGET_DURATIONS[value] if attribute_name == :target_duration
value
end
private
def sanitize_attributes!(attributes)
unsupported_attributes = (attributes.keys - SUPPORTED_ATTRIBUTES).present?
raise ArgumentError, "Attributes not supported: #{unsupported_attributes.join(", ")}" if unsupported_attributes
if attributes[:target_duration].present? && !TARGET_DURATIONS.key?(attributes[:target_duration])
raise ArgumentError, "Target duration not supported: #{attributes[:target_duration]}"
end
end
def set_attributes_for_actions(actions, attributes)
conflicted = conflicted_attributes(attributes, @default_attributes)
if conflicted.present?
raise ArgumentError, "#{conflicted.join(", ")} are already defined for all actions, but re-defined for #{actions.join(", ")}"
end
actions.each do |action|
action = action.to_s
if @action_attributes[action].blank?
@action_attributes[action] = attributes.dup
else
conflicted = conflicted_attributes(attributes, @action_attributes[action])
raise ArgumentError, "Attributes re-defined for action #{action}: #{conflicted.join(", ")}" if conflicted.present?
@action_attributes[action].merge!(attributes)
end
end
end
def conflicted_attributes(attributes, existing_attributes)
attributes.keys.filter { |attr| existing_attributes[attr].present? && existing_attributes[attr] != attributes[attr] }
end
end
end
end
...@@ -79,7 +79,7 @@ module Gitlab ...@@ -79,7 +79,7 @@ module Gitlab
if !health_endpoint && ::Gitlab::Metrics.record_duration_for_status?(status) if !health_endpoint && ::Gitlab::Metrics.record_duration_for_status?(status)
self.class.http_request_duration_seconds.observe({ method: method }, elapsed) self.class.http_request_duration_seconds.observe({ method: method }, elapsed)
record_apdex_if_needed(elapsed) record_apdex_if_needed(env, elapsed)
end end
[status, headers, body] [status, headers, body]
...@@ -113,14 +113,12 @@ module Gitlab ...@@ -113,14 +113,12 @@ module Gitlab
::Gitlab::ApplicationContext.current_context_attribute(:caller_id) ::Gitlab::ApplicationContext.current_context_attribute(:caller_id)
end end
def record_apdex_if_needed(elapsed) def record_apdex_if_needed(env, elapsed)
return unless Gitlab::Metrics::RailsSlis.request_apdex_counters_enabled? return unless Gitlab::Metrics::RailsSlis.request_apdex_counters_enabled?
Gitlab::Metrics::RailsSlis.request_apdex.increment( Gitlab::Metrics::RailsSlis.request_apdex.increment(
labels: labels_from_context, labels: labels_from_context,
# hardcoded 1s here will be replaced by a per-endpoint value. success: satisfactory?(env, elapsed)
# https://gitlab.com/gitlab-com/gl-infra/scalability/-/issues/1223
success: elapsed < 1
) )
end end
...@@ -130,6 +128,19 @@ module Gitlab ...@@ -130,6 +128,19 @@ module Gitlab
endpoint_id: endpoint_id.presence || ENDPOINT_MISSING endpoint_id: endpoint_id.presence || ENDPOINT_MISSING
} }
end end
def satisfactory?(env, elapsed)
target =
if env['api.endpoint'].present?
env['api.endpoint'].options[:for].try(:target_duration_for_app, env['api.endpoint'])
elsif env['action_controller.instance'].present? && env['action_controller.instance'].respond_to?(:target_duration)
env['action_controller.instance'].target_duration
end
target ||= Gitlab::EndpointAttributes::DEFAULT_TARGET_DURATION
elapsed < target.duration
end
end end
end end
end end
...@@ -21,7 +21,7 @@ module Gitlab ...@@ -21,7 +21,7 @@ module Gitlab
next if route_info[:controller].blank? || route_info[:action].blank? next if route_info[:controller].blank? || route_info[:action].blank?
controller = constantize_controller(route_info[:controller]) controller = constantize_controller(route_info[:controller])
next unless controller&.include?(::Gitlab::WithFeatureCategory) next unless controller&.include?(::Gitlab::EndpointAttributes)
next if controller == ApplicationController next if controller == ApplicationController
next if controller == Devise::UnlocksController next if controller == Devise::UnlocksController
......
# frozen_string_literal: true # frozen_string_literal: true
require 'spec_helper' require 'spec_helper'
RSpec.describe "Every controller" do RSpec.describe "Every controller" do
context "feature categories" do context "feature categories" do
let_it_be(:feature_categories) do let_it_be(:feature_categories) do
...@@ -65,9 +64,6 @@ RSpec.describe "Every controller" do ...@@ -65,9 +64,6 @@ RSpec.describe "Every controller" do
end end
def actions_defined_in_feature_category_config(controller) def actions_defined_in_feature_category_config(controller)
controller.send(:class_attributes)[:feature_category_config] controller.send(:class_attributes)[:endpoint_attributes_config].defined_actions
.values
.flatten
.map(&:to_s)
end end
end end
# frozen_string_literal: true
require 'spec_helper'
# rubocop:disable Rails/HttpPositionalArguments
RSpec.describe ::API::Base do
let(:app_hello) do
route = double(:route, request_method: 'GET', path: '/:version/test/hello')
double(:endpoint, route: route, options: { for: api_handler, path: ["hello"] }, namespace: '/test')
end
let(:app_hi) do
route = double(:route, request_method: 'GET', path: '/:version//test/hi')
double(:endpoint, route: route, options: { for: api_handler, path: ["hi"] }, namespace: '/test')
end
describe 'declare feature categories at handler level for all routes' do
let(:api_handler) do
Class.new(described_class) do
feature_category :foo
target_duration :fast
namespace '/test' do
get 'hello' do
end
post 'hi' do
end
end
end
end
it 'sets feature category for a particular route', :aggregate_failures do
expect(api_handler.feature_category_for_app(app_hello)).to eq(:foo)
expect(api_handler.feature_category_for_app(app_hi)).to eq(:foo)
end
it 'sets target duration for a particular route', :aggregate_failures do
expect(api_handler.target_duration_for_app(app_hello)).to be_a_target_duration(:fast)
expect(api_handler.target_duration_for_app(app_hi)).to be_a_target_duration(:fast)
end
end
describe 'declare feature categories at route level' do
let(:api_handler) do
Class.new(described_class) do
namespace '/test' do
get 'hello', feature_category: :foo, target_duration: :slow do
end
post 'hi', feature_category: :bar, target_duration: :fast do
end
end
end
end
it 'sets feature category for a particular route', :aggregate_failures do
expect(api_handler.feature_category_for_app(app_hello)).to eq(:foo)
expect(api_handler.feature_category_for_app(app_hi)).to eq(:bar)
end
it 'sets target duration for a particular route', :aggregate_failures do
expect(api_handler.target_duration_for_app(app_hello)).to be_a_target_duration(:slow)
expect(api_handler.target_duration_for_app(app_hi)).to be_a_target_duration(:fast)
end
end
describe 'declare feature categories at both handler level and route level' do
let(:api_handler) do
Class.new(described_class) do
feature_category :foo, ['/test/hello']
target_duration :slow, ['/test/hello']
namespace '/test' do
get 'hello' do
end
post 'hi', feature_category: :bar, target_duration: :fast do
end
end
end
end
it 'sets feature category for a particular route', :aggregate_failures do
expect(api_handler.feature_category_for_app(app_hello)).to eq(:foo)
expect(api_handler.feature_category_for_app(app_hi)).to eq(:bar)
end
it 'sets target duration for a particular route', :aggregate_failures do
expect(api_handler.target_duration_for_app(app_hello)).to be_a_target_duration(:slow)
expect(api_handler.target_duration_for_app(app_hi)).to be_a_target_duration(:fast)
end
end
end
# rubocop:enable Rails/HttpPositionalArguments
# frozen_string_literal: true
require 'fast_spec_helper'
require_relative "../../../lib/gitlab/endpoint_attributes"
RSpec.describe Gitlab::EndpointAttributes do
let(:base_controller) do
Class.new do
include ::Gitlab::EndpointAttributes
end
end
let(:controller) do
Class.new(base_controller) do
feature_category :foo, %w(update edit)
feature_category :bar, %w(index show)
feature_category :quux, %w(destroy)
target_duration :fast, %w(do_a)
target_duration :slow, %w(do_b do_c)
end
end
let(:subclass) do
Class.new(controller) do
feature_category :baz, %w(subclass_index)
target_duration :very_fast, %w(superclass_do_something)
end
end
it "is nil when nothing was defined" do
expect(base_controller.feature_category_for_action("hello")).to be_nil
end
it "returns the expected category", :aggregate_failures do
expect(controller.feature_category_for_action("update")).to eq(:foo)
expect(controller.feature_category_for_action("index")).to eq(:bar)
expect(controller.feature_category_for_action("destroy")).to eq(:quux)
end
it "falls back to medium when target_duration was not defined", :aggregate_failures do
expect(base_controller.target_duration_for_action("hello")).to be_a_target_duration(:medium)
expect(controller.target_duration_for_action("update")).to be_a_target_duration(:medium)
expect(controller.target_duration_for_action("index")).to be_a_target_duration(:medium)
expect(controller.target_duration_for_action("destroy")).to be_a_target_duration(:medium)
end
it "returns the expected target_duration", :aggregate_failures do
expect(controller.target_duration_for_action("do_a")).to be_a_target_duration(:fast)
expect(controller.target_duration_for_action("do_b")).to be_a_target_duration(:slow)
expect(controller.target_duration_for_action("do_c")).to be_a_target_duration(:slow)
end
it "returns feature category for an implied action if not specify actions" do
klass = Class.new(base_controller) do
feature_category :foo
end
expect(klass.feature_category_for_action("index")).to eq(:foo)
expect(klass.feature_category_for_action("show")).to eq(:foo)
end
it "returns expected duration for an implied action if not specify actions" do
klass = Class.new(base_controller) do
feature_category :foo
target_duration :slow
end
expect(klass.target_duration_for_action("index")).to be_a_target_duration(:slow)
expect(klass.target_duration_for_action("show")).to be_a_target_duration(:slow)
end
it "returns the expected category for categories defined in subclasses" do
expect(subclass.feature_category_for_action("subclass_index")).to eq(:baz)
end
it "falls back to superclass's feature category" do
expect(subclass.feature_category_for_action("update")).to eq(:foo)
end
it "returns the expected target_duration for categories defined in subclasses" do
expect(subclass.target_duration_for_action("superclass_do_something")).to be_a_target_duration(:very_fast)
end
it "falls back to superclass's expected duration" do
expect(subclass.target_duration_for_action("do_a")).to be_a_target_duration(:fast)
end
it "raises an error when defining for the controller and for individual actions" do
expect do
Class.new(base_controller) do
feature_category :hello
feature_category :goodbye, [:world]
end
end.to raise_error(ArgumentError, "feature_category are already defined for all actions, but re-defined for world")
end
it "raises an error when multiple calls define the same action" do
expect do
Class.new(base_controller) do
feature_category :hello, [:world]
feature_category :goodbye, ["world"]
end
end.to raise_error(ArgumentError, "Attributes re-defined for action world: feature_category")
end
it "raises an error when multiple calls define the same action" do
expect do
Class.new(base_controller) do
target_duration :fast, [:world]
target_duration :slow, ["world"]
end
end.to raise_error(ArgumentError, "Attributes re-defined for action world: target_duration")
end
it "does not raise an error when multiple calls define the same action and configs" do
expect do
Class.new(base_controller) do
feature_category :hello, [:world]
feature_category :hello, ["world"]
target_duration :fast, [:moon]
target_duration :fast, ["moon"]
end
end.not_to raise_error
end
it "raises an error if the expected duration is not supported" do
expect do
Class.new(base_controller) do
target_duration :super_slow
end
end.to raise_error(ArgumentError, "Target duration not supported: super_slow")
end
end
...@@ -71,7 +71,7 @@ RSpec.describe Gitlab::Metrics::RequestsRackMiddleware, :aggregate_failures do ...@@ -71,7 +71,7 @@ RSpec.describe Gitlab::Metrics::RequestsRackMiddleware, :aggregate_failures do
expect(described_class).not_to receive(:http_health_requests_total) expect(described_class).not_to receive(:http_health_requests_total)
expect(described_class) expect(described_class)
.to receive_message_chain(:http_request_duration_seconds, :observe) .to receive_message_chain(:http_request_duration_seconds, :observe)
.with({ method: 'get' }, a_positive_execution_time) .with({ method: 'get' }, a_positive_execution_time)
subject.call(env) subject.call(env)
end end
...@@ -161,6 +161,165 @@ RSpec.describe Gitlab::Metrics::RequestsRackMiddleware, :aggregate_failures do ...@@ -161,6 +161,165 @@ RSpec.describe Gitlab::Metrics::RequestsRackMiddleware, :aggregate_failures do
subject.call(env) subject.call(env)
end end
end end
context 'SLI satisfactory' do
where(:target, :duration, :success) do
[
[:very_fast, 0.1, true],
[:very_fast, 0.25, false],
[:very_fast, 0.3, false],
[:fast, 0.3, true],
[:fast, 0.5, false],
[:fast, 0.6, false],
[:medium, 0.6, true],
[:medium, 1.0, false],
[:medium, 1.2, false],
[:slow, 4.5, true],
[:slow, 5.0, false],
[:slow, 6, false]
]
end
with_them do
context 'Grape API handler having expected duration setup' do
let(:api_handler) do
target_duration = target # target is a DSL provided by Rspec, it's invisible to the inner block
Class.new(::API::Base) do
feature_category :hello_world, ['/projects/:id/archive']
target_duration target_duration, ['/projects/:id/archive']
end
end
let(:endpoint) do
route = double(:route, request_method: 'GET', path: '/:version/projects/:id/archive(.:format)')
double(:endpoint, route: route,
options: { for: api_handler, path: [":id/archive"] },
namespace: "/projects")
end
let(:env) { { 'api.endpoint' => endpoint, 'REQUEST_METHOD' => 'GET' } }
before do
::Gitlab::ApplicationContext.push(feature_category: 'hello_world', caller_id: 'GET /projects/:id/archive')
allow(Gitlab::Metrics::System).to receive(:monotonic_time).and_return(100, 100 + duration)
end
it "captures SLI metrics" do
expect(Gitlab::Metrics::RailsSlis.request_apdex).to receive(:increment).with(
labels: { feature_category: 'hello_world', endpoint_id: 'GET /projects/:id/archive' },
success: success
)
subject.call(env)
end
end
context 'Rails controller having expected duration setup' do
let(:controller) do
target_duration = target # target is a DSL provided by Rspec, it's invisible to the inner block
Class.new(ApplicationController) do
feature_category :hello_world, [:index, :show]
target_duration target_duration, [:index, :show]
end
end
let(:env) do
controller_instance = controller.new
controller_instance.action_name = :index
{ 'action_controller.instance' => controller_instance, 'REQUEST_METHOD' => 'GET' }
end
before do
::Gitlab::ApplicationContext.push(feature_category: 'hello_world', caller_id: 'AnonymousController#index')
allow(Gitlab::Metrics::System).to receive(:monotonic_time).and_return(100, 100 + duration)
end
it "captures SLI metrics" do
expect(Gitlab::Metrics::RailsSlis.request_apdex).to receive(:increment).with(
labels: { feature_category: 'hello_world', endpoint_id: 'AnonymousController#index' },
success: success
)
subject.call(env)
end
end
end
context 'Grape API without expected duration' do
let(:endpoint) do
route = double(:route, request_method: 'GET', path: '/:version/projects/:id/archive(.:format)')
double(:endpoint, route: route,
options: { for: api_handler, path: [":id/archive"] },
namespace: "/projects")
end
let(:env) { { 'api.endpoint' => endpoint, 'REQUEST_METHOD' => 'GET' } }
let(:api_handler) { Class.new(::API::Base) }
it "falls back request's expectation to medium (1 second)" do
allow(Gitlab::Metrics::System).to receive(:monotonic_time).and_return(100, 100.9)
expect(Gitlab::Metrics::RailsSlis.request_apdex).to receive(:increment).with(
labels: { feature_category: 'unknown', endpoint_id: 'unknown' },
success: true
)
subject.call(env)
allow(Gitlab::Metrics::System).to receive(:monotonic_time).and_return(100, 101)
expect(Gitlab::Metrics::RailsSlis.request_apdex).to receive(:increment).with(
labels: { feature_category: 'unknown', endpoint_id: 'unknown' },
success: false
)
subject.call(env)
end
end
context 'Rails controller without expected duration' do
let(:controller) { Class.new(ApplicationController) }
let(:env) do
controller_instance = controller.new
controller_instance.action_name = :index
{ 'action_controller.instance' => controller_instance, 'REQUEST_METHOD' => 'GET' }
end
it "falls back request's expectation to medium (1 second)" do
allow(Gitlab::Metrics::System).to receive(:monotonic_time).and_return(100, 100.9)
expect(Gitlab::Metrics::RailsSlis.request_apdex).to receive(:increment).with(
labels: { feature_category: 'unknown', endpoint_id: 'unknown' },
success: true
)
subject.call(env)
allow(Gitlab::Metrics::System).to receive(:monotonic_time).and_return(100, 101)
expect(Gitlab::Metrics::RailsSlis.request_apdex).to receive(:increment).with(
labels: { feature_category: 'unknown', endpoint_id: 'unknown' },
success: false
)
subject.call(env)
end
end
context 'An unknown request' do
let(:env) do
{ 'REQUEST_METHOD' => 'GET' }
end
it "falls back request's expectation to medium (1 second)" do
allow(Gitlab::Metrics::System).to receive(:monotonic_time).and_return(100, 100.9)
expect(Gitlab::Metrics::RailsSlis.request_apdex).to receive(:increment).with(
labels: { feature_category: 'unknown', endpoint_id: 'unknown' },
success: true
)
subject.call(env)
allow(Gitlab::Metrics::System).to receive(:monotonic_time).and_return(100, 101)
expect(Gitlab::Metrics::RailsSlis.request_apdex).to receive(:increment).with(
labels: { feature_category: 'unknown', endpoint_id: 'unknown' },
success: false
)
subject.call(env)
end
end
end
end end
describe '.initialize_metrics', :prometheus do describe '.initialize_metrics', :prometheus do
......
...@@ -6,7 +6,7 @@ RSpec.describe Gitlab::RequestEndpoints do ...@@ -6,7 +6,7 @@ RSpec.describe Gitlab::RequestEndpoints do
it 'selects all feature API classes' do it 'selects all feature API classes' do
api_classes = described_class.all_api_endpoints.map { |route| route.app.options[:for] } api_classes = described_class.all_api_endpoints.map { |route| route.app.options[:for] }
expect(api_classes).to all(include(Gitlab::WithFeatureCategory)) expect(api_classes).to all(include(Gitlab::EndpointAttributes))
end end
end end
...@@ -16,7 +16,7 @@ RSpec.describe Gitlab::RequestEndpoints do ...@@ -16,7 +16,7 @@ RSpec.describe Gitlab::RequestEndpoints do
controller_classes = all_controller_actions.map(&:first) controller_classes = all_controller_actions.map(&:first)
all_actions = all_controller_actions.map(&:last) all_actions = all_controller_actions.map(&:last)
expect(controller_classes).to all(include(Gitlab::WithFeatureCategory)) expect(controller_classes).to all(include(Gitlab::EndpointAttributes))
expect(controller_classes).not_to include(ApplicationController, Devise::UnlocksController) expect(controller_classes).not_to include(ApplicationController, Devise::UnlocksController)
expect(all_actions).to all(be_a(String)) expect(all_actions).to all(be_a(String))
end end
......
# frozen_string_literal: true
require 'fast_spec_helper'
require_relative "../../../lib/gitlab/with_feature_category"
RSpec.describe Gitlab::WithFeatureCategory do
describe ".feature_category_for_action" do
let(:base_controller) do
Class.new do
include ::Gitlab::WithFeatureCategory
end
end
let(:controller) do
Class.new(base_controller) do
feature_category :foo, %w(update edit)
feature_category :bar, %w(index show)
feature_category :quux, %w(destroy)
end
end
let(:subclass) do
Class.new(controller) do
feature_category :baz, %w(subclass_index)
end
end
it "is nil when nothing was defined" do
expect(base_controller.feature_category_for_action("hello")).to be_nil
end
it "returns the expected category", :aggregate_failures do
expect(controller.feature_category_for_action("update")).to eq(:foo)
expect(controller.feature_category_for_action("index")).to eq(:bar)
expect(controller.feature_category_for_action("destroy")).to eq(:quux)
end
it "returns the expected category for categories defined in subclasses" do
expect(subclass.feature_category_for_action("subclass_index")).to eq(:baz)
end
it "raises an error when defining for the controller and for individual actions" do
expect do
Class.new(base_controller) do
feature_category :hello
feature_category :goodbye, [:world]
end
end.to raise_error(ArgumentError, "hello is defined for all actions, but other categories are set")
end
it "raises an error when multiple calls define the same action" do
expect do
Class.new(base_controller) do
feature_category :hello, [:world]
feature_category :goodbye, ["world"]
end
end.to raise_error(ArgumentError, "Actions have multiple feature categories: world")
end
it "does not raise an error when multiple calls define the same action and feature category" do
expect do
Class.new(base_controller) do
feature_category :hello, [:world]
feature_category :hello, ["world"]
end
end.not_to raise_error
end
end
end
# frozen_string_literal: true
RSpec::Matchers.define :be_a_target_duration do |expected|
match do |actual|
actual.is_a?(::Gitlab::EndpointAttributes::Config::Duration) && actual.name == expected
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