Merge pull request #457 from epimorphics/hotfix/reduce-log-noise

Hotfix: Improved Logging Instrumentation

CHANGELOG.md

@@ -1,5 +1,15 @@
 # Changes to the UKHPI app by version and date
 
+## 1.7.6 - 2024-10
+
+- (Jon) Split the error logging into it's own method as well as adjusted the
+  logged message to be either the response message or the response status
+- (Jon) Renamed `render_error` method to `handle_error`
+- (Jon) Set the Internal Error Instrumentation to an `unless` statement to
+  ensure the application does not report internal errors to the Prometheus
+  metrics when the error is a 404 thereby reducing the noise in the Slack alerts
+  channel
+
 ## 1.7.5 - 2024-09
 
 - (Jon) Created a `local` makefile target to allow for local development without

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
 
@@ -54,20 +54,28 @@ def detailed_request_log(duration)
       body: request.body.gets&.gsub("\n", '\n'),
       method: request.method,
       status: response.status,
-      message: Rack::Utils::HTTP_STATUS_CODES[response.status]
+      message: response.message || Rack::Utils::HTTP_STATUS_CODES[response.status]
     }
 
-    case response.status
-    when 500..599
+    if (500..599).include?(Rack::Utils::SYMBOL_TO_STATUS_CODE[response.status])
       log_fields[:message] = env['action_dispatch.exception'].to_s
-      Rails.logger.error(JSON.generate(log_fields))
+    end
+
+    log_response(response.status, log_fields)
+  end
+  # rubocop:enable Metrics/AbcSize, Metrics/MethodLength
+
+  # Log the error with the appropriate log level based on the status code
+  def log_response(status, error_log)
+    case status
+    when 500..599
+      Rails.logger.error(JSON.generate(error_log))
     when 400..499
-      Rails.logger.warn(JSON.generate(log_fields))
+      Rails.logger.warn(JSON.generate(error_log))
     else
-      Rails.logger.info(JSON.generate(log_fields))
+      Rails.logger.info(JSON.generate(error_log))
     end
   end
-  # rubocop:enable Metrics/AbcSize
 
   # Notify subscriber(s) of an internal error event with the payload of the
   # exception once done

app/controllers/exceptions_controller.rb

@@ -4,14 +4,15 @@
 class ExceptionsController < ApplicationController
   layout 'application'
 
-  def render_error
+  def handle_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)
+
+    # add the exception to the prometheus metrics but only on errors that are 404
+    instrument_internal_error(exception) unless status_code == 404
 
     render :error_page,
            locals: { status: status_code, sentry_code: sentry_code },
@@ -22,7 +23,7 @@ def render_error
   private
 
   def maybe_report_to_sentry(exception, status_code)
-    return nil if Rails.env.development? # Why are we reporting to Senty in dev?
+    return nil if Rails.env.development? # Why are we reporting to Sentry in dev?
     return nil unless status_code >= 500
 
     sevent = Sentry.capture_exception(exception)

app/lib/version.rb

@@ -3,7 +3,7 @@
 module Version
   MAJOR = 1
   MINOR = 7
-  PATCH = 5
+  PATCH = 6
   SUFFIX = nil
   VERSION = "#{MAJOR}.#{MINOR}.#{PATCH}#{SUFFIX && ".#{SUFFIX}"}"
 end

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_error).call(env)
   end