From 20c5881c4631a8cb6db049843586976e342a65c1 Mon Sep 17 00:00:00 2001 From: Marius Bobin <mbobin@gitlab.com> Date: Wed, 24 Apr 2024 15:43:14 +0000 Subject: [PATCH] Goodbye sentry-raven Remove sentry-raven integration Changelog: removed --- Gemfile | 1 - Gemfile.checksum | 1 - Gemfile.lock | 3 - app/assets/javascripts/sentry/init_sentry.js | 3 +- .../javascripts/sentry/legacy_constants.js | 46 ---- app/assets/javascripts/sentry/legacy_index.js | 34 --- .../sentry/legacy_sentry_config.js | 64 ------ .../application_settings/_sentry.html.haml | 8 +- app/views/layouts/_head.html.haml | 4 +- .../enable_new_sentry_integration.yml | 8 - .../enable_old_sentry_integration.yml | 8 - config/webpack.config.js | 1 - .../content_security_policy/config_loader.rb | 13 -- lib/gitlab/error_tracking.rb | 86 ++++--- lib/gitlab/error_tracking/log_formatter.rb | 11 +- .../concerns/processes_exceptions.rb | 8 +- .../processor/sanitizer_processor.rb | 3 - lib/gitlab/gon_helper.rb | 12 +- locale/gitlab.pot | 15 -- package.json | 3 +- spec/features/sentry_js_spec.rb | 55 +---- spec/frontend/sentry/init_sentry_spec.js | 15 +- spec/frontend/sentry/legacy_index_spec.js | 51 ----- .../sentry/legacy_sentry_config_spec.js | 215 ------------------ .../config_loader_spec.rb | 41 ---- .../error_tracking/log_formatter_spec.rb | 8 +- .../context_payload_processor_spec.rb | 31 +-- .../processor/grpc_error_processor_spec.rb | 31 --- .../sanitize_error_message_processor_spec.rb | 14 -- .../processor/sidekiq_processor_spec.rb | 42 ---- spec/lib/gitlab/error_tracking_spec.rb | 115 +++------- spec/lib/gitlab/gon_helper_spec.rb | 28 --- spec/requests/api/helpers_spec.rb | 1 - spec/support/helpers/stub_configuration.rb | 3 - yarn.lock | 76 +------ 35 files changed, 147 insertions(+), 911 deletions(-) delete mode 100644 app/assets/javascripts/sentry/legacy_constants.js delete mode 100644 app/assets/javascripts/sentry/legacy_index.js delete mode 100644 app/assets/javascripts/sentry/legacy_sentry_config.js delete mode 100644 config/feature_flags/development/enable_new_sentry_integration.yml delete mode 100644 config/feature_flags/development/enable_old_sentry_integration.yml delete mode 100644 spec/frontend/sentry/legacy_index_spec.js delete mode 100644 spec/frontend/sentry/legacy_sentry_config_spec.js diff --git a/Gemfile b/Gemfile index 3a67459e3d50..bf8e095c287a 100644 --- a/Gemfile +++ b/Gemfile @@ -359,7 +359,6 @@ gem 'gitlab-license', '~> 2.4', feature_category: :shared gem 'rack-attack', '~> 6.7.0' # rubocop:todo Gemfile/MissingFeatureCategory # Sentry integration -gem 'sentry-raven', '~> 3.1', feature_category: :error_tracking gem 'sentry-ruby', '~> 5.10.0', feature_category: :error_tracking gem 'sentry-rails', '~> 5.10.0', feature_category: :error_tracking gem 'sentry-sidekiq', '~> 5.10.0', feature_category: :error_tracking diff --git a/Gemfile.checksum b/Gemfile.checksum index 5e1275b43778..69e4643d4cef 100644 --- a/Gemfile.checksum +++ b/Gemfile.checksum @@ -623,7 +623,6 @@ {"name":"selenium-webdriver","version":"4.19.0","platform":"ruby","checksum":"4c8bd1d6016a456154b4ba71a3bb4d532a0ae185a38acf9cec0acbd38b4e5066"}, {"name":"semver_dialects","version":"2.0.2","platform":"ruby","checksum":"60059c9f416f931b5212d862fad2879d6b9affb8e0b9afb0d91b793639c116fe"}, {"name":"sentry-rails","version":"5.10.0","platform":"ruby","checksum":"99aa2fac136c26942eb1897c65de65dac88ad43ac5eb183ff20711287a137ebd"}, -{"name":"sentry-raven","version":"3.1.2","platform":"ruby","checksum":"103d3b122958810d34898ce2e705bcf549ddb9d855a70ce9a3970ee2484f364a"}, {"name":"sentry-ruby","version":"5.10.0","platform":"ruby","checksum":"115c24c0aee1309210f3a2988fb118e2bec1f11609feeda90e694388b1183619"}, {"name":"sentry-sidekiq","version":"5.10.0","platform":"ruby","checksum":"cc81018d0733fb1be3fb5641c9e0b61030bbeaa1d0b23ca64797d70def7aea1a"}, {"name":"sexp_processor","version":"4.17.1","platform":"ruby","checksum":"91110946720307f30bf1d549e90d9a529fef40d1fc471c069c8cca7667015da0"}, diff --git a/Gemfile.lock b/Gemfile.lock index 46b3967fb302..558a898f5567 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -1677,8 +1677,6 @@ GEM sentry-rails (5.10.0) railties (>= 5.0) sentry-ruby (~> 5.10.0) - sentry-raven (3.1.2) - faraday (>= 1.0) sentry-ruby (5.10.0) concurrent-ruby (~> 1.0, >= 1.0.2) sentry-sidekiq (5.10.0) @@ -2233,7 +2231,6 @@ DEPENDENCIES selenium-webdriver (~> 4.19) semver_dialects (~> 2.0, >= 2.0.2) sentry-rails (~> 5.10.0) - sentry-raven (~> 3.1) sentry-ruby (~> 5.10.0) sentry-sidekiq (~> 5.10.0) shoulda-matchers (~> 5.1.0) diff --git a/app/assets/javascripts/sentry/init_sentry.js b/app/assets/javascripts/sentry/init_sentry.js index 722741b50e46..8c8d8b655dea 100644 --- a/app/assets/javascripts/sentry/init_sentry.js +++ b/app/assets/javascripts/sentry/init_sentry.js @@ -1,3 +1,4 @@ +/* eslint-disable no-restricted-imports */ import { BrowserClient, getCurrentHub, @@ -9,7 +10,7 @@ import { // exports captureException, SDK_VERSION, -} from 'sentrybrowser'; +} from '@sentry/browser'; const initSentry = () => { if (!gon?.sentry_dsn) { diff --git a/app/assets/javascripts/sentry/legacy_constants.js b/app/assets/javascripts/sentry/legacy_constants.js deleted file mode 100644 index d04011dab2fd..000000000000 --- a/app/assets/javascripts/sentry/legacy_constants.js +++ /dev/null @@ -1,46 +0,0 @@ -import { __ } from '~/locale'; - -// https://docs.sentry.io/platforms/javascript/configuration/filtering/#decluttering-sentry -export const IGNORE_ERRORS = [ - // Random plugins/extensions - 'top.GLOBALS', - // See: http://blog.errorception.com/2012/03/tale-of-unfindable-js-error. html - 'originalCreateNotification', - 'canvas.contentDocument', - 'MyApp_RemoveAllHighlights', - 'http://tt.epicplay.com', - __("Can't find variable: ZiteReader"), - __('jigsaw is not defined'), - __('ComboSearch is not defined'), - 'http://loading.retry.widdit.com/', - 'atomicFindClose', - // Facebook borked - 'fb_xd_fragment', - // ISP "optimizing" proxy - `Cache-Control: no-transform` seems to - // reduce this. (thanks @acdha) - 'bmi_SafeAddOnload', - 'EBCallBackMessageReceived', - // See http://toolbar.conduit.com/Developer/HtmlAndGadget/Methods/JSInjection.aspx - 'conduitPage', - // Exclude errors from polling when navigating away from a page - 'TypeError: Failed to fetch', -]; - -export const DENY_URLS = [ - // Facebook flakiness - /graph\.facebook\.com/i, - // Facebook blocked - /connect\.facebook\.net\/en_US\/all\.js/i, - // Woopra flakiness - /eatdifferent\.com\.woopra-ns\.com/i, - /static\.woopra\.com\/js\/woopra\.js/i, - // Chrome extensions - /extensions\//i, - /^chrome:\/\//i, - // Other plugins - /127\.0\.0\.1:4001\/isrunning/i, // Cacaoweb - /webappstoolbarba\.texthelp\.com\//i, - /metrics\.itunes\.apple\.com\.edgesuite\.net\//i, -]; - -export const SAMPLE_RATE = 0.95; diff --git a/app/assets/javascripts/sentry/legacy_index.js b/app/assets/javascripts/sentry/legacy_index.js deleted file mode 100644 index 688f8eb0a445..000000000000 --- a/app/assets/javascripts/sentry/legacy_index.js +++ /dev/null @@ -1,34 +0,0 @@ -import '../webpack'; - -import * as Sentry5 from 'sentrybrowser5'; -import LegacySentryConfig from './legacy_sentry_config'; - -const index = function index() { - // Configuration for legacy versions of Sentry SDK (v5) - LegacySentryConfig.init({ - dsn: gon.sentry_dsn, - currentUserId: gon.current_user_id, - 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, - feature_category: gon.feature_category, - }, - }); -}; - -index(); - -// The _Sentry object is globally exported so it can be used by -// ./sentry_browser_wrapper.js -// This hack allows us to load a single version of `~/sentry/sentry_browser_wrapper` -// in the browser, see app/views/layouts/_head.html.haml to find how it is imported. - -// eslint-disable-next-line no-underscore-dangle -window._Sentry = Sentry5; - -export default index; diff --git a/app/assets/javascripts/sentry/legacy_sentry_config.js b/app/assets/javascripts/sentry/legacy_sentry_config.js deleted file mode 100644 index 137c31f5526c..000000000000 --- a/app/assets/javascripts/sentry/legacy_sentry_config.js +++ /dev/null @@ -1,64 +0,0 @@ -import * as Sentry5 from 'sentrybrowser5'; -import $ from 'jquery'; -import { __ } from '~/locale'; -import { IGNORE_ERRORS, DENY_URLS, SAMPLE_RATE } from './legacy_constants'; - -const SentryConfig = { - IGNORE_ERRORS, - DENYLIST_URLS: DENY_URLS, - SAMPLE_RATE, - init(options = {}) { - this.options = options; - - this.configure(); - this.bindSentryErrors(); - if (this.options.currentUserId) this.setUser(); - }, - - configure() { - const { dsn, release, tags, whitelistUrls, environment } = this.options; - - Sentry5.init({ - dsn, - release, - whitelistUrls, - environment, - ignoreErrors: this.IGNORE_ERRORS, // TODO: Remove in favor of https://gitlab.com/gitlab-org/gitlab/issues/35144 - blacklistUrls: this.DENYLIST_URLS, - sampleRate: SAMPLE_RATE, - }); - - Sentry5.setTags(tags); - }, - - setUser() { - Sentry5.setUser({ - id: this.options.currentUserId, - }); - }, - - bindSentryErrors() { - $(document).on('ajaxError.sentry', this.handleSentryErrors); - }, - - handleSentryErrors(event, req, config, err) { - const error = err || req.statusText; - const { responseText = __('Unknown response text') } = req; - const { type, url, data } = config; - const { status } = req; - - Sentry5.captureMessage(error, { - extra: { - type, - url, - data, - status, - response: responseText, - error, - event, - }, - }); - }, -}; - -export default SentryConfig; diff --git a/app/views/admin/application_settings/_sentry.html.haml b/app/views/admin/application_settings/_sentry.html.haml index 5079f374c84e..7058a4b5cca3 100644 --- a/app/views/admin/application_settings/_sentry.html.haml +++ b/app/views/admin/application_settings/_sentry.html.haml @@ -1,12 +1,8 @@ = gitlab_ui_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) - - if Feature.disabled?(:enable_new_sentry_integration) - %fieldset.gl-text-secondary - = safe_format(s_('AdminSettings|GitLab uses the %{bold_start}Rails%{bold_end} and %{bold_start}Browser JavaScript%{bold_end} Sentry SDKs to send events to Sentry. For changes to Rails integration settings to take effect, enable the %{code_start}enable_new_sentry_integration%{code_end} feature flag and restart GitLab.'), tag_pair(tag.b, :bold_start, :bold_end), tag_pair(tag.code, :code_start, :code_end)) - - else - %fieldset.gl-text-secondary - = safe_format(s_('AdminSettings|GitLab uses the %{bold_start}Rails%{bold_end} and %{bold_start}Browser JavaScript%{bold_end} Sentry SDKs to send events to Sentry. For changes to Rails integration settings to take effect, restart GitLab.'), tag_pair(tag.b, :bold_start, :bold_end)) + %fieldset.gl-text-secondary + = safe_format(s_('AdminSettings|GitLab uses the %{bold_start}Rails%{bold_end} and %{bold_start}Browser JavaScript%{bold_end} Sentry SDKs to send events to Sentry. For changes to Rails integration settings to take effect, restart GitLab.'), tag_pair(tag.b, :bold_start, :bold_end)) %fieldset .form-group diff --git a/app/views/layouts/_head.html.haml b/app/views/layouts/_head.html.haml index 728997d39c79..ea770a076f3c 100644 --- a/app/views/layouts/_head.html.haml +++ b/app/views/layouts/_head.html.haml @@ -48,10 +48,8 @@ = render 'layouts/loading_hints' = render_if_exists 'layouts/header/translations' - - if Feature.enabled?(:enable_new_sentry_integration) && Gitlab::CurrentSettings.sentry_enabled + - if Gitlab::CurrentSettings.sentry_enabled = webpack_bundle_tag 'sentry' - - elsif Gitlab.config.sentry.enabled - = webpack_bundle_tag 'legacy_sentry' = webpack_bundle_tag 'performance_bar' if performance_bar_enabled? = yield :page_specific_javascripts diff --git a/config/feature_flags/development/enable_new_sentry_integration.yml b/config/feature_flags/development/enable_new_sentry_integration.yml deleted file mode 100644 index 00665f80ed6e..000000000000 --- a/config/feature_flags/development/enable_new_sentry_integration.yml +++ /dev/null @@ -1,8 +0,0 @@ ---- -name: enable_new_sentry_integration -introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/72428 -rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/344832 -milestone: '14.9' -type: development -group: group::pipeline execution -default_enabled: false diff --git a/config/feature_flags/development/enable_old_sentry_integration.yml b/config/feature_flags/development/enable_old_sentry_integration.yml deleted file mode 100644 index 4911dbfdc78e..000000000000 --- a/config/feature_flags/development/enable_old_sentry_integration.yml +++ /dev/null @@ -1,8 +0,0 @@ ---- -name: enable_old_sentry_integration -introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/72428 -rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/344832 -milestone: '14.9' -type: development -group: group::pipeline execution -default_enabled: true diff --git a/config/webpack.config.js b/config/webpack.config.js index ca5506a1e391..f6cd978f3007 100644 --- a/config/webpack.config.js +++ b/config/webpack.config.js @@ -261,7 +261,6 @@ module.exports = { */ return { default: defaultEntries, - legacy_sentry: './sentry/legacy_index.js', sentry: './sentry/index.js', performance_bar: './performance_bar/index.js', jira_connect_app: './jira_connect/subscriptions/index.js', diff --git a/lib/gitlab/content_security_policy/config_loader.rb b/lib/gitlab/content_security_policy/config_loader.rb index 6bd266ecb8f5..6a2eaea7aaa0 100644 --- a/lib/gitlab/content_security_policy/config_loader.rb +++ b/lib/gitlab/content_security_policy/config_loader.rb @@ -133,7 +133,6 @@ def allow_zuora(directives) end def allow_sentry(directives) - allow_legacy_sentry(directives) if legacy_sentry_configured? return unless sentry_client_side_dsn_enabled? sentry_uri = URI(Gitlab::CurrentSettings.sentry_clientside_dsn) @@ -141,18 +140,6 @@ def allow_sentry(directives) append_to_directive(directives, 'connect_src', "#{sentry_uri.scheme}://#{sentry_uri.host}") end - def allow_legacy_sentry(directives) - # Support for Sentry setup via configuration files will be removed in 16.0 - # in favor of Gitlab::CurrentSettings. - sentry_uri = URI(Gitlab.config.sentry.clientside_dsn) - - append_to_directive(directives, 'connect_src', "#{sentry_uri.scheme}://#{sentry_uri.host}") - end - - def legacy_sentry_configured? - Gitlab.config.sentry&.enabled && Gitlab.config.sentry&.clientside_dsn - end - def sentry_client_side_dsn_enabled? Gitlab::CurrentSettings.try(:sentry_enabled) && Gitlab::CurrentSettings.try(:sentry_clientside_dsn) end diff --git a/lib/gitlab/error_tracking.rb b/lib/gitlab/error_tracking.rb index 6bf30e152f86..de6be9d1617e 100644 --- a/lib/gitlab/error_tracking.rb +++ b/lib/gitlab/error_tracking.rb @@ -27,34 +27,12 @@ module ErrorTracking ].freeze class << self - def configure(&block) - configure_raven(&block) - configure_sentry(&block) - end - - def configure_raven - Raven.configure do |config| - config.dsn = sentry_dsn - config.release = Gitlab.revision - config.current_environment = Gitlab.config.sentry.environment - - # Sanitize fields based on those sanitized from Rails. - config.sanitize_fields = Rails.application.config.filter_parameters.map(&:to_s) - - # Sanitize authentication headers - config.sanitize_http_headers = %w[Authorization Private-Token] - config.before_send = method(:before_send_raven) - - yield config if block_given? - end - end - - def configure_sentry + def configure Sentry.init do |config| - config.dsn = new_sentry_dsn + config.dsn = sentry_dsn config.release = Gitlab.revision - config.environment = new_sentry_environment - config.before_send = method(:before_send_sentry) + config.environment = sentry_environment + config.before_send = method(:before_send) config.background_worker_threads = 0 config.send_default_pii = true config.send_modules = false @@ -134,18 +112,6 @@ def log_and_raise_exception(exception, extra = {}) private - def before_send_raven(event, hint) - return unless Feature.enabled?(:enable_old_sentry_integration) - - before_send(event, hint) - end - - def before_send_sentry(event, hint) - return unless Feature.enabled?(:enable_new_sentry_integration) - - before_send(event, hint) - end - def before_send(event, hint) # Don't report Sidekiq retry errors to Sentry return if hint[:exception].is_a?(Gitlab::SidekiqMiddleware::RetryError) @@ -170,7 +136,6 @@ def process_exception(exception, extra:, tags: {}, trackers: default_trackers) def default_trackers [].tap do |destinations| - destinations << Raven if Raven.configuration.server # There is a possibility that this method is called before Sentry is # configured. Since Sentry 4.0, some methods of Sentry are forwarded to # to `nil`, hence we have to check the client as well. @@ -179,27 +144,58 @@ def default_trackers end end + # Some configuration attributes like `dsn`, and `environment` + # can be configured both via `ENV` and `Application Settings`. + # The reason being is while GitLab.com uses application_settings + # in Geo installations, we can't override values in the primary database. + # Setting this value in application_settings would propagate the value + # to all Geo nodes, which doesn't solve that particular problem. def sentry_dsn + env_sentry_dsn || database_sentry_dsn + end + + def sentry_environment + env_sentry_environment || database_sentry_environment + end + + def database_sentry_dsn + return unless sentry_configurable? + return unless database_sentry_enabled? + + Gitlab::CurrentSettings.sentry_dsn + end + + def env_sentry_dsn return unless sentry_configurable? - return unless Gitlab.config.sentry.enabled + return unless env_sentry_enabled? Gitlab.config.sentry.dsn end - def new_sentry_dsn + def env_sentry_environment return unless sentry_configurable? - return unless Gitlab::CurrentSettings.respond_to?(:sentry_enabled?) - return unless Gitlab::CurrentSettings.sentry_enabled? + return unless env_sentry_enabled? - Gitlab::CurrentSettings.sentry_dsn + Gitlab.config.sentry.environment end - def new_sentry_environment + def database_sentry_environment + return unless sentry_configurable? + return unless database_sentry_enabled? return unless Gitlab::CurrentSettings.respond_to?(:sentry_environment) Gitlab::CurrentSettings.sentry_environment end + def database_sentry_enabled? + Gitlab::CurrentSettings.respond_to?(:sentry_enabled?) && + Gitlab::CurrentSettings.sentry_enabled? + end + + def env_sentry_enabled? + Gitlab.config.sentry.enabled + end + def sentry_configurable? Rails.env.production? || Rails.env.development? end diff --git a/lib/gitlab/error_tracking/log_formatter.rb b/lib/gitlab/error_tracking/log_formatter.rb index d004c4e20bbc..98b64ff335f8 100644 --- a/lib/gitlab/error_tracking/log_formatter.rb +++ b/lib/gitlab/error_tracking/log_formatter.rb @@ -3,7 +3,7 @@ module Gitlab module ErrorTracking class LogFormatter - # Note: all the accesses to Raven's contexts here are to keep the + # Note: all the accesses to Sentry's contexts here are to keep the # backward-compatibility to Sentry's built-in integrations. In future, # they can be removed. def generate_log(exception, context_payload) @@ -20,21 +20,24 @@ def generate_log(exception, context_payload) private def append_user_to_log!(payload, context_payload) - user_context = Raven.context.user.merge(context_payload[:user]) + user_context = context_payload[:user] + user_context = Sentry.get_current_scope.user.merge(user_context) if Sentry.initialized? user_context.each do |key, value| payload["user.#{key}"] = value end end def append_tags_to_log!(payload, context_payload) - tags_context = Raven.context.tags.merge(context_payload[:tags]) + tags_context = context_payload[:tags] + tags_context = Sentry.get_current_scope.tags.merge(tags_context) if Sentry.initialized? tags_context.each do |key, value| payload["tags.#{key}"] = value end end def append_extra_to_log!(payload, context_payload) - extra = Raven.context.extra.merge(context_payload[:extra]) + extra = context_payload[:extra] + extra = Sentry.get_current_scope.extra.merge(extra) if Sentry.initialized? extra = extra.except(:server) # The extra value for sidekiq is a hash whose keys are strings. diff --git a/lib/gitlab/error_tracking/processor/concerns/processes_exceptions.rb b/lib/gitlab/error_tracking/processor/concerns/processes_exceptions.rb index 4b6c69a8b33f..daf570536d6e 100644 --- a/lib/gitlab/error_tracking/processor/concerns/processes_exceptions.rb +++ b/lib/gitlab/error_tracking/processor/concerns/processes_exceptions.rb @@ -8,11 +8,7 @@ module ProcessesExceptions private def extract_exceptions_from(event) - exceptions = if event.is_a?(Raven::Event) - event.instance_variable_get(:@interfaces)[:exception]&.values - else - event&.exception&.instance_variable_get(:@values) - end + exceptions = event&.exception&.instance_variable_get(:@values) Array.wrap(exceptions) end @@ -27,7 +23,7 @@ def set_exception_message(exception, message) def valid_exception?(exception) case exception - when Raven::SingleExceptionInterface, Sentry::SingleExceptionInterface + when Sentry::SingleExceptionInterface exception&.value.present? else false diff --git a/lib/gitlab/error_tracking/processor/sanitizer_processor.rb b/lib/gitlab/error_tracking/processor/sanitizer_processor.rb index e6114f8e2066..577b0f2697de 100644 --- a/lib/gitlab/error_tracking/processor/sanitizer_processor.rb +++ b/lib/gitlab/error_tracking/processor/sanitizer_processor.rb @@ -15,9 +15,6 @@ module SanitizerProcessor # For more information, please visit: # https://docs.sentry.io/platforms/ruby/guides/rails/configuration/filtering/#using-beforesend def self.call(event) - # Raven::Event instances don't need this processing. - return event unless event.is_a?(Sentry::Event) - if event.request.present? event.request.cookies = {} event.request.data = {} diff --git a/lib/gitlab/gon_helper.rb b/lib/gitlab/gon_helper.rb index a3dd12ebabc0..de4dde89f0a0 100644 --- a/lib/gitlab/gon_helper.rb +++ b/lib/gitlab/gon_helper.rb @@ -20,14 +20,10 @@ def add_gon_variables add_browsersdk_tracking - if Gitlab.config.sentry.enabled - gon.sentry_dsn = Gitlab.config.sentry.clientside_dsn - gon.sentry_environment = Gitlab.config.sentry.environment - end - - # Support for Sentry setup via configuration files will be removed in 17.0 - # in favor of Gitlab::CurrentSettings. - if Feature.enabled?(:enable_new_sentry_integration) && Gitlab::CurrentSettings.sentry_enabled + # Sentry configurations for the browser client are done + # via `Gitlab::CurrentSettings` from the Admin panel: + # `/admin/application_settings/metrics_and_profiling` + if Gitlab::CurrentSettings.sentry_enabled gon.sentry_dsn = Gitlab::CurrentSettings.sentry_clientside_dsn gon.sentry_environment = Gitlab::CurrentSettings.sentry_environment gon.sentry_clientside_traces_sample_rate = Gitlab::CurrentSettings.sentry_clientside_traces_sample_rate diff --git a/locale/gitlab.pot b/locale/gitlab.pot index d1f4bf53cdd5..6d83e7235c63 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -3739,9 +3739,6 @@ msgstr "" msgid "AdminSettings|Git abuse rate limit" msgstr "" -msgid "AdminSettings|GitLab uses the %{bold_start}Rails%{bold_end} and %{bold_start}Browser JavaScript%{bold_end} Sentry SDKs to send events to Sentry. For changes to Rails integration settings to take effect, enable the %{code_start}enable_new_sentry_integration%{code_end} feature flag and restart GitLab." -msgstr "" - msgid "AdminSettings|GitLab uses the %{bold_start}Rails%{bold_end} and %{bold_start}Browser JavaScript%{bold_end} Sentry SDKs to send events to Sentry. For changes to Rails integration settings to take effect, restart GitLab." msgstr "" @@ -10002,9 +9999,6 @@ msgstr "" msgid "Can't find HEAD commit for this branch" msgstr "" -msgid "Can't find variable: ZiteReader" -msgstr "" - msgid "Can't scan the code?" msgstr "" @@ -12687,9 +12681,6 @@ msgstr "" msgid "Colorize messages" msgstr "" -msgid "ComboSearch is not defined" -msgstr "" - msgid "Comma-separated list of branches to be automatically inspected. Leave blank to include all branches." msgstr "" @@ -54895,9 +54886,6 @@ msgstr "" msgid "Unknown format" msgstr "" -msgid "Unknown response text" -msgstr "" - msgid "Unknown user" msgstr "" @@ -61113,9 +61101,6 @@ msgstr "" msgid "it is too large" msgstr "" -msgid "jigsaw is not defined" -msgstr "" - msgid "key result" msgstr "" diff --git a/package.json b/package.json index 8384f45bef8b..65162844fe6f 100644 --- a/package.json +++ b/package.json @@ -68,6 +68,7 @@ "@mattiasbuelens/web-streams-adapter": "^0.1.0", "@rails/actioncable": "7.0.8-1", "@rails/ujs": "7.0.8-1", + "@sentry/browser": "7.102.0", "@snowplow/browser-plugin-client-hints": "^3.9.0", "@snowplow/browser-plugin-form-tracking": "^3.9.0", "@snowplow/browser-plugin-ga-cookies": "^3.9.0", @@ -196,8 +197,6 @@ "sass": "^1.69.7", "scrollparent": "^2.0.1", "semver": "^7.3.4", - "sentrybrowser": "npm:@sentry/browser@7.102.0", - "sentrybrowser5": "npm:@sentry/browser@5.30.0", "sortablejs": "^1.10.2", "string-hash": "1.1.3", "style-loader": "^2.0.0", diff --git a/spec/features/sentry_js_spec.rb b/spec/features/sentry_js_spec.rb index 583dcba62208..672dc3010aa6 100644 --- a/spec/features/sentry_js_spec.rb +++ b/spec/features/sentry_js_spec.rb @@ -3,57 +3,22 @@ require 'spec_helper' RSpec.describe 'Sentry', feature_category: :error_tracking do - context 'when enable_new_sentry_integration is disabled' do - before do - stub_feature_flags(enable_new_sentry_integration: false) - end - - it 'does not load sentry if sentry is disabled' do - allow(Gitlab.config.sentry).to receive(:enabled).and_return(false) - - visit new_user_session_path - - expect(has_requested_legacy_sentry).to eq(false) - end + it 'does not load sentry if sentry settings are disabled' do + allow(Gitlab::CurrentSettings).to receive(:sentry_enabled).and_return(false) - it 'loads legacy sentry if sentry config is enabled', :js do - allow(Gitlab.config.sentry).to receive(:enabled).and_return(true) + visit new_user_session_path - visit new_user_session_path - - expect(has_requested_legacy_sentry).to eq(true) - expect(evaluate_script('window._Sentry.SDK_VERSION')).to match(%r{^5\.}) - end + expect(has_requested_sentry).to eq(false) end - context 'when enable_new_sentry_integration is enabled' do - before do - stub_feature_flags(enable_new_sentry_integration: true) - end - - it 'does not load sentry if sentry settings are disabled' do - allow(Gitlab::CurrentSettings).to receive(:sentry_enabled).and_return(false) + it 'loads sentry if sentry settings are enabled', :js do + allow(Gitlab::CurrentSettings).to receive(:sentry_enabled).and_return(true) + allow(Gitlab::CurrentSettings).to receive(:sentry_clientside_dsn).and_return('https://mockdsn@example.com/1') - visit new_user_session_path + visit new_user_session_path - expect(has_requested_sentry).to eq(false) - end - - it 'loads sentry if sentry settings are enabled', :js do - allow(Gitlab::CurrentSettings).to receive(:sentry_enabled).and_return(true) - allow(Gitlab::CurrentSettings).to receive(:sentry_clientside_dsn).and_return('https://mockdsn@example.com/1') - - visit new_user_session_path - - expect(has_requested_sentry).to eq(true) - expect(evaluate_script('window._Sentry.SDK_VERSION')).to match(%r{^7\.}) - end - end - - def has_requested_legacy_sentry - page.all('script', visible: false).one? do |elm| - elm[:src] =~ %r{/legacy_sentry.*\.chunk\.js\z} - end + expect(has_requested_sentry).to eq(true) + expect(evaluate_script('window._Sentry.SDK_VERSION')).to match(%r{^7\.}) end def has_requested_sentry diff --git a/spec/frontend/sentry/init_sentry_spec.js b/spec/frontend/sentry/init_sentry_spec.js index 118a48cc1dee..0ed271962267 100644 --- a/spec/frontend/sentry/init_sentry_spec.js +++ b/spec/frontend/sentry/init_sentry_spec.js @@ -1,3 +1,4 @@ +/* eslint-disable no-restricted-imports */ import { BrowserClient, defaultStackParser, @@ -8,8 +9,8 @@ import { // exports captureException, SDK_VERSION, -} from 'sentrybrowser'; -import * as Sentry from 'sentrybrowser'; +} from '@sentry/browser'; +import * as Sentry from '@sentry/browser'; import { initSentry } from '~/sentry/init_sentry'; @@ -23,14 +24,14 @@ const mockFeatureCategory = 'my_feature_category'; const mockPage = 'index:page'; const mockSentryClientsideTracesSampleRate = 0.1; -jest.mock('sentrybrowser', () => { +jest.mock('@sentry/browser', () => { return { - ...jest.createMockFromModule('sentrybrowser'), + ...jest.createMockFromModule('@sentry/browser'), // unmock actual configuration options - defaultStackParser: jest.requireActual('sentrybrowser').defaultStackParser, - makeFetchTransport: jest.requireActual('sentrybrowser').makeFetchTransport, - defaultIntegrations: jest.requireActual('sentrybrowser').defaultIntegrations, + defaultStackParser: jest.requireActual('@sentry/browser').defaultStackParser, + makeFetchTransport: jest.requireActual('@sentry/browser').makeFetchTransport, + defaultIntegrations: jest.requireActual('@sentry/browser').defaultIntegrations, }; }); diff --git a/spec/frontend/sentry/legacy_index_spec.js b/spec/frontend/sentry/legacy_index_spec.js deleted file mode 100644 index fad1760ffc50..000000000000 --- a/spec/frontend/sentry/legacy_index_spec.js +++ /dev/null @@ -1,51 +0,0 @@ -import index from '~/sentry/legacy_index'; - -import LegacySentryConfig from '~/sentry/legacy_sentry_config'; - -describe('Sentry init', () => { - const dsn = 'https://123@sentry.gitlab.test/123'; - const environment = 'test'; - const currentUserId = '1'; - const gitlabUrl = 'gitlabUrl'; - const revision = 'revision'; - const featureCategory = 'my_feature_category'; - - beforeEach(() => { - window.gon = { - sentry_dsn: dsn, - sentry_environment: environment, - current_user_id: currentUserId, - gitlab_url: gitlabUrl, - revision, - feature_category: featureCategory, - }; - - jest.spyOn(LegacySentryConfig, 'init').mockImplementation(); - }); - - it('exports legacy version of Sentry in the global object', () => { - // eslint-disable-next-line no-underscore-dangle - expect(window._Sentry.SDK_VERSION).toMatch(/^5\./); - }); - - describe('when called', () => { - beforeEach(() => { - index(); - }); - - it('configures legacy sentry', () => { - expect(LegacySentryConfig.init).toHaveBeenCalledTimes(1); - expect(LegacySentryConfig.init).toHaveBeenCalledWith({ - dsn, - currentUserId, - whitelistUrls: [gitlabUrl, 'webpack-internal://'], - environment, - release: revision, - tags: { - revision, - feature_category: featureCategory, - }, - }); - }); - }); -}); diff --git a/spec/frontend/sentry/legacy_sentry_config_spec.js b/spec/frontend/sentry/legacy_sentry_config_spec.js deleted file mode 100644 index 94256784ca24..000000000000 --- a/spec/frontend/sentry/legacy_sentry_config_spec.js +++ /dev/null @@ -1,215 +0,0 @@ -import * as Sentry5 from 'sentrybrowser5'; -import LegacySentryConfig from '~/sentry/legacy_sentry_config'; - -describe('LegacySentryConfig', () => { - describe('IGNORE_ERRORS', () => { - it('should be an array of strings', () => { - const areStrings = LegacySentryConfig.IGNORE_ERRORS.every( - (error) => typeof error === 'string', - ); - - expect(areStrings).toBe(true); - }); - }); - - describe('DENYLIST_URLS', () => { - it('should be an array of regexps', () => { - const areRegExps = LegacySentryConfig.DENYLIST_URLS.every((url) => url instanceof RegExp); - - expect(areRegExps).toBe(true); - }); - }); - - describe('SAMPLE_RATE', () => { - it('should be a finite number', () => { - expect(typeof LegacySentryConfig.SAMPLE_RATE).toEqual('number'); - }); - }); - - describe('init', () => { - const options = { - currentUserId: 1, - }; - - beforeEach(() => { - jest.spyOn(LegacySentryConfig, 'configure'); - jest.spyOn(LegacySentryConfig, 'bindSentryErrors'); - jest.spyOn(LegacySentryConfig, 'setUser'); - - LegacySentryConfig.init(options); - }); - - it('should set the options property', () => { - expect(LegacySentryConfig.options).toEqual(options); - }); - - it('should call the configure method', () => { - expect(LegacySentryConfig.configure).toHaveBeenCalled(); - }); - - it('should call the error bindings method', () => { - expect(LegacySentryConfig.bindSentryErrors).toHaveBeenCalled(); - }); - - it('should call setUser', () => { - expect(LegacySentryConfig.setUser).toHaveBeenCalled(); - }); - - it('should not call setUser if there is no current user ID', () => { - LegacySentryConfig.setUser.mockClear(); - options.currentUserId = undefined; - - LegacySentryConfig.init(options); - - expect(LegacySentryConfig.setUser).not.toHaveBeenCalled(); - }); - }); - - describe('configure', () => { - const sentryConfig = {}; - const options = { - dsn: 'https://123@sentry.gitlab.test/123', - whitelistUrls: ['//gitlabUrl', 'webpack-internal://'], - environment: 'test', - release: 'revision', - tags: { - revision: 'revision', - feature_category: 'my_feature_category', - }, - }; - - beforeEach(() => { - jest.spyOn(Sentry5, 'init').mockImplementation(); - jest.spyOn(Sentry5, 'setTags').mockImplementation(); - - sentryConfig.options = options; - sentryConfig.IGNORE_ERRORS = 'ignore_errors'; - sentryConfig.DENYLIST_URLS = 'blacklist_urls'; - - LegacySentryConfig.configure.call(sentryConfig); - }); - - it('should call Sentry5.init', () => { - expect(Sentry5.init).toHaveBeenCalledWith({ - dsn: options.dsn, - release: options.release, - sampleRate: 0.95, - whitelistUrls: options.whitelistUrls, - environment: 'test', - ignoreErrors: sentryConfig.IGNORE_ERRORS, - blacklistUrls: sentryConfig.DENYLIST_URLS, - }); - }); - - it('should call Sentry5.setTags', () => { - expect(Sentry5.setTags).toHaveBeenCalledWith(options.tags); - }); - - it('should set environment from options', () => { - sentryConfig.options.environment = 'development'; - - LegacySentryConfig.configure.call(sentryConfig); - - expect(Sentry5.init).toHaveBeenCalledWith({ - dsn: options.dsn, - release: options.release, - sampleRate: 0.95, - whitelistUrls: options.whitelistUrls, - environment: 'development', - ignoreErrors: sentryConfig.IGNORE_ERRORS, - blacklistUrls: sentryConfig.DENYLIST_URLS, - }); - }); - }); - - describe('setUser', () => { - let sentryConfig; - - beforeEach(() => { - sentryConfig = { options: { currentUserId: 1 } }; - jest.spyOn(Sentry5, 'setUser'); - - LegacySentryConfig.setUser.call(sentryConfig); - }); - - it('should call .setUser', () => { - expect(Sentry5.setUser).toHaveBeenCalledWith({ - id: sentryConfig.options.currentUserId, - }); - }); - }); - - describe('handleSentryErrors', () => { - let event; - let req; - let config; - let err; - - beforeEach(() => { - event = {}; - req = { status: 'status', responseText: 'Unknown response text', statusText: 'statusText' }; - config = { type: 'type', url: 'url', data: 'data' }; - err = {}; - - jest.spyOn(Sentry5, 'captureMessage'); - - LegacySentryConfig.handleSentryErrors(event, req, config, err); - }); - - it('should call Sentry5.captureMessage', () => { - expect(Sentry5.captureMessage).toHaveBeenCalledWith(err, { - extra: { - type: config.type, - url: config.url, - data: config.data, - status: req.status, - response: req.responseText, - error: err, - event, - }, - }); - }); - - describe('if no err is provided', () => { - beforeEach(() => { - LegacySentryConfig.handleSentryErrors(event, req, config); - }); - - it('should use req.statusText as the error value', () => { - expect(Sentry5.captureMessage).toHaveBeenCalledWith(req.statusText, { - extra: { - type: config.type, - url: config.url, - data: config.data, - status: req.status, - response: req.responseText, - error: req.statusText, - event, - }, - }); - }); - }); - - describe('if no req.responseText is provided', () => { - beforeEach(() => { - req.responseText = undefined; - - LegacySentryConfig.handleSentryErrors(event, req, config, err); - }); - - it('should use `Unknown response text` as the response', () => { - expect(Sentry5.captureMessage).toHaveBeenCalledWith(err, { - extra: { - type: config.type, - url: config.url, - data: config.data, - status: req.status, - response: 'Unknown response text', - error: err, - event, - }, - }); - }); - }); - }); -}); diff --git a/spec/lib/gitlab/content_security_policy/config_loader_spec.rb b/spec/lib/gitlab/content_security_policy/config_loader_spec.rb index 30f10e3b34df..8bd585d4f9f0 100644 --- a/spec/lib/gitlab/content_security_policy/config_loader_spec.rb +++ b/spec/lib/gitlab/content_security_policy/config_loader_spec.rb @@ -346,28 +346,14 @@ end context 'when sentry is configured' do - let(:legacy_dsn) { 'dummy://abc@legacy-sentry.example.com/1' } let(:dsn) { 'dummy://def@sentry.example.com/2' } before do stub_config_setting(host: 'gitlab.example.com') end - context 'when legacy sentry is configured' do - before do - allow(Gitlab.config.sentry).to receive(:enabled).and_return(true) - allow(Gitlab.config.sentry).to receive(:clientside_dsn).and_return(legacy_dsn) - allow(Gitlab::CurrentSettings).to receive(:sentry_enabled).and_return(false) - end - - it 'adds legacy sentry path to CSP' do - expect(connect_src).to eq("'self' ws://gitlab.example.com dummy://legacy-sentry.example.com") - end - end - context 'when sentry is configured' do before do - allow(Gitlab.config.sentry).to receive(:enabled).and_return(false) allow(Gitlab::CurrentSettings).to receive(:sentry_enabled).and_return(true) allow(Gitlab::CurrentSettings).to receive(:sentry_clientside_dsn).and_return(dsn) end @@ -379,8 +365,6 @@ context 'when sentry settings are from older schemas and sentry setting are missing' do before do - allow(Gitlab.config.sentry).to receive(:enabled).and_return(false) - allow(Gitlab::CurrentSettings).to receive(:respond_to?).with(:sentry_enabled).and_return(false) allow(Gitlab::CurrentSettings).to receive(:sentry_enabled).and_raise(NoMethodError) @@ -392,31 +376,6 @@ expect(connect_src).to eq("'self' ws://gitlab.example.com") end end - - context 'when legacy sentry and sentry are both configured' do - let(:connect_src_expectation) do - # rubocop:disable Lint/PercentStringArray - %w[ - 'self' - ws://gitlab.example.com - dummy://legacy-sentry.example.com - dummy://sentry.example.com - ].join(' ') - # rubocop:enable Lint/PercentStringArray - end - - before do - allow(Gitlab.config.sentry).to receive(:enabled).and_return(true) - allow(Gitlab.config.sentry).to receive(:clientside_dsn).and_return(legacy_dsn) - - allow(Gitlab::CurrentSettings).to receive(:sentry_enabled).and_return(true) - allow(Gitlab::CurrentSettings).to receive(:sentry_clientside_dsn).and_return(dsn) - end - - it 'adds both sentry paths to CSP' do - expect(connect_src).to eq(connect_src_expectation) - end - end end describe 'Customer portal frames' do diff --git a/spec/lib/gitlab/error_tracking/log_formatter_spec.rb b/spec/lib/gitlab/error_tracking/log_formatter_spec.rb index 15d201401f44..7773e33a5fff 100644 --- a/spec/lib/gitlab/error_tracking/log_formatter_spec.rb +++ b/spec/lib/gitlab/error_tracking/log_formatter_spec.rb @@ -27,9 +27,9 @@ end before do - Raven.context.user[:user_flag] = 'flag' - Raven.context.tags[:shard] = 'catchall' - Raven.context.extra[:some_info] = 'info' + Sentry.get_current_scope.user[:user_flag] = 'flag' + Sentry.get_current_scope.tags[:shard] = 'catchall' + Sentry.get_current_scope.extra[:some_info] = 'info' allow(exception).to receive(:backtrace).and_return( [ @@ -40,7 +40,7 @@ end after do - ::Raven::Context.clear! + Sentry.get_current_scope.clear end it 'appends error-related log fields and filters sensitive Sidekiq arguments' do diff --git a/spec/lib/gitlab/error_tracking/processor/context_payload_processor_spec.rb b/spec/lib/gitlab/error_tracking/processor/context_payload_processor_spec.rb index c9b632b50e16..cf0e45eb8339 100644 --- a/spec/lib/gitlab/error_tracking/processor/context_payload_processor_spec.rb +++ b/spec/lib/gitlab/error_tracking/processor/context_payload_processor_spec.rb @@ -4,18 +4,29 @@ RSpec.describe Gitlab::ErrorTracking::Processor::ContextPayloadProcessor do describe '.call' do - let(:required_options) do + let(:event) { Sentry::Event.new(configuration: Sentry.configuration) } + let(:result_hash) { described_class.call(event).to_hash } + let(:payload) do { - configuration: Raven.configuration, - context: Raven.context, - breadcrumbs: Raven.breadcrumbs + user: { ip_address: '127.0.0.1' }, + tags: { priority: 'high' }, + extra: { sidekiq: { class: 'SomeWorker', args: ['[FILTERED]', 1, 2] } } } end - let(:event) { Raven::Event.new(required_options.merge(payload)) } - let(:result_hash) { described_class.call(event).to_hash } + around do |example| + Sentry.with_scope do |scope| + scope.set_user(payload[:user]) + scope.set_tags(payload[:tags]) + scope.set_extras(payload[:extra]) + + example.run + end + end before do + Sentry.get_current_scope.apply_to_event(event) + allow_next_instance_of(Gitlab::ErrorTracking::ContextPayloadGenerator) do |generator| allow(generator).to receive(:generate).and_return( user: { username: 'root' }, @@ -25,14 +36,6 @@ end end - let(:payload) do - { - user: { ip_address: '127.0.0.1' }, - tags: { priority: 'high' }, - extra: { sidekiq: { class: 'SomeWorker', args: ['[FILTERED]', 1, 2] } } - } - end - it 'merges the context payload into event payload', :aggregate_failures do expect(result_hash[:user]).to include(ip_address: '127.0.0.1', username: 'root') diff --git a/spec/lib/gitlab/error_tracking/processor/grpc_error_processor_spec.rb b/spec/lib/gitlab/error_tracking/processor/grpc_error_processor_spec.rb index 3399c6dd9f4c..2c886738f09d 100644 --- a/spec/lib/gitlab/error_tracking/processor/grpc_error_processor_spec.rb +++ b/spec/lib/gitlab/error_tracking/processor/grpc_error_processor_spec.rb @@ -4,19 +4,6 @@ RSpec.describe Gitlab::ErrorTracking::Processor::GrpcErrorProcessor, :sentry, feature_category: :integrations do describe '.call' do - let(:raven_required_options) do - { - configuration: Raven.configuration, - context: Raven.context, - breadcrumbs: Raven.breadcrumbs - } - end - - let(:raven_event) do - Raven::Event - .from_exception(exception, raven_required_options.merge(data)) - end - let(:sentry_event) do Sentry.get_current_client.event_from_exception(exception) end @@ -52,12 +39,6 @@ it { expect(result_hash).to include(data) } end - context 'with Raven event' do - let(:event) { raven_event } - - it_behaves_like 'leaves data unchanged' - end - context 'with Sentry event' do let(:event) { sentry_event } @@ -97,12 +78,6 @@ end end - context 'with Raven event' do - let(:event) { raven_event } - - it_behaves_like 'processes the exception' - end - context 'with Sentry event' do let(:event) { sentry_event } @@ -160,12 +135,6 @@ end end - context 'with Raven event' do - let(:event) { raven_event } - - it_behaves_like 'processes the exception' - end - context 'with Sentry event' do let(:event) { sentry_event } diff --git a/spec/lib/gitlab/error_tracking/processor/sanitize_error_message_processor_spec.rb b/spec/lib/gitlab/error_tracking/processor/sanitize_error_message_processor_spec.rb index 74b6df241781..8cf93287c2c6 100644 --- a/spec/lib/gitlab/error_tracking/processor/sanitize_error_message_processor_spec.rb +++ b/spec/lib/gitlab/error_tracking/processor/sanitize_error_message_processor_spec.rb @@ -20,20 +20,6 @@ end end - context 'with Raven event' do - let(:raven_required_options) do - { - configuration: Raven.configuration, - context: Raven.context, - breadcrumbs: Raven.breadcrumbs - } - end - - let(:event) { Raven::Event.from_exception(exception, raven_required_options) } - - it_behaves_like 'processes the exception' - end - context 'with Sentry event' do let(:event) { Sentry.get_current_client.event_from_exception(exception) } diff --git a/spec/lib/gitlab/error_tracking/processor/sidekiq_processor_spec.rb b/spec/lib/gitlab/error_tracking/processor/sidekiq_processor_spec.rb index bc4526758c0d..4dd780addf04 100644 --- a/spec/lib/gitlab/error_tracking/processor/sidekiq_processor_spec.rb +++ b/spec/lib/gitlab/error_tracking/processor/sidekiq_processor_spec.rb @@ -97,18 +97,6 @@ describe '.call' do let(:exception) { StandardError.new('Test exception') } - let(:raven_required_options) do - { - configuration: Raven.configuration, - context: Raven.context, - breadcrumbs: Raven.breadcrumbs - } - end - - let(:raven_event) do - Raven::Event.new(raven_required_options.merge(wrapped_value)) - end - let(:sentry_event) do Sentry.get_current_client.event_from_exception(exception) end @@ -158,12 +146,6 @@ end context 'when processing via the default error handler' do - context 'with Raven events' do - let(:event) { raven_event } - - include_examples 'Sidekiq arguments', args_in_job_hash: true - end - context 'with Sentry events' do let(:event) { sentry_event } @@ -172,12 +154,6 @@ end context 'when processing via Gitlab::ErrorTracking' do - context 'with Raven events' do - let(:event) { raven_event } - - include_examples 'Sidekiq arguments', args_in_job_hash: false - end - context 'with Sentry events' do let(:event) { sentry_event } @@ -208,12 +184,6 @@ end end - context 'with Raven events' do - let(:event) { raven_event } - - it_behaves_like 'handles jobstr fields' - end - context 'with Sentry events' do let(:event) { sentry_event } @@ -232,12 +202,6 @@ end end - context 'with Raven events' do - let(:event) { raven_event } - - it_behaves_like 'does nothing' - end - context 'with Sentry events' do let(:event) { sentry_event } @@ -255,12 +219,6 @@ end end - context 'with Raven events' do - let(:event) { raven_event } - - it_behaves_like 'does nothing' - end - context 'with Sentry events' do let(:event) { sentry_event } diff --git a/spec/lib/gitlab/error_tracking_spec.rb b/spec/lib/gitlab/error_tracking_spec.rb index 7b2c5ca27cb2..5c7b3e72fbe4 100644 --- a/spec/lib/gitlab/error_tracking_spec.rb +++ b/spec/lib/gitlab/error_tracking_spec.rb @@ -1,8 +1,6 @@ # frozen_string_literal: true require 'spec_helper' - -require 'raven/transports/dummy' require 'sentry/transport/dummy_transport' RSpec.describe Gitlab::ErrorTracking, feature_category: :shared do @@ -44,18 +42,11 @@ } end - let(:raven_event) do - event = Raven.client.transport.events.last[1] - Gitlab::Json.parse(event) - end - let(:sentry_event) do Sentry.get_current_client.transport.events.last end before do - stub_feature_flags(enable_old_sentry_integration: true) - stub_feature_flags(enable_new_sentry_integration: true) stub_sentry_settings allow(described_class).to receive(:sentry_configurable?).and_return(true) @@ -86,7 +77,6 @@ end it 'raises the exception' do - expect(Raven).to receive(:capture_exception).with(exception, sentry_payload) expect(Sentry).to receive(:capture_exception).with(exception, sentry_payload) expect do @@ -106,7 +96,6 @@ end it 'includes additional tags' do - expect(Raven).to receive(:capture_exception).with(exception, sentry_payload) expect(Sentry).to receive(:capture_exception).with(exception, sentry_payload) expect do @@ -126,7 +115,6 @@ end it 'logs the exception with all attributes passed' do - expect(Raven).to receive(:capture_exception).with(exception, sentry_payload) expect(Sentry).to receive(:capture_exception).with(exception, sentry_payload) described_class.track_and_raise_for_dev_exception( @@ -150,7 +138,6 @@ describe '.track_and_raise_exception' do it 'always raises the exception' do - expect(Raven).to receive(:capture_exception).with(exception, sentry_payload) expect(Sentry).to receive(:capture_exception).with(exception, sentry_payload) expect do @@ -195,7 +182,6 @@ end it 'only logs and raises the exception' do - expect(Raven).not_to receive(:capture_exception) expect(Sentry).not_to receive(:capture_exception) expect(Gitlab::ErrorTracking::Logger).to receive(:error).with(logger_payload) @@ -233,17 +219,13 @@ end before do - allow(Raven).to receive(:capture_exception).and_call_original allow(Sentry).to receive(:capture_exception).and_call_original allow(Gitlab::ErrorTracking::Logger).to receive(:error) end - it 'calls Raven.capture_exception' do + it 'calls Sentry.capture_exception' do track_exception - expect(Raven) - .to have_received(:capture_exception).with(exception, sentry_payload) - expect(Sentry) .to have_received(:capture_exception).with(exception, sentry_payload) end @@ -296,10 +278,6 @@ it 'includes the extra data from the exception in the tracking information' do track_exception - expect(Raven).to have_received(:capture_exception).with( - exception, a_hash_including(extra: a_hash_including(extra_info)) - ) - expect(Sentry).to have_received(:capture_exception).with( exception, a_hash_including(extra: a_hash_including(extra_info)) ) @@ -316,10 +294,6 @@ it 'just includes the other extra info' do track_exception - expect(Raven).to have_received(:capture_exception).with( - exception, a_hash_including(extra: a_hash_including(extra)) - ) - expect(Sentry).to have_received(:capture_exception).with( exception, a_hash_including(extra: a_hash_including(extra)) ) @@ -331,40 +305,10 @@ subject(:track_exception) { described_class.track_exception(exception, extra) } before do - allow(Raven).to receive(:capture_exception).and_call_original allow(Sentry).to receive(:capture_exception).and_call_original allow(Gitlab::ErrorTracking::Logger).to receive(:error) end - # This is a workaround for restoring Raven's user context below. - # Raven.user_context(&block) does not restore the user context correctly. - around do |example| - previous_user_context = Raven.context.user.dup - example.run - ensure - Raven.context.user = previous_user_context - end - - context 'with custom GitLab context when using Raven.capture_exception directly' do - subject(:track_exception) { Raven.capture_exception(exception) } - - it 'merges a default set of tags into the existing tags' do - allow(Raven.context).to receive(:tags).and_return(foo: 'bar') - - track_exception - - expect(raven_event['tags']).to include('correlation_id', 'feature_category', 'foo', 'locale', 'program') - end - - it 'merges the current user information into the existing user information' do - Raven.user_context(id: -1) - - track_exception - - expect(raven_event['user']).to eq('id' => -1, 'username' => user.username) - end - end - context 'with custom GitLab context when using Sentry.capture_exception directly' do subject(:track_exception) { Sentry.capture_exception(exception) } @@ -418,7 +362,6 @@ track_exception expected_data = [1, { 'id' => 2, 'name' => 'hello' }, 'some-value', 'another-value'] - expect(raven_event.dig('extra', 'sidekiq', 'args')).to eq(expected_data) expect(sentry_event.extra[:sidekiq]['args']).to eq(expected_data) end end @@ -429,7 +372,6 @@ it 'filters sensitive arguments before sending and logging' do track_exception - expect(raven_event.dig('extra', 'sidekiq', 'args')).to eq(['[FILTERED]', 1, 2]) expect(sentry_event.extra[:sidekiq]['args']).to eq(['[FILTERED]', 1, 2]) expect(Gitlab::ErrorTracking::Logger).to have_received(:error).with( hash_including( @@ -450,8 +392,6 @@ it 'sets the GRPC debug error string in the Sentry event and adds a custom fingerprint' do track_exception - expect(raven_event.dig('extra', 'grpc_debug_error_string')).to eq('{"hello":1}') - expect(raven_event['fingerprint']).to eq(['GRPC::DeadlineExceeded', '4:unknown cause.']) expect(sentry_event.extra[:grpc_debug_error_string]).to eq('{"hello":1}') expect(sentry_event.fingerprint).to eq(['GRPC::DeadlineExceeded', '4:unknown cause.']) end @@ -463,8 +403,6 @@ it 'does not do any processing on the event' do track_exception - expect(raven_event['extra']).not_to include('grpc_debug_error_string') - expect(raven_event['fingerprint']).to eq(['GRPC::DeadlineExceeded', '4:unknown cause']) expect(sentry_event.extra).not_to include(:grpc_debug_error_string) expect(sentry_event.fingerprint).to eq(['GRPC::DeadlineExceeded', '4:unknown cause']) end @@ -485,7 +423,6 @@ track_exception - expect(Raven.client.transport.events).to eq([]) expect(Sentry.get_current_client.transport.events).to eq([]) end end @@ -494,7 +431,6 @@ context 'when processing invalid URI exceptions' do let(:invalid_uri) { 'http://foo:bar' } - let(:raven_exception_values) { raven_event['exception']['values'] } let(:sentry_exception_values) { sentry_event.exception.to_hash[:values] } context 'when the error is a URI::InvalidURIError' do @@ -507,12 +443,6 @@ it 'filters the URI from the error message' do track_exception - expect(raven_exception_values).to include( - hash_including( - 'type' => 'URI::InvalidURIError', - 'value' => 'bad URI(is not URI?): [FILTERED]' - ) - ) expect(sentry_exception_values).to include( hash_including( type: 'URI::InvalidURIError', @@ -532,12 +462,6 @@ it 'filters the URI from the error message' do track_exception - expect(raven_exception_values).to include( - hash_including( - 'type' => 'Addressable::URI::InvalidURIError', - 'value' => 'Invalid port number: [FILTERED]' - ) - ) expect(sentry_exception_values).to include( hash_including( type: 'Addressable::URI::InvalidURIError', @@ -573,7 +497,7 @@ context 'when ENABLE_SENTRY_PERFORMANCE_MONITORING env is disabled' do before do stub_env('ENABLE_SENTRY_PERFORMANCE_MONITORING', false) - described_class.configure_sentry # Force re-initialization to reset traces_sample_rate setting + described_class.configure # Force re-initialization to reset traces_sample_rate setting end it 'does not set traces_sample_rate' do @@ -584,7 +508,7 @@ context 'when ENABLE_SENTRY_PERFORMANCE_MONITORING env is enabled' do before do stub_env('ENABLE_SENTRY_PERFORMANCE_MONITORING', true) - described_class.configure_sentry # Force re-initialization to reset traces_sample_rate setting + described_class.configure # Force re-initialization to reset traces_sample_rate setting end it 'sets traces_sample_rate' do @@ -592,4 +516,37 @@ end end end + + describe '.configure' do + using RSpec::Parameterized::TableSyntax + + let(:envdsn) { 'dummy://b44a0828b72421a6d8e99efd68d44fa8@example.com/41' } + let(:appdsn) { 'dummy://b44a0828b72421a6d8e99efd68d44fa8@example.com/42' } + + where(:env_enabled, :env_dsn, :env_env, :app_enabled, :app_dsn, :app_env, :dsn, :environment) do + true | ref(:envdsn) | 'eprd' | true | ref(:appdsn) | 'aprd' | ref(:envdsn) | 'eprd' + false | ref(:envdsn) | 'eprd' | true | ref(:appdsn) | 'aprd' | ref(:appdsn) | 'aprd' + true | ref(:envdsn) | 'eprd' | false | ref(:appdsn) | 'aprd' | ref(:envdsn) | 'eprd' + false | ref(:envdsn) | 'eprd' | false | ref(:appdsn) | 'aprd' | '' | '' + end + with_them do + before do + allow(Gitlab.config.sentry).to receive(:enabled) { env_enabled } + allow(Gitlab::CurrentSettings).to receive(:sentry_enabled?) { app_enabled } + + allow(Gitlab.config.sentry).to receive(:dsn) { env_dsn } + allow(Gitlab::CurrentSettings).to receive(:sentry_dsn) { app_dsn } + + allow(Gitlab.config.sentry).to receive(:environment).and_return('eprd') + allow(Gitlab::CurrentSettings).to receive(:sentry_environment).and_return('aprd') + + described_class.configure + end + + it 'selects the correct settings' do + expect(Sentry.configuration.dsn.to_s).to eq(dsn) + expect(Sentry.configuration.environment).to eq(environment) + end + end + end end diff --git a/spec/lib/gitlab/gon_helper_spec.rb b/spec/lib/gitlab/gon_helper_spec.rb index e95623a01e11..4fc9a5257928 100644 --- a/spec/lib/gitlab/gon_helper_spec.rb +++ b/spec/lib/gitlab/gon_helper_spec.rb @@ -60,19 +60,6 @@ let(:environment) { 'staging' } let(:sentry_clientside_traces_sample_rate) { 0.5 } - context 'with legacy sentry configuration' do - before do - stub_config(sentry: { enabled: true, clientside_dsn: clientside_dsn, environment: environment }) - end - - it 'sets sentry dsn and environment from config' do - expect(gon).to receive(:sentry_dsn=).with(clientside_dsn) - expect(gon).to receive(:sentry_environment=).with(environment) - - helper.add_gon_variables - end - end - context 'with sentry settings' do before do stub_application_setting(sentry_enabled: true) @@ -88,21 +75,6 @@ helper.add_gon_variables end - - context 'when enable_new_sentry_integration is disabled' do - before do - stub_feature_flags(enable_new_sentry_integration: false) - end - - it 'does not set sentry dsn and environment from config' do - expect(gon).not_to receive(:sentry_dsn=).with(clientside_dsn) - expect(gon).not_to receive(:sentry_environment=).with(environment) - expect(gon).not_to receive(:sentry_clientside_traces_sample_rate=) - .with(sentry_clientside_traces_sample_rate) - - helper.add_gon_variables - end - end end end end diff --git a/spec/requests/api/helpers_spec.rb b/spec/requests/api/helpers_spec.rb index 7304437bc425..28ddea7cdd90 100644 --- a/spec/requests/api/helpers_spec.rb +++ b/spec/requests/api/helpers_spec.rb @@ -1,7 +1,6 @@ # frozen_string_literal: true require 'spec_helper' -require 'raven/transports/dummy' require_relative '../../../config/initializers/sentry' RSpec.describe API::Helpers, :enable_admin_mode, feature_category: :system_access do diff --git a/spec/support/helpers/stub_configuration.rb b/spec/support/helpers/stub_configuration.rb index ea334fa8fac4..bc769ce0796c 100644 --- a/spec/support/helpers/stub_configuration.rb +++ b/spec/support/helpers/stub_configuration.rb @@ -109,15 +109,12 @@ def stub_storage_settings(messages) end def stub_sentry_settings(enabled: true) - allow(Gitlab.config.sentry).to receive(:enabled) { enabled } allow(Gitlab::CurrentSettings).to receive(:sentry_enabled?) { enabled } dsn = 'dummy://b44a0828b72421a6d8e99efd68d44fa8@example.com/42' - allow(Gitlab.config.sentry).to receive(:dsn) { dsn } allow(Gitlab::CurrentSettings).to receive(:sentry_dsn) { dsn } clientside_dsn = 'dummy://b44a0828b72421a6d8e99efd68d44fa8@example.com/43' - allow(Gitlab.config.sentry).to receive(:clientside_dsn) { clientside_dsn } allow(Gitlab::CurrentSettings) .to receive(:sentry_clientside_dsn) { clientside_dsn } end diff --git a/yarn.lock b/yarn.lock index e3ac3d27b0a5..8f750a1bb632 100644 --- a/yarn.lock +++ b/yarn.lock @@ -2080,16 +2080,18 @@ "@sentry/types" "7.102.0" "@sentry/utils" "7.102.0" -"@sentry/core@5.30.0": - version "5.30.0" - resolved "https://registry.yarnpkg.com/@sentry/core/-/core-5.30.0.tgz#6b203664f69e75106ee8b5a2fe1d717379b331f3" - integrity sha512-TmfrII8w1PQZSZgPpUESqjB+jC6MvZJZdLtE/0hZ+SrnKhW3x5WlYLvTXZpcWePYBku7rl2wn1RZu6uT0qCTeg== +"@sentry/browser@7.102.0": + version "7.102.0" + resolved "https://registry.yarnpkg.com/@sentry/browser/-/browser-7.102.0.tgz#335f51d01aabf8c4d2abc871855f9c2d19f8f70d" + integrity sha512-hIggcMnojIbWhbmlRfkykHmy6n7pjug0AHfF19HRUQxAx9KJfMH5YdWvohov0Hb9fS+jdvqgE+/4AWbEeXQrHw== dependencies: - "@sentry/hub" "5.30.0" - "@sentry/minimal" "5.30.0" - "@sentry/types" "5.30.0" - "@sentry/utils" "5.30.0" - tslib "^1.9.3" + "@sentry-internal/feedback" "7.102.0" + "@sentry-internal/replay-canvas" "7.102.0" + "@sentry-internal/tracing" "7.102.0" + "@sentry/core" "7.102.0" + "@sentry/replay" "7.102.0" + "@sentry/types" "7.102.0" + "@sentry/utils" "7.102.0" "@sentry/core@7.102.0": version "7.102.0" @@ -2099,24 +2101,6 @@ "@sentry/types" "7.102.0" "@sentry/utils" "7.102.0" -"@sentry/hub@5.30.0": - version "5.30.0" - resolved "https://registry.yarnpkg.com/@sentry/hub/-/hub-5.30.0.tgz#2453be9b9cb903404366e198bd30c7ca74cdc100" - integrity sha512-2tYrGnzb1gKz2EkMDQcfLrDTvmGcQPuWxLnJKXJvYTQDGLlEvi2tWz1VIHjunmOvJrB5aIQLhm+dcMRwFZDCqQ== - dependencies: - "@sentry/types" "5.30.0" - "@sentry/utils" "5.30.0" - tslib "^1.9.3" - -"@sentry/minimal@5.30.0": - version "5.30.0" - resolved "https://registry.yarnpkg.com/@sentry/minimal/-/minimal-5.30.0.tgz#ce3d3a6a273428e0084adcb800bc12e72d34637b" - integrity sha512-BwWb/owZKtkDX+Sc4zCSTNcvZUq7YcH3uAVlmh/gtR9rmUvbzAA3ewLuB3myi4wWRAMEtny6+J/FN/x+2wn9Xw== - dependencies: - "@sentry/hub" "5.30.0" - "@sentry/types" "5.30.0" - tslib "^1.9.3" - "@sentry/replay@7.102.0": version "7.102.0" resolved "https://registry.yarnpkg.com/@sentry/replay/-/replay-7.102.0.tgz#209b7adb68e89772824218ecab498d3a6fbc2c42" @@ -2127,24 +2111,11 @@ "@sentry/types" "7.102.0" "@sentry/utils" "7.102.0" -"@sentry/types@5.30.0": - version "5.30.0" - resolved "https://registry.yarnpkg.com/@sentry/types/-/types-5.30.0.tgz#19709bbe12a1a0115bc790b8942917da5636f402" - integrity sha512-R8xOqlSTZ+htqrfteCWU5Nk0CDN5ApUTvrlvBuiH1DyP6czDZ4ktbZB0hAgBlVcK0U+qpD3ag3Tqqpa5Q67rPw== - "@sentry/types@7.102.0": version "7.102.0" resolved "https://registry.yarnpkg.com/@sentry/types/-/types-7.102.0.tgz#b31e9faa54036053ab82c09c3c855035a4889c59" integrity sha512-FPfFBP0x3LkPARw1/6cWySLq1djIo8ao3Qo2KNBeE9CHdq8bsS1a8zzjJLuWG4Ww+wieLP8/lY3WTgrCz4jowg== -"@sentry/utils@5.30.0": - version "5.30.0" - resolved "https://registry.yarnpkg.com/@sentry/utils/-/utils-5.30.0.tgz#9a5bd7ccff85ccfe7856d493bffa64cabc41e980" - integrity sha512-zaYmoH0NWWtvnJjC9/CBseXMtKHm/tm40sz3YfJRxeQjyzRqNQPgivpd9R/oDJCYj999mzdW382p/qi2ypjLww== - dependencies: - "@sentry/types" "5.30.0" - tslib "^1.9.3" - "@sentry/utils@7.102.0": version "7.102.0" resolved "https://registry.yarnpkg.com/@sentry/utils/-/utils-7.102.0.tgz#66325f2567986cc3fd12fbdb980fb8ada170342b" @@ -12565,29 +12536,6 @@ send@0.17.2: range-parser "~1.2.1" statuses "~1.5.0" -"sentrybrowser5@npm:@sentry/browser@5.30.0": - version "5.30.0" - resolved "https://registry.yarnpkg.com/@sentry/browser/-/browser-5.30.0.tgz#c28f49d551db3172080caef9f18791a7fd39e3b3" - integrity sha512-rOb58ZNVJWh1VuMuBG1mL9r54nZqKeaIlwSlvzJfc89vyfd7n6tQ1UXMN383QBz/MS5H5z44Hy5eE+7pCrYAfw== - dependencies: - "@sentry/core" "5.30.0" - "@sentry/types" "5.30.0" - "@sentry/utils" "5.30.0" - tslib "^1.9.3" - -"sentrybrowser@npm:@sentry/browser@7.102.0": - version "7.102.0" - resolved "https://registry.yarnpkg.com/@sentry/browser/-/browser-7.102.0.tgz#335f51d01aabf8c4d2abc871855f9c2d19f8f70d" - integrity sha512-hIggcMnojIbWhbmlRfkykHmy6n7pjug0AHfF19HRUQxAx9KJfMH5YdWvohov0Hb9fS+jdvqgE+/4AWbEeXQrHw== - dependencies: - "@sentry-internal/feedback" "7.102.0" - "@sentry-internal/replay-canvas" "7.102.0" - "@sentry-internal/tracing" "7.102.0" - "@sentry/core" "7.102.0" - "@sentry/replay" "7.102.0" - "@sentry/types" "7.102.0" - "@sentry/utils" "7.102.0" - serialize-javascript@^2.1.2: version "2.1.2" resolved "https://registry.yarnpkg.com/serialize-javascript/-/serialize-javascript-2.1.2.tgz#ecec53b0e0317bdc95ef76ab7074b7384785fa61" @@ -13773,7 +13721,7 @@ tslib@2.3.0, tslib@~2.3.0: resolved "https://registry.yarnpkg.com/tslib/-/tslib-2.3.0.tgz#803b8cdab3e12ba581a4ca41c8839bbb0dacb09e" integrity sha512-N82ooyxVNm6h1riLCoyS9e3fuJ3AMG2zIZs2Gd1ATcSFjSA23Q0fzjjZeh0jbJvWVDZ0cJT8yaNNaaXHzueNjg== -tslib@^1.8.1, tslib@^1.9.0, tslib@^1.9.3: +tslib@^1.8.1, tslib@^1.9.0: version "1.14.1" resolved "https://registry.yarnpkg.com/tslib/-/tslib-1.14.1.tgz#cf2d38bdc34a134bcaf1091c41f6619e2f672d00" integrity sha512-Xni35NKzjgMrwevysHTCArtLDpPvye8zV/0E4EyYn43P7/7qvQwPh9BGkHewbMulVntbigmcT7rdX3BNo9wRJg== -- GitLab