diff options
author | GitLab Bot <gitlab-bot@gitlab.com> | 2021-09-21 21:11:18 +0300 |
---|---|---|
committer | GitLab Bot <gitlab-bot@gitlab.com> | 2021-09-21 21:11:18 +0300 |
commit | 79f759cc144c7020942f09762154c6758ee1d275 (patch) | |
tree | 5d159ea257006957a34e854949662ad67a8c3532 | |
parent | 9ef26edc4ae561ba072dee3900fef4408e227b48 (diff) |
Add latest changes from gitlab-org/gitlab@master
91 files changed, 1492 insertions, 932 deletions
diff --git a/app/assets/javascripts/header_search/components/app.vue b/app/assets/javascripts/header_search/components/app.vue index 580c27f6c61..c6590fd8eb3 100644 --- a/app/assets/javascripts/header_search/components/app.vue +++ b/app/assets/javascripts/header_search/components/app.vue @@ -3,6 +3,7 @@ import { GlSearchBoxByType, GlOutsideDirective as Outside } from '@gitlab/ui'; import { mapState, mapActions, mapGetters } from 'vuex'; import { visitUrl } from '~/lib/utils/url_utility'; import { __ } from '~/locale'; +import HeaderSearchAutocompleteItems from './header_search_autocomplete_items.vue'; import HeaderSearchDefaultItems from './header_search_default_items.vue'; import HeaderSearchScopedItems from './header_search_scoped_items.vue'; @@ -16,6 +17,7 @@ export default { GlSearchBoxByType, HeaderSearchDefaultItems, HeaderSearchScopedItems, + HeaderSearchAutocompleteItems, }, data() { return { @@ -41,7 +43,7 @@ export default { }, }, methods: { - ...mapActions(['setSearch']), + ...mapActions(['setSearch', 'fetchAutocompleteOptions']), openDropdown() { this.showDropdown = true; }, @@ -51,6 +53,13 @@ export default { submitSearch() { return visitUrl(this.searchQuery); }, + getAutocompleteOptions(searchTerm) { + if (!searchTerm) { + return; + } + + this.fetchAutocompleteOptions(); + }, }, }; </script> @@ -64,18 +73,20 @@ export default { :placeholder="$options.i18n.searchPlaceholder" @focus="openDropdown" @click="openDropdown" + @input="getAutocompleteOptions" @keydown.enter="submitSearch" @keydown.esc="closeDropdown" /> <div v-if="showSearchDropdown" data-testid="header-search-dropdown-menu" - class="header-search-dropdown-menu gl-overflow-y-auto gl-absolute gl-left-0 gl-z-index-1 gl-w-full gl-bg-white gl-border-1 gl-rounded-base gl-border-solid gl-border-gray-200 gl-shadow-x0-y2-b4-s0" + class="header-search-dropdown-menu gl-absolute gl-w-full gl-bg-white gl-border-1 gl-rounded-base gl-border-solid gl-border-gray-200 gl-shadow-x0-y2-b4-s0" > <div class="header-search-dropdown-content gl-overflow-y-auto gl-py-2"> <header-search-default-items v-if="showDefaultItems" /> <template v-else> <header-search-scoped-items /> + <header-search-autocomplete-items /> </template> </div> </div> diff --git a/app/assets/javascripts/header_search/components/header_search_autocomplete_items.vue b/app/assets/javascripts/header_search/components/header_search_autocomplete_items.vue new file mode 100644 index 00000000000..9bea2b280f7 --- /dev/null +++ b/app/assets/javascripts/header_search/components/header_search_autocomplete_items.vue @@ -0,0 +1,74 @@ +<script> +import { + GlDropdownItem, + GlDropdownSectionHeader, + GlDropdownDivider, + GlAvatar, + GlLoadingIcon, + GlSafeHtmlDirective as SafeHtml, +} from '@gitlab/ui'; +import { mapState, mapGetters } from 'vuex'; +import highlight from '~/lib/utils/highlight'; +import { GROUPS_CATEGORY, PROJECTS_CATEGORY, LARGE_AVATAR_PX, SMALL_AVATAR_PX } from '../constants'; + +export default { + name: 'HeaderSearchAutocompleteItems', + components: { + GlDropdownItem, + GlDropdownSectionHeader, + GlDropdownDivider, + GlAvatar, + GlLoadingIcon, + }, + directives: { + SafeHtml, + }, + computed: { + ...mapState(['search', 'loading']), + ...mapGetters(['autocompleteGroupedSearchOptions']), + }, + methods: { + highlightedName(val) { + return highlight(val, this.search); + }, + avatarSize(data) { + if (data.category === GROUPS_CATEGORY || data.category === PROJECTS_CATEGORY) { + return LARGE_AVATAR_PX; + } + + return SMALL_AVATAR_PX; + }, + }, +}; +</script> + +<template> + <div> + <template v-if="!loading"> + <div v-for="option in autocompleteGroupedSearchOptions" :key="option.category"> + <gl-dropdown-divider /> + <gl-dropdown-section-header>{{ option.category }}</gl-dropdown-section-header> + <gl-dropdown-item + v-for="(data, index) in option.data" + :id="`autocomplete-${option.category}-${index}`" + :key="index" + tabindex="-1" + :href="data.url" + > + <div class="gl-display-flex gl-align-items-center"> + <gl-avatar + v-if="data.avatar_url !== undefined" + :src="data.avatar_url" + :entity-id="data.id" + :entity-name="data.label" + :size="avatarSize(data)" + shape="square" + /> + <span v-safe-html="highlightedName(data.label)"></span> + </div> + </gl-dropdown-item> + </div> + </template> + <gl-loading-icon v-else size="lg" class="my-4" /> + </div> +</template> diff --git a/app/assets/javascripts/header_search/constants.js b/app/assets/javascripts/header_search/constants.js index fffed7bcbdb..2fadb1bd1ee 100644 --- a/app/assets/javascripts/header_search/constants.js +++ b/app/assets/javascripts/header_search/constants.js @@ -15,3 +15,11 @@ export const MSG_IN_ALL_GITLAB = __('in all GitLab'); export const MSG_IN_GROUP = __('in group'); export const MSG_IN_PROJECT = __('in project'); + +export const GROUPS_CATEGORY = 'Groups'; + +export const PROJECTS_CATEGORY = 'Projects'; + +export const LARGE_AVATAR_PX = 32; + +export const SMALL_AVATAR_PX = 16; diff --git a/app/assets/javascripts/header_search/index.js b/app/assets/javascripts/header_search/index.js index 2d37ee137fc..d7e21f55ea5 100644 --- a/app/assets/javascripts/header_search/index.js +++ b/app/assets/javascripts/header_search/index.js @@ -12,13 +12,13 @@ export const initHeaderSearchApp = () => { return false; } - const { searchPath, issuesPath, mrPath } = el.dataset; + const { searchPath, issuesPath, mrPath, autocompletePath } = el.dataset; let { searchContext } = el.dataset; searchContext = JSON.parse(searchContext); return new Vue({ el, - store: createStore({ searchPath, issuesPath, mrPath, searchContext }), + store: createStore({ searchPath, issuesPath, mrPath, autocompletePath, searchContext }), render(createElement) { return createElement(HeaderSearchApp); }, diff --git a/app/assets/javascripts/header_search/store/actions.js b/app/assets/javascripts/header_search/store/actions.js index 841aee04029..2c3b1bd4c0f 100644 --- a/app/assets/javascripts/header_search/store/actions.js +++ b/app/assets/javascripts/header_search/store/actions.js @@ -1,5 +1,19 @@ +import createFlash from '~/flash'; +import axios from '~/lib/utils/axios_utils'; +import { __ } from '~/locale'; import * as types from './mutation_types'; +export const fetchAutocompleteOptions = ({ commit, getters }) => { + commit(types.REQUEST_AUTOCOMPLETE); + return axios + .get(getters.autocompleteQuery) + .then(({ data }) => commit(types.RECEIVE_AUTOCOMPLETE_SUCCESS, data)) + .catch(() => { + commit(types.RECEIVE_AUTOCOMPLETE_ERROR); + createFlash({ message: __('There was an error fetching search autocomplete suggestions') }); + }); +}; + export const setSearch = ({ commit }, value) => { commit(types.SET_SEARCH, value); }; diff --git a/app/assets/javascripts/header_search/store/getters.js b/app/assets/javascripts/header_search/store/getters.js index d1e1fc8ad73..3f4e231ca55 100644 --- a/app/assets/javascripts/header_search/store/getters.js +++ b/app/assets/javascripts/header_search/store/getters.js @@ -23,6 +23,16 @@ export const searchQuery = (state) => { return `${state.searchPath}?${objectToQuery(query)}`; }; +export const autocompleteQuery = (state) => { + const query = { + term: state.search, + project_id: state.searchContext.project?.id, + project_ref: state.searchContext.ref, + }; + + return `${state.autocompletePath}?${objectToQuery(query)}`; +}; + export const scopedIssuesPath = (state) => { return ( state.searchContext.project_metadata?.issues_path || @@ -133,3 +143,25 @@ export const scopedSearchOptions = (state, getters) => { return options; }; + +export const autocompleteGroupedSearchOptions = (state) => { + const groupedOptions = {}; + const results = []; + + state.autocompleteOptions.forEach((option) => { + const category = groupedOptions[option.category]; + + if (category) { + category.data.push(option); + } else { + groupedOptions[option.category] = { + category: option.category, + data: [option], + }; + + results.push(groupedOptions[option.category]); + } + }); + + return results; +}; diff --git a/app/assets/javascripts/header_search/store/index.js b/app/assets/javascripts/header_search/store/index.js index 8b74f8662a5..06cca4be8a7 100644 --- a/app/assets/javascripts/header_search/store/index.js +++ b/app/assets/javascripts/header_search/store/index.js @@ -7,11 +7,17 @@ import createState from './state'; Vue.use(Vuex); -export const getStoreConfig = ({ searchPath, issuesPath, mrPath, searchContext }) => ({ +export const getStoreConfig = ({ + searchPath, + issuesPath, + mrPath, + autocompletePath, + searchContext, +}) => ({ actions, getters, mutations, - state: createState({ searchPath, issuesPath, mrPath, searchContext }), + state: createState({ searchPath, issuesPath, mrPath, autocompletePath, searchContext }), }); const createStore = (config) => new Vuex.Store(getStoreConfig(config)); diff --git a/app/assets/javascripts/header_search/store/mutation_types.js b/app/assets/javascripts/header_search/store/mutation_types.js index 0bc94ae055f..a2358621ce6 100644 --- a/app/assets/javascripts/header_search/store/mutation_types.js +++ b/app/assets/javascripts/header_search/store/mutation_types.js @@ -1 +1,5 @@ +export const REQUEST_AUTOCOMPLETE = 'REQUEST_AUTOCOMPLETE'; +export const RECEIVE_AUTOCOMPLETE_SUCCESS = 'RECEIVE_AUTOCOMPLETE_SUCCESS'; +export const RECEIVE_AUTOCOMPLETE_ERROR = 'RECEIVE_AUTOCOMPLETE_ERROR'; + export const SET_SEARCH = 'SET_SEARCH'; diff --git a/app/assets/javascripts/header_search/store/mutations.js b/app/assets/javascripts/header_search/store/mutations.js index 5b1438929d4..175b5406540 100644 --- a/app/assets/javascripts/header_search/store/mutations.js +++ b/app/assets/javascripts/header_search/store/mutations.js @@ -1,6 +1,18 @@ import * as types from './mutation_types'; export default { + [types.REQUEST_AUTOCOMPLETE](state) { + state.loading = true; + state.autocompleteOptions = []; + }, + [types.RECEIVE_AUTOCOMPLETE_SUCCESS](state, data) { + state.loading = false; + state.autocompleteOptions = data; + }, + [types.RECEIVE_AUTOCOMPLETE_ERROR](state) { + state.loading = false; + state.autocompleteOptions = []; + }, [types.SET_SEARCH](state, value) { state.search = value; }, diff --git a/app/assets/javascripts/header_search/store/state.js b/app/assets/javascripts/header_search/store/state.js index fb2c83dbbe3..3d4073f0583 100644 --- a/app/assets/javascripts/header_search/store/state.js +++ b/app/assets/javascripts/header_search/store/state.js @@ -1,8 +1,11 @@ -const createState = ({ searchPath, issuesPath, mrPath, searchContext }) => ({ +const createState = ({ searchPath, issuesPath, mrPath, autocompletePath, searchContext }) => ({ searchPath, issuesPath, mrPath, + autocompletePath, searchContext, search: '', + autocompleteOptions: [], + loading: false, }); export default createState; diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index a83458f3260..4742760ea10 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -70,6 +70,10 @@ class ApplicationController < ActionController::Base # concerns due to caching private data. DEFAULT_GITLAB_CACHE_CONTROL = "#{ActionDispatch::Http::Cache::Response::DEFAULT_CACHE_CONTROL}, no-store" + def self.endpoint_id_for_action(action_name) + "#{self.name}##{action_name}" + end + rescue_from Encoding::CompatibilityError do |exception| log_exception(exception) render "errors/encoding", layout: "errors", status: :internal_server_error @@ -457,7 +461,7 @@ class ApplicationController < ActionController::Base user: -> { context_user }, project: -> { @project if @project&.persisted? }, namespace: -> { @group if @group&.persisted? }, - caller_id: caller_id, + caller_id: self.class.endpoint_id_for_action(action_name), remote_ip: request.ip, feature_category: feature_category ) @@ -543,10 +547,6 @@ class ApplicationController < ActionController::Base auth_user if strong_memoized?(:auth_user) end - def caller_id - "#{self.class.name}##{action_name}" - end - def feature_category self.class.feature_category_for_action(action_name).to_s end diff --git a/app/controllers/metrics_controller.rb b/app/controllers/metrics_controller.rb index a0c307a0a03..d3dea2ce159 100644 --- a/app/controllers/metrics_controller.rb +++ b/app/controllers/metrics_controller.rb @@ -7,6 +7,7 @@ class MetricsController < ActionController::Base def index response = if Gitlab::Metrics.prometheus_metrics_enabled? + Gitlab::Metrics::RailsSlis.initialize_request_slis_if_needed! metrics_service.metrics_text else help_page = help_page_url('administration/monitoring/prometheus/gitlab_metrics', diff --git a/app/services/projects/update_service.rb b/app/services/projects/update_service.rb index b87564fcaef..a32e80af4b1 100644 --- a/app/services/projects/update_service.rb +++ b/app/services/projects/update_service.rb @@ -105,7 +105,7 @@ module Projects end update_pages_config if changing_pages_related_config? - update_pending_builds if shared_runners_toggled? + update_pending_builds if runners_settings_toggled? end def after_rename_service(project) @@ -181,13 +181,36 @@ module Projects end def update_pending_builds - update_params = { instance_runners_enabled: project.shared_runners_enabled } + update_params = { + instance_runners_enabled: project.shared_runners_enabled?, + namespace_traversal_ids: group_runner_traversal_ids + } - ::Ci::UpdatePendingBuildService.new(project, update_params).execute + ::Ci::UpdatePendingBuildService + .new(project, update_params) + .execute end - def shared_runners_toggled? - project.previous_changes.include?('shared_runners_enabled') + def shared_runners_settings_toggled? + project.previous_changes.include?(:shared_runners_enabled) + end + + def group_runners_settings_toggled? + return false unless project.ci_cd_settings.present? + + project.ci_cd_settings.previous_changes.include?(:group_runners_enabled) + end + + def runners_settings_toggled? + shared_runners_settings_toggled? || group_runners_settings_toggled? + end + + def group_runner_traversal_ids + if project.group_runners_enabled? + project.namespace.traversal_ids + else + [] + end end end end diff --git a/app/views/layouts/header/_default.html.haml b/app/views/layouts/header/_default.html.haml index 3e7155b2c0e..8d28823bfa4 100644 --- a/app/views/layouts/header/_default.html.haml +++ b/app/views/layouts/header/_default.html.haml @@ -34,7 +34,8 @@ #js-header-search.header-search{ data: { 'search-context' => search_context.to_json, 'search-path' => search_path, 'issues-path' => issues_dashboard_path, - 'mr-path' => merge_requests_dashboard_path } } + 'mr-path' => merge_requests_dashboard_path, + 'autocomplete-path' => search_autocomplete_path } } %input{ type: "text", placeholder: _('Search or jump to...'), class: 'form-control gl-form-input' } - else = render 'layouts/search' diff --git a/app/workers/authorized_project_update/user_refresh_from_replica_worker.rb b/app/workers/authorized_project_update/user_refresh_from_replica_worker.rb index 48e3d0837c7..daebb23baae 100644 --- a/app/workers/authorized_project_update/user_refresh_from_replica_worker.rb +++ b/app/workers/authorized_project_update/user_refresh_from_replica_worker.rb @@ -30,8 +30,6 @@ module AuthorizedProjectUpdate # does not allow us to deduplicate these jobs. # https://gitlab.com/gitlab-org/gitlab/-/issues/325291 def use_replica_if_available(&block) - return yield unless ::Gitlab::Database::LoadBalancing.enable? - ::Gitlab::Database::LoadBalancing::Session.current.use_replicas_for_read_queries(&block) end diff --git a/app/workers/container_expiration_policy_worker.rb b/app/workers/container_expiration_policy_worker.rb index a791fe5d350..5fcbd74ddad 100644 --- a/app/workers/container_expiration_policy_worker.rb +++ b/app/workers/container_expiration_policy_worker.rb @@ -45,8 +45,6 @@ class ContainerExpirationPolicyWorker # rubocop:disable Scalability/IdempotentWo # not perfomed with a delay # https://gitlab.com/gitlab-org/gitlab/-/merge_requests/63635#note_603771207 def use_replica_if_available(&blk) - return yield unless ::Gitlab::Database::LoadBalancing.enable? - ::Gitlab::Database::LoadBalancing::Session.current.use_replicas_for_read_queries(&blk) end diff --git a/config/feature_flags/development/request_apdex_counters.yml b/config/feature_flags/development/request_apdex_counters.yml new file mode 100644 index 00000000000..07d6cb7ac5e --- /dev/null +++ b/config/feature_flags/development/request_apdex_counters.yml @@ -0,0 +1,8 @@ +--- +name: request_apdex_counters +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/69154 +rollout_issue_url: https://gitlab.com/gitlab-com/gl-infra/scalability/-/issues/1099 +milestone: '14.3' +type: development +group: team::Scalability +default_enabled: false diff --git a/config/initializers/7_prometheus_metrics.rb b/config/initializers/7_prometheus_metrics.rb index d2d546a5438..587d393fd77 100644 --- a/config/initializers/7_prometheus_metrics.rb +++ b/config/initializers/7_prometheus_metrics.rb @@ -48,7 +48,7 @@ if !Rails.env.test? && Gitlab::Metrics.prometheus_metrics_enabled? Gitlab::Metrics.gauge(:deployments, 'GitLab Version', {}, :max).set({ version: Gitlab::VERSION, revision: Gitlab.revision }, 1) - unless Gitlab::Runtime.sidekiq? + if Gitlab::Runtime.web_server? Gitlab::Metrics::RequestsRackMiddleware.initialize_metrics end diff --git a/config/initializers/action_cable.rb b/config/initializers/action_cable.rb index 16d29f5910f..a7ef5cc332c 100644 --- a/config/initializers/action_cable.rb +++ b/config/initializers/action_cable.rb @@ -19,4 +19,4 @@ ActionCable::SubscriptionAdapter::Redis.redis_connector = lambda do |config| end Gitlab::ActionCable::RequestStoreCallbacks.install -Gitlab::Database::LoadBalancing::ActionCableCallbacks.install if Gitlab::Database::LoadBalancing.enable? +Gitlab::Database::LoadBalancing::ActionCableCallbacks.install diff --git a/config/initializers/load_balancing.rb b/config/initializers/load_balancing.rb index 2b58ae0f642..29700b37c7d 100644 --- a/config/initializers/load_balancing.rb +++ b/config/initializers/load_balancing.rb @@ -2,27 +2,51 @@ ActiveRecord::Base.singleton_class.attr_accessor :load_balancing_proxy -if Gitlab::Database::LoadBalancing.enable? - Gitlab::Database.main.disable_prepared_statements +Gitlab::Database.main.disable_prepared_statements - Gitlab::Application.configure do |config| - config.middleware.use(Gitlab::Database::LoadBalancing::RackMiddleware) - end +Gitlab::Application.configure do |config| + config.middleware.use(Gitlab::Database::LoadBalancing::RackMiddleware) +end + +# This hijacks the "connection" method to ensure both +# `ActiveRecord::Base.connection` and all models use the same load +# balancing proxy. +ActiveRecord::Base.singleton_class.prepend(Gitlab::Database::LoadBalancing::ActiveRecordProxy) + +# The load balancer needs to be configured immediately, and re-configured after +# forking. This ensures queries that run before forking use the load balancer, +# and queries running after a fork don't run into any errors when using dead +# database connections. +# +# See https://gitlab.com/gitlab-org/gitlab/-/merge_requests/63485 for more +# information. +setup = proc do + lb = Gitlab::Database::LoadBalancing::LoadBalancer.new( + Gitlab::Database::LoadBalancing.configuration, + primary_only: !Gitlab::Database::LoadBalancing.enable_replicas? + ) - # This hijacks the "connection" method to ensure both - # `ActiveRecord::Base.connection` and all models use the same load - # balancing proxy. - ActiveRecord::Base.singleton_class.prepend(Gitlab::Database::LoadBalancing::ActiveRecordProxy) + ActiveRecord::Base.load_balancing_proxy = + Gitlab::Database::LoadBalancing::ConnectionProxy.new(lb) - Gitlab::Database::LoadBalancing.configure_proxy + # Populate service discovery immediately if it is configured + Gitlab::Database::LoadBalancing.perform_service_discovery +end + +setup.call - # This needs to be executed after fork of clustered processes - Gitlab::Cluster::LifecycleEvents.on_worker_start do - # For Host-based LB, we need to re-connect as Rails discards connections on fork - Gitlab::Database::LoadBalancing.configure_proxy +# Database queries may be run before we fork, so we must set up the load +# balancer as early as possible. When we do fork, we need to make sure all the +# hosts are disconnected. +Gitlab::Cluster::LifecycleEvents.on_before_fork do + # When forking, we don't want to wait until the connections aren't in use any + # more, as this could delay the boot cycle. + Gitlab::Database::LoadBalancing.proxy.load_balancer.disconnect!(timeout: 0) +end - # Service discovery must be started after configuring the proxy, as service - # discovery depends on this. - Gitlab::Database::LoadBalancing.start_service_discovery - end +# Service discovery only needs to run in the worker processes, as the main one +# won't be running many (if any) database queries. +Gitlab::Cluster::LifecycleEvents.on_worker_start do + setup.call + Gitlab::Database::LoadBalancing.start_service_discovery end diff --git a/config/initializers/zz_metrics.rb b/config/initializers/zz_metrics.rb index 0fa67ea9f8c..8e69e1634f1 100644 --- a/config/initializers/zz_metrics.rb +++ b/config/initializers/zz_metrics.rb @@ -29,12 +29,8 @@ if Gitlab::Metrics.enabled? && !Rails.env.test? && !(Rails.env.development? && d Gitlab::Application.configure do |config| # We want to track certain metrics during the Load Balancing host resolving process. # Because of that, we need to have metrics code available earlier for Load Balancing. - if Gitlab::Database::LoadBalancing.enable? - config.middleware.insert_before Gitlab::Database::LoadBalancing::RackMiddleware, - Gitlab::Metrics::RackMiddleware - else - config.middleware.use(Gitlab::Metrics::RackMiddleware) - end + config.middleware.insert_before Gitlab::Database::LoadBalancing::RackMiddleware, + Gitlab::Metrics::RackMiddleware config.middleware.use(Gitlab::Middleware::RailsQueueDuration) config.middleware.use(Gitlab::Metrics::ElasticsearchRackMiddleware) diff --git a/doc/development/feature_categorization/index.md b/doc/development/feature_categorization/index.md index 07d1a981855..20325facc75 100644 --- a/doc/development/feature_categorization/index.md +++ b/doc/development/feature_categorization/index.md @@ -72,8 +72,8 @@ class SomeCrossCuttingConcernWorker end ``` -Workers marked as not owned workers will, when possible, use the -category of their caller (worker or HTTP endpoint) in metrics and logs. +When possible, workers marked as "not owned" use their caller's +category (worker or HTTP endpoint) in metrics and logs. For instance, `ReactiveCachingWorker` can have multiple feature categories in metrics and logs. diff --git a/doc/user/infrastructure/clusters/connect/index.md b/doc/user/infrastructure/clusters/connect/index.md index ada278292a9..472d9c0daf0 100644 --- a/doc/user/infrastructure/clusters/connect/index.md +++ b/doc/user/infrastructure/clusters/connect/index.md @@ -100,15 +100,15 @@ group, or instance, open the cluster's page according to the **Project-level clusters:** -1. Go to your project's homepage. +1. On the top bar, select **Menu > Projects** and find your project. 1. On the left sidebar, select **Infrastructure > Kubernetes clusters**. -**Group-level clusters:** +**[Group-level clusters](../../../group/clusters/index.md):** -1. Go to your group's homepage. +1. On the top bar, select **Menu > Groups** and find your group. 1. On the left sidebar, select **Kubernetes**. -**Instance-level clusters:** **(FREE SELF)** +**[Instance-level clusters](../../../instance/clusters/index.md):** 1. On the top bar, select **Menu > Admin**. 1. On the left sidebar, select **Kubernetes**. diff --git a/doc/user/instance/clusters/index.md b/doc/user/instance/clusters/index.md index 20cd30b6110..09975ba8cd8 100644 --- a/doc/user/instance/clusters/index.md +++ b/doc/user/instance/clusters/index.md @@ -4,7 +4,7 @@ group: unassigned info: To determine the technical writer assigned to the Stage/Group associated with this page, see https://about.gitlab.com/handbook/engineering/ux/technical-writing/#assignments --- -# Instance-level Kubernetes clusters +# Instance-level Kubernetes clusters **(FREE SELF)** > [Introduced](https://gitlab.com/gitlab-org/gitlab-foss/-/issues/39840) in GitLab 11.11. diff --git a/lib/api/base.rb b/lib/api/base.rb index 33e47c18fcd..fef82b01cd4 100644 --- a/lib/api/base.rb +++ b/lib/api/base.rb @@ -13,6 +13,10 @@ module API normalize_path(app.namespace, app.options[:path].first) end + def endpoint_id_for_route(route) + "#{route.request_method} #{route.origin}" + end + def route(methods, paths = ['/'], route_options = {}, &block) if category = route_options.delete(:feature_category) feature_category(category, Array(paths).map { |path| normalize_path(namespace, path) }) diff --git a/lib/api/helpers/common_helpers.rb b/lib/api/helpers/common_helpers.rb index 02942820982..855648f2ef0 100644 --- a/lib/api/helpers/common_helpers.rb +++ b/lib/api/helpers/common_helpers.rb @@ -34,7 +34,7 @@ module API end def endpoint_id - "#{request.request_method} #{route.origin}" + ::API::Base.endpoint_id_for_route(route) end end end diff --git a/lib/gitlab/checks/matching_merge_request.rb b/lib/gitlab/checks/matching_merge_request.rb index e37cbc0442b..c86779c7a37 100644 --- a/lib/gitlab/checks/matching_merge_request.rb +++ b/lib/gitlab/checks/matching_merge_request.rb @@ -13,23 +13,21 @@ module Gitlab end def match? - if ::Gitlab::Database::LoadBalancing.enable? - # When a user merges a merge request, the following sequence happens: - # - # 1. Sidekiq: MergeService runs and updates the merge request in a locked state. - # 2. Gitaly: The UserMergeBranch RPC runs. - # 3. Gitaly (gitaly-ruby): This RPC calls the pre-receive hook. - # 4. Rails: This hook makes an API request to /api/v4/internal/allowed. - # 5. Rails: This API check does a SQL query for locked merge - # requests with a matching SHA. - # - # Since steps 1 and 5 will happen on different database - # sessions, replication lag could erroneously cause step 5 to - # report no matching merge requests. To avoid this, we check - # the write location to ensure the replica can make this query. - track_session_metrics do - ::Gitlab::Database::LoadBalancing::Sticking.select_valid_host(:project, @project.id) - end + # When a user merges a merge request, the following sequence happens: + # + # 1. Sidekiq: MergeService runs and updates the merge request in a locked state. + # 2. Gitaly: The UserMergeBranch RPC runs. + # 3. Gitaly (gitaly-ruby): This RPC calls the pre-receive hook. + # 4. Rails: This hook makes an API request to /api/v4/internal/allowed. + # 5. Rails: This API check does a SQL query for locked merge + # requests with a matching SHA. + # + # Since steps 1 and 5 will happen on different database + # sessions, replication lag could erroneously cause step 5 to + # report no matching merge requests. To avoid this, we check + # the write location to ensure the replica can make this query. + track_session_metrics do + ::Gitlab::Database::LoadBalancing::Sticking.select_valid_host(:project, @project.id) end # rubocop: disable CodeReuse/ActiveRecord diff --git a/lib/gitlab/database/load_balancing.rb b/lib/gitlab/database/load_balancing.rb index bbfbf83222f..a5cfaf795f9 100644 --- a/lib/gitlab/database/load_balancing.rb +++ b/lib/gitlab/database/load_balancing.rb @@ -4,34 +4,22 @@ module Gitlab module Database module LoadBalancing # The exceptions raised for connection errors. - CONNECTION_ERRORS = if defined?(PG) - [ - PG::ConnectionBad, - PG::ConnectionDoesNotExist, - PG::ConnectionException, - PG::ConnectionFailure, - PG::UnableToSend, - # During a failover this error may be raised when - # writing to a primary. - PG::ReadOnlySqlTransaction - ].freeze - else - [].freeze - end - - ProxyNotConfiguredError = Class.new(StandardError) + CONNECTION_ERRORS = [ + PG::ConnectionBad, + PG::ConnectionDoesNotExist, + PG::ConnectionException, + PG::ConnectionFailure, + PG::UnableToSend, + # During a failover this error may be raised when + # writing to a primary. + PG::ReadOnlySqlTransaction, + # This error is raised when we can't connect to the database in the + # first place (e.g. it's offline or the hostname is incorrect). + ActiveRecord::ConnectionNotEstablished + ].freeze - # The connection proxy to use for load balancing (if enabled). def self.proxy - unless load_balancing_proxy = ActiveRecord::Base.load_balancing_proxy - Gitlab::ErrorTracking.track_exception( - ProxyNotConfiguredError.new( - "Attempting to access the database load balancing proxy, but it wasn't configured.\n" \ - "Did you forget to call '#{self.name}.configure_proxy'?" - )) - end - - load_balancing_proxy + ActiveRecord::Base.load_balancing_proxy end # Returns a Hash containing the load balancing configuration. @@ -39,8 +27,11 @@ module Gitlab @configuration ||= Configuration.for_model(ActiveRecord::Base) end - # Returns true if load balancing is to be enabled. - def self.enable? + # Returns `true` if the use of load balancing replicas should be enabled. + # + # This is disabled for Rake tasks to ensure e.g. database migrations + # always produce consistent results. + def self.enable_replicas? return false if Gitlab::Runtime.rake? configured? @@ -59,17 +50,12 @@ module Gitlab .start end - # Configures proxying of requests. - def self.configure_proxy - lb = LoadBalancer.new(configuration, primary_only: !enable?) - ActiveRecord::Base.load_balancing_proxy = ConnectionProxy.new(lb) + def self.perform_service_discovery + return unless configuration.service_discovery_enabled? - # Populate service discovery immediately if it is configured - if configuration.service_discovery_enabled? - ServiceDiscovery - .new(lb, **configuration.service_discovery) - .perform_service_discovery - end + ServiceDiscovery + .new(proxy.load_balancer, **configuration.service_discovery) + .perform_service_discovery end DB_ROLES = [ diff --git a/lib/gitlab/database/load_balancing/host.rb b/lib/gitlab/database/load_balancing/host.rb index acd7df0a263..bdbb80d6f31 100644 --- a/lib/gitlab/database/load_balancing/host.rb +++ b/lib/gitlab/database/load_balancing/host.rb @@ -9,19 +9,12 @@ module Gitlab delegate :connection, :release_connection, :enable_query_cache!, :disable_query_cache!, :query_cache_enabled, to: :pool - CONNECTION_ERRORS = - if defined?(PG) - [ - ActionView::Template::Error, - ActiveRecord::StatementInvalid, - PG::Error - ].freeze - else - [ - ActionView::Template::Error, - ActiveRecord::StatementInvalid - ].freeze - end + CONNECTION_ERRORS = [ + ActionView::Template::Error, + ActiveRecord::StatementInvalid, + ActiveRecord::ConnectionNotEstablished, + PG::Error + ].freeze # host - The address of the database. # load_balancer - The LoadBalancer that manages this Host. diff --git a/lib/gitlab/database/load_balancing/rack_middleware.rb b/lib/gitlab/database/load_balancing/rack_middleware.rb index f8a31622b7d..22871427bff 100644 --- a/lib/gitlab/database/load_balancing/rack_middleware.rb +++ b/lib/gitlab/database/load_balancing/rack_middleware.rb @@ -18,8 +18,6 @@ module Gitlab # namespace - The namespace to use for sticking. # id - The identifier to use for sticking. def self.stick_or_unstick(env, namespace, id) - return unless ::Gitlab::Database::LoadBalancing.enable? - ::Gitlab::Database::LoadBalancing::Sticking.unstick_or_continue_sticking(namespace, id) env[STICK_OBJECT] ||= Set.new diff --git a/lib/gitlab/database/load_balancing/sticking.rb b/lib/gitlab/database/load_balancing/sticking.rb index 20d42b9a694..8d55ade4880 100644 --- a/lib/gitlab/database/load_balancing/sticking.rb +++ b/lib/gitlab/database/load_balancing/sticking.rb @@ -22,8 +22,6 @@ module Gitlab # Sticks to the primary if a write was performed. def self.stick_if_necessary(namespace, id) - return unless LoadBalancing.enable? - stick(namespace, id) if Session.current.performed_write? end @@ -75,15 +73,11 @@ module Gitlab # Starts sticking to the primary for the given namespace and id, using # the latest WAL pointer from the primary. def self.stick(namespace, id) - return unless LoadBalancing.enable? - mark_primary_write_location(namespace, id) Session.current.use_primary! end def self.bulk_stick(namespace, ids) - return unless LoadBalancing.enable? - with_primary_write_location do |location| ids.each do |id| set_write_location_for(namespace, id, location) @@ -94,17 +88,7 @@ module Gitlab end def self.with_primary_write_location - return unless LoadBalancing.configured? - - # Load balancing could be enabled for the Web application server, - # but it's not activated for Sidekiq. We should update Redis with - # the write location just in case load balancing is being used. - location = - if LoadBalancing.enable? - load_balancer.primary_write_location - else - Gitlab::Database.main.get_write_location(ActiveRecord::Base.connection) - end + location = load_balancer.primary_write_location return if location.blank? diff --git a/lib/gitlab/import_export/json/streaming_serializer.rb b/lib/gitlab/import_export/json/streaming_serializer.rb index 9d28e1abeab..9ab8fa68d0e 100644 --- a/lib/gitlab/import_export/json/streaming_serializer.rb +++ b/lib/gitlab/import_export/json/streaming_serializer.rb @@ -171,7 +171,6 @@ module Gitlab def read_from_replica_if_available(&block) return yield unless ::Feature.enabled?(:load_balancing_for_export_workers, type: :development, default_enabled: :yaml) - return yield unless ::Gitlab::Database::LoadBalancing.enable? ::Gitlab::Database::LoadBalancing::Session.current.use_replicas_for_read_queries(&block) end diff --git a/lib/gitlab/metrics/exporter/web_exporter.rb b/lib/gitlab/metrics/exporter/web_exporter.rb index f378577f08e..c5fa1e545d7 100644 --- a/lib/gitlab/metrics/exporter/web_exporter.rb +++ b/lib/gitlab/metrics/exporter/web_exporter.rb @@ -15,6 +15,14 @@ module Gitlab end end + RailsMetricsInitializer = Struct.new(:app) do + def call(env) + Gitlab::Metrics::RailsSlis.initialize_request_slis_if_needed! + + app.call(env) + end + end + attr_reader :running # This exporter is always run on master process @@ -45,6 +53,15 @@ module Gitlab private + def rack_app + app = super + + Rack::Builder.app do + use RailsMetricsInitializer + run app + end + end + def start_working @running = true super diff --git a/lib/gitlab/metrics/rails_slis.rb b/lib/gitlab/metrics/rails_slis.rb new file mode 100644 index 00000000000..69e0c1e9fde --- /dev/null +++ b/lib/gitlab/metrics/rails_slis.rb @@ -0,0 +1,52 @@ +# frozen_string_literal: true + +module Gitlab + module Metrics + module RailsSlis + class << self + def request_apdex_counters_enabled? + Feature.enabled?(:request_apdex_counters) + end + + def initialize_request_slis_if_needed! + return unless request_apdex_counters_enabled? + return if Gitlab::Metrics::Sli.initialized?(:rails_request_apdex) + + Gitlab::Metrics::Sli.initialize_sli(:rails_request_apdex, possible_request_labels) + end + + def request_apdex + Gitlab::Metrics::Sli[:rails_request_apdex] + end + + private + + def possible_request_labels + possible_controller_labels + possible_api_labels + end + + def possible_api_labels + Gitlab::RequestEndpoints.all_api_endpoints.map do |route| + endpoint_id = API::Base.endpoint_id_for_route(route) + route_class = route.app.options[:for] + feature_category = route_class.feature_category_for_app(route.app) + + { + endpoint_id: endpoint_id, + feature_category: feature_category + } + end + end + + def possible_controller_labels + Gitlab::RequestEndpoints.all_controller_actions.map do |controller, action| + { + endpoint_id: controller.endpoint_id_for_action(action), + feature_category: controller.feature_category_for_action(action) + } + end + end + end + end + end +end diff --git a/lib/gitlab/metrics/requests_rack_middleware.rb b/lib/gitlab/metrics/requests_rack_middleware.rb index 6ba336d37cd..8ab94eee5db 100644 --- a/lib/gitlab/metrics/requests_rack_middleware.rb +++ b/lib/gitlab/metrics/requests_rack_middleware.rb @@ -16,6 +16,7 @@ module Gitlab HEALTH_ENDPOINT = %r{^/-/(liveness|readiness|health|metrics)/?$}.freeze FEATURE_CATEGORY_DEFAULT = 'unknown' + ENDPOINT_MISSING = 'unknown' # These were the top 5 categories at a point in time, chosen as a # reasonable default. If we initialize every category we'll end up @@ -77,6 +78,8 @@ module Gitlab if !health_endpoint && ::Gitlab::Metrics.record_duration_for_status?(status) self.class.http_request_duration_seconds.observe({ method: method }, elapsed) + + record_apdex_if_needed(elapsed) end [status, headers, body] @@ -105,6 +108,28 @@ module Gitlab def feature_category ::Gitlab::ApplicationContext.current_context_attribute(:feature_category) end + + def endpoint_id + ::Gitlab::ApplicationContext.current_context_attribute(:caller_id) + end + + def record_apdex_if_needed(elapsed) + return unless Gitlab::Metrics::RailsSlis.request_apdex_counters_enabled? + + Gitlab::Metrics::Sli[:rails_request_apdex].increment( + labels: labels_from_context, + # hardcoded 1s here will be replaced by a per-endpoint value. + # https://gitlab.com/gitlab-com/gl-infra/scalability/-/issues/1223 + success: elapsed < 1 + ) + end + + def labels_from_context + { + feature_category: feature_category.presence || FEATURE_CATEGORY_DEFAULT, + endpoint_id: endpoint_id.presence || ENDPOINT_MISSING + } + end end end end diff --git a/lib/gitlab/metrics/sli.rb b/lib/gitlab/metrics/sli.rb new file mode 100644 index 00000000000..de73db0755d --- /dev/null +++ b/lib/gitlab/metrics/sli.rb @@ -0,0 +1,83 @@ +# frozen_string_literal: true + +module Gitlab + module Metrics + class Sli + SliNotInitializedError = Class.new(StandardError) + + COUNTER_PREFIX = 'gitlab_sli' + + class << self + INITIALIZATION_MUTEX = Mutex.new + + def [](name) + known_slis[name] || initialize_sli(name, []) + end + + def initialize_sli(name, possible_label_combinations) + INITIALIZATION_MUTEX.synchronize do + sli = new(name) + sli.initialize_counters(possible_label_combinations) + known_slis[name] = sli + end + end + + def initialized?(name) + known_slis.key?(name) && known_slis[name].initialized? + end + + private + + def known_slis + @known_slis ||= {} + end + end + + attr_reader :name + + def initialize(name) + @name = name + @initialized_with_combinations = false + end + + def initialize_counters(possible_label_combinations) + @initialized_with_combinations = possible_label_combinations.any? + possible_label_combinations.each do |label_combination| + total_counter.get(label_combination) + success_counter.get(label_combination) + end + end + + def increment(labels:, success:) + total_counter.increment(labels) + success_counter.increment(labels) if success + end + + def initialized? + @initialized_with_combinations + end + + private + + def total_counter + prometheus.counter(total_counter_name.to_sym, "Total number of measurements for #{name}") + end + + def success_counter + prometheus.counter(success_counter_name.to_sym, "Number of successful measurements for #{name}") + end + + def total_counter_name + "#{COUNTER_PREFIX}:#{name}:total" + end + + def success_counter_name + "#{COUNTER_PREFIX}:#{name}:success_total" + end + + def prometheus + Gitlab::Metrics + end + end + end +end diff --git a/lib/gitlab/metrics/subscribers/active_record.rb b/lib/gitlab/metrics/subscribers/active_record.rb index 59b2f88041f..df0582149a9 100644 --- a/lib/gitlab/metrics/subscribers/active_record.rb +++ b/lib/gitlab/metrics/subscribers/active_record.rb @@ -47,13 +47,11 @@ module Gitlab buckets SQL_DURATION_BUCKET end - if ::Gitlab::Database::LoadBalancing.enable? - db_role = ::Gitlab::Database::LoadBalancing.db_role_for_connection(payload[:connection]) - return if db_role.blank? + db_role = ::Gitlab::Database::LoadBalancing.db_role_for_connection(payload[:connection]) + return if db_role.blank? - increment_db_role_counters(db_role, payload) - observe_db_role_duration(db_role, event) - end + increment_db_role_counters(db_role, payload) + observe_db_role_duration(db_role, event) end def self.db_counter_payload @@ -64,7 +62,7 @@ module Gitlab payload[key] = Gitlab::SafeRequestStore[key].to_i end - if ::Gitlab::SafeRequestStore.active? && ::Gitlab::Database::LoadBalancing.enable? + if ::Gitlab::SafeRequestStore.active? load_balancing_metric_counter_keys.each do |counter| payload[counter] = ::Gitlab::SafeRequestStore[counter].to_i end diff --git a/lib/gitlab/metrics/subscribers/load_balancing.rb b/lib/gitlab/metrics/subscribers/load_balancing.rb index 333fc63ebef..bd77e8c3c3f 100644 --- a/lib/gitlab/metrics/subscribers/load_balancing.rb +++ b/lib/gitlab/metrics/subscribers/load_balancing.rb @@ -10,7 +10,7 @@ module Gitlab LOG_COUNTERS = { true => :caught_up_replica_pick_ok, false => :caught_up_replica_pick_fail }.freeze def caught_up_replica_pick(event) - return unless Gitlab::SafeRequestStore.active? && ::Gitlab::Database::LoadBalancing.enable? + return unless Gitlab::SafeRequestStore.active? result = event.payload[:result] counter_name = counter(result) @@ -20,13 +20,13 @@ module Gitlab # we want to update Prometheus counter after the controller/action are set def web_transaction_completed(_event) - return unless Gitlab::SafeRequestStore.active? && ::Gitlab::Database::LoadBalancing.enable? + return unless Gitlab::SafeRequestStore.active? LOG_COUNTERS.keys.each { |result| increment_prometheus_for_result_label(result) } end def self.load_balancing_payload - return {} unless Gitlab::SafeRequestStore.active? && ::Gitlab::Database::LoadBalancing.enable? + return {} unless Gitlab::SafeRequestStore.active? {}.tap do |payload| LOG_COUNTERS.values.each do |counter| diff --git a/lib/gitlab/request_endpoints.rb b/lib/gitlab/request_endpoints.rb new file mode 100644 index 00000000000..04fdffe3d28 --- /dev/null +++ b/lib/gitlab/request_endpoints.rb @@ -0,0 +1,41 @@ +# frozen_string_literal: true + +module Gitlab + module RequestEndpoints + class << self + def all_api_endpoints + # This compile does not do anything if the routes were already built + # but if they weren't, the routes will be drawn and available for the rest of + # application. + API::API.compile! + API::API.routes.select { |route| route.app.options[:for] < API::Base } + end + + def all_controller_actions + # This will return tuples of all controller actions defined in the routes + # Only for controllers inheriting ApplicationController + # Excluding controllers from gems (OAuth, Sidekiq) + Rails.application.routes.routes.filter_map do |route| + route_info = route.required_defaults.presence + next unless route_info + next if route_info[:controller].blank? || route_info[:action].blank? + + controller = constantize_controller(route_info[:controller]) + next unless controller&.include?(::Gitlab::WithFeatureCategory) + next if controller == ApplicationController + next if controller == Devise::UnlocksController + + [controller, route_info[:action]] + end + end + + private + + def constantize_controller(name) + "#{name.camelize}Controller".constantize + rescue NameError + nil # some controllers, like the omniauth ones are dynamic + end + end + end +end diff --git a/lib/gitlab/sidekiq_middleware.rb b/lib/gitlab/sidekiq_middleware.rb index 5ad4d7dcbb3..c97b1632bf8 100644 --- a/lib/gitlab/sidekiq_middleware.rb +++ b/lib/gitlab/sidekiq_middleware.rb @@ -39,7 +39,7 @@ module Gitlab # DuplicateJobs::Server should be placed at the bottom, but before the SidekiqServerMiddleware, # so we can compare the latest WAL location against replica chain.add ::Gitlab::SidekiqMiddleware::DuplicateJobs::Server - chain.add ::Gitlab::Database::LoadBalancing::SidekiqServerMiddleware if load_balancing_enabled? + chain.add ::Gitlab::Database::LoadBalancing::SidekiqServerMiddleware end end @@ -52,7 +52,7 @@ module Gitlab chain.add ::Labkit::Middleware::Sidekiq::Client # Sidekiq Client Middleware should be placed before DuplicateJobs::Client middleware, # so we can store WAL location before we deduplicate the job. - chain.add ::Gitlab::Database::LoadBalancing::SidekiqClientMiddleware if load_balancing_enabled? + chain.add ::Gitlab::Database::LoadBalancing::SidekiqClientMiddleware chain.add ::Gitlab::SidekiqMiddleware::DuplicateJobs::Client chain.add ::Gitlab::SidekiqStatus::ClientMiddleware chain.add ::Gitlab::SidekiqMiddleware::AdminMode::Client @@ -61,10 +61,5 @@ module Gitlab chain.add ::Gitlab::SidekiqMiddleware::ClientMetrics end end - - def self.load_balancing_enabled? - ::Gitlab::Database::LoadBalancing.enable? - end - private_class_method :load_balancing_enabled? end end diff --git a/lib/gitlab/sidekiq_middleware/server_metrics.rb b/lib/gitlab/sidekiq_middleware/server_metrics.rb index 2d9767e0266..bea98403997 100644 --- a/lib/gitlab/sidekiq_middleware/server_metrics.rb +++ b/lib/gitlab/sidekiq_middleware/server_metrics.rb @@ -53,10 +53,7 @@ module Gitlab def initialize @metrics = self.class.metrics - - if ::Gitlab::Database::LoadBalancing.enable? - @metrics[:sidekiq_load_balancing_count] = ::Gitlab::Metrics.counter(:sidekiq_load_balancing_count, 'Sidekiq jobs with load balancing') - end + @metrics[:sidekiq_load_balancing_count] = ::Gitlab::Metrics.counter(:sidekiq_load_balancing_count, 'Sidekiq jobs with load balancing') end def call(worker, job, queue) @@ -128,8 +125,6 @@ module Gitlab private def with_load_balancing_settings(job) - return unless ::Gitlab::Database::LoadBalancing.enable? - keys = %w[load_balancing_strategy worker_data_consistency] return unless keys.all? { |k| job.key?(k) } diff --git a/lib/peek/views/active_record.rb b/lib/peek/views/active_record.rb index a3fe206c86f..6a41de8f0b0 100644 --- a/lib/peek/views/active_record.rb +++ b/lib/peek/views/active_record.rb @@ -44,10 +44,8 @@ module Peek count[item[:transaction]] += 1 end - if ::Gitlab::Database::LoadBalancing.enable? - count[item[:db_role]] ||= 0 - count[item[:db_role]] += 1 - end + count[item[:db_role]] ||= 0 + count[item[:db_role]] += 1 end def setup_subscribers @@ -72,8 +70,6 @@ module Peek end def db_role(data) - return unless ::Gitlab::Database::LoadBalancing.enable? - role = ::Gitlab::Database::LoadBalancing.db_role_for_connection(data[:connection]) || ::Gitlab::Database::LoadBalancing::ROLE_UNKNOWN diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 6bceb97029c..cd7703d2083 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -32837,9 +32837,15 @@ msgstr "" msgid "SuperSonics|You do not have an active subscription" msgstr "" +msgid "SuperSonics|You have successfully added a license that activates on %{date}. Please see the subscription history table below for more details." +msgstr "" + msgid "SuperSonics|You'll be charged for %{trueUpLinkStart}users over license%{trueUpLinkEnd} on a quarterly or annual basis, depending on the terms of your agreement." msgstr "" +msgid "SuperSonics|Your future dated license was successfully added" +msgstr "" + msgid "SuperSonics|Your subscription" msgstr "" @@ -34157,6 +34163,9 @@ msgstr "" msgid "There was an error fetching projects" msgstr "" +msgid "There was an error fetching search autocomplete suggestions" +msgstr "" + msgid "There was an error fetching stage total counts" msgstr "" diff --git a/package.json b/package.json index 2d29bcdbc4f..e7d1cc4a51c 100644 --- a/package.json +++ b/package.json @@ -68,7 +68,7 @@ "@tiptap/extension-bold": "^2.0.0-beta.15", "@tiptap/extension-bullet-list": "^2.0.0-beta.15", "@tiptap/extension-code": "^2.0.0-beta.16", - "@tiptap/extension-code-block-lowlight": "2.0.0-beta.38", + "@tiptap/extension-code-block-lowlight": "2.0.0-beta.39", "@tiptap/extension-document": "^2.0.0-beta.13", "@tiptap/extension-dropcursor": "^2.0.0-beta.19", "@tiptap/extension-gapcursor": "^2.0.0-beta.19", @@ -160,7 +160,7 @@ "portal-vue": "^2.1.7", "prismjs": "^1.21.0", "prosemirror-inputrules": "^1.1.3", - "prosemirror-markdown": "^1.5.2", + "prosemirror-markdown": "^1.6.0", "prosemirror-model": "^1.14.3", "prosemirror-state": "^1.3.4", "prosemirror-tables": "^1.1.1", @@ -244,7 +244,7 @@ "postcss": "^7.0.14", "prettier": "2.2.1", "prosemirror-schema-basic": "^1.1.2", - "prosemirror-schema-list": "^1.1.5", + "prosemirror-schema-list": "^1.1.6", "prosemirror-test-builder": "^1.0.4", "purgecss": "^4.0.3", "purgecss-from-html": "^4.0.3", diff --git a/spec/controllers/application_controller_spec.rb b/spec/controllers/application_controller_spec.rb index 218aa04dd3f..93491246e2c 100644 --- a/spec/controllers/application_controller_spec.rb +++ b/spec/controllers/application_controller_spec.rb @@ -967,6 +967,14 @@ RSpec.describe ApplicationController do end end + describe '.endpoint_id_for_action' do + controller(described_class) { } + + it 'returns an expected endpoint id' do + expect(controller.class.endpoint_id_for_action('hello')).to eq('AnonymousController#hello') + end + end + describe '#current_user' do controller(described_class) do def index; end diff --git a/spec/controllers/every_controller_spec.rb b/spec/controllers/every_controller_spec.rb index a1c377eff76..7d9d1d158b2 100644 --- a/spec/controllers/every_controller_spec.rb +++ b/spec/controllers/every_controller_spec.rb @@ -9,16 +9,7 @@ RSpec.describe "Every controller" do end let_it_be(:controller_actions) do - # This will return tuples of all controller actions defined in the routes - # Only for controllers inheriting ApplicationController - # Excluding controllers from gems (OAuth, Sidekiq) - Rails.application.routes.routes - .map { |route| route.required_defaults.presence } - .compact - .select { |route| route[:controller].present? && route[:action].present? } - .map { |route| [constantize_controller(route[:controller]), route[:action]] } - .select { |(controller, action)| controller&.include?(::Gitlab::WithFeatureCategory) } - .reject { |(controller, action)| controller == ApplicationController || controller == Devise::UnlocksController } + Gitlab::RequestEndpoints.all_controller_actions end let_it_be(:routes_without_category) do diff --git a/spec/controllers/metrics_controller_spec.rb b/spec/controllers/metrics_controller_spec.rb index 9fa90dde997..4f74af295c6 100644 --- a/spec/controllers/metrics_controller_spec.rb +++ b/spec/controllers/metrics_controller_spec.rb @@ -67,6 +67,12 @@ RSpec.describe MetricsController, :request_store do expect(response.body).to match(/^prometheus_counter 1$/) end + it 'initializes the rails request SLIs' do + expect(Gitlab::Metrics::RailsSlis).to receive(:initialize_request_slis_if_needed!).and_call_original + + get :index + end + context 'prometheus metrics are disabled' do before do allow(Gitlab::Metrics).to receive(:prometheus_metrics_enabled?).and_return(false) diff --git a/spec/frontend/header_search/components/app_spec.js b/spec/frontend/header_search/components/app_spec.js index 2cbcb73ce5b..2ea2693a978 100644 --- a/spec/frontend/header_search/components/app_spec.js +++ b/spec/frontend/header_search/components/app_spec.js @@ -3,6 +3,7 @@ import Vue from 'vue'; import Vuex from 'vuex'; import { shallowMountExtended } from 'helpers/vue_test_utils_helper'; import HeaderSearchApp from '~/header_search/components/app.vue'; +import HeaderSearchAutocompleteItems from '~/header_search/components/header_search_autocomplete_items.vue'; import HeaderSearchDefaultItems from '~/header_search/components/header_search_default_items.vue'; import HeaderSearchScopedItems from '~/header_search/components/header_search_scoped_items.vue'; import { ENTER_KEY, ESC_KEY } from '~/lib/utils/keys'; @@ -20,6 +21,7 @@ describe('HeaderSearchApp', () => { const actionSpies = { setSearch: jest.fn(), + fetchAutocompleteOptions: jest.fn(), }; const createComponent = (initialState) => { @@ -46,6 +48,8 @@ describe('HeaderSearchApp', () => { const findHeaderSearchDropdown = () => wrapper.findByTestId('header-search-dropdown-menu'); const findHeaderSearchDefaultItems = () => wrapper.findComponent(HeaderSearchDefaultItems); const findHeaderSearchScopedItems = () => wrapper.findComponent(HeaderSearchScopedItems); + const findHeaderSearchAutocompleteItems = () => + wrapper.findComponent(HeaderSearchAutocompleteItems); describe('template', () => { it('always renders Header Search Input', () => { @@ -74,11 +78,11 @@ describe('HeaderSearchApp', () => { }); describe.each` - search | showDefault | showScoped - ${null} | ${true} | ${false} - ${''} | ${true} | ${false} - ${MOCK_SEARCH} | ${false} | ${true} - `('Header Search Dropdown Items', ({ search, showDefault, showScoped }) => { + search | showDefault | showScoped | showAutocomplete + ${null} | ${true} | ${false} | ${false} + ${''} | ${true} | ${false} | ${false} + ${MOCK_SEARCH} | ${false} | ${true} | ${true} + `('Header Search Dropdown Items', ({ search, showDefault, showScoped, showAutocomplete }) => { describe(`when search is ${search}`, () => { beforeEach(() => { createComponent({ search }); @@ -93,6 +97,10 @@ describe('HeaderSearchApp', () => { it(`should${showScoped ? '' : ' not'} render the Scoped Dropdown Items`, () => { expect(findHeaderSearchScopedItems().exists()).toBe(showScoped); }); + + it(`should${showAutocomplete ? '' : ' not'} render the Autocomplete Dropdown Items`, () => { + expect(findHeaderSearchAutocompleteItems().exists()).toBe(showAutocomplete); + }); }); }); }); @@ -139,12 +147,18 @@ describe('HeaderSearchApp', () => { }); }); - it('calls setSearch when search input event is fired', async () => { - findHeaderSearchInput().vm.$emit('input', MOCK_SEARCH); + describe('onInput', () => { + beforeEach(() => { + findHeaderSearchInput().vm.$emit('input', MOCK_SEARCH); + }); - await wrapper.vm.$nextTick(); + it('calls setSearch with search term', () => { + expect(actionSpies.setSearch).toHaveBeenCalledWith(expect.any(Object), MOCK_SEARCH); + }); - expect(actionSpies.setSearch).toHaveBeenCalledWith(expect.any(Object), MOCK_SEARCH); + it('calls fetchAutocompleteOptions', () => { + expect(actionSpies.fetchAutocompleteOptions).toHaveBeenCalled(); + }); }); it('submits a search onKey-Enter', async () => { diff --git a/spec/frontend/header_search/components/header_search_autocomplete_items_spec.js b/spec/frontend/header_search/components/header_search_autocomplete_items_spec.js new file mode 100644 index 00000000000..6b84e63989d --- /dev/null +++ b/spec/frontend/header_search/components/header_search_autocomplete_items_spec.js @@ -0,0 +1,108 @@ +import { GlDropdownItem, GlLoadingIcon, GlAvatar } from '@gitlab/ui'; +import { shallowMount } from '@vue/test-utils'; +import Vue from 'vue'; +import Vuex from 'vuex'; +import HeaderSearchAutocompleteItems from '~/header_search/components/header_search_autocomplete_items.vue'; +import { + GROUPS_CATEGORY, + LARGE_AVATAR_PX, + PROJECTS_CATEGORY, + SMALL_AVATAR_PX, +} from '~/header_search/constants'; +import { MOCK_GROUPED_AUTOCOMPLETE_OPTIONS, MOCK_AUTOCOMPLETE_OPTIONS } from '../mock_data'; + +Vue.use(Vuex); + +describe('HeaderSearchAutocompleteItems', () => { + let wrapper; + + const createComponent = (initialState, mockGetters) => { + const store = new Vuex.Store({ + state: { + loading: false, + ...initialState, + }, + getters: { + autocompleteGroupedSearchOptions: () => MOCK_GROUPED_AUTOCOMPLETE_OPTIONS, + ...mockGetters, + }, + }); + + wrapper = shallowMount(HeaderSearchAutocompleteItems, { + store, + }); + }; + + afterEach(() => { + wrapper.destroy(); + }); + + const findDropdownItems = () => wrapper.findAllComponents(GlDropdownItem); + const findDropdownItemTitles = () => findDropdownItems().wrappers.map((w) => w.text()); + const findDropdownItemLinks = () => findDropdownItems().wrappers.map((w) => w.attributes('href')); + const findGlLoadingIcon = () => wrapper.findComponent(GlLoadingIcon); + const findGlAvatar = () => wrapper.findComponent(GlAvatar); + + describe('template', () => { + describe('when loading is true', () => { + beforeEach(() => { + createComponent({ loading: true }); + }); + + it('renders GlLoadingIcon', () => { + expect(findGlLoadingIcon().exists()).toBe(true); + }); + + it('does not render autocomplete options', () => { + expect(findDropdownItems()).toHaveLength(0); + }); + }); + + describe('when loading is false', () => { + beforeEach(() => { + createComponent({ loading: false }); + }); + + it('does not render GlLoadingIcon', () => { + expect(findGlLoadingIcon().exists()).toBe(false); + }); + + describe('Dropdown items', () => { + it('renders item for each option in autocomplete option', () => { + expect(findDropdownItems()).toHaveLength(MOCK_AUTOCOMPLETE_OPTIONS.length); + }); + + it('renders titles correctly', () => { + const expectedTitles = MOCK_AUTOCOMPLETE_OPTIONS.map((o) => o.label); + expect(findDropdownItemTitles()).toStrictEqual(expectedTitles); + }); + + it('renders links correctly', () => { + const expectedLinks = MOCK_AUTOCOMPLETE_OPTIONS.map((o) => o.url); + expect(findDropdownItemLinks()).toStrictEqual(expectedLinks); + }); + }); + describe.each` + item | showAvatar | avatarSize + ${{ data: [{ category: PROJECTS_CATEGORY, avatar_url: null }] }} | ${true} | ${String(LARGE_AVATAR_PX)} + ${{ data: [{ category: GROUPS_CATEGORY, avatar_url: '/123' }] }} | ${true} | ${String(LARGE_AVATAR_PX)} + ${{ data: [{ category: 'Help', avatar_url: '' }] }} | ${true} | ${String(SMALL_AVATAR_PX)} + ${{ data: [{ category: 'Settings' }] }} | ${false} | ${false} + `('GlAvatar', ({ item, showAvatar, avatarSize }) => { + describe(`when category is ${item.data[0].category} and avatar_url is ${item.data[0].avatar_url}`, () => { + beforeEach(() => { + createComponent({}, { autocompleteGroupedSearchOptions: () => [item] }); + }); + + it(`should${showAvatar ? '' : ' not'} render`, () => { + expect(findGlAvatar().exists()).toBe(showAvatar); + }); + + it(`should set avatarSize to ${avatarSize}`, () => { + expect(findGlAvatar().exists() && findGlAvatar().attributes('size')).toBe(avatarSize); + }); + }); + }); + }); + }); +}); diff --git a/spec/frontend/header_search/mock_data.js b/spec/frontend/header_search/mock_data.js index 5963ad9c279..915b3a4a678 100644 --- a/spec/frontend/header_search/mock_data.js +++ b/spec/frontend/header_search/mock_data.js @@ -19,6 +19,8 @@ export const MOCK_MR_PATH = '/dashboard/merge_requests'; export const MOCK_ALL_PATH = '/'; +export const MOCK_AUTOCOMPLETE_PATH = '/autocomplete'; + export const MOCK_PROJECT = { id: 123, name: 'MockProject', @@ -81,3 +83,70 @@ export const MOCK_SCOPED_SEARCH_OPTIONS = [ url: MOCK_ALL_PATH, }, ]; + +export const MOCK_AUTOCOMPLETE_OPTIONS = [ + { + category: 'Projects', + id: 1, + label: 'MockProject1', + url: 'project/1', + }, + { + category: 'Projects', + id: 2, + label: 'MockProject2', + url: 'project/2', + }, + { + category: 'Groups', + id: 1, + label: 'MockGroup1', + url: 'group/1', + }, + { + category: 'Help', + label: 'GitLab Help', + url: 'help/gitlab', + }, +]; + +export const MOCK_GROUPED_AUTOCOMPLETE_OPTIONS = [ + { + category: 'Projects', + data: [ + { + category: 'Projects', + id: 1, + label: 'MockProject1', + url: 'project/1', + }, + { + category: 'Projects', + id: 2, + label: 'MockProject2', + url: 'project/2', + }, + ], + }, + { + category: 'Groups', + data: [ + { + category: 'Groups', + id: 1, + label: 'MockGroup1', + url: 'group/1', + }, + ], + }, + { + category: 'Help', + data: [ + { + category: 'Help', + label: 'GitLab Help', + url: 'help/gitlab', + }, + ], + }, +]; diff --git a/spec/frontend/header_search/store/actions_spec.js b/spec/frontend/header_search/store/actions_spec.js index 4530df0d91c..ee2c72df77b 100644 --- a/spec/frontend/header_search/store/actions_spec.js +++ b/spec/frontend/header_search/store/actions_spec.js @@ -1,18 +1,50 @@ +import MockAdapter from 'axios-mock-adapter'; import testAction from 'helpers/vuex_action_helper'; +import createFlash from '~/flash'; import * as actions from '~/header_search/store/actions'; import * as types from '~/header_search/store/mutation_types'; import createState from '~/header_search/store/state'; -import { MOCK_SEARCH } from '../mock_data'; +import axios from '~/lib/utils/axios_utils'; +import { MOCK_SEARCH, MOCK_AUTOCOMPLETE_OPTIONS } from '../mock_data'; + +jest.mock('~/flash'); describe('Header Search Store Actions', () => { let state; + let mock; + + const flashCallback = (callCount) => { + expect(createFlash).toHaveBeenCalledTimes(callCount); + createFlash.mockClear(); + }; beforeEach(() => { state = createState({}); + mock = new MockAdapter(axios); }); afterEach(() => { state = null; + mock.restore(); + }); + + describe.each` + axiosMock | type | expectedMutations | flashCallCount + ${{ method: 'onGet', code: 200, res: MOCK_AUTOCOMPLETE_OPTIONS }} | ${'success'} | ${[{ type: types.REQUEST_AUTOCOMPLETE }, { type: types.RECEIVE_AUTOCOMPLETE_SUCCESS, payload: MOCK_AUTOCOMPLETE_OPTIONS }]} | ${0} + ${{ method: 'onGet', code: 500, res: null }} | ${'error'} | ${[{ type: types.REQUEST_AUTOCOMPLETE }, { type: types.RECEIVE_AUTOCOMPLETE_ERROR }]} | ${1} + `('fetchAutocompleteOptions', ({ axiosMock, type, expectedMutations, flashCallCount }) => { + describe(`on ${type}`, () => { + beforeEach(() => { + mock[axiosMock.method]().replyOnce(axiosMock.code, axiosMock.res); + }); + it(`should dispatch the correct mutations`, () => { + return testAction({ + action: actions.fetchAutocompleteOptions, + state, + expectedMutations, + }).then(() => flashCallback(flashCallCount)); + }); + }); }); describe('setSearch', () => { diff --git a/spec/frontend/header_search/store/getters_spec.js b/spec/frontend/header_search/store/getters_spec.js index 2ad0a082f6a..d55db07188e 100644 --- a/spec/frontend/header_search/store/getters_spec.js +++ b/spec/frontend/header_search/store/getters_spec.js @@ -5,6 +5,7 @@ import { MOCK_SEARCH_PATH, MOCK_ISSUE_PATH, MOCK_MR_PATH, + MOCK_AUTOCOMPLETE_PATH, MOCK_SEARCH_CONTEXT, MOCK_DEFAULT_SEARCH_OPTIONS, MOCK_SCOPED_SEARCH_OPTIONS, @@ -12,6 +13,8 @@ import { MOCK_GROUP, MOCK_ALL_PATH, MOCK_SEARCH, + MOCK_AUTOCOMPLETE_OPTIONS, + MOCK_GROUPED_AUTOCOMPLETE_OPTIONS, } from '../mock_data'; describe('Header Search Store Getters', () => { @@ -22,6 +25,7 @@ describe('Header Search Store Getters', () => { searchPath: MOCK_SEARCH_PATH, issuesPath: MOCK_ISSUE_PATH, mrPath: MOCK_MR_PATH, + autocompletePath: MOCK_AUTOCOMPLETE_PATH, searchContext: MOCK_SEARCH_CONTEXT, ...initialState, }); @@ -56,6 +60,29 @@ describe('Header Search Store Getters', () => { }); describe.each` + project | ref | expectedPath + ${null} | ${null} | ${`${MOCK_AUTOCOMPLETE_PATH}?term=${MOCK_SEARCH}&project_id=undefined&project_ref=null`} + ${MOCK_PROJECT} | ${null} | ${`${MOCK_AUTOCOMPLETE_PATH}?term=${MOCK_SEARCH}&project_id=${MOCK_PROJECT.id}&project_ref=null`} + ${MOCK_PROJECT} | ${MOCK_PROJECT.id} | ${`${MOCK_AUTOCOMPLETE_PATH}?term=${MOCK_SEARCH}&project_id=${MOCK_PROJECT.id}&project_ref=${MOCK_PROJECT.id}`} + `('autocompleteQuery', ({ project, ref, expectedPath }) => { + describe(`when project is ${project?.name} and project ref is ${ref}`, () => { + beforeEach(() => { + createState({ + searchContext: { + project, + ref, + }, + }); + state.search = MOCK_SEARCH; + }); + + it(`should return ${expectedPath}`, () => { + expect(getters.autocompleteQuery(state)).toBe(expectedPath); + }); + }); + }); + + describe.each` group | group_metadata | project | project_metadata | expectedPath ${null} | ${null} | ${null} | ${null} | ${MOCK_ISSUE_PATH} ${{ name: 'Test Group' }} | ${{ issues_path: 'group/path' }} | ${null} | ${null} | ${'group/path'} @@ -208,4 +235,17 @@ describe('Header Search Store Getters', () => { ); }); }); + + describe('autocompleteGroupedSearchOptions', () => { + beforeEach(() => { + createState(); + state.autocompleteOptions = MOCK_AUTOCOMPLETE_OPTIONS; + }); + + it('returns the correct grouped array', () => { + expect(getters.autocompleteGroupedSearchOptions(state)).toStrictEqual( + MOCK_GROUPED_AUTOCOMPLETE_OPTIONS, + ); + }); + }); }); diff --git a/spec/frontend/header_search/store/mutations_spec.js b/spec/frontend/header_search/store/mutations_spec.js index 8196c06099d..7f9b7631a7e 100644 --- a/spec/frontend/header_search/store/mutations_spec.js +++ b/spec/frontend/header_search/store/mutations_spec.js @@ -1,7 +1,7 @@ import * as types from '~/header_search/store/mutation_types'; import mutations from '~/header_search/store/mutations'; import createState from '~/header_search/store/state'; -import { MOCK_SEARCH } from '../mock_data'; +import { MOCK_SEARCH, MOCK_AUTOCOMPLETE_OPTIONS } from '../mock_data'; describe('Header Search Store Mutations', () => { let state; @@ -10,6 +10,33 @@ describe('Header Search Store Mutations', () => { state = createState({}); }); + describe('REQUEST_AUTOCOMPLETE', () => { + it('sets loading to true and empties autocompleteOptions array', () => { + mutations[types.REQUEST_AUTOCOMPLETE](state); + + expect(state.loading).toBe(true); + expect(state.autocompleteOptions).toStrictEqual([]); + }); + }); + + describe('RECEIVE_AUTOCOMPLETE_SUCCESS', () => { + it('sets loading to false and sets autocompleteOptions array', () => { + mutations[types.RECEIVE_AUTOCOMPLETE_SUCCESS](state, MOCK_AUTOCOMPLETE_OPTIONS); + + expect(state.loading).toBe(false); + expect(state.autocompleteOptions).toStrictEqual(MOCK_AUTOCOMPLETE_OPTIONS); + }); + }); + + describe('RECEIVE_AUTOCOMPLETE_ERROR', () => { + it('sets loading to false and empties autocompleteOptions array', () => { + mutations[types.RECEIVE_AUTOCOMPLETE_ERROR](state); + + expect(state.loading).toBe(false); + expect(state.autocompleteOptions).toStrictEqual([]); + }); + }); + describe('SET_SEARCH', () => { it('sets search to value', () => { mutations[types.SET_SEARCH](state, MOCK_SEARCH); diff --git a/spec/initializers/lograge_spec.rb b/spec/initializers/lograge_spec.rb index 4d2aa6e74de..9e58fa289ac 100644 --- a/spec/initializers/lograge_spec.rb +++ b/spec/initializers/lograge_spec.rb @@ -230,39 +230,21 @@ RSpec.describe 'lograge', type: :request do end end - context 'when load balancing is enabled' do - before do - allow(Gitlab::Database::LoadBalancing).to receive(:enable?).and_return(true) - end - - context 'with db payload' do - context 'when RequestStore is enabled', :request_store do - it 'includes db counters for load balancing' do - subscriber.process_action(event) - - expect(log_data).to include(*db_load_balancing_logging_keys) - end - end - - context 'when RequestStore is disabled' do - it 'does not include db counters for load balancing' do - subscriber.process_action(event) + context 'with db payload' do + context 'when RequestStore is enabled', :request_store do + it 'includes db counters for load balancing' do + subscriber.process_action(event) - expect(log_data).not_to include(*db_load_balancing_logging_keys) - end + expect(log_data).to include(*db_load_balancing_logging_keys) end end - end - context 'when load balancing is disabled' do - before do - allow(Gitlab::Database::LoadBalancing).to receive(:enable?).and_return(false) - end + context 'when RequestStore is disabled' do + it 'does not include db counters for load balancing' do + subscriber.process_action(event) - it 'does not include db counters for load balancing' do - subscriber.process_action(event) - - expect(log_data).not_to include(*db_load_balancing_logging_keys) + expect(log_data).not_to include(*db_load_balancing_logging_keys) + end end end end diff --git a/spec/lib/api/every_api_endpoint_spec.rb b/spec/lib/api/every_api_endpoint_spec.rb index ebf75e733d0..a9f325edefd 100644 --- a/spec/lib/api/every_api_endpoint_spec.rb +++ b/spec/lib/api/every_api_endpoint_spec.rb @@ -9,7 +9,7 @@ RSpec.describe 'Every API endpoint' do end let_it_be(:api_endpoints) do - API::API.routes.map do |route| + Gitlab::RequestEndpoints.all_api_endpoints.map do |route| [route.app.options[:for], API::Base.path_for_app(route.app)] end end diff --git a/spec/lib/api/helpers_spec.rb b/spec/lib/api/helpers_spec.rb index 587fe60860a..c966527ca5b 100644 --- a/spec/lib/api/helpers_spec.rb +++ b/spec/lib/api/helpers_spec.rb @@ -32,10 +32,6 @@ RSpec.describe API::Helpers do helper end - before do - allow(Gitlab::Database::LoadBalancing).to receive(:enable?).and_return(true) - end - it 'handles sticking when a user could be found' do allow_any_instance_of(API::Helpers).to receive(:initial_current_user).and_return(user) diff --git a/spec/lib/gitlab/checks/matching_merge_request_spec.rb b/spec/lib/gitlab/checks/matching_merge_request_spec.rb index 2e562a5a350..dce6a271918 100644 --- a/spec/lib/gitlab/checks/matching_merge_request_spec.rb +++ b/spec/lib/gitlab/checks/matching_merge_request_spec.rb @@ -31,35 +31,22 @@ RSpec.describe Gitlab::Checks::MatchingMergeRequest do expect(matcher.match?).to be false end - context 'with load balancing disabled', :request_store, :redis do - before do - expect(::Gitlab::Database::LoadBalancing).to receive(:enable?).at_least(:once).and_return(false) - expect(::Gitlab::Database::LoadBalancing::Sticking).not_to receive(:unstick_or_continue_sticking) - expect(::Gitlab::Database::LoadBalancing::Sticking).not_to receive(:select_valid_replicas) - end - - it 'does not attempt to stick to primary' do - expect(subject.match?).to be true - end - - it 'increments no counters' do - expect { subject.match? } - .to change { total_counter.get }.by(0) - .and change { stale_counter.get }.by(0) - end - end - - context 'with load balancing enabled', :db_load_balancing do + context 'with load balancing enabled' do let(:session) { ::Gitlab::Database::LoadBalancing::Session.current } let(:all_caught_up) { true } before do + Gitlab::Database::LoadBalancing::Session.clear_session allow(::Gitlab::Database::LoadBalancing::Sticking).to receive(:all_caught_up?).and_return(all_caught_up) expect(::Gitlab::Database::LoadBalancing::Sticking).to receive(:select_valid_host).with(:project, project.id).and_call_original allow(::Gitlab::Database::LoadBalancing::Sticking).to receive(:select_caught_up_replicas).with(:project, project.id).and_return(all_caught_up) end + after do + Gitlab::Database::LoadBalancing::Session.clear_session + end + shared_examples 'secondary that has caught up to a primary' do it 'continues to use the secondary' do expect(session.use_primary?).to be false diff --git a/spec/lib/gitlab/database/bulk_update_spec.rb b/spec/lib/gitlab/database/bulk_update_spec.rb index dbafada26ca..33dea809d7d 100644 --- a/spec/lib/gitlab/database/bulk_update_spec.rb +++ b/spec/lib/gitlab/database/bulk_update_spec.rb @@ -122,7 +122,7 @@ RSpec.describe Gitlab::Database::BulkUpdate do stub_const('ActiveRecordBasePreparedStatementsInverted', klass) - c = ActiveRecord::Base.connection.instance_variable_get(:@config) + c = ActiveRecord::Base.retrieve_connection.instance_variable_get(:@config) inverted = c.merge(prepared_statements: !ActiveRecord::Base.connection.prepared_statements) ActiveRecordBasePreparedStatementsInverted.establish_connection(inverted) diff --git a/spec/lib/gitlab/database/consistency_spec.rb b/spec/lib/gitlab/database/consistency_spec.rb index 35fa65512ae..b3d166d58bb 100644 --- a/spec/lib/gitlab/database/consistency_spec.rb +++ b/spec/lib/gitlab/database/consistency_spec.rb @@ -7,7 +7,7 @@ RSpec.describe Gitlab::Database::Consistency do Gitlab::Database::LoadBalancing::Session.current end - describe '.with_read_consistency' do + describe '.with_read_consistency', :db_load_balancing do it 'sticks to primary database' do expect(session).not_to be_using_primary diff --git a/spec/lib/gitlab/database/load_balancing/host_spec.rb b/spec/lib/gitlab/database/load_balancing/host_spec.rb index e2011692228..b040c7a76bd 100644 --- a/spec/lib/gitlab/database/load_balancing/host_spec.rb +++ b/spec/lib/gitlab/database/load_balancing/host_spec.rb @@ -172,6 +172,14 @@ RSpec.describe Gitlab::Database::LoadBalancing::Host do expect(host).not_to be_online end + + it 'returns false when ActiveRecord::ConnectionNotEstablished is raised' do + allow(host) + .to receive(:check_replica_status?) + .and_raise(ActiveRecord::ConnectionNotEstablished) + + expect(host).not_to be_online + end end end diff --git a/spec/lib/gitlab/database/load_balancing/load_balancer_spec.rb b/spec/lib/gitlab/database/load_balancing/load_balancer_spec.rb index 86fae14b961..4784f4270b2 100644 --- a/spec/lib/gitlab/database/load_balancing/load_balancer_spec.rb +++ b/spec/lib/gitlab/database/load_balancing/load_balancer_spec.rb @@ -277,31 +277,26 @@ RSpec.describe Gitlab::Database::LoadBalancing::LoadBalancer, :request_store do end describe '#connection_error?' do - before do - stub_const('Gitlab::Database::LoadBalancing::LoadBalancer::CONNECTION_ERRORS', - [NotImplementedError]) - end - it 'returns true for a connection error' do - error = NotImplementedError.new + error = ActiveRecord::ConnectionNotEstablished.new expect(lb.connection_error?(error)).to eq(true) end it 'returns true for a wrapped connection error' do - wrapped = wrapped_exception(ActiveRecord::StatementInvalid, NotImplementedError) + wrapped = wrapped_exception(ActiveRecord::StatementInvalid, ActiveRecord::ConnectionNotEstablished) expect(lb.connection_error?(wrapped)).to eq(true) end it 'returns true for a wrapped connection error from a view' do - wrapped = wrapped_exception(ActionView::Template::Error, NotImplementedError) + wrapped = wrapped_exception(ActionView::Template::Error, ActiveRecord::ConnectionNotEstablished) expect(lb.connection_error?(wrapped)).to eq(true) end it 'returns true for deeply wrapped/nested errors' do - top = twice_wrapped_exception(ActionView::Template::Error, ActiveRecord::StatementInvalid, NotImplementedError) + top = twice_wrapped_exception(ActionView::Template::Error, ActiveRecord::StatementInvalid, ActiveRecord::ConnectionNotEstablished) expect(lb.connection_error?(top)).to eq(true) end diff --git a/spec/lib/gitlab/database/load_balancing/rack_middleware_spec.rb b/spec/lib/gitlab/database/load_balancing/rack_middleware_spec.rb index ea0c7f781fd..5453e36e08f 100644 --- a/spec/lib/gitlab/database/load_balancing/rack_middleware_spec.rb +++ b/spec/lib/gitlab/database/load_balancing/rack_middleware_spec.rb @@ -20,11 +20,6 @@ RSpec.describe Gitlab::Database::LoadBalancing::RackMiddleware, :redis do end describe '.stick_or_unstick' do - before do - allow(Gitlab::Database::LoadBalancing).to receive(:enable?) - .and_return(true) - end - it 'sticks or unsticks a single object and updates the Rack environment' do expect(Gitlab::Database::LoadBalancing::Sticking) .to receive(:unstick_or_continue_sticking) diff --git a/spec/lib/gitlab/database/load_balancing/sidekiq_client_middleware_spec.rb b/spec/lib/gitlab/database/load_balancing/sidekiq_client_middleware_spec.rb index f683ade978a..18d9cc9e170 100644 --- a/spec/lib/gitlab/database/load_balancing/sidekiq_client_middleware_spec.rb +++ b/spec/lib/gitlab/database/load_balancing/sidekiq_client_middleware_spec.rb @@ -5,14 +5,13 @@ require 'spec_helper' RSpec.describe Gitlab::Database::LoadBalancing::SidekiqClientMiddleware do let(:middleware) { described_class.new } - let(:load_balancer) { double.as_null_object } + let(:load_balancer) { Gitlab::Database::LoadBalancing.proxy.load_balancer } let(:worker_class) { 'TestDataConsistencyWorker' } let(:job) { { "job_id" => "a180b47c-3fd6-41b8-81e9-34da61c3400e" } } before do skip_feature_flags_yaml_validation skip_default_enabled_yaml_check - allow(::Gitlab::Database::LoadBalancing).to receive_message_chain(:proxy, :load_balancer).and_return(load_balancer) end after do diff --git a/spec/lib/gitlab/database/load_balancing/sidekiq_server_middleware_spec.rb b/spec/lib/gitlab/database/load_balancing/sidekiq_server_middleware_spec.rb index 9f23eb0094f..d254bdeefad 100644 --- a/spec/lib/gitlab/database/load_balancing/sidekiq_server_middleware_spec.rb +++ b/spec/lib/gitlab/database/load_balancing/sidekiq_server_middleware_spec.rb @@ -5,7 +5,7 @@ require 'spec_helper' RSpec.describe Gitlab::Database::LoadBalancing::SidekiqServerMiddleware do let(:middleware) { described_class.new } - let(:load_balancer) { double.as_null_object } + let(:load_balancer) { Gitlab::Database::LoadBalancing.proxy.load_balancer } let(:worker) { worker_class.new } let(:job) { { "retry" => 3, "job_id" => "a180b47c-3fd6-41b8-81e9-34da61c3400e", 'database_replica_location' => '0/D525E3A8' } } @@ -13,7 +13,6 @@ RSpec.describe Gitlab::Database::LoadBalancing::SidekiqServerMiddleware do before do skip_feature_flags_yaml_validation skip_default_enabled_yaml_check - allow(::Gitlab::Database::LoadBalancing).to receive_message_chain(:proxy, :load_balancer).and_return(load_balancer) replication_lag!(false) end diff --git a/spec/lib/gitlab/database/load_balancing/sticking_spec.rb b/spec/lib/gitlab/database/load_balancing/sticking_spec.rb index cf52e59db3a..1087ff6dd78 100644 --- a/spec/lib/gitlab/database/load_balancing/sticking_spec.rb +++ b/spec/lib/gitlab/database/load_balancing/sticking_spec.rb @@ -8,39 +8,24 @@ RSpec.describe Gitlab::Database::LoadBalancing::Sticking, :redis do end describe '.stick_if_necessary' do - context 'when sticking is disabled' do - it 'does not perform any sticking' do - expect(described_class).not_to receive(:stick) + it 'does not stick if no write was performed' do + allow(Gitlab::Database::LoadBalancing::Session.current) + .to receive(:performed_write?) + .and_return(false) - described_class.stick_if_necessary(:user, 42) - end - end - - context 'when sticking is enabled' do - before do - allow(Gitlab::Database::LoadBalancing).to receive(:enable?) - .and_return(true) - end - - it 'does not stick if no write was performed' do - allow(Gitlab::Database::LoadBalancing::Session.current) - .to receive(:performed_write?) - .and_return(false) + expect(described_class).not_to receive(:stick) - expect(described_class).not_to receive(:stick) - - described_class.stick_if_necessary(:user, 42) - end + described_class.stick_if_necessary(:user, 42) + end - it 'sticks to the primary if a write was performed' do - allow(Gitlab::Database::LoadBalancing::Session.current) - .to receive(:performed_write?) - .and_return(true) + it 'sticks to the primary if a write was performed' do + allow(Gitlab::Database::LoadBalancing::Session.current) + .to receive(:performed_write?) + .and_return(true) - expect(described_class).to receive(:stick).with(:user, 42) + expect(described_class).to receive(:stick).with(:user, 42) - described_class.stick_if_necessary(:user, 42) - end + described_class.stick_if_necessary(:user, 42) end end @@ -155,35 +140,22 @@ RSpec.describe Gitlab::Database::LoadBalancing::Sticking, :redis do end RSpec.shared_examples 'sticking' do - context 'when sticking is disabled' do - it 'does not perform any sticking', :aggregate_failures do - expect(described_class).not_to receive(:set_write_location_for) - expect(Gitlab::Database::LoadBalancing::Session.current).not_to receive(:use_primary!) + before do + lb = double(:lb, primary_write_location: 'foo') - described_class.bulk_stick(:user, ids) - end + allow(described_class).to receive(:load_balancer).and_return(lb) end - context 'when sticking is enabled' do - before do - allow(Gitlab::Database::LoadBalancing).to receive(:configured?).and_return(true) - - lb = double(:lb, primary_write_location: 'foo') - - allow(described_class).to receive(:load_balancer).and_return(lb) + it 'sticks an entity to the primary', :aggregate_failures do + ids.each do |id| + expect(described_class).to receive(:set_write_location_for) + .with(:user, id, 'foo') end - it 'sticks an entity to the primary', :aggregate_failures do - ids.each do |id| - expect(described_class).to receive(:set_write_location_for) - .with(:user, id, 'foo') - end - - expect(Gitlab::Database::LoadBalancing::Session.current) - .to receive(:use_primary!) + expect(Gitlab::Database::LoadBalancing::Session.current) + .to receive(:use_primary!) - subject - end + subject end end @@ -202,63 +174,15 @@ RSpec.describe Gitlab::Database::LoadBalancing::Sticking, :redis do end describe '.mark_primary_write_location' do - context 'when enabled' do - before do - allow(Gitlab::Database::LoadBalancing).to receive(:enable?).and_return(true) - allow(Gitlab::Database::LoadBalancing).to receive(:configured?).and_return(true) - end + it 'updates the write location with the load balancer' do + lb = double(:lb, primary_write_location: 'foo') - it 'updates the write location with the load balancer' do - lb = double(:lb, primary_write_location: 'foo') - - allow(described_class).to receive(:load_balancer).and_return(lb) - - expect(described_class).to receive(:set_write_location_for) - .with(:user, 42, 'foo') - - described_class.mark_primary_write_location(:user, 42) - end - end - - context 'when load balancing is configured but not enabled' do - before do - allow(Gitlab::Database::LoadBalancing).to receive(:enable?).and_return(false) - allow(Gitlab::Database::LoadBalancing).to receive(:configured?).and_return(true) - end - - it 'updates the write location with the main ActiveRecord connection' do - allow(described_class).to receive(:load_balancer).and_return(nil) - expect(ActiveRecord::Base).to receive(:connection).and_call_original - expect(described_class).to receive(:set_write_location_for) - .with(:user, 42, anything) - - described_class.mark_primary_write_location(:user, 42) - end - - context 'when write location is nil' do - before do - allow(Gitlab::Database.main).to receive(:get_write_location).and_return(nil) - end - - it 'does not update the write location' do - expect(described_class).not_to receive(:set_write_location_for) - - described_class.mark_primary_write_location(:user, 42) - end - end - end - - context 'when load balancing is disabled' do - before do - allow(Gitlab::Database::LoadBalancing).to receive(:enable?).and_return(false) - allow(Gitlab::Database::LoadBalancing).to receive(:configured?).and_return(false) - end + allow(described_class).to receive(:load_balancer).and_return(lb) - it 'updates the write location with the main ActiveRecord connection' do - expect(described_class).not_to receive(:set_write_location_for) + expect(described_class).to receive(:set_write_location_for) + .with(:user, 42, 'foo') - described_class.mark_primary_write_location(:user, 42) - end + described_class.mark_primary_write_location(:user, 42) end end diff --git a/spec/lib/gitlab/database/load_balancing_spec.rb b/spec/lib/gitlab/database/load_balancing_spec.rb index f40ad444081..58b03286260 100644 --- a/spec/lib/gitlab/database/load_balancing_spec.rb +++ b/spec/lib/gitlab/database/load_balancing_spec.rb @@ -4,38 +4,14 @@ require 'spec_helper' RSpec.describe Gitlab::Database::LoadBalancing do describe '.proxy' do - before do - @previous_proxy = ActiveRecord::Base.load_balancing_proxy + it 'returns the connection proxy' do + proxy = double(:connection_proxy) - ActiveRecord::Base.load_balancing_proxy = connection_proxy - end - - after do - ActiveRecord::Base.load_balancing_proxy = @previous_proxy - end - - context 'when configured' do - let(:connection_proxy) { double(:connection_proxy) } - - it 'returns the connection proxy' do - expect(subject.proxy).to eq(connection_proxy) - end - end - - context 'when not configured' do - let(:connection_proxy) { nil } - - it 'returns nil' do - expect(subject.proxy).to be_nil - end - - it 'tracks an error to sentry' do - expect(Gitlab::ErrorTracking).to receive(:track_exception).with( - an_instance_of(subject::ProxyNotConfiguredError) - ) + allow(ActiveRecord::Base) + .to receive(:load_balancing_proxy) + .and_return(proxy) - subject.proxy - end + expect(described_class.proxy).to eq(proxy) end end @@ -50,46 +26,53 @@ RSpec.describe Gitlab::Database::LoadBalancing do end end - describe '.enable?' do - before do - allow(described_class.configuration) - .to receive(:hosts) - .and_return(%w(foo)) - end - - it 'returns false when no hosts are specified' do - allow(described_class.configuration).to receive(:hosts).and_return([]) + describe '.enable_replicas?' do + context 'when hosts are specified' do + before do + allow(described_class.configuration) + .to receive(:hosts) + .and_return(%w(foo)) + end - expect(described_class.enable?).to eq(false) - end + it 'returns true' do + expect(described_class.enable_replicas?).to eq(true) + end - it 'returns true when Sidekiq is being used' do - allow(Gitlab::Runtime).to receive(:sidekiq?).and_return(true) + it 'returns true when Sidekiq is being used' do + allow(Gitlab::Runtime).to receive(:sidekiq?).and_return(true) - expect(described_class.enable?).to eq(true) - end + expect(described_class.enable_replicas?).to eq(true) + end - it 'returns false when running inside a Rake task' do - allow(Gitlab::Runtime).to receive(:rake?).and_return(true) + it 'returns false when running inside a Rake task' do + allow(Gitlab::Runtime).to receive(:rake?).and_return(true) - expect(described_class.enable?).to eq(false) + expect(described_class.enable_replicas?).to eq(false) + end end - it 'returns true when load balancing should be enabled' do - allow(Gitlab::Runtime).to receive(:sidekiq?).and_return(false) + context 'when no hosts are specified but service discovery is enabled' do + it 'returns true' do + allow(described_class.configuration).to receive(:hosts).and_return([]) + allow(Gitlab::Runtime).to receive(:sidekiq?).and_return(false) - expect(described_class.enable?).to eq(true) - end + allow(described_class.configuration) + .to receive(:service_discovery_enabled?) + .and_return(true) - it 'returns true when service discovery is enabled' do - allow(described_class.configuration).to receive(:hosts).and_return([]) - allow(Gitlab::Runtime).to receive(:sidekiq?).and_return(false) + expect(described_class.enable_replicas?).to eq(true) + end + end - allow(described_class.configuration) - .to receive(:service_discovery_enabled?) - .and_return(true) + context 'when no hosts are specified and service discovery is disabled' do + it 'returns false' do + allow(described_class.configuration).to receive(:hosts).and_return([]) + allow(described_class.configuration) + .to receive(:service_discovery_enabled?) + .and_return(false) - expect(described_class.enable?).to eq(true) + expect(described_class.enable_replicas?).to eq(false) + end end end @@ -121,41 +104,6 @@ RSpec.describe Gitlab::Database::LoadBalancing do end end - describe '.configure_proxy' do - before do - allow(ActiveRecord::Base).to receive(:load_balancing_proxy=) - end - - it 'configures the connection proxy' do - described_class.configure_proxy - - expect(ActiveRecord::Base).to have_received(:load_balancing_proxy=) - .with(Gitlab::Database::LoadBalancing::ConnectionProxy) - end - - context 'when service discovery is enabled' do - it 'runs initial service discovery when configuring the connection proxy' do - discover = instance_spy(Gitlab::Database::LoadBalancing::ServiceDiscovery) - - allow(described_class.configuration) - .to receive(:service_discovery) - .and_return({ record: 'foo' }) - - expect(Gitlab::Database::LoadBalancing::ServiceDiscovery) - .to receive(:new) - .with( - an_instance_of(Gitlab::Database::LoadBalancing::LoadBalancer), - an_instance_of(Hash) - ) - .and_return(discover) - - expect(discover).to receive(:perform_service_discovery) - - described_class.configure_proxy - end - end - end - describe '.start_service_discovery' do it 'does not start if service discovery is disabled' do expect(Gitlab::Database::LoadBalancing::ServiceDiscovery) @@ -191,15 +139,41 @@ RSpec.describe Gitlab::Database::LoadBalancing do end end - describe '.db_role_for_connection' do - context 'when the load balancing is not configured' do - let(:connection) { ActiveRecord::Base.connection } + describe '.perform_service_discovery' do + it 'does nothing if service discovery is disabled' do + expect(Gitlab::Database::LoadBalancing::ServiceDiscovery) + .not_to receive(:new) - it 'returns primary' do - expect(described_class.db_role_for_connection(connection)).to eq(:primary) - end + described_class.perform_service_discovery end + it 'performs service discovery when enabled' do + allow(described_class.configuration) + .to receive(:service_discovery_enabled?) + .and_return(true) + + sv = instance_spy(Gitlab::Database::LoadBalancing::ServiceDiscovery) + cfg = Gitlab::Database::LoadBalancing::Configuration + .new(ActiveRecord::Base) + lb = Gitlab::Database::LoadBalancing::LoadBalancer.new(cfg) + proxy = Gitlab::Database::LoadBalancing::ConnectionProxy.new(lb) + + allow(described_class) + .to receive(:proxy) + .and_return(proxy) + + expect(Gitlab::Database::LoadBalancing::ServiceDiscovery) + .to receive(:new) + .with(lb, cfg.service_discovery) + .and_return(sv) + + expect(sv).to receive(:perform_service_discovery) + + described_class.perform_service_discovery + end + end + + describe '.db_role_for_connection' do context 'when the NullPool is used for connection' do let(:pool) { ActiveRecord::ConnectionAdapters::NullPool.new } let(:connection) { double(:connection, pool: pool) } @@ -274,10 +248,6 @@ RSpec.describe Gitlab::Database::LoadBalancing do end end - before do - model.singleton_class.prepend ::Gitlab::Database::LoadBalancing::ActiveRecordProxy - end - where(:queries, :include_transaction, :expected_results) do [ # Read methods diff --git a/spec/lib/gitlab/database/with_lock_retries_spec.rb b/spec/lib/gitlab/database/with_lock_retries_spec.rb index 0b960830d89..c2c818aa106 100644 --- a/spec/lib/gitlab/database/with_lock_retries_spec.rb +++ b/spec/lib/gitlab/database/with_lock_retries_spec.rb @@ -7,7 +7,7 @@ RSpec.describe Gitlab::Database::WithLockRetries do let(:logger) { Gitlab::Database::WithLockRetries::NULL_LOGGER } let(:subject) { described_class.new(env: env, logger: logger, allow_savepoints: allow_savepoints, timing_configuration: timing_configuration) } let(:allow_savepoints) { true } - let(:connection) { ActiveRecord::Base.connection } + let(:connection) { ActiveRecord::Base.retrieve_connection } let(:timing_configuration) do [ diff --git a/spec/lib/gitlab/database_spec.rb b/spec/lib/gitlab/database_spec.rb index a9a8d5e6314..c00fa3541e9 100644 --- a/spec/lib/gitlab/database_spec.rb +++ b/spec/lib/gitlab/database_spec.rb @@ -185,10 +185,23 @@ RSpec.describe Gitlab::Database do describe '.db_config_name' do it 'returns the db_config name for the connection' do - connection = ActiveRecord::Base.connection + model = ActiveRecord::Base - expect(described_class.db_config_name(connection)).to be_a(String) - expect(described_class.db_config_name(connection)).to eq(connection.pool.db_config.name) + # This is a ConnectionProxy + expect(described_class.db_config_name(model.connection)) + .to eq('unknown') + + # This is an actual connection + expect(described_class.db_config_name(model.retrieve_connection)) + .to eq('main') + end + + context 'when replicas are configured', :db_load_balancing do + it 'returns the name for a replica' do + replica = ActiveRecord::Base.connection.load_balancer.host + + expect(described_class.db_config_name(replica)).to eq('main_replica') + end end end @@ -279,7 +292,7 @@ RSpec.describe Gitlab::Database do expect(event).not_to be_nil expect(event.duration).to be > 0.0 expect(event.payload).to a_hash_including( - connection: be_a(ActiveRecord::ConnectionAdapters::AbstractAdapter) + connection: be_a(Gitlab::Database::LoadBalancing::ConnectionProxy) ) end end @@ -296,7 +309,7 @@ RSpec.describe Gitlab::Database do expect(event).not_to be_nil expect(event.duration).to be > 0.0 expect(event.payload).to a_hash_including( - connection: be_a(ActiveRecord::ConnectionAdapters::AbstractAdapter) + connection: be_a(Gitlab::Database::LoadBalancing::ConnectionProxy) ) end end @@ -319,7 +332,7 @@ RSpec.describe Gitlab::Database do expect(event).not_to be_nil expect(event.duration).to be > 0.0 expect(event.payload).to a_hash_including( - connection: be_a(ActiveRecord::ConnectionAdapters::AbstractAdapter) + connection: be_a(Gitlab::Database::LoadBalancing::ConnectionProxy) ) end end @@ -340,7 +353,7 @@ RSpec.describe Gitlab::Database do expect(event).not_to be_nil expect(event.duration).to be > 0.0 expect(event.payload).to a_hash_including( - connection: be_a(ActiveRecord::ConnectionAdapters::AbstractAdapter) + connection: be_a(Gitlab::Database::LoadBalancing::ConnectionProxy) ) end end diff --git a/spec/lib/gitlab/import_export/json/streaming_serializer_spec.rb b/spec/lib/gitlab/import_export/json/streaming_serializer_spec.rb index 9e30564b437..b63f9acb279 100644 --- a/spec/lib/gitlab/import_export/json/streaming_serializer_spec.rb +++ b/spec/lib/gitlab/import_export/json/streaming_serializer_spec.rb @@ -158,26 +158,15 @@ RSpec.describe Gitlab::ImportExport::Json::StreamingSerializer do end describe 'load balancing' do - context 'when feature flag load_balancing_for_export_workers is enabled' do + context 'when feature flag load_balancing_for_export_workers is enabled', :db_load_balancing do before do stub_feature_flags(load_balancing_for_export_workers: true) end - context 'when enabled', :db_load_balancing do - it 'reads from replica' do - expect(Gitlab::Database::LoadBalancing::Session.current).to receive(:use_replicas_for_read_queries).and_call_original + it 'reads from replica' do + expect(Gitlab::Database::LoadBalancing::Session.current).to receive(:use_replicas_for_read_queries).and_call_original - subject.execute - end - end - - context 'when disabled' do - it 'reads from primary' do - allow(Gitlab::Database::LoadBalancing).to receive(:enable?).and_return(false) - expect(Gitlab::Database::LoadBalancing::Session.current).not_to receive(:use_replicas_for_read_queries) - - subject.execute - end + subject.execute end end diff --git a/spec/lib/gitlab/instrumentation_helper_spec.rb b/spec/lib/gitlab/instrumentation_helper_spec.rb index 85daf50717c..6e95480830c 100644 --- a/spec/lib/gitlab/instrumentation_helper_spec.rb +++ b/spec/lib/gitlab/instrumentation_helper_spec.rb @@ -107,69 +107,44 @@ RSpec.describe Gitlab::InstrumentationHelper do end end - context 'when load balancing is enabled' do - before do - allow(Gitlab::Database::LoadBalancing).to receive(:enable?).and_return(true) - end - - it 'includes DB counts' do - subject - - expect(payload).to include(db_replica_count: 0, - db_replica_cached_count: 0, - db_primary_count: 0, - db_primary_cached_count: 0, - db_primary_wal_count: 0, - db_replica_wal_count: 0, - db_primary_wal_cached_count: 0, - db_replica_wal_cached_count: 0) - end - - context 'when replica caught up search was made' do - before do - Gitlab::SafeRequestStore[:caught_up_replica_pick_ok] = 2 - Gitlab::SafeRequestStore[:caught_up_replica_pick_fail] = 1 - end + it 'includes DB counts' do + subject - it 'includes related metrics' do - subject + expect(payload).to include(db_replica_count: 0, + db_replica_cached_count: 0, + db_primary_count: 0, + db_primary_cached_count: 0, + db_primary_wal_count: 0, + db_replica_wal_count: 0, + db_primary_wal_cached_count: 0, + db_replica_wal_cached_count: 0) + end - expect(payload).to include(caught_up_replica_pick_ok: 2) - expect(payload).to include(caught_up_replica_pick_fail: 1) - end + context 'when replica caught up search was made' do + before do + Gitlab::SafeRequestStore[:caught_up_replica_pick_ok] = 2 + Gitlab::SafeRequestStore[:caught_up_replica_pick_fail] = 1 end - context 'when only a single counter was updated' do - before do - Gitlab::SafeRequestStore[:caught_up_replica_pick_ok] = 1 - Gitlab::SafeRequestStore[:caught_up_replica_pick_fail] = nil - end - - it 'includes only that counter into logging' do - subject + it 'includes related metrics' do + subject - expect(payload).to include(caught_up_replica_pick_ok: 1) - expect(payload).not_to include(:caught_up_replica_pick_fail) - end + expect(payload).to include(caught_up_replica_pick_ok: 2) + expect(payload).to include(caught_up_replica_pick_fail: 1) end end - context 'when load balancing is disabled' do + context 'when only a single counter was updated' do before do - allow(Gitlab::Database::LoadBalancing).to receive(:enable?).and_return(false) + Gitlab::SafeRequestStore[:caught_up_replica_pick_ok] = 1 + Gitlab::SafeRequestStore[:caught_up_replica_pick_fail] = nil end - it 'does not include DB counts' do + it 'includes only that counter into logging' do subject - expect(payload).not_to include(db_replica_count: 0, - db_replica_cached_count: 0, - db_primary_count: 0, - db_primary_cached_count: 0, - db_primary_wal_count: 0, - db_replica_wal_count: 0, - db_primary_wal_cached_count: 0, - db_replica_wal_cached_count: 0) + expect(payload).to include(caught_up_replica_pick_ok: 1) + expect(payload).not_to include(:caught_up_replica_pick_fail) end end end diff --git a/spec/lib/gitlab/metrics/exporter/web_exporter_spec.rb b/spec/lib/gitlab/metrics/exporter/web_exporter_spec.rb index ce98c807e2e..9deaecbf41b 100644 --- a/spec/lib/gitlab/metrics/exporter/web_exporter_spec.rb +++ b/spec/lib/gitlab/metrics/exporter/web_exporter_spec.rb @@ -30,6 +30,15 @@ RSpec.describe Gitlab::Metrics::Exporter::WebExporter do expect(readiness_probe.json).to include(status: 'ok') expect(readiness_probe.json).to include('web_exporter' => [{ 'status': 'ok' }]) end + + it 'initializes request metrics', :prometheus do + expect(Gitlab::Metrics::RailsSlis).to receive(:initialize_request_slis_if_needed!).and_call_original + + http = Net::HTTP.new(exporter.server.config[:BindAddress], exporter.server.config[:Port]) + response = http.request(Net::HTTP::Get.new('/metrics')) + + expect(response.body).to include('gitlab_sli:rails_request_apdex') + end end describe '#mark_as_not_running!' do diff --git a/spec/lib/gitlab/metrics/rails_slis_spec.rb b/spec/lib/gitlab/metrics/rails_slis_spec.rb new file mode 100644 index 00000000000..16fcb9d46a2 --- /dev/null +++ b/spec/lib/gitlab/metrics/rails_slis_spec.rb @@ -0,0 +1,58 @@ +# frozen_string_literal: true +require 'spec_helper' + +RSpec.describe Gitlab::Metrics::RailsSlis do + # Limit what routes we'll initialize so we don't have to load the entire thing + before do + api_route = API::API.routes.find do |route| + API::Base.endpoint_id_for_route(route) == "GET /api/:version/version" + end + + allow(Gitlab::RequestEndpoints).to receive(:all_api_endpoints).and_return([api_route]) + allow(Gitlab::RequestEndpoints).to receive(:all_controller_actions).and_return([[ProjectsController, 'show']]) + end + + describe '.initialize_request_slis_if_needed!' do + it "initializes the SLI for all possible endpoints if they weren't" do + possible_labels = [ + { + endpoint_id: "GET /api/:version/version", + feature_category: :not_owned + }, + { + endpoint_id: "ProjectsController#show", + feature_category: :projects + } + ] + + expect(Gitlab::Metrics::Sli).to receive(:initialized?).with(:rails_request_apdex) { false } + expect(Gitlab::Metrics::Sli).to receive(:initialize_sli).with(:rails_request_apdex, array_including(*possible_labels)).and_call_original + + described_class.initialize_request_slis_if_needed! + end + + it 'does not initialize the SLI if they were initialized already' do + expect(Gitlab::Metrics::Sli).to receive(:initialized?).with(:rails_request_apdex) { true } + expect(Gitlab::Metrics::Sli).not_to receive(:initialize_sli) + + described_class.initialize_request_slis_if_needed! + end + + it 'does not initialize anything if the feature flag is disabled' do + stub_feature_flags(request_apdex_counters: false) + + expect(Gitlab::Metrics::Sli).not_to receive(:initialize_sli) + expect(Gitlab::Metrics::Sli).not_to receive(:initialized?) + + described_class.initialize_request_slis_if_needed! + end + end + + describe '.request_apdex' do + it 'returns the initialized request apdex SLI object' do + described_class.initialize_request_slis_if_needed! + + expect(described_class.request_apdex).to be_initialized + end + end +end diff --git a/spec/lib/gitlab/metrics/requests_rack_middleware_spec.rb b/spec/lib/gitlab/metrics/requests_rack_middleware_spec.rb index 9d5c4bdf9e2..e0962614763 100644 --- a/spec/lib/gitlab/metrics/requests_rack_middleware_spec.rb +++ b/spec/lib/gitlab/metrics/requests_rack_middleware_spec.rb @@ -36,6 +36,7 @@ RSpec.describe Gitlab::Metrics::RequestsRackMiddleware, :aggregate_failures do it 'tracks request count and duration' do expect(described_class).to receive_message_chain(:http_requests_total, :increment).with(method: 'get', status: '200', feature_category: 'unknown') expect(described_class).to receive_message_chain(:http_request_duration_seconds, :observe).with({ method: 'get' }, a_positive_execution_time) + expect(Gitlab::Metrics::RailsSlis.request_apdex).to receive(:increment).with(labels: { feature_category: 'unknown', endpoint_id: 'unknown' }, success: true) subject.call(env) end @@ -82,9 +83,10 @@ RSpec.describe Gitlab::Metrics::RequestsRackMiddleware, :aggregate_failures do context '@app.call returns an error code' do let(:status) { '500' } - it 'tracks count but not duration' do + it 'tracks count but not duration or apdex' do expect(described_class).to receive_message_chain(:http_requests_total, :increment).with(method: 'get', status: '500', feature_category: 'unknown') expect(described_class).not_to receive(:http_request_duration_seconds) + expect(Gitlab::Metrics::RailsSlis).not_to receive(:request_apdex) subject.call(env) end @@ -104,20 +106,23 @@ RSpec.describe Gitlab::Metrics::RequestsRackMiddleware, :aggregate_failures do expect(described_class).to receive_message_chain(:rack_uncaught_errors_count, :increment) expect(described_class).to receive_message_chain(:http_requests_total, :increment).with(method: 'get', status: 'undefined', feature_category: 'unknown') expect(described_class.http_request_duration_seconds).not_to receive(:observe) + expect(Gitlab::Metrics::RailsSlis).not_to receive(:request_apdex) expect { subject.call(env) }.to raise_error(StandardError) end end - context 'feature category header' do - context 'when a feature category context is present' do + context 'application context' do + context 'when a context is present' do before do - ::Gitlab::ApplicationContext.push(feature_category: 'issue_tracking') + ::Gitlab::ApplicationContext.push(feature_category: 'issue_tracking', caller_id: 'IssuesController#show') end - it 'adds the feature category to the labels for http_requests_total' do + it 'adds the feature category to the labels for required metrics' do expect(described_class).to receive_message_chain(:http_requests_total, :increment).with(method: 'get', status: '200', feature_category: 'issue_tracking') expect(described_class).not_to receive(:http_health_requests_total) + expect(Gitlab::Metrics::RailsSlis.request_apdex) + .to receive(:increment).with(labels: { feature_category: 'issue_tracking', endpoint_id: 'IssuesController#show' }, success: true) subject.call(env) end @@ -127,6 +132,7 @@ RSpec.describe Gitlab::Metrics::RequestsRackMiddleware, :aggregate_failures do expect(described_class).to receive_message_chain(:http_health_requests_total, :increment).with(method: 'get', status: '200') expect(described_class).not_to receive(:http_requests_total) + expect(Gitlab::Metrics::RailsSlis).not_to receive(:request_apdex) subject.call(env) end @@ -140,15 +146,17 @@ RSpec.describe Gitlab::Metrics::RequestsRackMiddleware, :aggregate_failures do it 'adds the feature category to the labels for http_requests_total' do expect(described_class).to receive_message_chain(:http_requests_total, :increment).with(method: 'get', status: 'undefined', feature_category: 'issue_tracking') + expect(Gitlab::Metrics::RailsSlis).not_to receive(:request_apdex) expect { subject.call(env) }.to raise_error(StandardError) end end - context 'when the feature category context is not available' do - it 'sets the feature category to unknown' do + context 'when the context is not available' do + it 'sets the required labels to unknown' do expect(described_class).to receive_message_chain(:http_requests_total, :increment).with(method: 'get', status: '200', feature_category: 'unknown') expect(described_class).not_to receive(:http_health_requests_total) + expect(Gitlab::Metrics::RailsSlis.request_apdex).to receive(:increment).with(labels: { feature_category: 'unknown', endpoint_id: 'unknown' }, success: true) subject.call(env) end diff --git a/spec/lib/gitlab/metrics/sli_spec.rb b/spec/lib/gitlab/metrics/sli_spec.rb new file mode 100644 index 00000000000..8ba4bf29568 --- /dev/null +++ b/spec/lib/gitlab/metrics/sli_spec.rb @@ -0,0 +1,99 @@ +# frozen_string_literal: true + +require 'fast_spec_helper' + +RSpec.describe Gitlab::Metrics::Sli do + let(:prometheus) { double("prometheus") } + + before do + stub_const("Gitlab::Metrics", prometheus) + end + + describe 'Class methods' do + before do + described_class.instance_variable_set(:@known_slis, nil) + end + + describe '.[]' do + it 'warns about an uninitialized SLI but returns and stores a new one' do + sli = described_class[:bar] + + expect(described_class[:bar]).to be(sli) + end + + it 'returns the same object for multiple accesses' do + sli = described_class.initialize_sli(:huzzah, []) + + 2.times do + expect(described_class[:huzzah]).to be(sli) + end + end + end + + describe '.initialized?' do + before do + fake_total_counter(:boom) + fake_success_counter(:boom) + end + + it 'is true when an SLI was initialized with labels' do + expect { described_class.initialize_sli(:boom, [{ hello: :world }]) } + .to change { described_class.initialized?(:boom) }.from(false).to(true) + end + + it 'is false when an SLI was not initialized with labels' do + expect { described_class.initialize_sli(:boom, []) } + .not_to change { described_class.initialized?(:boom) }.from(false) + end + end + end + + describe '#initialize_counters' do + it 'initializes counters for the passed label combinations' do + counters = [fake_total_counter(:hey), fake_success_counter(:hey)] + + described_class.new(:hey).initialize_counters([{ foo: 'bar' }, { foo: 'baz' }]) + + expect(counters).to all(have_received(:get).with({ foo: 'bar' })) + expect(counters).to all(have_received(:get).with({ foo: 'baz' })) + end + end + + describe "#increment" do + let!(:sli) { described_class.new(:heyo) } + let!(:total_counter) { fake_total_counter(:heyo) } + let!(:success_counter) { fake_success_counter(:heyo) } + + it 'increments both counters for labels successes' do + sli.increment(labels: { hello: "world" }, success: true) + + expect(total_counter).to have_received(:increment).with({ hello: 'world' }) + expect(success_counter).to have_received(:increment).with({ hello: 'world' }) + end + + it 'only increments the total counters for labels when not successful' do + sli.increment(labels: { hello: "world" }, success: false) + + expect(total_counter).to have_received(:increment).with({ hello: 'world' }) + expect(success_counter).not_to have_received(:increment).with({ hello: 'world' }) + end + end + + def fake_prometheus_counter(name) + fake_counter = double("prometheus counter: #{name}") + + allow(fake_counter).to receive(:get) + allow(fake_counter).to receive(:increment) + allow(prometheus).to receive(:counter).with(name.to_sym, anything).and_return(fake_counter) + + fake_counter + end + + def fake_total_counter(name) + fake_prometheus_counter("gitlab_sli:#{name}:total") + end + + def fake_success_counter(name) + fake_prometheus_counter("gitlab_sli:#{name}:success_total") + end +end diff --git a/spec/lib/gitlab/metrics/subscribers/active_record_spec.rb b/spec/lib/gitlab/metrics/subscribers/active_record_spec.rb index 3ffbcbea03c..b6b6c1ffa1f 100644 --- a/spec/lib/gitlab/metrics/subscribers/active_record_spec.rb +++ b/spec/lib/gitlab/metrics/subscribers/active_record_spec.rb @@ -7,7 +7,7 @@ RSpec.describe Gitlab::Metrics::Subscribers::ActiveRecord do let(:env) { {} } let(:subscriber) { described_class.new } - let(:connection) { ActiveRecord::Base.connection } + let(:connection) { ActiveRecord::Base.retrieve_connection } let(:db_config_name) { ::Gitlab::Database.db_config_name(connection) } describe '#transaction' do @@ -135,7 +135,7 @@ RSpec.describe Gitlab::Metrics::Subscribers::ActiveRecord do end it_behaves_like 'record ActiveRecord metrics' - it_behaves_like 'store ActiveRecord info in RequestStore' + it_behaves_like 'store ActiveRecord info in RequestStore', :primary end end @@ -195,11 +195,7 @@ RSpec.describe Gitlab::Metrics::Subscribers::ActiveRecord do with_them do let(:payload) { { name: name, sql: sql(sql_query, comments: comments), connection: connection } } - before do - allow(Gitlab::Database::LoadBalancing).to receive(:enable?).and_return(true) - end - - context 'query using a connection to a replica' do + context 'query using a connection to a replica', :db_load_balancing do before do allow(Gitlab::Database::LoadBalancing).to receive(:db_role_for_connection).and_return(:replica) end diff --git a/spec/lib/gitlab/metrics/subscribers/load_balancing_spec.rb b/spec/lib/gitlab/metrics/subscribers/load_balancing_spec.rb index 21a6573c6fd..bc6effd0438 100644 --- a/spec/lib/gitlab/metrics/subscribers/load_balancing_spec.rb +++ b/spec/lib/gitlab/metrics/subscribers/load_balancing_spec.rb @@ -5,10 +5,6 @@ require 'spec_helper' RSpec.describe Gitlab::Metrics::Subscribers::LoadBalancing, :request_store do let(:subscriber) { described_class.new } - before do - allow(Gitlab::Database::LoadBalancing).to receive(:enable?).and_return(true) - end - describe '#caught_up_replica_pick' do shared_examples 'having payload result value' do |result, counter_name| subject { subscriber.caught_up_replica_pick(event) } diff --git a/spec/lib/gitlab/request_endpoints_spec.rb b/spec/lib/gitlab/request_endpoints_spec.rb new file mode 100644 index 00000000000..226605f6e2e --- /dev/null +++ b/spec/lib/gitlab/request_endpoints_spec.rb @@ -0,0 +1,24 @@ +# frozen_string_literal: true +require 'spec_helper' + +RSpec.describe Gitlab::RequestEndpoints do + describe '.all_api_endpoints' do + it 'selects all feature API classes' do + api_classes = described_class.all_api_endpoints.map { |route| route.app.options[:for] } + + expect(api_classes).to all(include(Gitlab::WithFeatureCategory)) + end + end + + describe '.all_controller_actions' do + it 'selects all feature controllers and action names' do + all_controller_actions = described_class.all_controller_actions + controller_classes = all_controller_actions.map(&:first) + all_actions = all_controller_actions.map(&:last) + + expect(controller_classes).to all(include(Gitlab::WithFeatureCategory)) + expect(controller_classes).not_to include(ApplicationController, Devise::UnlocksController) + expect(all_actions).to all(be_a(String)) + end + end +end diff --git a/spec/lib/gitlab/sidekiq_logging/structured_logger_spec.rb b/spec/lib/gitlab/sidekiq_logging/structured_logger_spec.rb index a98038cd3f8..44b593d8c0a 100644 --- a/spec/lib/gitlab/sidekiq_logging/structured_logger_spec.rb +++ b/spec/lib/gitlab/sidekiq_logging/structured_logger_spec.rb @@ -243,8 +243,22 @@ RSpec.describe Gitlab::SidekiqLogging::StructuredLogger do expected_end_payload.merge( 'db_duration_s' => a_value >= 0.1, 'db_count' => a_value >= 1, - 'db_cached_count' => 0, - 'db_write_count' => 0 + "db_replica_#{db_config_name}_count" => 0, + 'db_replica_duration_s' => a_value >= 0, + 'db_primary_count' => a_value >= 1, + "db_primary_#{db_config_name}_count" => a_value >= 1, + 'db_primary_duration_s' => a_value > 0, + "db_primary_#{db_config_name}_duration_s" => a_value > 0 + ) + end + + let(:end_payload) do + start_payload.merge(db_payload_defaults).merge( + 'message' => 'TestWorker JID-da883554ee4fe414012f5f42: done: 0.0 sec', + 'job_status' => 'done', + 'duration_s' => 0.0, + 'completed_at' => timestamp.to_f, + 'cpu_s' => 1.111112 ) end @@ -274,59 +288,9 @@ RSpec.describe Gitlab::SidekiqLogging::StructuredLogger do end end - context 'when load balancing is disabled' do - before do - allow(Gitlab::Database::LoadBalancing).to receive(:enable?).and_return(false) - end - - let(:expected_end_payload_with_db) do - expected_end_payload.merge( - 'db_duration_s' => a_value >= 0.1, - 'db_count' => a_value >= 1, - 'db_cached_count' => 0, - 'db_write_count' => 0 - ) - end - - include_examples 'performs database queries' - end - context 'when load balancing is enabled', :db_load_balancing do - let(:db_config_name) { ::Gitlab::Database.db_config_name(ApplicationRecord.connection) } - - let(:expected_db_payload_defaults) do - metrics = - ::Gitlab::Metrics::Subscribers::ActiveRecord.load_balancing_metric_counter_keys + - ::Gitlab::Metrics::Subscribers::ActiveRecord.load_balancing_metric_duration_keys + - ::Gitlab::Metrics::Subscribers::ActiveRecord.db_counter_keys + - [:db_duration_s] - - metrics.each_with_object({}) do |key, result| - result[key.to_s] = 0 - end - end - - let(:expected_end_payload_with_db) do - expected_end_payload.merge(expected_db_payload_defaults).merge( - 'db_duration_s' => a_value >= 0.1, - 'db_count' => a_value >= 1, - "db_replica_#{db_config_name}_count" => 0, - 'db_replica_duration_s' => a_value >= 0, - 'db_primary_count' => a_value >= 1, - "db_primary_#{db_config_name}_count" => a_value >= 1, - 'db_primary_duration_s' => a_value > 0, - "db_primary_#{db_config_name}_duration_s" => a_value > 0 - ) - end - - let(:end_payload) do - start_payload.merge(expected_db_payload_defaults).merge( - 'message' => 'TestWorker JID-da883554ee4fe414012f5f42: done: 0.0 sec', - 'job_status' => 'done', - 'duration_s' => 0.0, - 'completed_at' => timestamp.to_f, - 'cpu_s' => 1.111112 - ) + let(:db_config_name) do + ::Gitlab::Database.db_config_name(ApplicationRecord.retrieve_connection) end include_examples 'performs database queries' diff --git a/spec/lib/gitlab/sidekiq_middleware/server_metrics_spec.rb b/spec/lib/gitlab/sidekiq_middleware/server_metrics_spec.rb index dc0b4239120..914f5a30c3a 100644 --- a/spec/lib/gitlab/sidekiq_middleware/server_metrics_spec.rb +++ b/spec/lib/gitlab/sidekiq_middleware/server_metrics_spec.rb @@ -250,60 +250,30 @@ RSpec.describe Gitlab::SidekiqMiddleware::ServerMetrics do end end - context 'when load_balancing is enabled' do - before do - allow(::Gitlab::Database::LoadBalancing).to receive(:enable?).and_return(true) - end - - describe '#call' do - context 'when worker declares data consistency' do - include_context 'worker declaring data consistency' - - it 'increments load balancing counter with defined data consistency' do - process_job - - expect(load_balancing_metric).to have_received(:increment).with( - a_hash_including( - data_consistency: :delayed, - load_balancing_strategy: 'replica' - ), 1) - end - end - - context 'when worker does not declare data consistency' do - it 'increments load balancing counter with default data consistency' do - process_job - - expect(load_balancing_metric).to have_received(:increment).with( - a_hash_including( - data_consistency: :always, - load_balancing_strategy: 'primary' - ), 1) - end - end - end - end - - context 'when load_balancing is disabled' do - include_context 'worker declaring data consistency' + describe '#call' do + context 'when worker declares data consistency' do + include_context 'worker declaring data consistency' - before do - allow(::Gitlab::Database::LoadBalancing).to receive(:enable?).and_return(false) - end - - describe '#initialize' do - it 'does not set load_balancing metrics' do - expect(Gitlab::Metrics).not_to receive(:counter).with(:sidekiq_load_balancing_count, anything) + it 'increments load balancing counter with defined data consistency' do + process_job - subject + expect(load_balancing_metric).to have_received(:increment).with( + a_hash_including( + data_consistency: :delayed, + load_balancing_strategy: 'replica' + ), 1) end end - describe '#call' do - it 'does not increment load balancing counter' do + context 'when worker does not declare data consistency' do + it 'increments load balancing counter with default data consistency' do process_job - expect(load_balancing_metric).not_to have_received(:increment) + expect(load_balancing_metric).to have_received(:increment).with( + a_hash_including( + data_consistency: :always, + load_balancing_strategy: 'primary' + ), 1) end end end diff --git a/spec/lib/gitlab/sidekiq_middleware_spec.rb b/spec/lib/gitlab/sidekiq_middleware_spec.rb index dde364098db..e687c8e8cf7 100644 --- a/spec/lib/gitlab/sidekiq_middleware_spec.rb +++ b/spec/lib/gitlab/sidekiq_middleware_spec.rb @@ -28,9 +28,8 @@ RSpec.describe Gitlab::SidekiqMiddleware do stub_const('TestWorker', worker_class) end - shared_examples "a middleware chain" do |load_balancing_enabled| + shared_examples "a middleware chain" do before do - allow(::Gitlab::Database::LoadBalancing).to receive(:enable?).and_return(load_balancing_enabled) configurator.call(chain) end @@ -45,10 +44,10 @@ RSpec.describe Gitlab::SidekiqMiddleware do end end - shared_examples "a middleware chain for mailer" do |load_balancing_enabled| + shared_examples "a middleware chain for mailer" do let(:worker_class) { ActiveJob::QueueAdapters::SidekiqAdapter::JobWrapper } - it_behaves_like "a middleware chain", load_balancing_enabled + it_behaves_like "a middleware chain" end describe '.server_configurator' do @@ -105,25 +104,8 @@ RSpec.describe Gitlab::SidekiqMiddleware do end context "all optional middlewares on" do - context "when load balancing is enabled" do - before do - allow(::Gitlab::Database::LoadBalancing).to receive_message_chain(:proxy, :load_balancer, :release_host) - end - - it_behaves_like "a middleware chain", true - it_behaves_like "a middleware chain for mailer", true - end - - context "when load balancing is disabled" do - let(:disabled_sidekiq_middlewares) do - [ - Gitlab::Database::LoadBalancing::SidekiqServerMiddleware - ] - end - - it_behaves_like "a middleware chain", false - it_behaves_like "a middleware chain for mailer", false - end + it_behaves_like "a middleware chain" + it_behaves_like "a middleware chain for mailer" end context "all optional middlewares off" do @@ -135,36 +117,16 @@ RSpec.describe Gitlab::SidekiqMiddleware do ) end - context "when load balancing is enabled" do - let(:disabled_sidekiq_middlewares) do - [ - Gitlab::SidekiqMiddleware::ServerMetrics, - Gitlab::SidekiqMiddleware::ArgumentsLogger, - Gitlab::SidekiqMiddleware::MemoryKiller - ] - end - - before do - allow(::Gitlab::Database::LoadBalancing).to receive_message_chain(:proxy, :load_balancer, :release_host) - end - - it_behaves_like "a middleware chain", true - it_behaves_like "a middleware chain for mailer", true + let(:disabled_sidekiq_middlewares) do + [ + Gitlab::SidekiqMiddleware::ServerMetrics, + Gitlab::SidekiqMiddleware::ArgumentsLogger, + Gitlab::SidekiqMiddleware::MemoryKiller + ] end - context "when load balancing is disabled" do - let(:disabled_sidekiq_middlewares) do - [ - Gitlab::SidekiqMiddleware::ServerMetrics, - Gitlab::SidekiqMiddleware::ArgumentsLogger, - Gitlab::SidekiqMiddleware::MemoryKiller, - Gitlab::Database::LoadBalancing::SidekiqServerMiddleware - ] - end - - it_behaves_like "a middleware chain", false - it_behaves_like "a middleware chain for mailer", false - end + it_behaves_like "a middleware chain" + it_behaves_like "a middleware chain for mailer" end end @@ -186,30 +148,7 @@ RSpec.describe Gitlab::SidekiqMiddleware do ] end - context "when load balancing is disabled" do - let(:disabled_sidekiq_middlewares) do - [ - Gitlab::Database::LoadBalancing::SidekiqClientMiddleware - ] - end - - it_behaves_like "a middleware chain", false - it_behaves_like "a middleware chain for mailer", false - - # Sidekiq documentation states that the worker class could be a string - # or a class reference. We should test for both - context "worker_class as string value" do - let(:worker_args) { [worker_class.to_s, { 'args' => job_args }, queue, redis_pool] } - let(:middleware_expected_args) { [worker_class.to_s, hash_including({ 'args' => job_args }), queue, redis_pool] } - - it_behaves_like "a middleware chain", false - it_behaves_like "a middleware chain for mailer", false - end - end - - context "when load balancing is enabled" do - it_behaves_like "a middleware chain", true - it_behaves_like "a middleware chain for mailer", true - end + it_behaves_like "a middleware chain" + it_behaves_like "a middleware chain for mailer" end end diff --git a/spec/lib/peek/views/active_record_spec.rb b/spec/lib/peek/views/active_record_spec.rb index 6d50922904e..c89f6a21b35 100644 --- a/spec/lib/peek/views/active_record_spec.rb +++ b/spec/lib/peek/views/active_record_spec.rb @@ -53,154 +53,82 @@ RSpec.describe Peek::Views::ActiveRecord, :request_store do allow(connection_primary_2).to receive(:transaction_open?).and_return(true) allow(connection_unknown).to receive(:transaction_open?).and_return(false) allow(::Gitlab::Database).to receive(:db_config_name).and_return('the_db_config_name') + + allow(Gitlab::Database::LoadBalancing).to receive(:db_role_for_connection).with(connection_replica).and_return(:replica) + allow(Gitlab::Database::LoadBalancing).to receive(:db_role_for_connection).with(connection_primary_1).and_return(:primary) + allow(Gitlab::Database::LoadBalancing).to receive(:db_role_for_connection).with(connection_primary_2).and_return(:primary) + allow(Gitlab::Database::LoadBalancing).to receive(:db_role_for_connection).with(connection_unknown).and_return(nil) end - context 'when database load balancing is not enabled' do - it 'subscribes and store data into peek views' do - Timecop.freeze(2021, 2, 23, 10, 0) do - ActiveSupport::Notifications.publish('sql.active_record', Time.current, Time.current + 1.second, '1', event_1) - ActiveSupport::Notifications.publish('sql.active_record', Time.current, Time.current + 2.seconds, '2', event_2) - ActiveSupport::Notifications.publish('sql.active_record', Time.current, Time.current + 3.seconds, '3', event_3) - ActiveSupport::Notifications.publish('sql.active_record', Time.current, Time.current + 4.seconds, '4', event_4) - end + it 'includes db role data and db_config_name name' do + Timecop.freeze(2021, 2, 23, 10, 0) do + ActiveSupport::Notifications.publish('sql.active_record', Time.current, Time.current + 1.second, '1', event_1) + ActiveSupport::Notifications.publish('sql.active_record', Time.current, Time.current + 2.seconds, '2', event_2) + ActiveSupport::Notifications.publish('sql.active_record', Time.current, Time.current + 3.seconds, '3', event_3) + ActiveSupport::Notifications.publish('sql.active_record', Time.current, Time.current + 4.seconds, '4', event_4) + end - expect(subject.results).to match( - calls: 4, - summary: { - "Cached" => 1, - "In a transaction" => 1 - }, - duration: '10000.00ms', - warnings: ["active-record duration: 10000.0 over 3000"], - details: contain_exactly( - a_hash_including( - start: be_a(Time), - cached: '', - transaction: '', - duration: 1000.0, - sql: 'SELECT * FROM users WHERE id = 10', - db_config_name: "Config name: the_db_config_name" - ), - a_hash_including( - start: be_a(Time), - cached: 'Cached', - transaction: '', - duration: 2000.0, - sql: 'SELECT * FROM users WHERE id = 10', - db_config_name: "Config name: the_db_config_name" - ), - a_hash_including( - start: be_a(Time), - cached: '', - transaction: 'In a transaction', - duration: 3000.0, - sql: 'UPDATE users SET admin = true WHERE id = 10', - db_config_name: "Config name: the_db_config_name" - ), - a_hash_including( - start: be_a(Time), - cached: '', - transaction: '', - duration: 4000.0, - sql: 'SELECT VERSION()', - db_config_name: "Config name: the_db_config_name" - ) + expect(subject.results).to match( + calls: 4, + summary: { + "Cached" => 1, + "In a transaction" => 1, + "Role: Primary" => 2, + "Role: Replica" => 1, + "Role: Unknown" => 1 + }, + duration: '10000.00ms', + warnings: ["active-record duration: 10000.0 over 3000"], + details: contain_exactly( + a_hash_including( + start: be_a(Time), + cached: '', + transaction: '', + duration: 1000.0, + sql: 'SELECT * FROM users WHERE id = 10', + db_role: 'Role: Primary', + db_config_name: "Config name: the_db_config_name" + ), + a_hash_including( + start: be_a(Time), + cached: 'Cached', + transaction: '', + duration: 2000.0, + sql: 'SELECT * FROM users WHERE id = 10', + db_role: 'Role: Replica', + db_config_name: "Config name: the_db_config_name" + ), + a_hash_including( + start: be_a(Time), + cached: '', + transaction: 'In a transaction', + duration: 3000.0, + sql: 'UPDATE users SET admin = true WHERE id = 10', + db_role: 'Role: Primary', + db_config_name: "Config name: the_db_config_name" + ), + a_hash_including( + start: be_a(Time), + cached: '', + transaction: '', + duration: 4000.0, + sql: 'SELECT VERSION()', + db_role: 'Role: Unknown', + db_config_name: "Config name: the_db_config_name" ) ) - end - - context 'when the GITLAB_MULTIPLE_DATABASE_METRICS env var is disabled' do - before do - stub_env('GITLAB_MULTIPLE_DATABASE_METRICS', nil) - end - - it 'does not include db_config_name field' do - ActiveSupport::Notifications.publish('sql.active_record', Time.current, Time.current + 1.second, '1', event_1) - - expect(subject.results[:details][0][:db_config_name]).to be_nil - end - end + ) end - context 'when database load balancing is enabled' do + context 'when the GITLAB_MULTIPLE_DATABASE_METRICS env var is disabled' do before do - allow(Gitlab::Database::LoadBalancing).to receive(:enable?).and_return(true) - allow(Gitlab::Database::LoadBalancing).to receive(:db_role_for_connection).with(connection_replica).and_return(:replica) - allow(Gitlab::Database::LoadBalancing).to receive(:db_role_for_connection).with(connection_primary_1).and_return(:primary) - allow(Gitlab::Database::LoadBalancing).to receive(:db_role_for_connection).with(connection_primary_2).and_return(:primary) - allow(Gitlab::Database::LoadBalancing).to receive(:db_role_for_connection).with(connection_unknown).and_return(nil) + stub_env('GITLAB_MULTIPLE_DATABASE_METRICS', nil) end - it 'includes db role data and db_config_name name' do - Timecop.freeze(2021, 2, 23, 10, 0) do - ActiveSupport::Notifications.publish('sql.active_record', Time.current, Time.current + 1.second, '1', event_1) - ActiveSupport::Notifications.publish('sql.active_record', Time.current, Time.current + 2.seconds, '2', event_2) - ActiveSupport::Notifications.publish('sql.active_record', Time.current, Time.current + 3.seconds, '3', event_3) - ActiveSupport::Notifications.publish('sql.active_record', Time.current, Time.current + 4.seconds, '4', event_4) - end - - expect(subject.results).to match( - calls: 4, - summary: { - "Cached" => 1, - "In a transaction" => 1, - "Role: Primary" => 2, - "Role: Replica" => 1, - "Role: Unknown" => 1 - }, - duration: '10000.00ms', - warnings: ["active-record duration: 10000.0 over 3000"], - details: contain_exactly( - a_hash_including( - start: be_a(Time), - cached: '', - transaction: '', - duration: 1000.0, - sql: 'SELECT * FROM users WHERE id = 10', - db_role: 'Role: Primary', - db_config_name: "Config name: the_db_config_name" - ), - a_hash_including( - start: be_a(Time), - cached: 'Cached', - transaction: '', - duration: 2000.0, - sql: 'SELECT * FROM users WHERE id = 10', - db_role: 'Role: Replica', - db_config_name: "Config name: the_db_config_name" - ), - a_hash_including( - start: be_a(Time), - cached: '', - transaction: 'In a transaction', - duration: 3000.0, - sql: 'UPDATE users SET admin = true WHERE id = 10', - db_role: 'Role: Primary', - db_config_name: "Config name: the_db_config_name" - ), - a_hash_including( - start: be_a(Time), - cached: '', - transaction: '', - duration: 4000.0, - sql: 'SELECT VERSION()', - db_role: 'Role: Unknown', - db_config_name: "Config name: the_db_config_name" - ) - ) - ) - end - - context 'when the GITLAB_MULTIPLE_DATABASE_METRICS env var is disabled' do - before do - stub_env('GITLAB_MULTIPLE_DATABASE_METRICS', nil) - end - - it 'does not include db_config_name field' do - ActiveSupport::Notifications.publish('sql.active_record', Time.current, Time.current + 1.second, '1', event_1) + it 'does not include db_config_name field' do + ActiveSupport::Notifications.publish('sql.active_record', Time.current, Time.current + 1.second, '1', event_1) - expect(subject.results[:details][0][:db_config_name]).to be_nil - end + expect(subject.results[:details][0][:db_config_name]).to be_nil end end end diff --git a/spec/models/application_record_spec.rb b/spec/models/application_record_spec.rb index efb92ddaea0..f0212da3041 100644 --- a/spec/models/application_record_spec.rb +++ b/spec/models/application_record_spec.rb @@ -194,7 +194,7 @@ RSpec.describe ApplicationRecord do end context 'with database load balancing' do - let(:session) { double(:session) } + let(:session) { Gitlab::Database::LoadBalancing::Session.new } before do allow(::Gitlab::Database::LoadBalancing::Session).to receive(:current).and_return(session) diff --git a/spec/models/ci/pipeline_spec.rb b/spec/models/ci/pipeline_spec.rb index a8395a83177..14c7f7f227b 100644 --- a/spec/models/ci/pipeline_spec.rb +++ b/spec/models/ci/pipeline_spec.rb @@ -2816,8 +2816,9 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do extra_update_queries = 4 # transition ... => :canceled, queue pop extra_generic_commit_status_validation_queries = 2 # name_uniqueness_across_types + extra_load_balancer_queries = 3 - expect(control2.count).to eq(control1.count + extra_update_queries + extra_generic_commit_status_validation_queries) + expect(control2.count).to eq(control1.count + extra_update_queries + extra_generic_commit_status_validation_queries + extra_load_balancer_queries) end end diff --git a/spec/models/ci/runner_spec.rb b/spec/models/ci/runner_spec.rb index 31e854c852e..192af5d22a9 100644 --- a/spec/models/ci/runner_spec.rb +++ b/spec/models/ci/runner_spec.rb @@ -383,9 +383,6 @@ RSpec.describe Ci::Runner do it 'sticks the runner to the primary and calls the original method' do runner = create(:ci_runner) - allow(Gitlab::Database::LoadBalancing).to receive(:enable?) - .and_return(true) - expect(Gitlab::Database::LoadBalancing::Sticking).to receive(:stick) .with(:runner, runner.id) diff --git a/spec/services/projects/update_service_spec.rb b/spec/services/projects/update_service_spec.rb index 115f3098185..3138dbc56db 100644 --- a/spec/services/projects/update_service_spec.rb +++ b/spec/services/projects/update_service_spec.rb @@ -441,26 +441,62 @@ RSpec.describe Projects::UpdateService do end end - context 'when updating #shared_runners', :https_pages_enabled do - let!(:pending_build) { create(:ci_pending_build, project: project, instance_runners_enabled: true) } + context 'when updating runners settings' do + let(:settings) do + { instance_runners_enabled: true, namespace_traversal_ids: [123] } + end - subject(:call_service) do - update_project(project, admin, shared_runners_enabled: shared_runners_enabled) + let!(:pending_build) do + create(:ci_pending_build, project: project, **settings) + end + + context 'when project has shared runners enabled' do + let(:project) { create(:project, shared_runners_enabled: true) } + + it 'updates builds queue when shared runners get disabled' do + expect { update_project(project, admin, shared_runners_enabled: false) } + .to change { pending_build.reload.instance_runners_enabled }.to(false) + + expect(pending_build.reload.instance_runners_enabled).to be false + end + end + + context 'when project has shared runners disabled' do + let(:project) { create(:project, shared_runners_enabled: false) } + + it 'updates builds queue when shared runners get enabled' do + expect { update_project(project, admin, shared_runners_enabled: true) } + .to not_change { pending_build.reload.instance_runners_enabled } + + expect(pending_build.reload.instance_runners_enabled).to be true + end end - context 'when shared runners is toggled' do - let(:shared_runners_enabled) { false } + context 'when project has group runners enabled' do + let(:project) { create(:project, group_runners_enabled: true) } + + before do + project.ci_cd_settings.update!(group_runners_enabled: true) + end + + it 'updates builds queue when group runners get disabled' do + update_project(project, admin, group_runners_enabled: false) - it 'updates ci pending builds' do - expect { call_service }.to change { pending_build.reload.instance_runners_enabled }.to(false) + expect(pending_build.reload.namespace_traversal_ids).to be_empty end end - context 'when shared runners is not toggled' do - let(:shared_runners_enabled) { true } + context 'when project has group runners disabled' do + let(:project) { create(:project, :in_subgroup, group_runners_enabled: false) } + + before do + project.reload.ci_cd_settings.update!(group_runners_enabled: false) + end + + it 'updates builds queue when group runners get enabled' do + update_project(project, admin, group_runners_enabled: true) - it 'updates ci pending builds' do - expect { call_service }.to not_change { pending_build.reload.instance_runners_enabled } + expect(pending_build.reload.namespace_traversal_ids).to include(project.namespace.id) end end end diff --git a/spec/services/user_project_access_changed_service_spec.rb b/spec/services/user_project_access_changed_service_spec.rb index f8835fefc84..3af23fafef5 100644 --- a/spec/services/user_project_access_changed_service_spec.rb +++ b/spec/services/user_project_access_changed_service_spec.rb @@ -47,8 +47,6 @@ RSpec.describe UserProjectAccessChangedService do let(:service) { UserProjectAccessChangedService.new([1, 2]) } before do - allow(Gitlab::Database::LoadBalancing).to receive(:enable?).and_return(true) - expect(AuthorizedProjectsWorker).to receive(:bulk_perform_and_wait) .with([[1], [2]]) .and_return(10) diff --git a/spec/support/database_load_balancing.rb b/spec/support/database_load_balancing.rb index f22c69ea613..d5573162c13 100644 --- a/spec/support/database_load_balancing.rb +++ b/spec/support/database_load_balancing.rb @@ -2,8 +2,6 @@ RSpec.configure do |config| config.before(:each, :db_load_balancing) do - allow(Gitlab::Database::LoadBalancing).to receive(:enable?).and_return(true) - config = Gitlab::Database::LoadBalancing::Configuration .new(ActiveRecord::Base, [Gitlab::Database.main.config['host']]) lb = ::Gitlab::Database::LoadBalancing::LoadBalancer.new(config) diff --git a/spec/support/shared_contexts/lib/gitlab/sidekiq_logging/structured_logger_shared_context.rb b/spec/support/shared_contexts/lib/gitlab/sidekiq_logging/structured_logger_shared_context.rb index 5a72b330707..b7966e25b38 100644 --- a/spec/support/shared_contexts/lib/gitlab/sidekiq_logging/structured_logger_shared_context.rb +++ b/spec/support/shared_contexts/lib/gitlab/sidekiq_logging/structured_logger_shared_context.rb @@ -39,17 +39,25 @@ RSpec.shared_context 'structured_logger' do ) end + let(:db_payload_defaults) do + metrics = + ::Gitlab::Metrics::Subscribers::ActiveRecord.load_balancing_metric_counter_keys + + ::Gitlab::Metrics::Subscribers::ActiveRecord.load_balancing_metric_duration_keys + + ::Gitlab::Metrics::Subscribers::ActiveRecord.db_counter_keys + + [:db_duration_s] + + metrics.each_with_object({}) do |key, result| + result[key.to_s] = 0 + end + end + let(:end_payload) do - start_payload.merge( + start_payload.merge(db_payload_defaults).merge( 'message' => 'TestWorker JID-da883554ee4fe414012f5f42: done: 0.0 sec', 'job_status' => 'done', 'duration_s' => 0.0, 'completed_at' => timestamp.to_f, - 'cpu_s' => 1.111112, - 'db_duration_s' => 0.0, - 'db_cached_count' => 0, - 'db_count' => 0, - 'db_write_count' => 0 + 'cpu_s' => 1.111112 ) end diff --git a/spec/support/shared_contexts/lib/gitlab/sidekiq_middleware/server_metrics_shared_context.rb b/spec/support/shared_contexts/lib/gitlab/sidekiq_middleware/server_metrics_shared_context.rb index 37e3805e4e0..0d992f33c61 100644 --- a/spec/support/shared_contexts/lib/gitlab/sidekiq_middleware/server_metrics_shared_context.rb +++ b/spec/support/shared_contexts/lib/gitlab/sidekiq_middleware/server_metrics_shared_context.rb @@ -15,6 +15,7 @@ RSpec.shared_context 'server metrics with mocked prometheus' do let(:redis_seconds_metric) { double('redis seconds metric') } let(:elasticsearch_seconds_metric) { double('elasticsearch seconds metric') } let(:elasticsearch_requests_total) { double('elasticsearch calls total metric') } + let(:load_balancing_metric) { double('load balancing metric') } before do allow(Gitlab::Metrics).to receive(:histogram).and_call_original @@ -31,6 +32,7 @@ RSpec.shared_context 'server metrics with mocked prometheus' do allow(Gitlab::Metrics).to receive(:counter).with(:sidekiq_jobs_retried_total, anything).and_return(retried_total_metric) allow(Gitlab::Metrics).to receive(:counter).with(:sidekiq_redis_requests_total, anything).and_return(redis_requests_total) allow(Gitlab::Metrics).to receive(:counter).with(:sidekiq_elasticsearch_requests_total, anything).and_return(elasticsearch_requests_total) + allow(Gitlab::Metrics).to receive(:counter).with(:sidekiq_load_balancing_count, anything).and_return(load_balancing_metric) allow(Gitlab::Metrics).to receive(:gauge).with(:sidekiq_running_jobs, anything, {}, :all).and_return(running_jobs_metric) allow(Gitlab::Metrics).to receive(:gauge).with(:sidekiq_concurrency, anything, {}, :all).and_return(concurrency_metric) diff --git a/spec/support/shared_examples/metrics/active_record_subscriber_shared_examples.rb b/spec/support/shared_examples/metrics/active_record_subscriber_shared_examples.rb index c6d6ff6bc1d..c06083ba952 100644 --- a/spec/support/shared_examples/metrics/active_record_subscriber_shared_examples.rb +++ b/spec/support/shared_examples/metrics/active_record_subscriber_shared_examples.rb @@ -4,14 +4,20 @@ RSpec.shared_examples 'store ActiveRecord info in RequestStore' do |db_role| let(:db_config_name) { ::Gitlab::Database.db_config_names.first } let(:expected_payload_defaults) do + result = {} metrics = ::Gitlab::Metrics::Subscribers::ActiveRecord.load_balancing_metric_counter_keys + - ::Gitlab::Metrics::Subscribers::ActiveRecord.load_balancing_metric_duration_keys + ::Gitlab::Metrics::Subscribers::ActiveRecord.db_counter_keys - metrics.each_with_object({}) do |key, result| + metrics.each do |key| result[key] = 0 end + + ::Gitlab::Metrics::Subscribers::ActiveRecord.load_balancing_metric_duration_keys.each do |key| + result[key] = 0.0 + end + + result end def transform_hash(hash, another_hash) @@ -36,8 +42,8 @@ RSpec.shared_examples 'store ActiveRecord info in RequestStore' do |db_role| "db_primary_#{db_config_name}_cached_count": record_cached_query ? 1 : 0, db_primary_count: record_query ? 1 : 0, "db_primary_#{db_config_name}_count": record_query ? 1 : 0, - db_primary_duration_s: record_query ? 0.002 : 0, - "db_primary_#{db_config_name}_duration_s": record_query ? 0.002 : 0, + db_primary_duration_s: record_query ? 0.002 : 0.0, + "db_primary_#{db_config_name}_duration_s": record_query ? 0.002 : 0.0, db_primary_wal_count: record_wal_query ? 1 : 0, "db_primary_#{db_config_name}_wal_count": record_wal_query ? 1 : 0, db_primary_wal_cached_count: record_wal_query && record_cached_query ? 1 : 0, @@ -52,19 +58,29 @@ RSpec.shared_examples 'store ActiveRecord info in RequestStore' do |db_role| "db_replica_#{db_config_name}_cached_count": record_cached_query ? 1 : 0, db_replica_count: record_query ? 1 : 0, "db_replica_#{db_config_name}_count": record_query ? 1 : 0, - db_replica_duration_s: record_query ? 0.002 : 0, - "db_replica_#{db_config_name}_duration_s": record_query ? 0.002 : 0, + db_replica_duration_s: record_query ? 0.002 : 0.0, + "db_replica_#{db_config_name}_duration_s": record_query ? 0.002 : 0.0, db_replica_wal_count: record_wal_query ? 1 : 0, "db_replica_#{db_config_name}_wal_count": record_wal_query ? 1 : 0, db_replica_wal_cached_count: record_wal_query && record_cached_query ? 1 : 0, "db_replica_#{db_config_name}_wal_cached_count": record_wal_query && record_cached_query ? 1 : 0 }) else - { + transform_hash(expected_payload_defaults, { db_count: record_query ? 1 : 0, db_write_count: record_write_query ? 1 : 0, - db_cached_count: record_cached_query ? 1 : 0 - } + db_cached_count: record_cached_query ? 1 : 0, + db_primary_cached_count: 0, + "db_primary_#{db_config_name}_cached_count": 0, + db_primary_count: 0, + "db_primary_#{db_config_name}_count": 0, + db_primary_duration_s: 0.0, + "db_primary_#{db_config_name}_duration_s": 0.0, + db_primary_wal_count: 0, + "db_primary_#{db_config_name}_wal_count": 0, + db_primary_wal_cached_count: 0, + "db_primary_#{db_config_name}_wal_cached_count": 0 + }) end expect(described_class.db_counter_payload).to eq(expected) @@ -89,7 +105,7 @@ RSpec.shared_examples 'store ActiveRecord info in RequestStore' do |db_role| end RSpec.shared_examples 'record ActiveRecord metrics in a metrics transaction' do |db_role| - let(:db_config_name) { ::Gitlab::Database.db_config_name(ApplicationRecord.connection) } + let(:db_config_name) { ::Gitlab::Database.db_config_name(ApplicationRecord.retrieve_connection) } it 'increments only db counters' do if record_query diff --git a/yarn.lock b/yarn.lock index 91c00287ed0..d0795b5e02f 100644 --- a/yarn.lock +++ b/yarn.lock @@ -1517,10 +1517,10 @@ dependencies: prosemirror-inputrules "^1.1.3" -"@tiptap/extension-code-block-lowlight@2.0.0-beta.38": - version "2.0.0-beta.38" - resolved "https://registry.yarnpkg.com/@tiptap/extension-code-block-lowlight/-/extension-code-block-lowlight-2.0.0-beta.38.tgz#2d0dc97ce2a25f4a8bd57fa179dbe591f3d66440" - integrity sha512-1JVmesa0RuKMYUKLbW34hCqHjPm5gRKv9NOU6jhI90xJHWG0pe+Am6MEfWosilB66DtcE43zP08aiKWgWBvI2A== +"@tiptap/extension-code-block-lowlight@2.0.0-beta.39": + version "2.0.0-beta.39" + resolved "https://registry.yarnpkg.com/@tiptap/extension-code-block-lowlight/-/extension-code-block-lowlight-2.0.0-beta.39.tgz#f890fd33dfdda946d747798f050e08729f40517d" + integrity sha512-mXiHKc2hL6PcAAIMEiSxtscw60ILdV5Mq2sNlZ9QPwGAkiizb7TI6nOPfBWU2QB9l78MtW0+FBgV0K20Qs30cQ== dependencies: "@tiptap/extension-code-block" "^2.0.0-beta.18" "@types/lowlight" "^0.0.3" @@ -9609,10 +9609,10 @@ prosemirror-keymap@^1.0.0, prosemirror-keymap@^1.1.2, prosemirror-keymap@^1.1.3, prosemirror-state "^1.0.0" w3c-keyname "^2.2.0" -prosemirror-markdown@^1.5.2: - version "1.5.2" - resolved "https://registry.yarnpkg.com/prosemirror-markdown/-/prosemirror-markdown-1.5.2.tgz#f188ad14caa8c2f499b4d3eb6082e19f1d9d366e" - integrity sha512-e9rVnRULVACEjCvIBOj5P2dGTE/nz8kKspA/GWZXVgtQgqeJEvQ+tUNeZkeRZJ2/I3XPzuWjeoWnwJmkMnIKrg== +prosemirror-markdown@^1.6.0: + version "1.6.0" + resolved "https://registry.yarnpkg.com/prosemirror-markdown/-/prosemirror-markdown-1.6.0.tgz#141c88e03c8892f2e93cf58b1382ab0b6088d012" + integrity sha512-y/gRpJIIrNArtkyMax7ypYafb+ZMjddbVHI+AwlcUfCLCCXK57cOmfBMKYVq9kdEKJYVdYHdoyWsVNn1nWLHUg== dependencies: markdown-it "^10.0.0" prosemirror-model "^1.0.0" @@ -9631,10 +9631,10 @@ prosemirror-schema-basic@^1.1.2: dependencies: prosemirror-model "^1.2.0" -prosemirror-schema-list@^1.1.4, prosemirror-schema-list@^1.1.5: - version "1.1.5" - resolved "https://registry.yarnpkg.com/prosemirror-schema-list/-/prosemirror-schema-list-1.1.5.tgz#e7ad9e337ea3d77da6d6a4250f3d7bd51ae980a4" - integrity sha512-9gadhga/wySVfb/iZ2vOpndbG0XroeLw0HkkZN5demNbOea6U5oQtJmvyYWC7ZVf3WkhmVdVsOXrllM9JcC20A== +prosemirror-schema-list@^1.1.4, prosemirror-schema-list@^1.1.5, prosemirror-schema-list@^1.1.6: + version "1.1.6" + resolved "https://registry.yarnpkg.com/prosemirror-schema-list/-/prosemirror-schema-list-1.1.6.tgz#c3e13fe2f74750e4a53ff88d798dc0c4ccca6707" + integrity sha512-aFGEdaCWmJzouZ8DwedmvSsL50JpRkqhQ6tcpThwJONVVmCgI36LJHtoQ4VGZbusMavaBhXXr33zyD2IVsTlkw== dependencies: prosemirror-model "^1.0.0" prosemirror-transform "^1.0.0" |