diff options
| -rw-r--r-- | app/controllers/admin/application_settings_controller.rb | 2 | ||||
| -rw-r--r-- | app/models/application_setting.rb | 37 | ||||
| -rw-r--r-- | app/views/admin/application_settings/_form.html.haml | 16 | ||||
| -rw-r--r-- | config/gitlab.yml.example | 5 | ||||
| -rw-r--r-- | config/initializers/1_settings.rb | 6 | ||||
| -rw-r--r-- | db/migrate/20170706151212_add_performance_bar_allowed_group_id_to_application_settings.rb | 9 | ||||
| -rw-r--r-- | db/schema.rb | 3 | ||||
| -rw-r--r-- | doc/administration/monitoring/performance/img/performance_bar_configuration_settings.png | bin | 0 -> 20385 bytes | |||
| -rw-r--r-- | doc/administration/monitoring/performance/performance_bar.md | 50 | ||||
| -rw-r--r-- | lib/gitlab/performance_bar.rb | 18 | ||||
| -rw-r--r-- | spec/features/user_can_display_performance_bar_spec.rb | 16 | ||||
| -rw-r--r-- | spec/lib/gitlab/performance_bar_spec.rb | 4 | ||||
| -rw-r--r-- | spec/models/application_setting_spec.rb | 145 |
13 files changed, 257 insertions, 54 deletions
diff --git a/app/controllers/admin/application_settings_controller.rb b/app/controllers/admin/application_settings_controller.rb index f978ce478c7..1cc060e4de8 100644 --- a/app/controllers/admin/application_settings_controller.rb +++ b/app/controllers/admin/application_settings_controller.rb @@ -126,6 +126,8 @@ class Admin::ApplicationSettingsController < Admin::ApplicationController :metrics_port, :metrics_sample_interval, :metrics_timeout, + :performance_bar_allowed_group_id, + :performance_bar_enabled, :recaptcha_enabled, :recaptcha_private_key, :recaptcha_site_key, diff --git a/app/models/application_setting.rb b/app/models/application_setting.rb index 668caef0d2c..c6d8e45c86d 100644 --- a/app/models/application_setting.rb +++ b/app/models/application_setting.rb @@ -234,6 +234,7 @@ class ApplicationSetting < ActiveRecord::Base koding_url: nil, max_artifacts_size: Settings.artifacts['max_size'], max_attachment_size: Settings.gitlab['max_attachment_size'], + performance_bar_allowed_group_id: nil, plantuml_enabled: false, plantuml_url: nil, recaptcha_enabled: false, @@ -336,6 +337,42 @@ class ApplicationSetting < ActiveRecord::Base super(levels.map { |level| Gitlab::VisibilityLevel.level_value(level) }) end + def performance_bar_allowed_group_id=(group_full_path) + group = Group.find_by_full_path(group_full_path) + return unless group && group.id != performance_bar_allowed_group_id + + super(group.id) + Gitlab::PerformanceBar.expire_allowed_user_ids_cache + end + + def performance_bar_allowed_group + Group.find_by_id(performance_bar_allowed_group_id) + end + + # Return true is the Performance Bar is available globally or for the + # `performance_team` feature group + def performance_bar_enabled? + feature = Feature.get(:performance_bar) + + feature.on? || feature.groups_value.include?('performance_team') + end + + # - If `enable` is true, enable the `performance_bar` feature for the + # `performance_team` feature group + # - If `enable` is false, disable the `performance_bar` feature globally + def performance_bar_enabled=(enable) + feature = Feature.get(:performance_bar) + performance_bar_on = performance_bar_enabled? + + if enable && !performance_bar_on + feature.enable_group(:performance_team) + Gitlab::PerformanceBar.expire_allowed_user_ids_cache + elsif !enable && performance_bar_on + feature.disable + Gitlab::PerformanceBar.expire_allowed_user_ids_cache + end + end + # Choose one of the available repository storage options. Currently all have # equal weighting. def pick_repository_storage diff --git a/app/views/admin/application_settings/_form.html.haml b/app/views/admin/application_settings/_form.html.haml index 5f5eeb8b9a9..7f1e13c7989 100644 --- a/app/views/admin/application_settings/_form.html.haml +++ b/app/views/admin/application_settings/_form.html.haml @@ -333,6 +333,22 @@ Environment variable `prometheus_multiproc_dir` does not exist or is not pointing to a valid directory. %fieldset + %legend Profiling - Performance Bar + %p + Enable the Performance Bar for a given group. + = link_to icon('question-circle'), help_page_path('administration/monitoring/performance/performance_bar') + .form-group + .col-sm-offset-2.col-sm-10 + .checkbox + = f.label :performance_bar_enabled do + = f.check_box :performance_bar_enabled + Enable the Performance Bar + .form-group + = f.label :performance_bar_allowed_group_id, 'Allowed group', class: 'control-label col-sm-2' + .col-sm-10 + = f.text_field :performance_bar_allowed_group_id, class: 'form-control', placeholder: 'my-org/my-group', value: @application_setting.performance_bar_allowed_group&.full_path + + %fieldset %legend Background Jobs %p These settings require a restart to take effect. diff --git a/config/gitlab.yml.example b/config/gitlab.yml.example index d6284f26814..4b81fd90f59 100644 --- a/config/gitlab.yml.example +++ b/config/gitlab.yml.example @@ -459,11 +459,6 @@ production: &base # is the normal way to deploy Gitaly. token: - # Performance bar settings - performance_bar: - # This setting controls what group can see the performance bar. - # allowed_group: my-org/performance-group - # # 4. Advanced settings # ========================== diff --git a/config/initializers/1_settings.rb b/config/initializers/1_settings.rb index 55664399111..cb11d2c34f4 100644 --- a/config/initializers/1_settings.rb +++ b/config/initializers/1_settings.rb @@ -486,12 +486,6 @@ Settings['gitaly'] ||= Settingslogic.new({}) Settings.gitaly['enabled'] = true if Settings.gitaly['enabled'].nil? # -# Performance bar -# -Settings['performance_bar'] ||= Settingslogic.new({}) -Settings.performance_bar['allowed_group'] ||= nil - -# # Webpack settings # Settings['webpack'] ||= Settingslogic.new({}) diff --git a/db/migrate/20170706151212_add_performance_bar_allowed_group_id_to_application_settings.rb b/db/migrate/20170706151212_add_performance_bar_allowed_group_id_to_application_settings.rb new file mode 100644 index 00000000000..fe9970ddc71 --- /dev/null +++ b/db/migrate/20170706151212_add_performance_bar_allowed_group_id_to_application_settings.rb @@ -0,0 +1,9 @@ +class AddPerformanceBarAllowedGroupIdToApplicationSettings < ActiveRecord::Migration + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + + def change + add_column :application_settings, :performance_bar_allowed_group_id, :integer + end +end diff --git a/db/schema.rb b/db/schema.rb index 40f30a10a01..20dfe13bc7b 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -11,7 +11,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 20170703102400) do +ActiveRecord::Schema.define(version: 20170706151212) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -126,6 +126,7 @@ ActiveRecord::Schema.define(version: 20170703102400) do t.boolean "prometheus_metrics_enabled", default: false, null: false t.boolean "help_page_hide_commercial_content", default: false t.string "help_page_support_url" + t.integer "performance_bar_allowed_group_id" end create_table "audit_events", force: :cascade do |t| diff --git a/doc/administration/monitoring/performance/img/performance_bar_configuration_settings.png b/doc/administration/monitoring/performance/img/performance_bar_configuration_settings.png Binary files differnew file mode 100644 index 00000000000..2d64ef8c5fc --- /dev/null +++ b/doc/administration/monitoring/performance/img/performance_bar_configuration_settings.png diff --git a/doc/administration/monitoring/performance/performance_bar.md b/doc/administration/monitoring/performance/performance_bar.md index 409a74d2f91..6ee21eae194 100644 --- a/doc/administration/monitoring/performance/performance_bar.md +++ b/doc/administration/monitoring/performance/performance_bar.md @@ -1,9 +1,5 @@ # Performance Bar ->**Note:** -Available since GitLab 9.4. For installations from source you'll have to -configure it yourself. - A Performance Bar can be displayed, to dig into the performance of a page. When activated, it looks as follows: @@ -21,38 +17,44 @@ It allows you to: - profile the code used to generate the page, line by line  -## Enable the Performance Bar +## Enable the Performance Bar via the Admin panel + +GitLab Performance Bar is disabled by default. To enable it for a given group, +navigate to the Admin area in **Settings > Profiling - Performance Bar** +(`/admin/application_settings`). + +The only required setting you need to set is the full path of the group that +will be allowed to display the Performance Bar. +Make sure _Enable the Performance Bar_ is checked and hit +**Save** to save the changes. + +--- + + -By default, the Performance Bar is disabled. You can enable it for a group -and/or users. Note that it's possible to enable it for a group and for -individual users at the same time. +--- -1. Edit `/etc/gitlab/gitlab.rb` -1. Find the following line, and set it to the group's **full path** that should -be allowed to use the Performance Bar: +## Enable the Performance Bar via the API - ```ruby - gitlab_rails['performance_bar_allowed_group'] = 'your-org/your-performance-group' - ``` +Under the hood, the Performance Bar activation is done via the `performance_bar` +[Feature Flag](../../../development/features_flags.md). -1. Save the file and [reconfigure GitLab][reconfigure] for the changes to - take effect -1. The Performance Bar can then be enabled via the - [Features API](../../../api/features.md#set-or-create-a-feature) (see below). +That means you can also enable or disable it via the +[Features API](../../../api/features.md#set-or-create-a-feature). -### Enable for the `performance_team` feature group +### For the `performance_team` feature group -The `performance_team` feature group maps to the group specified by the -`performance_bar_allowed_group` setting you've set in the previous step. +The `performance_team` feature group maps to the group specified in your [Admin +area](#enable-the-performance-bar-via-the-admin-panel). ``` curl --data "feature_group=performance_team" --data "value=true" --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" https://gitlab.example.com/api/v4/features/performance_bar ``` -### Enable for specific users +### For specific users -It's possible to enable the Performance Bar for specific users in addition to a -group, or even instead of a group: +It's also possible to enable the Performance Bar for specific users in addition +to a group, or even instead of a group: ``` curl --data "user=my_username" --data "value=true" --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" https://gitlab.example.com/api/v4/features/performance_bar diff --git a/lib/gitlab/performance_bar.rb b/lib/gitlab/performance_bar.rb index f48d0506994..aa033571de0 100644 --- a/lib/gitlab/performance_bar.rb +++ b/lib/gitlab/performance_bar.rb @@ -1,27 +1,29 @@ module Gitlab module PerformanceBar + include Gitlab::CurrentSettings + ALLOWED_USER_IDS_KEY = 'performance_bar_allowed_user_ids'.freeze # The time (in seconds) after which a set of allowed user IDs is expired # automatically. ALLOWED_USER_IDS_TIME_TO_LIVE = 10.minutes def self.enabled?(current_user = nil) - Feature.enabled?(:gitlab_performance_bar, current_user) + Feature.enabled?(:performance_bar, current_user) end def self.allowed_user?(user) - return false unless allowed_group_name + return false unless allowed_group_id allowed_user_ids.include?(user.id) end - def self.allowed_group_name - Gitlab.config.performance_bar.allowed_group + def self.allowed_group_id + current_application_settings.performance_bar_allowed_group_id end def self.allowed_user_ids - Rails.cache.fetch(cache_key, expires_in: ALLOWED_USER_IDS_TIME_TO_LIVE) do - group = Group.find_by_full_path(allowed_group_name) + Rails.cache.fetch(ALLOWED_USER_IDS_KEY, expires_in: ALLOWED_USER_IDS_TIME_TO_LIVE) do + group = Group.find_by_id(allowed_group_id) if group GroupMembersFinder.new(group).execute.pluck(:user_id) @@ -31,8 +33,8 @@ module Gitlab end end - def self.cache_key - "#{ALLOWED_USER_IDS_KEY}:#{allowed_group_name}" + def self.expire_allowed_user_ids_cache + Rails.cache.delete(ALLOWED_USER_IDS_KEY) end end end diff --git a/spec/features/user_can_display_performance_bar_spec.rb b/spec/features/user_can_display_performance_bar_spec.rb index 24fff1a3052..a3adb8c2882 100644 --- a/spec/features/user_can_display_performance_bar_spec.rb +++ b/spec/features/user_can_display_performance_bar_spec.rb @@ -38,17 +38,17 @@ describe 'User can display performance bar', :js do visit root_path end - context 'when the gitlab_performance_bar feature is disabled' do + context 'when the performance_bar feature is disabled' do before do - Feature.disable('gitlab_performance_bar') + Feature.disable(:performance_bar) end it_behaves_like 'performance bar is disabled' end - context 'when the gitlab_performance_bar feature is enabled' do + context 'when the performance_bar feature is enabled' do before do - Feature.enable('gitlab_performance_bar') + Feature.enable(:performance_bar) end it_behaves_like 'performance bar is disabled' @@ -62,17 +62,17 @@ describe 'User can display performance bar', :js do visit root_path end - context 'when the gitlab_performance_bar feature is disabled' do + context 'when the performance_bar feature is disabled' do before do - Feature.disable('gitlab_performance_bar') + Feature.disable(:performance_bar) end it_behaves_like 'performance bar is disabled' end - context 'when the gitlab_performance_bar feature is enabled' do + context 'when the performance_bar feature is enabled' do before do - Feature.enable('gitlab_performance_bar') + Feature.enable(:performance_bar) end it_behaves_like 'performance bar is enabled' diff --git a/spec/lib/gitlab/performance_bar_spec.rb b/spec/lib/gitlab/performance_bar_spec.rb index 6414bdb80ed..e77a1ebbb0d 100644 --- a/spec/lib/gitlab/performance_bar_spec.rb +++ b/spec/lib/gitlab/performance_bar_spec.rb @@ -10,7 +10,7 @@ describe Gitlab::PerformanceBar do actor = double('actor') expect(Feature).to receive(:enabled?) - .with(:gitlab_performance_bar, actor).and_return(false) + .with(:performance_bar, actor).and_return(false) expect(described_class.enabled?(actor)).to be_falsy end @@ -19,7 +19,7 @@ describe Gitlab::PerformanceBar do actor = double('actor') expect(Feature).to receive(:enabled?) - .with(:gitlab_performance_bar, actor).and_return(true) + .with(:performance_bar, actor).and_return(true) expect(described_class.enabled?(actor)).to be_truthy end diff --git a/spec/models/application_setting_spec.rb b/spec/models/application_setting_spec.rb index 166a4474abf..4b7281d593a 100644 --- a/spec/models/application_setting_spec.rb +++ b/spec/models/application_setting_spec.rb @@ -214,6 +214,151 @@ describe ApplicationSetting, models: true do end end + describe 'performance bar settings' do + before do + Flipper.unregister_groups + Flipper.register(:performance_team) + end + + after do + Flipper.unregister_groups + end + + describe 'performance_bar_allowed_group_id=' do + it 'does not persist an invalid group path' do + setting.performance_bar_allowed_group_id = 'foo' + + expect(setting.performance_bar_allowed_group_id).to be_nil + end + + context 'with a path to an existing group' do + let(:group) { create(:group) } + + it 'persists a valid group path and clears allowed user IDs cache' do + expect(Gitlab::PerformanceBar).to receive(:expire_allowed_user_ids_cache) + + setting.performance_bar_allowed_group_id = group.full_path + + expect(setting.performance_bar_allowed_group_id).to eq(group.id) + end + + context 'when the given path is the same' do + before do + setting.performance_bar_allowed_group_id = group.full_path + end + + it 'clears the cached allowed user IDs' do + expect(Gitlab::PerformanceBar).not_to receive(:expire_allowed_user_ids_cache) + + setting.performance_bar_allowed_group_id = group.full_path + end + end + end + end + + describe 'performance_bar_allowed_group' do + context 'with no performance_bar_allowed_group_id saved' do + it 'returns nil' do + expect(setting.performance_bar_allowed_group).to be_nil + end + end + + context 'with a performance_bar_allowed_group_id saved' do + let(:group) { create(:group) } + + before do + setting.performance_bar_allowed_group_id = group.full_path + end + + it 'returns the group' do + expect(setting.performance_bar_allowed_group).to eq(group) + end + end + end + + describe 'performance_bar_enabled?' do + context 'with the Performance Bar is enabled globally' do + before do + Feature.enable(:performance_bar) + end + + it 'returns true' do + expect(setting).to be_performance_bar_enabled + end + end + + context 'with the Performance Bar is enabled for the performance_team group' do + before do + Feature.enable_group(:performance_bar, :performance_team) + end + + it 'returns true' do + expect(setting).to be_performance_bar_enabled + end + end + + context 'with the Performance Bar is enabled for a specific user' do + before do + Feature.enable(:performance_team, create(:user)) + end + + it 'returns false' do + expect(setting).not_to be_performance_bar_enabled + end + end + end + + describe 'performance_bar_enabled=' do + context 'when the performance bar is enabled' do + before do + Feature.enable(:performance_bar) + end + + context 'when passing true' do + it 'does not clear allowed user IDs cache' do + expect(Gitlab::PerformanceBar).not_to receive(:expire_allowed_user_ids_cache) + setting.performance_bar_enabled = true + + expect(setting).to be_performance_bar_enabled + end + end + + context 'when passing false' do + it 'disables the performance bar and clears allowed user IDs cache' do + expect(Gitlab::PerformanceBar).to receive(:expire_allowed_user_ids_cache) + setting.performance_bar_enabled = false + + expect(setting).not_to be_performance_bar_enabled + end + end + end + + context 'when the performance bar is disabled' do + before do + Feature.disable(:performance_bar) + end + + context 'when passing true' do + it 'enables the performance bar and clears allowed user IDs cache' do + expect(Gitlab::PerformanceBar).to receive(:expire_allowed_user_ids_cache) + setting.performance_bar_enabled = true + + expect(setting).to be_performance_bar_enabled + end + end + + context 'when passing false' do + it 'does not clear allowed user IDs cache' do + expect(Gitlab::PerformanceBar).not_to receive(:expire_allowed_user_ids_cache) + setting.performance_bar_enabled = false + + expect(setting).not_to be_performance_bar_enabled + end + end + end + end + end + describe 'usage ping settings' do context 'when the usage ping is disabled in gitlab.yml' do before do |
