Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove ActiveJob keys for both Sidekiq and DelayedJob #898

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion lib/raven/integrations/delayed_job.rb
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
require 'delayed_job'
require 'raven/utils/context_filter'

module Delayed
module Plugins
Expand Down Expand Up @@ -29,7 +30,7 @@ class Raven < ::Delayed::Plugin
extra[:handler] = job.handler[0...1000] if job.handler

if job.respond_to?('payload_object') && job.payload_object.respond_to?('job_data')
extra[:active_job] = job.payload_object.job_data
extra[:active_job] = Utils::ContextFilter.filter_context(job.payload_object.job_data)
end
::Raven.capture_exception(e,
:logger => 'delayed_job',
Expand Down
4 changes: 2 additions & 2 deletions lib/raven/integrations/sidekiq/error_handler.rb
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
require 'raven/integrations/sidekiq/context_filter'
require 'raven/utils/context_filter'

module Raven
module Sidekiq
class ErrorHandler
SIDEKIQ_NAME = "Sidekiq".freeze

def call(ex, context)
context = ContextFilter.filter_context(context)
context = Utils::ContextFilter.filter_context(context)
Raven.context.transaction.push transaction_from_context(context)
Raven.capture_exception(
ex,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
module Raven
module Sidekiq
module Utils
module ContextFilter
class << self
ACTIVEJOB_RESERVED_PREFIX = "_aj_".freeze
ACTIVEJOB_RESERVED_PREFIX_REGEX = /^_aj_/.freeze
HAS_GLOBALID = const_defined?('GlobalID')

# Once an ActiveJob is queued, ActiveRecord references get serialized into
Expand All @@ -25,7 +25,7 @@ def filter_context(context)
private

def filter_context_hash(key, value)
(key = key[3..-1]) if key [0..3] == ACTIVEJOB_RESERVED_PREFIX
key = key.to_s.sub(ACTIVEJOB_RESERVED_PREFIX_REGEX, "") if key.match(ACTIVEJOB_RESERVED_PREFIX_REGEX)
[key, filter_context(value)]
end

Expand Down
2 changes: 1 addition & 1 deletion spec/raven/integrations/sidekiq/error_handler_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@
aj_context["_aj_globalid"] = GlobalID.new('gid://app/model/id')
expected_context = aj_context.dup
expected_context.delete("_aj_globalid")
expected_context["_globalid"] = "gid://app/model/id"
expected_context["globalid"] = "gid://app/model/id"
expected_options = {
:message => exception.message,
:extra => { :sidekiq => expected_context }
Expand Down
41 changes: 41 additions & 0 deletions spec/raven/utils/context_filter_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
require "spec_helper"

require "raven/utils/context_filter"

RSpec.describe Raven::Utils::ContextFilter do
context "filters out ActiveJob keys from context" do
let(:context) do
{ :_aj_globalid => "gid://app/model/id", :key => "value" }
end
let(:expected_context) do
{ "globalid" => "gid://app/model/id", :key => "value" }
end

it "removes reserved keys" do
new_context = described_class.filter_context(context)

expect(new_context).to eq(expected_context)
end
end

context "filters out ActiveJob keys from nested context" do
let(:context) do
{
:_aj_globalid => "gid://app/model/id",
:arguments => { :key => "value", :_aj_symbol_keys => ["key"] }
}
end
let(:expected_context) do
{
"globalid" => "gid://app/model/id",
:arguments => { :key => "value", "symbol_keys" => ["key"] }
}
end

it "removes reserved keys" do
new_context = described_class.filter_context(context)

expect(new_context).to eq(expected_context)
end
end
end