From 74096016eea016015fc68c561833b9f1acbf745a Mon Sep 17 00:00:00 2001 From: Bogdan Marc Date: Mon, 23 Dec 2024 12:21:26 +0000 Subject: [PATCH] Revert "Merge pull request #468 from epimorphics/spike/reduce-metric-alerts" This reverts commit b18c3df43f1afacba99ffb217ee23e0656a35faf, reversing changes made to 625b8e21cbff0bb77a15f94477ef238f2250cbc2. --- CHANGELOG.md | 13 ------------- app/controllers/application_controller.rb | 23 +++++------------------ app/controllers/exceptions_controller.rb | 7 +++---- app/lib/version.rb | 2 +- config/initializers/exceptions.rb | 2 +- 5 files changed, 10 insertions(+), 37 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7b5bc2e8..e3e8b368 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,18 +1,5 @@ # Changes to the UKHPI app by version and date -## 2.0.1 - 2024-12 - -- (Jon) Improves error metrics reporting to ensure that logging always happens - with the appropriate severity depending on the exception status while reducing - the types of errors that can trigger a an error metric and therefore a - notification in slack - [GH-149](https://github.com/epimorphics/hmlr-linked-data/issues/149) -- (Jon) Updated the `maybe_report_to_sentry` method to call the - `instrument_internal_error` method for reporting internal errors to the - Prometheus metrics when necessary -- (Jon) Renamed the `handle_error` method to `handle_exceptions` to better - reflect the method's purpose - ## 2.0.0 - 2024-11 - (Bogdan) Updated all gems by regenerating `Gemfile.lock` diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index ad98b087..34285f77 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -39,7 +39,7 @@ def log_request_result private - # rubocop:disable Metrics/AbcSize, Metrics/MethodLength + # rubocop:disable Metrics/AbcSize def detailed_request_log(duration) env = request.env @@ -67,27 +67,14 @@ def detailed_request_log(duration) Rails.logger.info(JSON.generate(log_fields)) end end - # rubocop:enable Metrics/AbcSize, Metrics/MethodLength + # rubocop:enable Metrics/AbcSize # Notify subscriber(s) of an internal error event with the payload of the # exception once done - # @param [exc] exp the exception that caused the error + # @param [Exception] exp the exception that caused the error # @return [ActiveSupport::Notifications::Event] provides an object-oriented # interface to the event - def instrument_internal_error(exc) # rubocop:disable Metrics/AbcSize, Metrics/CyclomaticComplexity, Metrics/PerceivedComplexity - err = { - message: exc&.message || exc, - status: exc&.status || Rack::Utils::SYMBOL_TO_STATUS_CODE[exc] - } - err[:type] = exc.class&.name if exc&.class - err[:cause] = exc&.cause if exc&.cause - err[:backtrace] = exc&.backtrace if exc&.backtrace && Rails.env.development? - # Log the exception to the Rails logger with the appropriate severity - Rails.logger.send(err[:status] < 500 ? :warn : :error, JSON.generate(err)) - # Return unless the status code is 500 or greater to ensure subscribers are NOT notified - return unless err[:status] >= 500 - - # Instrument the internal error event to notify subscribers of the error - ActiveSupport::Notifications.instrument('internal_error.application', exception: err) + def instrument_internal_error(exception) + ActiveSupport::Notifications.instrument('internal_error.application', exception: exception) end end diff --git a/app/controllers/exceptions_controller.rb b/app/controllers/exceptions_controller.rb index e0b9fc75..11c20945 100644 --- a/app/controllers/exceptions_controller.rb +++ b/app/controllers/exceptions_controller.rb @@ -4,12 +4,14 @@ class ExceptionsController < ApplicationController layout 'application' - def handle_exceptions + def render_error env = request.env exception = env['action_dispatch.exception'] status_code = ActionDispatch::ExceptionWrapper.new(env, exception).status_code sentry_code = maybe_report_to_sentry(exception, status_code) + # add the exception to the prometheus metrics + instrument_internal_error(exception) render :error_page, locals: { status: status_code, sentry_code: sentry_code }, @@ -23,9 +25,6 @@ def maybe_report_to_sentry(exception, status_code) return nil if Rails.env.development? # Why are we reporting to Senty in dev? return nil unless status_code >= 500 - # add the exception to the prometheus metrics - instrument_internal_error(exception) - sevent = Sentry.capture_exception(exception) sevent&.event_id end diff --git a/app/lib/version.rb b/app/lib/version.rb index 4d205e68..da348ade 100644 --- a/app/lib/version.rb +++ b/app/lib/version.rb @@ -3,7 +3,7 @@ module Version MAJOR = 2 MINOR = 0 - PATCH = 1 + PATCH = 0 SUFFIX = nil VERSION = "#{MAJOR}.#{MINOR}.#{PATCH}#{SUFFIX && ".#{SUFFIX}"}" end diff --git a/config/initializers/exceptions.rb b/config/initializers/exceptions.rb index 48551584..b0de5388 100644 --- a/config/initializers/exceptions.rb +++ b/config/initializers/exceptions.rb @@ -3,5 +3,5 @@ # Custom error handling via a Rack middleware action Rails.application.config.exceptions_app = lambda do |env| - ExceptionsController.action(:handle_exceptions).call(env) + ExceptionsController.action(:render_error).call(env) end