From 802630378e5ed7ffae500034bc926b8318524e43 Mon Sep 17 00:00:00 2001 From: Ari Pollak Date: Fri, 28 Nov 2014 14:19:16 -0500 Subject: Hide potentially sensitive ActiveJob params from logs * Show GlobalID instead of full object .inspect output --- activejob/lib/active_job/logging.rb | 17 ++++++++++++++++- activejob/test/cases/logging_test.rb | 28 ++++++++++++++++++---------- 2 files changed, 34 insertions(+), 11 deletions(-) (limited to 'activejob') diff --git a/activejob/lib/active_job/logging.rb b/activejob/lib/active_job/logging.rb index 21d2fda3ff..74c02e149a 100644 --- a/activejob/lib/active_job/logging.rb +++ b/activejob/lib/active_job/logging.rb @@ -6,6 +6,8 @@ module ActiveJob module Logging #:nodoc: extend ActiveSupport::Concern + FILTERED = '[FILTERED]'.freeze + included do cattr_accessor(:logger) { ActiveSupport::TaggedLogging.new(ActiveSupport::Logger.new(STDOUT)) } @@ -84,8 +86,21 @@ module ActiveJob event.payload[:adapter].name.demodulize.remove('Adapter') + "(#{event.payload[:job].queue_name})" end + def global_id_or_filtered(argument) + if argument.is_a?(GlobalID::Identification) + argument.to_global_id.to_s + else + FILTERED + end + end + def args_info(job) - job.arguments.any? ? " with arguments: #{job.arguments.map(&:inspect).join(", ")}" : "" + if job.arguments.any? + ' with arguments: ' + + job.arguments.map { |arg| global_id_or_filtered(arg) }.join(', ') + else + "" + end end def scheduled_at(event) diff --git a/activejob/test/cases/logging_test.rb b/activejob/test/cases/logging_test.rb index 3d4e561117..e308232073 100644 --- a/activejob/test/cases/logging_test.rb +++ b/activejob/test/cases/logging_test.rb @@ -42,37 +42,45 @@ class AdapterTest < ActiveSupport::TestCase def test_uses_active_job_as_tag - HelloJob.perform_later "Cristian" + HelloJob.perform_later nil assert_match(/\[ActiveJob\]/, @logger.messages) end def test_uses_job_name_as_tag - LoggingJob.perform_later "Dummy" + LoggingJob.perform_later nil assert_match(/\[LoggingJob\]/, @logger.messages) end def test_uses_job_id_as_tag - LoggingJob.perform_later "Dummy" + LoggingJob.perform_later nil assert_match(/\[LOGGING-JOB-ID\]/, @logger.messages) end def test_logs_correct_queue_name original_queue_name = LoggingJob.queue_name LoggingJob.queue_as :php_jobs - LoggingJob.perform_later("Dummy") + LoggingJob.perform_later nil assert_match(/to .*?\(php_jobs\).*/, @logger.messages) ensure LoggingJob.queue_name = original_queue_name end + def test_globalid_parameter_logging + person = Person.new(123) + LoggingJob.perform_later person + assert_match(%r{Enqueued.*gid://aj/Person/123}, @logger.messages) + assert_match(%r{Dummy, here is it: #}, @logger.messages) + assert_match(%r{Performing.*gid://aj/Person/123}, @logger.messages) + end + def test_enqueue_job_logging HelloJob.perform_later "Cristian" - assert_match(/Enqueued HelloJob \(Job ID: .*?\) to .*?:.*Cristian/, @logger.messages) + assert_match(/Enqueued HelloJob \(Job ID: .*?\) to .*?:.*\[FILTERED\]/, @logger.messages) end def test_perform_job_logging LoggingJob.perform_later "Dummy" - assert_match(/Performing LoggingJob from .*? with arguments:.*Dummy/, @logger.messages) + assert_match(/Performing LoggingJob from .*? with arguments:.*\[FILTERED\]/, @logger.messages) assert_match(/Dummy, here is it: Dummy/, @logger.messages) assert_match(/Performed LoggingJob from .*? in .*ms/, @logger.messages) end @@ -82,8 +90,8 @@ class AdapterTest < ActiveSupport::TestCase assert_match(/\[LoggingJob\] \[.*?\]/, @logger.messages) assert_match(/\[ActiveJob\] Enqueued NestedJob \(Job ID: .*\) to/, @logger.messages) assert_match(/\[ActiveJob\] \[NestedJob\] \[NESTED-JOB-ID\] Performing NestedJob from/, @logger.messages) - assert_match(/\[ActiveJob\] \[NestedJob\] \[NESTED-JOB-ID\] Enqueued LoggingJob \(Job ID: .*?\) to .* with arguments: "NestedJob"/, @logger.messages) - assert_match(/\[ActiveJob\].*\[LoggingJob\] \[LOGGING-JOB-ID\] Performing LoggingJob from .* with arguments: "NestedJob"/, @logger.messages) + assert_match(/\[ActiveJob\] \[NestedJob\] \[NESTED-JOB-ID\] Enqueued LoggingJob \(Job ID: .*?\) to .* with arguments: \[FILTERED\]/, @logger.messages) + assert_match(/\[ActiveJob\].*\[LoggingJob\] \[LOGGING-JOB-ID\] Performing LoggingJob from .* with arguments: \[FILTERED\]/, @logger.messages) assert_match(/\[ActiveJob\].*\[LoggingJob\] \[LOGGING-JOB-ID\] Dummy, here is it: NestedJob/, @logger.messages) assert_match(/\[ActiveJob\].*\[LoggingJob\] \[LOGGING-JOB-ID\] Performed LoggingJob from .* in/, @logger.messages) assert_match(/\[ActiveJob\] \[NestedJob\] \[NESTED-JOB-ID\] Performed NestedJob from .* in/, @logger.messages) @@ -91,14 +99,14 @@ class AdapterTest < ActiveSupport::TestCase def test_enqueue_at_job_logging HelloJob.set(wait_until: 24.hours.from_now).perform_later "Cristian" - assert_match(/Enqueued HelloJob \(Job ID: .*\) to .*? at.*Cristian/, @logger.messages) + assert_match(/Enqueued HelloJob \(Job ID: .*\) to .*? at.*\[FILTERED\]/, @logger.messages) rescue NotImplementedError skip end def test_enqueue_in_job_logging HelloJob.set(wait: 2.seconds).perform_later "Cristian" - assert_match(/Enqueued HelloJob \(Job ID: .*\) to .*? at.*Cristian/, @logger.messages) + assert_match(/Enqueued HelloJob \(Job ID: .*\) to .*? at.*\[FILTERED\]/, @logger.messages) rescue NotImplementedError skip end -- cgit v1.2.3 From 25c8ea264ea4d690da00439d1414083750a52616 Mon Sep 17 00:00:00 2001 From: Ari Pollak Date: Mon, 1 Dec 2014 11:13:49 -0500 Subject: Only filter GIDable objects; remove cosmetic changes --- activejob/lib/active_job/logging.rb | 10 ++++------ activejob/test/cases/logging_test.rb | 20 ++++++++++---------- 2 files changed, 14 insertions(+), 16 deletions(-) (limited to 'activejob') diff --git a/activejob/lib/active_job/logging.rb b/activejob/lib/active_job/logging.rb index 74c02e149a..010c26dda4 100644 --- a/activejob/lib/active_job/logging.rb +++ b/activejob/lib/active_job/logging.rb @@ -6,8 +6,6 @@ module ActiveJob module Logging #:nodoc: extend ActiveSupport::Concern - FILTERED = '[FILTERED]'.freeze - included do cattr_accessor(:logger) { ActiveSupport::TaggedLogging.new(ActiveSupport::Logger.new(STDOUT)) } @@ -86,20 +84,20 @@ module ActiveJob event.payload[:adapter].name.demodulize.remove('Adapter') + "(#{event.payload[:job].queue_name})" end - def global_id_or_filtered(argument) + def global_id_or_inspected(argument) if argument.is_a?(GlobalID::Identification) argument.to_global_id.to_s else - FILTERED + argument.inspect end end def args_info(job) if job.arguments.any? ' with arguments: ' + - job.arguments.map { |arg| global_id_or_filtered(arg) }.join(', ') + job.arguments.map { |arg| global_id_or_inspected(arg) }.join(', ') else - "" + '' end end diff --git a/activejob/test/cases/logging_test.rb b/activejob/test/cases/logging_test.rb index e308232073..745aedb6bd 100644 --- a/activejob/test/cases/logging_test.rb +++ b/activejob/test/cases/logging_test.rb @@ -42,24 +42,24 @@ class AdapterTest < ActiveSupport::TestCase def test_uses_active_job_as_tag - HelloJob.perform_later nil + HelloJob.perform_later "Cristian" assert_match(/\[ActiveJob\]/, @logger.messages) end def test_uses_job_name_as_tag - LoggingJob.perform_later nil + LoggingJob.perform_later "Dummy" assert_match(/\[LoggingJob\]/, @logger.messages) end def test_uses_job_id_as_tag - LoggingJob.perform_later nil + LoggingJob.perform_later "Dummy" assert_match(/\[LOGGING-JOB-ID\]/, @logger.messages) end def test_logs_correct_queue_name original_queue_name = LoggingJob.queue_name LoggingJob.queue_as :php_jobs - LoggingJob.perform_later nil + LoggingJob.perform_later("Dummy") assert_match(/to .*?\(php_jobs\).*/, @logger.messages) ensure LoggingJob.queue_name = original_queue_name @@ -75,12 +75,12 @@ class AdapterTest < ActiveSupport::TestCase def test_enqueue_job_logging HelloJob.perform_later "Cristian" - assert_match(/Enqueued HelloJob \(Job ID: .*?\) to .*?:.*\[FILTERED\]/, @logger.messages) + assert_match(/Enqueued HelloJob \(Job ID: .*?\) to .*?:.*Cristian/, @logger.messages) end def test_perform_job_logging LoggingJob.perform_later "Dummy" - assert_match(/Performing LoggingJob from .*? with arguments:.*\[FILTERED\]/, @logger.messages) + assert_match(/Performing LoggingJob from .*? with arguments:.*Dummy/, @logger.messages) assert_match(/Dummy, here is it: Dummy/, @logger.messages) assert_match(/Performed LoggingJob from .*? in .*ms/, @logger.messages) end @@ -90,8 +90,8 @@ class AdapterTest < ActiveSupport::TestCase assert_match(/\[LoggingJob\] \[.*?\]/, @logger.messages) assert_match(/\[ActiveJob\] Enqueued NestedJob \(Job ID: .*\) to/, @logger.messages) assert_match(/\[ActiveJob\] \[NestedJob\] \[NESTED-JOB-ID\] Performing NestedJob from/, @logger.messages) - assert_match(/\[ActiveJob\] \[NestedJob\] \[NESTED-JOB-ID\] Enqueued LoggingJob \(Job ID: .*?\) to .* with arguments: \[FILTERED\]/, @logger.messages) - assert_match(/\[ActiveJob\].*\[LoggingJob\] \[LOGGING-JOB-ID\] Performing LoggingJob from .* with arguments: \[FILTERED\]/, @logger.messages) + assert_match(/\[ActiveJob\] \[NestedJob\] \[NESTED-JOB-ID\] Enqueued LoggingJob \(Job ID: .*?\) to .* with arguments: "NestedJob"/, @logger.messages) + assert_match(/\[ActiveJob\].*\[LoggingJob\] \[LOGGING-JOB-ID\] Performing LoggingJob from .* with arguments: "NestedJob"/, @logger.messages) assert_match(/\[ActiveJob\].*\[LoggingJob\] \[LOGGING-JOB-ID\] Dummy, here is it: NestedJob/, @logger.messages) assert_match(/\[ActiveJob\].*\[LoggingJob\] \[LOGGING-JOB-ID\] Performed LoggingJob from .* in/, @logger.messages) assert_match(/\[ActiveJob\] \[NestedJob\] \[NESTED-JOB-ID\] Performed NestedJob from .* in/, @logger.messages) @@ -99,14 +99,14 @@ class AdapterTest < ActiveSupport::TestCase def test_enqueue_at_job_logging HelloJob.set(wait_until: 24.hours.from_now).perform_later "Cristian" - assert_match(/Enqueued HelloJob \(Job ID: .*\) to .*? at.*\[FILTERED\]/, @logger.messages) + assert_match(/Enqueued HelloJob \(Job ID: .*\) to .*? at.*Cristian/, @logger.messages) rescue NotImplementedError skip end def test_enqueue_in_job_logging HelloJob.set(wait: 2.seconds).perform_later "Cristian" - assert_match(/Enqueued HelloJob \(Job ID: .*\) to .*? at.*\[FILTERED\]/, @logger.messages) + assert_match(/Enqueued HelloJob \(Job ID: .*\) to .*? at.*Cristian/, @logger.messages) rescue NotImplementedError skip end -- cgit v1.2.3 From e5e44ba8db1d01aa0f58d137d547807f0be9a5f7 Mon Sep 17 00:00:00 2001 From: Ari Pollak Date: Thu, 4 Dec 2014 16:51:59 -0500 Subject: Use try instead of checking for GlobalID::Identification --- activejob/lib/active_job/logging.rb | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) (limited to 'activejob') diff --git a/activejob/lib/active_job/logging.rb b/activejob/lib/active_job/logging.rb index 010c26dda4..97d11aebfc 100644 --- a/activejob/lib/active_job/logging.rb +++ b/activejob/lib/active_job/logging.rb @@ -85,11 +85,7 @@ module ActiveJob end def global_id_or_inspected(argument) - if argument.is_a?(GlobalID::Identification) - argument.to_global_id.to_s - else - argument.inspect - end + argument.try(:to_global_id).try(:to_s) || argument.inspect end def args_info(job) -- cgit v1.2.3 From e2fffbff98a3c60d3efb8d98a3554bea28dca532 Mon Sep 17 00:00:00 2001 From: Ari Pollak Date: Thu, 4 Dec 2014 18:25:05 -0500 Subject: Inline to_global_id conversion --- activejob/lib/active_job/logging.rb | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) (limited to 'activejob') diff --git a/activejob/lib/active_job/logging.rb b/activejob/lib/active_job/logging.rb index 97d11aebfc..cd29e6908e 100644 --- a/activejob/lib/active_job/logging.rb +++ b/activejob/lib/active_job/logging.rb @@ -84,14 +84,10 @@ module ActiveJob event.payload[:adapter].name.demodulize.remove('Adapter') + "(#{event.payload[:job].queue_name})" end - def global_id_or_inspected(argument) - argument.try(:to_global_id).try(:to_s) || argument.inspect - end - def args_info(job) if job.arguments.any? ' with arguments: ' + - job.arguments.map { |arg| global_id_or_inspected(arg) }.join(', ') + job.arguments.map { |arg| arg.try(:to_global_id).try(:to_s) || arg.inspect }.join(', ') else '' end -- cgit v1.2.3