Commit 400dd57b authored by Stan Hu's avatar Stan Hu

Standardize error message field in structured logs

Some of our log messages emitted `error` as a string, and others emitted
`error.message`. In Elasticsearch, if the `error.message` is mapping is
used, `error` is an object capable of holding other fields. As a result
of these mixed types, Elasticsearch would flag a mapping conflict and
dropped certain log messages, depending on which value arrived first for
the day.

To avoid this mapping conflict, we now standardize on `error.message`.

Relates to https://gitlab.com/gitlab-org/gitlab/-/issues/344015

Changelog: fixed
parent dfa79015
...@@ -528,7 +528,9 @@ module Integrations ...@@ -528,7 +528,9 @@ module Integrations
yield yield
rescue StandardError => error rescue StandardError => error
@error = error @error = error
log_error("Error sending message", client_url: client_url, error: @error.message) payload = { client_url: client_url }
Gitlab::ExceptionLogFormatter.format!(error, payload)
log_error("Error sending message", payload)
nil nil
end end
......
...@@ -14,13 +14,13 @@ module Ci ...@@ -14,13 +14,13 @@ module Ci
sync_coverage_rules sync_coverage_rules
success success
rescue StandardError => error rescue StandardError => error
log_error( payload = {
pipeline: pipeline&.to_param, pipeline: pipeline&.to_param,
error: error.class.name, source: "#{__FILE__}:#{__LINE__}"
message: error.message, }
source: "#{__FILE__}:#{__LINE__}",
backtrace: error.backtrace Gitlab::ExceptionLogFormatter.format!(error, payload)
) log_error(payload)
error("Failed to update approval rules") error("Failed to update approval rules")
ensure ensure
[:project_rule_vulnerabilities_allowed, :project_rule_scanners, :project_rule_severity_levels, :project_vulnerability_report, :reports].each do |memoization| [:project_rule_vulnerabilities_allowed, :project_rule_scanners, :project_rule_severity_levels, :project_vulnerability_report, :reports].each do |memoization|
......
...@@ -129,6 +129,13 @@ RSpec.describe Ci::SyncReportsToApprovalRulesService, '#execute' do ...@@ -129,6 +129,13 @@ RSpec.describe Ci::SyncReportsToApprovalRulesService, '#execute' do
allow_next_instance_of(Gitlab::Ci::Reports::LicenseScanning::Report) do |instance| allow_next_instance_of(Gitlab::Ci::Reports::LicenseScanning::Report) do |instance|
allow(instance).to receive(:violates?).and_raise('heck') allow(instance).to receive(:violates?).and_raise('heck')
end end
expect(Gitlab::AppLogger).to receive(:error).with(
hash_including(pipeline: anything,
'exception.class' => anything,
'exception.message' => anything,
'exception.backtrace' => anything,
source: anything)).and_call_original
end end
specify { expect(subject[:status]).to be(:error) } specify { expect(subject[:status]).to be(:error) }
......
...@@ -972,7 +972,9 @@ RSpec.describe Integrations::Jira do ...@@ -972,7 +972,9 @@ RSpec.describe Integrations::Jira do
expect(jira_integration).to receive(:log_error).with( expect(jira_integration).to receive(:log_error).with(
'Error sending message', 'Error sending message',
client_url: 'http://jira.example.com', client_url: 'http://jira.example.com',
error: error_message 'exception.class' => anything,
'exception.message' => error_message,
'exception.backtrace' => anything
) )
expect(jira_integration.test(nil)).to eq(success: false, result: error_message) expect(jira_integration.test(nil)).to eq(success: false, result: error_message)
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment