Support both global and per Application/Server backtrace cleaners

* (Hopefully) addressed @rosa's feedback on project style, code quality, and implementation requirements
* Updated the design to support both a global MissionControl::Jobs wide backtrace cleaner and
  Application/Server specific backtrace cleaners
* With this commit, the implementation relies less on jobs_helper.rb and
  more on a new before_action in the jobs_controller.
* Added more UI testing for compleness

Author: hms

README.md

@@ -57,6 +57,7 @@ Besides `base_controller_class`, you can also set the following for `MissionCont
 - `internal_query_count_limit`: in count queries, the maximum number of records that will be counted if the adapter needs to limit these queries. True counts above this number will be returned as `INFINITY`. This keeps count queries fast—defaults to `500,000`
 - `scheduled_job_delay_threshold`: the time duration before a scheduled job is considered delayed. Defaults to `1.minute` (a job is considered delayed if it hasn't transitioned from the `scheduled` status 1 minute after the scheduled time).
 - `show_console_help`: whether to show the console help. If you don't want the console help message, set this to `false`—defaults to `true`.
+- `backtrace_cleaner`: a backtrace cleaner used for optionally filtering backtraces on the Failed Jobs detail page. Defaults to `Rails::BacktraceCleaner.new`. See the [Advanced configuration](#advanced-configuration) section for how to configure/override this setting on a per application/server basis.
 
 This library extends Active Job with a querying interface and the following setting:
 - `config.active_job.default_page_size`: the internal batch size that Active Job will use when sending queries to the underlying adapter and the batch size for the bulk operations defined above—defaults to `1000`.
@@ -107,7 +108,24 @@ SERVERS_BY_APP.each do |app, servers|
       ActiveJob::QueueAdapters::SolidQueueAdapter.new
     end
 
-    [ server, queue_adapter ]
+    # Default:
+    # 
+    # @return Array<String, ActiveJob::QueueAdapters::Base)
+    # An array where:
+    # * the String represents the symbolic name for this server within the UI
+    # * ActiveJob::QueueAdapters::Base adapter instance used to access this Application Server/Service
+    [ server, queue_adapter ] 
+    
+    # Optional return formats:
+    # 
+    # @return Array<String, Array<ActiveJob::QueueAdapters::Base>>
+    # * This is equivalent, and behaves identically to, the format the default format above. 
+    # [ server, [ queue_adapter ]]  # without optional backtrace cleaner
+    # 
+    # @return Array<String, Array<ActiveJob::QueueAdapters::Base, ActiveSupport::BacktraceCleaner>>
+    # * This format adds an optional ActiveSupport::BacktraceCleaner to override the system wide
+    #   backtrace cleaner for *this* Application Server/Service.
+    # [ server, [ queue_adapter, BacktraceCleaner.new ]]  # with optional backtrace cleaner
   end.to_h
 
   MissionControl::Jobs.applications.add(app, queue_adapters_by_name)

app/controllers/mission_control/jobs/jobs_controller.rb

@@ -2,6 +2,7 @@ class MissionControl::Jobs::JobsController < MissionControl::Jobs::ApplicationCo
   include MissionControl::Jobs::JobScoped, MissionControl::Jobs::JobFilters
 
   skip_before_action :set_job, only: :index
+  before_action :set_backtrace_cleaner, only: :show
 
   def index
     @job_class_names = jobs_with_status.job_class_names
@@ -15,6 +16,7 @@ def show
   end
 
   private
+
     def jobs_relation
       filtered_jobs
     end
@@ -36,4 +38,12 @@ def filtered_jobs
     def jobs_status
       params[:status].presence&.inquiry
     end
+
+    def set_backtrace_cleaner
+      @backtrace_cleaner = @application.servers[server_id]&.backtrace_cleaner
+    end
+
+    def server_id
+      params[:server_id]
+    end
 end

app/helpers/mission_control/jobs/jobs_helper.rb

@@ -11,16 +11,16 @@ def failed_job_error(job)
     "#{job.last_execution_error.error_class}: #{job.last_execution_error.message}"
   end
 
-  def backtrace_cleaner = Rails.backtrace_cleaner
-
-  def backtrace_cleaner? = backtrace_cleaner.present?
-
-  def should_clean_backtrace? = params["clean_backtrace"] == "true"
-
-  def failed_job_backtrace(job, clean: false)
-    return job.last_execution_error.backtrace.join("\n") if !clean || !backtrace_cleaner?
+  def clean_backtrace?
+    params["clean_backtrace"] == "true"
+  end
 
-    backtrace_cleaner.clean(job.last_execution_error.backtrace).join("\n")
+  def failed_job_backtrace(job)
+    if clean_backtrace? && @backtrace_cleaner
+      @backtrace_cleaner.clean(job.last_execution_error.backtrace).join("\n")
+    else
+      job.last_execution_error.backtrace.join("\n")
+    end
   end
 
   def attribute_names_for_job_status(status)

app/views/mission_control/jobs/jobs/_error_information.html.erb

@@ -14,7 +14,9 @@
     </tbody>
   </table>
 
-  <%= render "mission_control/jobs/jobs/failed/backtrace_toggle", application: @application, job: @job if backtrace_cleaner? %>
+  <% if @backtrace_cleaner %>
+    <%= render "mission_control/jobs/jobs/failed/backtrace_toggle", application: @application, job: job %>
+  <% end %>
 
-  <pre class="is-family-monospace mb-4 backtrace-selector"><%= failed_job_backtrace(job, clean: should_clean_backtrace?) %></pre>
+  <pre class="is-family-monospace mb-4 backtrace-content"><%= failed_job_backtrace(job) %></pre>
 <% end %>

app/views/mission_control/jobs/jobs/failed/_backtrace_toggle.html.erb

@@ -1,11 +1,13 @@
+<%# locals: (application:, job:) %>
+
 <div class="is-flex is-justify-content-flex-end mb-2 mr-2 backtrace-toggle-selector">
   <div class="tabs is-toggle is-toggle-rounded is-small">
     <ul>
-      <li class="<%= 'is-active' if should_clean_backtrace? %>">
-        <%= link_to("Clean", application_job_path(application, job.job_id, clean_backtrace: true)) %>
+      <li class="<%= class_names('backtrace-clean-link', 'is-active' => clean_backtrace?) %>">
+        <%= link_to "Clean", application_job_path(application, job.job_id, clean_backtrace: true) %>
       </li>
-      <li class="<%= 'is-active' unless should_clean_backtrace? %>">
-        <%= link_to("Full", application_job_path(application, job.job_id, clean_backtrace: false)) %>
+      <li class="<%= class_names('backtrace-full-link', 'is-active' => !clean_backtrace?) %>">
+        <%= link_to "Full", application_job_path(application, job.job_id, clean_backtrace: false) %>
       </li>
     </ul>
   </div>

lib/mission_control/jobs.rb

@@ -19,6 +19,6 @@ module Jobs
     mattr_accessor :internal_query_count_limit, default: 500_000 # Hard limit to keep unlimited count queries fast enough
     mattr_accessor :show_console_help, default: true
     mattr_accessor :scheduled_job_delay_threshold, default: 1.minute
-    mattr_accessor :backtrace_cleaner, default: proc { Rails.backtrace_cleaner }
+    mattr_accessor :backtrace_cleaner
   end
 end

lib/mission_control/jobs/application.rb

@@ -11,7 +11,11 @@ def initialize(name:)
 
   def add_servers(queue_adapters_by_name)
     queue_adapters_by_name.each do |name, queue_adapter|
-      servers << MissionControl::Jobs::Server.new(name: name.to_s, queue_adapter: queue_adapter, application: self)
+      adapter, cleaner = queue_adapter
+      cleaner ||= MissionControl::Jobs.backtrace_cleaner
+
+      servers << MissionControl::Jobs::Server.new(name: name.to_s, queue_adapter: adapter,
+        backtrace_cleaner: cleaner, application: self)
     end
   end
 end

lib/mission_control/jobs/engine.rb

@@ -15,6 +15,7 @@ class Engine < ::Rails::Engine
 
       config.before_initialize do
         config.mission_control.jobs.applications = MissionControl::Jobs::Applications.new
+        config.mission_control.jobs.backtrace_cleaner ||= Rails::BacktraceCleaner.new
 
         config.mission_control.jobs.each do |key, value|
           MissionControl::Jobs.public_send("#{key}=", value)

lib/mission_control/jobs/server.rb

@@ -4,12 +4,13 @@ class MissionControl::Jobs::Server
   include MissionControl::Jobs::IdentifiedByName
   include Serializable, RecurringTasks, Workers
 
-  attr_reader :name, :queue_adapter, :application
+  attr_reader :name, :queue_adapter, :application, :backtrace_cleaner
 
-  def initialize(name:, queue_adapter:, application:)
+  def initialize(name:, queue_adapter:, application:, backtrace_cleaner:)
     super(name: name)
     @queue_adapter = queue_adapter
     @application = application
+    @backtrace_cleaner = backtrace_cleaner
   end
 
   def activating(&block)

test/system/show_failed_job_test.rb

@@ -30,13 +30,47 @@ class ShowFailedJobsTest < ApplicationSystemTestCase
     assert_text /there are no failed jobs/i
   end
 
-  test "show backtrace cleaner buttons" do
+  test "Has Clean/Full buttons when a backtrace cleaner is configured" do
     visit jobs_path(:failed)
-    within_job_row /FailingJob\s*2/ do
+    within_job_row(/FailingJob\s*2/) do
+      click_on "RuntimeError: This always fails!"
+    end
+
+    assert_selector ".backtrace-toggle-selector"
+  end
+
+  test "Does not offer Clean/Full buttons when a backtrace cleaner is not configured" do
+    setup do
+      # grab the current state
+      @backtrace_cleaner = MissionControl::Jobs.backtrace_cleaner
+      @applications = MissionControl::Jobs.backtrace_cleaner
+
+      # reset the state
+      MissionControl::Jobs.backtrace_cleaner = nil
+      MissionControl::Jobs.applications = Applications.new
+
+      # Setup the application with what we had before *minus* a backtrace cleaner
+      @applications.each do |application|
+        MissionControl::Jobs.applications.add(application.name).tap do |it|
+          application.servers.each do |server|
+            it.add_servers(server.name, server.queue_adapter)
+          end
+        end
+      end
+    end
+
+    teardown do
+      # reset back to the known state before the start of the test
+      MissionControl::Jobs.backtrace_cleaner = @backtrace_cleaner
+      MissionControl::Jobs.applications = @application
+    end
+
+    visit jobs_path(:failed)
+    within_job_row(/FailingJob\s*2/) do
       click_on "RuntimeError: This always fails!"
     end
 
-    assert_text /clean/i
+    assert_no_selector ".backtrace-toggle-selector"
   end
 
   test "click on 'clean' shows a backtrace cleaned by the Rails default backtrace cleaner" do
@@ -45,9 +79,18 @@ class ShowFailedJobsTest < ApplicationSystemTestCase
       click_on "RuntimeError: This always fails!"
     end
 
-    click_on "Clean"
-    within ".backtrace-selector" do
-      assert_no_selector "*"
+    assert_selector ".backtrace-toggle-selector"
+
+    within ".backtrace-toggle-selector" do
+      click_on "Clean"
     end
+
+    assert_selector "pre.backtrace-content", text: /.*/, visible: true
+
+    within ".backtrace-toggle-selector" do
+      click_on "Full"
+    end
+
+    assert_selector "pre.backtrace-content", text: /.*/, visible: true
   end
 end