Skip to content

Commit

Permalink
Log error in a full string instead of json
Browse files Browse the repository at this point in the history
In order to avoid complexity around parsing JSON, we log errors in one full string now

Co-authored-by: Neamah Al-Selwi <neamah.alselwi@digital.cabinet-office.gov.uk>
Co-authored-by: Tuomas Nylund <tuomas.nylund@digital.cabinet-office.gov.uk>
  • Loading branch information
3 people committed Jul 5, 2023
1 parent 0a6f772 commit e13795b
Show file tree
Hide file tree
Showing 3 changed files with 13 additions and 24 deletions.
8 changes: 1 addition & 7 deletions lib/govuk_app_config/govuk_json_logging.rb
Original file line number Diff line number Diff line change
Expand Up @@ -13,15 +13,9 @@ def self.configure
$stdout,
level: Rails.logger.level,
formatter: proc { |severity, datetime, _progname, msg|
begin
message = JSON.parse(msg)
rescue JSON::ParserError, TypeError => _e
message = msg
end

hash = {
"@timestamp": datetime.utc.iso8601(3),
message: message,
message: msg,
level: severity,
tags: %w[rails],
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,12 +38,9 @@ def log_error(request, wrapper)

trace = wrapper.application_trace
trace = wrapper.framework_trace if trace.empty?
exception.set_backtrace(trace)

logger.fatal({
exception_class: exception.class.to_s,
exception_message: exception.message,
stacktrace: trace,
}.to_json)
logger.fatal(exception.full_message)
end
end
end
Expand Down
22 changes: 10 additions & 12 deletions spec/lib/govuk_json_logging_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,10 @@ def self.headers

after { Rails.application = nil }

# By storing origin stdout in a constant and redirect `$stdout` to a fake one,
# We are able to inspect and test what is printed
# BUT it also suppress all the normal log outputs
# I.E. puts doesn't work anymore :D
original_stderr = nil
original_stdout = nil

Expand Down Expand Up @@ -97,12 +101,9 @@ def app
))

error_log_json_msg = error_log_json["message"]
expect(error_log_json_msg).to match(hash_including(
"exception_class" => "StandardError",
"exception_message" => "default exception",
))
expect(error_log_json_msg).to have_key("stacktrace")
expect(error_log_json_msg["stacktrace"]).to be_a(Array)
expect(error_log_json_msg).to include("StandardError")
expect(error_log_json_msg).to include("default exception")
expect(error_log_json_msg).to match(/from.*:[0-9]+:in.*/)
end

it "logs errors thrown by the application with no govuk_request_id" do
Expand All @@ -115,12 +116,9 @@ def app
expect(error_log_line).not_to be_empty
error_log_json = JSON.parse(error_log_line)
error_log_json_msg = error_log_json["message"]
expect(error_log_json_msg).to match(hash_including(
"exception_class" => "StandardError",
"exception_message" => "default exception",
))
expect(error_log_json_msg).to have_key("stacktrace")
expect(error_log_json_msg["stacktrace"]).to be_a(Array)
expect(error_log_json_msg).to include("StandardError")
expect(error_log_json_msg).to include("default exception")
expect(error_log_json_msg).to match(/from.*:[0-9]+:in.*/)
end

it "logs to stdout in JSON format with govuk_request_id" do
Expand Down

0 comments on commit e13795b

Please sign in to comment.