From aebb2f70257882dd530b820f3cfdd67621d2a3fd Mon Sep 17 00:00:00 2001 From: Roger Meier Date: Sun, 7 Apr 2019 21:21:52 +0200 Subject: [PATCH] feat: allow Sentry configuration to be passed on gitlab.yml --- app/assets/javascripts/raven/index.js | 7 +++- app/assets/javascripts/raven/raven_config.js | 2 +- .../application_setting_implementation.rb | 16 ++++++++ .../application_settings/_logging.html.haml | 7 ++++ .../unreleased/feat-sentry-environment.yml | 5 +++ config/gitlab.yml.example | 7 ++++ config/initializers/1_settings.rb | 8 ++++ config/initializers/sentry.rb | 1 + lib/gitlab/gon_helper.rb | 7 +++- spec/javascripts/raven/index_spec.js | 10 ++--- spec/javascripts/raven/raven_config_spec.js | 10 ++--- .../application_setting_examples.rb | 37 +++++++++++++++++++ 12 files changed, 103 insertions(+), 14 deletions(-) create mode 100644 changelogs/unreleased/feat-sentry-environment.yml diff --git a/app/assets/javascripts/raven/index.js b/app/assets/javascripts/raven/index.js index edc2293915f..4dd0175e528 100644 --- a/app/assets/javascripts/raven/index.js +++ b/app/assets/javascripts/raven/index.js @@ -4,8 +4,11 @@ const index = function index() { RavenConfig.init({ sentryDsn: gon.sentry_dsn, currentUserId: gon.current_user_id, - whitelistUrls: [gon.gitlab_url], - isProduction: process.env.NODE_ENV, + whitelistUrls: + process.env.NODE_ENV === 'production' + ? [gon.gitlab_url] + : [gon.gitlab_url, 'webpack-internal://'], + environment: gon.sentry_environment, release: gon.revision, tags: { revision: gon.revision, diff --git a/app/assets/javascripts/raven/raven_config.js b/app/assets/javascripts/raven/raven_config.js index 338006ce2b9..b4a8c263954 100644 --- a/app/assets/javascripts/raven/raven_config.js +++ b/app/assets/javascripts/raven/raven_config.js @@ -61,7 +61,7 @@ const RavenConfig = { release: this.options.release, tags: this.options.tags, whitelistUrls: this.options.whitelistUrls, - environment: this.options.isProduction ? 'production' : 'development', + environment: this.options.environment, ignoreErrors: this.IGNORE_ERRORS, ignoreUrls: this.IGNORE_URLS, shouldSendCallback: this.shouldSendSample.bind(this), diff --git a/app/models/application_setting_implementation.rb b/app/models/application_setting_implementation.rb index b413ffddb9d..557215ff4dc 100644 --- a/app/models/application_setting_implementation.rb +++ b/app/models/application_setting_implementation.rb @@ -183,6 +183,22 @@ module ApplicationSettingImplementation clientside_sentry_dsn.strip! if clientside_sentry_dsn.present? end + def sentry_enabled + Gitlab.config.sentry.enabled || read_attribute(:sentry_enabled) + end + + def sentry_dsn + Gitlab.config.sentry.dsn || read_attribute(:sentry_dsn) + end + + def clientside_sentry_enabled + Gitlab.config.sentry.enabled || read_attribute(:clientside_sentry_enabled) + end + + def clientside_sentry_dsn + Gitlab.config.sentry.dsn || read_attribute(:clientside_sentry_dsn) + end + def performance_bar_allowed_group Group.find_by_id(performance_bar_allowed_group_id) end diff --git a/app/views/admin/application_settings/_logging.html.haml b/app/views/admin/application_settings/_logging.html.haml index 41b787515b5..4145ef94de8 100644 --- a/app/views/admin/application_settings/_logging.html.haml +++ b/app/views/admin/application_settings/_logging.html.haml @@ -1,6 +1,13 @@ = form_for @application_setting, url: admin_application_settings_path(anchor: 'js-logging-settings'), html: { class: 'fieldset-form' } do |f| = form_errors(@application_setting) + %p + %strong + NOTE: + These settings will be removed from the UI in a GitLab 12.0 release and made available within gitlab.yml. + The specific client side DSN setting is already handled as a component from a Sentry perspective anb will be removed. + In addition, you will be able to define a Sentry Environment to differentiate between multiple deployments. For example, development, staging, and production. + %fieldset .form-group .form-check diff --git a/changelogs/unreleased/feat-sentry-environment.yml b/changelogs/unreleased/feat-sentry-environment.yml new file mode 100644 index 00000000000..44ea19375f8 --- /dev/null +++ b/changelogs/unreleased/feat-sentry-environment.yml @@ -0,0 +1,5 @@ +--- +title: Allow Sentry configuration to be passed on gitlab.yml +merge_request: 27091 +author: Roger Meier +type: added diff --git a/config/gitlab.yml.example b/config/gitlab.yml.example index bdac5b2a6a1..06530194907 100644 --- a/config/gitlab.yml.example +++ b/config/gitlab.yml.example @@ -315,6 +315,13 @@ production: &base # path: shared/registry # issuer: gitlab-issuer + + ## Error Reporting and Logging with Sentry + sentry: + # enabled: false + # dsn: https://@sentry.io/ + # environment: 'production' # e.g. development, staging, production + # # 2. GitLab CI settings # ========================== diff --git a/config/initializers/1_settings.rb b/config/initializers/1_settings.rb index e9b36873d75..cddf5bf33f5 100644 --- a/config/initializers/1_settings.rb +++ b/config/initializers/1_settings.rb @@ -215,6 +215,14 @@ Settings.registry['issuer'] ||= nil Settings.registry['host_port'] ||= [Settings.registry['host'], Settings.registry['port']].compact.join(':') Settings.registry['path'] = Settings.absolute(Settings.registry['path'] || File.join(Settings.shared['path'], 'registry')) +# +# Error Reporting and Logging with Sentry +# +Settings['sentry'] ||= Settingslogic.new({}) +Settings.sentry['enabled'] ||= false +Settings.sentry['dsn'] ||= nil +Settings.sentry['environment'] ||= nil + # # Pages # diff --git a/config/initializers/sentry.rb b/config/initializers/sentry.rb index 680cfa6f0ed..e5589ce0ad1 100644 --- a/config/initializers/sentry.rb +++ b/config/initializers/sentry.rb @@ -14,6 +14,7 @@ def configure_sentry Raven.configure do |config| config.dsn = Gitlab::CurrentSettings.current_application_settings.sentry_dsn config.release = Gitlab.revision + config.current_environment = Gitlab.config.sentry.environment.presence # Sanitize fields based on those sanitized from Rails. config.sanitize_fields = Rails.application.config.filter_parameters.map(&:to_s) diff --git a/lib/gitlab/gon_helper.rb b/lib/gitlab/gon_helper.rb index e00309e7946..582c3065189 100644 --- a/lib/gitlab/gon_helper.rb +++ b/lib/gitlab/gon_helper.rb @@ -15,7 +15,12 @@ module Gitlab gon.relative_url_root = Gitlab.config.gitlab.relative_url_root gon.shortcuts_path = Gitlab::Routing.url_helpers.help_page_path('shortcuts') gon.user_color_scheme = Gitlab::ColorSchemes.for_user(current_user).css_class - gon.sentry_dsn = Gitlab::CurrentSettings.clientside_sentry_dsn if Gitlab::CurrentSettings.clientside_sentry_enabled + + if Gitlab::CurrentSettings.clientside_sentry_enabled + gon.sentry_dsn = Gitlab::CurrentSettings.clientside_sentry_dsn + gon.sentry_environment = Gitlab.config.sentry.environment + end + gon.gitlab_url = Gitlab.config.gitlab.url gon.revision = Gitlab.revision gon.gitlab_logo = ActionController::Base.helpers.asset_path('gitlab_logo.png') diff --git a/spec/javascripts/raven/index_spec.js b/spec/javascripts/raven/index_spec.js index a503a54029f..6b9fe923624 100644 --- a/spec/javascripts/raven/index_spec.js +++ b/spec/javascripts/raven/index_spec.js @@ -5,19 +5,19 @@ describe('RavenConfig options', () => { const sentryDsn = 'sentryDsn'; const currentUserId = 'currentUserId'; const gitlabUrl = 'gitlabUrl'; - const isProduction = 'isProduction'; + const environment = 'test'; const revision = 'revision'; let indexReturnValue; beforeEach(() => { window.gon = { sentry_dsn: sentryDsn, + sentry_environment: environment, current_user_id: currentUserId, gitlab_url: gitlabUrl, revision, }; - process.env.NODE_ENV = isProduction; process.env.HEAD_COMMIT_SHA = revision; spyOn(RavenConfig, 'init'); @@ -25,12 +25,12 @@ describe('RavenConfig options', () => { indexReturnValue = index(); }); - it('should init with .sentryDsn, .currentUserId, .whitelistUrls and .isProduction', () => { + it('should init with .sentryDsn, .currentUserId, .whitelistUrls and environment', () => { expect(RavenConfig.init).toHaveBeenCalledWith({ sentryDsn, currentUserId, - whitelistUrls: [gitlabUrl], - isProduction, + whitelistUrls: [gitlabUrl, 'webpack-internal://'], + environment, release: revision, tags: { revision, diff --git a/spec/javascripts/raven/raven_config_spec.js b/spec/javascripts/raven/raven_config_spec.js index 5cc59cc28d3..af634a0c196 100644 --- a/spec/javascripts/raven/raven_config_spec.js +++ b/spec/javascripts/raven/raven_config_spec.js @@ -69,8 +69,8 @@ describe('RavenConfig', () => { let ravenConfig; const options = { sentryDsn: '//sentryDsn', - whitelistUrls: ['//gitlabUrl'], - isProduction: true, + whitelistUrls: ['//gitlabUrl', 'webpack-internal://'], + environment: 'test', release: 'revision', tags: { revision: 'revision', @@ -95,7 +95,7 @@ describe('RavenConfig', () => { release: options.release, tags: options.tags, whitelistUrls: options.whitelistUrls, - environment: 'production', + environment: 'test', ignoreErrors: ravenConfig.IGNORE_ERRORS, ignoreUrls: ravenConfig.IGNORE_URLS, shouldSendCallback: jasmine.any(Function), @@ -106,8 +106,8 @@ describe('RavenConfig', () => { expect(raven.install).toHaveBeenCalled(); }); - it('should set .environment to development if isProduction is false', () => { - ravenConfig.options.isProduction = false; + it('should set environment from options', () => { + ravenConfig.options.environment = 'development'; RavenConfig.configure.call(ravenConfig); diff --git a/spec/support/shared_examples/application_setting_examples.rb b/spec/support/shared_examples/application_setting_examples.rb index e7ec24c5b7e..d8f7ba1185e 100644 --- a/spec/support/shared_examples/application_setting_examples.rb +++ b/spec/support/shared_examples/application_setting_examples.rb @@ -249,4 +249,41 @@ RSpec.shared_examples 'application settings examples' do expect(setting.password_authentication_enabled_for_web?).to be_falsey end + + describe 'sentry settings' do + context 'when the sentry settings are not set in gitlab.yml' do + it 'fallbacks to the settings in the database' do + setting.sentry_enabled = true + setting.sentry_dsn = 'https://b44a0828b72421a6d8e99efd68d44fa8@example.com/40' + setting.clientside_sentry_enabled = true + setting.clientside_sentry_dsn = 'https://b44a0828b72421a6d8e99efd68d44fa8@example.com/41' + + allow(Gitlab.config.sentry).to receive(:enabled).and_return(false) + allow(Gitlab.config.sentry).to receive(:dsn).and_return(nil) + + expect(setting.sentry_enabled).to eq true + expect(setting.sentry_dsn).to eq 'https://b44a0828b72421a6d8e99efd68d44fa8@example.com/40' + expect(setting.clientside_sentry_enabled).to eq true + expect(setting.clientside_sentry_dsn). to eq 'https://b44a0828b72421a6d8e99efd68d44fa8@example.com/41' + end + end + + context 'when the sentry settings are set in gitlab.yml' do + it 'does not fallback to the settings in the database' do + setting.sentry_enabled = false + setting.sentry_dsn = 'https://b44a0828b72421a6d8e99efd68d44fa8@example.com/40' + setting.clientside_sentry_enabled = false + setting.clientside_sentry_dsn = 'https://b44a0828b72421a6d8e99efd68d44fa8@example.com/41' + + allow(Gitlab.config.sentry).to receive(:enabled).and_return(true) + allow(Gitlab.config.sentry).to receive(:dsn).and_return('https://b44a0828b72421a6d8e99efd68d44fa8@example.com/42') + + expect(setting).not_to receive(:read_attribute) + expect(setting.sentry_enabled).to eq true + expect(setting.sentry_dsn).to eq 'https://b44a0828b72421a6d8e99efd68d44fa8@example.com/42' + expect(setting.clientside_sentry_enabled).to eq true + expect(setting.clientside_sentry_dsn). to eq 'https://b44a0828b72421a6d8e99efd68d44fa8@example.com/42' + end + end + end end