Commit 80acfadf authored by Quang-Minh Nguyen's avatar Quang-Minh Nguyen

Clean up and fix feedback on Gitlab::ErrorTracking review

Issue https://gitlab.com/gitlab-com/gl-infra/scalability/-/issues/846
parent 1e6e9484
...@@ -32,7 +32,7 @@ module Gitlab ...@@ -32,7 +32,7 @@ module Gitlab
end end
def self.current_context_include?(attribute_name) def self.current_context_include?(attribute_name)
Labkit::Context.current.to_h.include?(Labkit::Context.log_key(attribute_name)) current.include?(Labkit::Context.log_key(attribute_name))
end end
def initialize(**args) def initialize(**args)
......
...@@ -108,8 +108,8 @@ module Gitlab ...@@ -108,8 +108,8 @@ module Gitlab
end end
if logging if logging
formater = Gitlab::ErrorTracking::LogFormatter.new formatter = Gitlab::ErrorTracking::LogFormatter.new
log_hash = formater.generate_log(exception, context_payload) log_hash = formatter.generate_log(exception, context_payload)
Gitlab::ErrorTracking::Logger.error(log_hash) Gitlab::ErrorTracking::Logger.error(log_hash)
end end
......
...@@ -8,24 +8,22 @@ module Gitlab ...@@ -8,24 +8,22 @@ module Gitlab
end end
def generate(exception, extra = {}) def generate(exception, extra = {})
payload = {} {
extra: extra_payload(exception, extra),
append_extra!(payload, exception, extra) tags: tags_payload,
append_tags!(payload) user: user_payload
append_user!(payload) }
payload
end end
private private
def append_extra!(payload, exception, extra) def extra_payload(exception, extra)
inline_extra = exception.try(:sentry_extra_data) inline_extra = exception.try(:sentry_extra_data)
if inline_extra.present? && inline_extra.is_a?(Hash) if inline_extra.present? && inline_extra.is_a?(Hash)
extra = extra.merge(inline_extra) extra = extra.merge(inline_extra)
end end
payload[:extra] = sanitize_request_parameters(extra) sanitize_request_parameters(extra)
end end
def sanitize_request_parameters(parameters) def sanitize_request_parameters(parameters)
...@@ -33,11 +31,8 @@ module Gitlab ...@@ -33,11 +31,8 @@ module Gitlab
filter.filter(parameters) filter.filter(parameters)
end end
def append_tags!(payload) def tags_payload
payload[:tags] = {} extra_tags_from_env.merge!(
payload[:tags]
.merge!(extra_tags_from_env)
.merge!(
program: Gitlab.process_name, program: Gitlab.process_name,
locale: I18n.locale, locale: I18n.locale,
feature_category: current_context['meta.feature_category'], feature_category: current_context['meta.feature_category'],
...@@ -45,8 +40,8 @@ module Gitlab ...@@ -45,8 +40,8 @@ module Gitlab
) )
end end
def append_user!(payload) def user_payload
payload[:user] = { {
username: current_context['meta.user'] username: current_context['meta.user']
} }
end end
......
...@@ -37,22 +37,18 @@ module Gitlab ...@@ -37,22 +37,18 @@ module Gitlab
extra = Raven.context.extra.merge(context_payload[:extra]) extra = Raven.context.extra.merge(context_payload[:extra])
extra = extra.except(:server) extra = extra.except(:server)
sidekiq_extra = extra[:sidekiq]
# The extra value for sidekiq is a hash whose keys are strings. # The extra value for sidekiq is a hash whose keys are strings.
if sidekiq_extra.is_a?(Hash) && sidekiq_extra.key?('args') if extra[:sidekiq].is_a?(Hash) && extra[:sidekiq].key?('args')
sidekiq_extra = sidekiq_extra.dup sidekiq_extra = extra.delete(:sidekiq)
sidekiq_extra['args'] = Gitlab::ErrorTracking::Processor::SidekiqProcessor.loggable_arguments( sidekiq_extra['args'] = Gitlab::ErrorTracking::Processor::SidekiqProcessor.loggable_arguments(
sidekiq_extra['args'], sidekiq_extra['class'] sidekiq_extra['args'], sidekiq_extra['class']
) )
payload["extra.sidekiq"] = sidekiq_extra
end end
extra[:sidekiq] = sidekiq_extra if sidekiq_extra
extra.each do |key, value| extra.each do |key, value|
payload["extra.#{key}"] = value payload["extra.#{key}"] = value
end end
payload
end end
end end
end end
......
# frozen_string_literal: true # frozen_string_literal: true
require 'spec_helper' require 'fast_spec_helper'
require 'rspec-parameterized' require 'rspec-parameterized'
RSpec.describe Gitlab::ErrorTracking::ContextPayloadGenerator do RSpec.describe Gitlab::ErrorTracking::ContextPayloadGenerator do
......
# frozen_string_literal: true # frozen_string_literal: true
require 'spec_helper' require 'fast_spec_helper'
RSpec.describe Gitlab::ErrorTracking::LogFormatter do RSpec.describe Gitlab::ErrorTracking::LogFormatter do
let(:exception) { StandardError.new('boom') } let(:exception) { StandardError.new('boom') }
...@@ -43,7 +43,7 @@ RSpec.describe Gitlab::ErrorTracking::LogFormatter do ...@@ -43,7 +43,7 @@ RSpec.describe Gitlab::ErrorTracking::LogFormatter do
::Raven::Context.clear! ::Raven::Context.clear!
end end
it 'appends error-related log fields' do it 'appends error-related log fields and filters sensitive Sidekiq arguments' do
payload = described_class.new.generate_log(exception, context_payload) payload = described_class.new.generate_log(exception, context_payload)
expect(payload).to eql( expect(payload).to eql(
......
# frozen_string_literal: true # frozen_string_literal: true
require 'spec_helper' require 'fast_spec_helper'
RSpec.describe Gitlab::ErrorTracking::Processor::ContextPayloadProcessor do RSpec.describe Gitlab::ErrorTracking::Processor::ContextPayloadProcessor do
subject(:processor) { described_class.new } subject(:processor) { described_class.new }
......
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