From 2833237679a513d54b3d31a9a478ce85b7334929 Mon Sep 17 00:00:00 2001 From: Bogdan Marc Date: Fri, 22 Nov 2024 11:32:25 +0000 Subject: [PATCH 01/12] Removed version lock for rubygems --- Dockerfile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Dockerfile b/Dockerfile index 1f36aff..1597029 100644 --- a/Dockerfile +++ b/Dockerfile @@ -13,7 +13,7 @@ RUN apk add --update \ tzdata \ yarn \ && rm -rf /var/cache/apk/* \ - && gem install rubygems-update -v 3.4.22 \ + && gem install rubygems-update \ && update_rubygems \ && gem install bundler:$BUNDLER_VERSION \ && bundle config --global frozen 1 From 2ba3bfca569ce2be9bfbe45bc0995786a143428a Mon Sep 17 00:00:00 2001 From: Bogdan Marc Date: Fri, 22 Nov 2024 14:07:57 +0000 Subject: [PATCH 02/12] Changed the way we update rubygems in Dockerfile --- Dockerfile | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/Dockerfile b/Dockerfile index 1597029..8f6a344 100644 --- a/Dockerfile +++ b/Dockerfile @@ -13,8 +13,7 @@ RUN apk add --update \ tzdata \ yarn \ && rm -rf /var/cache/apk/* \ - && gem install rubygems-update \ - && update_rubygems \ + && gem update --system \ && gem install bundler:$BUNDLER_VERSION \ && bundle config --global frozen 1 From f39874de786fcaa951d801faba5dc95c6a1878e3 Mon Sep 17 00:00:00 2001 From: Bogdan Marc Date: Fri, 22 Nov 2024 15:28:57 +0000 Subject: [PATCH 03/12] Fix rubygems version --- Dockerfile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Dockerfile b/Dockerfile index 8f6a344..3e4d0db 100644 --- a/Dockerfile +++ b/Dockerfile @@ -13,7 +13,7 @@ RUN apk add --update \ tzdata \ yarn \ && rm -rf /var/cache/apk/* \ - && gem update --system \ + && gem update --system 3.4.22 \ && gem install bundler:$BUNDLER_VERSION \ && bundle config --global frozen 1 From 1f60080af5cb33a4af0a699d3a02dfe48edffead Mon Sep 17 00:00:00 2001 From: Bogdan Marc Date: Fri, 22 Nov 2024 15:47:44 +0000 Subject: [PATCH 04/12] Revert to original rubygems update method --- Dockerfile | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Dockerfile b/Dockerfile index 3e4d0db..1f36aff 100644 --- a/Dockerfile +++ b/Dockerfile @@ -13,7 +13,8 @@ RUN apk add --update \ tzdata \ yarn \ && rm -rf /var/cache/apk/* \ - && gem update --system 3.4.22 \ + && gem install rubygems-update -v 3.4.22 \ + && update_rubygems \ && gem install bundler:$BUNDLER_VERSION \ && bundle config --global frozen 1 From 625b8e21cbff0bb77a15f94477ef238f2250cbc2 Mon Sep 17 00:00:00 2001 From: Bogdan Marc Date: Tue, 26 Nov 2024 10:10:05 +0000 Subject: [PATCH 05/12] Changed the way we update rubygems --- Dockerfile | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/Dockerfile b/Dockerfile index 1f36aff..8f6a344 100644 --- a/Dockerfile +++ b/Dockerfile @@ -13,8 +13,7 @@ RUN apk add --update \ tzdata \ yarn \ && rm -rf /var/cache/apk/* \ - && gem install rubygems-update -v 3.4.22 \ - && update_rubygems \ + && gem update --system \ && gem install bundler:$BUNDLER_VERSION \ && bundle config --global frozen 1 From ac73ff00cc008e8652cdae459d12f75edc520dd9 Mon Sep 17 00:00:00 2001 From: "Jon R. Humphrey" Date: Mon, 16 Dec 2024 17:06:55 +0000 Subject: [PATCH 06/12] fix: remove error instrumentation from standard error handling --- app/controllers/exceptions_controller.rb | 2 -- 1 file changed, 2 deletions(-) diff --git a/app/controllers/exceptions_controller.rb b/app/controllers/exceptions_controller.rb index 11c2094..8258309 100644 --- a/app/controllers/exceptions_controller.rb +++ b/app/controllers/exceptions_controller.rb @@ -10,8 +10,6 @@ def render_error 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 }, From d4e436b8bcf754d3dfbdbb8d642841094723bc22 Mon Sep 17 00:00:00 2001 From: "Jon R. Humphrey" Date: Mon, 16 Dec 2024 17:08:23 +0000 Subject: [PATCH 07/12] fix: only trigger instrumentation on internal errors --- app/controllers/exceptions_controller.rb | 3 +++ 1 file changed, 3 insertions(+) diff --git a/app/controllers/exceptions_controller.rb b/app/controllers/exceptions_controller.rb index 8258309..cba7da5 100644 --- a/app/controllers/exceptions_controller.rb +++ b/app/controllers/exceptions_controller.rb @@ -23,6 +23,9 @@ 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 From 6cbc0cfca99b46361bf38981e93303e07a23b573 Mon Sep 17 00:00:00 2001 From: "Jon R. Humphrey" Date: Mon, 16 Dec 2024 17:09:28 +0000 Subject: [PATCH 08/12] fix: improve error metrics reporting - ensures that logging always happens with the appropriate severity depending on the exception status - reduces the types of errors that can trigger a an error metric and therefore a notification in slack (e.g. :bad_request is set at WARN level and therefore not alerted on, whereas :internal_server_error is set at ERROR level and does trigger alerts - includes minor rubocop lint disabling where needed --- app/controllers/application_controller.rb | 23 ++++++++++++++++++----- 1 file changed, 18 insertions(+), 5 deletions(-) diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 34285f7..ad98b08 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 + # rubocop:disable Metrics/AbcSize, Metrics/MethodLength def detailed_request_log(duration) env = request.env @@ -67,14 +67,27 @@ def detailed_request_log(duration) Rails.logger.info(JSON.generate(log_fields)) end end - # rubocop:enable Metrics/AbcSize + # rubocop:enable Metrics/AbcSize, Metrics/MethodLength # Notify subscriber(s) of an internal error event with the payload of the # exception once done - # @param [Exception] exp the exception that caused the error + # @param [exc] exp the exception that caused the error # @return [ActiveSupport::Notifications::Event] provides an object-oriented # interface to the event - def instrument_internal_error(exception) - ActiveSupport::Notifications.instrument('internal_error.application', exception: exception) + 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) end end From c620a76c1890ac1e297401a5a063c6c95562b41c Mon Sep 17 00:00:00 2001 From: "Jon R. Humphrey" Date: Mon, 16 Dec 2024 17:10:22 +0000 Subject: [PATCH 09/12] style: Renamed the `handle_error` method to `handle_exceptions` --- app/controllers/exceptions_controller.rb | 2 +- config/initializers/exceptions.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/app/controllers/exceptions_controller.rb b/app/controllers/exceptions_controller.rb index cba7da5..e0b9fc7 100644 --- a/app/controllers/exceptions_controller.rb +++ b/app/controllers/exceptions_controller.rb @@ -4,7 +4,7 @@ class ExceptionsController < ApplicationController layout 'application' - def render_error + def handle_exceptions env = request.env exception = env['action_dispatch.exception'] status_code = ActionDispatch::ExceptionWrapper.new(env, exception).status_code diff --git a/config/initializers/exceptions.rb b/config/initializers/exceptions.rb index b0de538..4855158 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(:render_error).call(env) + ExceptionsController.action(:handle_exceptions).call(env) end From 5f2d21fd16f89cdbbe23d53f773a827dc3501320 Mon Sep 17 00:00:00 2001 From: "Jon R. Humphrey" Date: Fri, 20 Dec 2024 10:17:25 +0000 Subject: [PATCH 10/12] docs: updated CHANGELOG --- CHANGELOG.md | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index e3e8b36..8c2622c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,18 @@ # 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 neccessary +- (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` From 019d0ca2b7b58c10550d24281c261502ac608d4b Mon Sep 17 00:00:00 2001 From: "Jon R. Humphrey" Date: Fri, 20 Dec 2024 10:17:40 +0000 Subject: [PATCH 11/12] build: updated patch version cadence --- app/lib/version.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/lib/version.rb b/app/lib/version.rb index da348ad..4d205e6 100644 --- a/app/lib/version.rb +++ b/app/lib/version.rb @@ -3,7 +3,7 @@ module Version MAJOR = 2 MINOR = 0 - PATCH = 0 + PATCH = 1 SUFFIX = nil VERSION = "#{MAJOR}.#{MINOR}.#{PATCH}#{SUFFIX && ".#{SUFFIX}"}" end From 3ef38d435839b1adf0b17c43ea1126c5af739a41 Mon Sep 17 00:00:00 2001 From: "Jon R. Humphrey" Date: Fri, 20 Dec 2024 10:19:23 +0000 Subject: [PATCH 12/12] docs: fix CHANGELOG typo --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8c2622c..7b5bc2e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,7 +9,7 @@ [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 neccessary + Prometheus metrics when necessary - (Jon) Renamed the `handle_error` method to `handle_exceptions` to better reflect the method's purpose