Merge branch 'mb-add-sentry-to-application-settings' into 'master'

Add Sentry config to application settings

See merge request gitlab-org/gitlab!73381

app/helpers/application_settings_helper.rb

@@ -404,6 +404,10 @@ module ApplicationSettingsHelper
       :keep_latest_artifact,
       :whats_new_variant,
       :user_deactivation_emails_enabled,
+      :sentry_enabled,
+      :sentry_dsn,
+      :sentry_clientside_dsn,
+      :sentry_environment,
       :sidekiq_job_limiter_mode,
       :sidekiq_job_limiter_compression_threshold_bytes,
       :sidekiq_job_limiter_limit_bytes,

app/models/application_setting.rb

@@ -536,6 +536,18 @@ class ApplicationSetting < ApplicationRecord
   validates :sidekiq_job_limiter_limit_bytes,
             numericality: { only_integer: true, greater_than_or_equal_to: 0 }
 
+  validates :sentry_enabled,
+    inclusion: { in: [true, false], message: _('must be a boolean value') }
+  validates :sentry_dsn,
+    addressable_url: true, presence: true, length: { maximum: 255 },
+    if: :sentry_enabled?
+  validates :sentry_clientside_dsn,
+    addressable_url: true, allow_blank: true, length: { maximum: 255 },
+    if: :sentry_enabled?
+  validates :sentry_environment,
+    presence: true, length: { maximum: 255 },
+    if: :sentry_enabled?
+
   attr_encrypted :asset_proxy_secret_key,
                  mode: :per_attribute_iv,
                  key: Settings.attr_encrypted_db_key_base_truncated,

app/views/admin/application_settings/_sentry.html.haml

+= form_for @application_setting, url: metrics_and_profiling_admin_application_settings_path(anchor: 'js-sentry-settings'), html: { class: 'fieldset-form', id: 'sentry-settings' } do |f|
+  = form_errors(@application_setting)
+
+  %span.text-muted
+    = _('Changing any setting here requires an application restart')
+
+  %fieldset
+    .form-group
+      .form-check
+        = f.check_box :sentry_enabled, class: 'form-check-input'
+        = f.label :sentry_enabled, _('Enable Sentry error tracking'), class: 'form-check-label'
+    .form-group
+      = f.label :sentry_dsn, _('DSN'), class: 'label-light'
+      = f.text_field :sentry_dsn, class: 'form-control gl-form-input', placeholder: 'https://public@sentry.example.com/1'
+    .form-group
+      = f.label :sentry_clientside_dsn, _('Clientside DSN'), class: 'label-light'
+      = f.text_field :sentry_clientside_dsn, class: 'form-control gl-form-input', placeholder: 'https://public@sentry.example.com/2'
+    .form-group
+      = f.label :sentry_environment, _('Environment'), class: 'label-light'
+      = f.text_field :sentry_environment, class: 'form-control gl-form-input', placeholder: Rails.env
+
+  = f.submit _('Save changes'), class: 'gl-button btn btn-confirm'

app/views/admin/application_settings/metrics_and_profiling.html.haml

@@ -54,3 +54,15 @@
     = render 'usage'
 
 = render_if_exists 'admin/application_settings/pseudonymizer_settings', expanded: expanded_by_default?
+
+- if Feature.enabled?(:configure_sentry_in_application_settings, default_enabled: :yaml)
+  %section.settings.as-sentry.no-animate#js-sentry-settings{ class: ('expanded' if expanded_by_default?), data: { qa_selector: 'sentry_settings_content' } }
+    .settings-header
+      %h4
+        = _('Sentry')
+      %button.btn.gl-button.btn-default.js-settings-toggle{ type: 'button' }
+        = expanded_by_default? ? _('Collapse') : _('Expand')
+      %p
+        = _('Configure Sentry integration for error tracking')
+    .settings-content
+      = render 'sentry'

config/feature_flags/development/configure_sentry_in_application_settings.yml

+---
+name: configure_sentry_in_application_settings
+introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/73381
+rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/344832
+milestone: '14.5'
+type: development
+group: group::pipeline execution
+default_enabled: false

db/migrate/20211021125908_add_sentry_settings_to_application_settings.rb

+# frozen_string_literal: true
+
+class AddSentrySettingsToApplicationSettings < Gitlab::Database::Migration[1.0]
+  # rubocop:disable Migration/AddLimitToTextColumns
+  def change
+    add_column :application_settings, :sentry_enabled, :boolean, default: false, null: false
+    add_column :application_settings, :sentry_dsn,            :text
+    add_column :application_settings, :sentry_clientside_dsn, :text
+    add_column :application_settings, :sentry_environment,    :text
+  end
+  # rubocop:enable Migration/AddLimitToTextColumns
+end

db/migrate/20211021134458_add_limits_to_sentry_settings_on_application_settings.rb

+# frozen_string_literal: true
+
+class AddLimitsToSentrySettingsOnApplicationSettings < Gitlab::Database::Migration[1.0]
+  disable_ddl_transaction!
+
+  def up
+    add_text_limit :application_settings, :sentry_dsn,            255
+    add_text_limit :application_settings, :sentry_clientside_dsn, 255
+    add_text_limit :application_settings, :sentry_environment,    255
+  end
+
+  def down
+    remove_text_limit :application_settings, :sentry_dsn
+    remove_text_limit :application_settings, :sentry_clientside_dsn
+    remove_text_limit :application_settings, :sentry_environment
+  end
+end

db/schema_migrations/20211021125908

+d6fbe3efc3e45b750d82e277e30b7b0048b960d9f9f5b4f7c6a7a1ed869e76b5
\ No newline at end of file

db/schema_migrations/20211021134458

+1baa8db0d42a8d99e48b61930f5c42d1af5f86555488419b6551e1dbf417d3ad
\ No newline at end of file

db/structure.sql

@@ -10457,6 +10457,10 @@ CREATE TABLE application_settings (
     encrypted_content_validation_api_key bytea,
     encrypted_content_validation_api_key_iv bytea,
     content_validation_endpoint_enabled boolean DEFAULT false NOT NULL,
+    sentry_enabled boolean DEFAULT false NOT NULL,
+    sentry_dsn text,
+    sentry_clientside_dsn text,
+    sentry_environment text,
     CONSTRAINT app_settings_container_reg_cleanup_tags_max_list_size_positive CHECK ((container_registry_cleanup_tags_service_max_list_size >= 0)),
     CONSTRAINT app_settings_dep_proxy_ttl_policies_worker_capacity_positive CHECK ((dependency_proxy_ttl_group_policy_worker_capacity >= 0)),
     CONSTRAINT app_settings_ext_pipeline_validation_service_url_text_limit CHECK ((char_length(external_pipeline_validation_service_url) <= 255)),
@@ -10465,9 +10469,12 @@ CREATE TABLE application_settings (
     CONSTRAINT app_settings_yaml_max_size_positive CHECK ((max_yaml_size_bytes > 0)),
     CONSTRAINT check_17d9558205 CHECK ((char_length((kroki_url)::text) <= 1024)),
     CONSTRAINT check_2dba05b802 CHECK ((char_length(gitpod_url) <= 255)),
+    CONSTRAINT check_3def0f1829 CHECK ((char_length(sentry_clientside_dsn) <= 255)),
+    CONSTRAINT check_4f8b811780 CHECK ((char_length(sentry_dsn) <= 255)),
     CONSTRAINT check_51700b31b5 CHECK ((char_length(default_branch_name) <= 255)),
     CONSTRAINT check_57123c9593 CHECK ((char_length(help_page_documentation_base_url) <= 255)),
     CONSTRAINT check_5a84c3ffdc CHECK ((char_length(content_validation_endpoint_url) <= 255)),
+    CONSTRAINT check_5bcba483c4 CHECK ((char_length(sentry_environment) <= 255)),
     CONSTRAINT check_718b4458ae CHECK ((char_length(personal_access_token_prefix) <= 20)),
     CONSTRAINT check_7227fad848 CHECK ((char_length(rate_limiting_response_text) <= 255)),
     CONSTRAINT check_85a39b68ff CHECK ((char_length(encrypted_ci_jwt_signing_key_iv) <= 255)),

ee/spec/models/application_setting_spec.rb

@@ -178,6 +178,42 @@ RSpec.describe ApplicationSetting do
       it { is_expected.to allow_value(nil).for(:new_user_signups_cap) }
       it { is_expected.not_to allow_value(max_active_user_count + 1).for(:new_user_signups_cap) }
     end
+
+    context 'Sentry validations' do
+      context 'when Sentry is enabled' do
+        before do
+          setting.sentry_enabled = true
+        end
+
+        it { is_expected.to allow_value(false).for(:sentry_enabled) }
+        it { is_expected.not_to allow_value(nil).for(:sentry_enabled) }
+
+        it { is_expected.to allow_value('http://example.com').for(:sentry_dsn) }
+        it { is_expected.not_to allow_value("http://#{'a' * 255}.com").for(:sentry_dsn) }
+        it { is_expected.not_to allow_value('example').for(:sentry_dsn) }
+        it { is_expected.not_to allow_value(nil).for(:sentry_dsn) }
+
+        it { is_expected.to allow_value('http://example.com').for(:sentry_clientside_dsn) }
+        it { is_expected.to allow_value(nil).for(:sentry_clientside_dsn) }
+        it { is_expected.not_to allow_value('example').for(:sentry_clientside_dsn) }
+        it { is_expected.not_to allow_value("http://#{'a' * 255}.com").for(:sentry_clientside_dsn) }
+
+        it { is_expected.to allow_value('production').for(:sentry_environment) }
+        it { is_expected.not_to allow_value(nil).for(:sentry_environment) }
+        it { is_expected.not_to allow_value('a' * 256).for(:sentry_environment) }
+      end
+
+      context 'when Sentry is disabled' do
+        before do
+          setting.sentry_enabled = false
+        end
+
+        it { is_expected.not_to allow_value(nil).for(:sentry_enabled) }
+        it { is_expected.to allow_value(nil).for(:sentry_dsn) }
+        it { is_expected.to allow_value(nil).for(:sentry_clientside_dsn) }
+        it { is_expected.to allow_value(nil).for(:sentry_environment) }
+      end
+    end
   end
 
   describe '#should_check_namespace_plan?' do

locale/gitlab.pot

@@ -6553,6 +6553,9 @@ msgstr ""
 msgid "Changes to the title have not been saved"
 msgstr ""
 
+msgid "Changing any setting here requires an application restart"
+msgstr ""
+
 msgid "Changing group URL can have unintended side effects."
 msgstr ""
 
@@ -7196,6 +7199,9 @@ msgstr ""
 msgid "Clients"
 msgstr ""
 
+msgid "Clientside DSN"
+msgstr ""
+
 msgid "Clone"
 msgstr ""
 
@@ -8689,6 +8695,9 @@ msgstr ""
 msgid "Configure Secret Detection in `.gitlab-ci.yml`, creating this file if it does not already exist"
 msgstr ""
 
+msgid "Configure Sentry integration for error tracking"
+msgstr ""
+
 msgid "Configure Tracing"
 msgstr ""
 
@@ -10397,6 +10406,9 @@ msgstr ""
 msgid "DORA4Metrics|The chart displays the median time between a merge request being merged and deployed to production environment(s) that are based on the %{linkStart}deployment_tier%{linkEnd} value."
 msgstr ""
 
+msgid "DSN"
+msgstr ""
+
 msgid "Dashboard"
 msgstr ""
 
@@ -12709,6 +12721,9 @@ msgstr ""
 msgid "Enable SSL verification"
 msgstr ""
 
+msgid "Enable Sentry error tracking"
+msgstr ""
+
 msgid "Enable Service Ping"
 msgstr ""
 

spec/requests/api/settings_spec.rb

@@ -612,5 +612,46 @@ RSpec.describe API::Settings, 'Settings', :do_not_mock_admin_mode_setting do
         expect(json_response.slice(*settings.keys)).to eq(settings)
       end
     end
+
+    context 'Sentry settings' do
+      let(:settings) do
+        {
+          sentry_enabled: true,
+          sentry_dsn: 'http://sentry.example.com',
+          sentry_clientside_dsn: 'http://sentry.example.com',
+          sentry_environment: 'production'
+        }
+      end
+
+      let(:attribute_names) { settings.keys.map(&:to_s) }
+
+      it 'includes the attributes in the API' do
+        get api('/application/settings', admin)
+
+        expect(response).to have_gitlab_http_status(:ok)
+        attribute_names.each do |attribute|
+          expect(json_response.keys).to include(attribute)
+        end
+      end
+
+      it 'allows updating the settings' do
+        put api('/application/settings', admin), params: settings
+
+        expect(response).to have_gitlab_http_status(:ok)
+        settings.each do |attribute, value|
+          expect(ApplicationSetting.current.public_send(attribute)).to eq(value)
+        end
+      end
+
+      context 'missing sentry_dsn value when sentry_enabled is true' do
+        it 'returns a blank parameter error message' do
+          put api('/application/settings', admin), params: { sentry_enabled: true }
+
+          expect(response).to have_gitlab_http_status(:bad_request)
+          message = json_response['message']
+          expect(message["sentry_dsn"]).to include(a_string_matching("can't be blank"))
+        end
+      end
+    end
   end
 end