refactor(platform): PAYMENTS-11567 JobPayload.for never raises so worker instrumentation needs no guards

Author: Willem Homan

lib/bigcommerce/prometheus/integrations/resque/job_metrics.rb

@@ -137,26 +137,21 @@ def install_hooks
           # - perform duration: after super
           # The duration timer starts immediately before super so the
           # observation matches the documented fork-to-waitpid window and
-          # excludes payload parsing and the queue-latency push. The ensure
-          # is guarded so instrumentation failures can never mask a job
-          # exception.
+          # excludes payload parsing and the queue-latency push.
+          # JobPayload.for never raises and the record_* methods rescue
+          # internally, so instrumentation cannot break the job or mask
+          # its exceptions.
           module WorkerInstrumentation
             def perform_with_fork(job, &block)
-              payload = begin
-                JobPayload.for(job)
-              rescue StandardError
-                nil
-              end
-              JobMetrics.record_queue_latency(payload) if payload
+              payload = JobPayload.for(job)
+              JobMetrics.record_queue_latency(payload)
               started_at = Process.clock_gettime(Process::CLOCK_MONOTONIC)
               super
             ensure
-              if payload && started_at
-                JobMetrics.record_perform_duration(
-                  payload,
-                  Process.clock_gettime(Process::CLOCK_MONOTONIC) - started_at
-                )
-              end
+              JobMetrics.record_perform_duration(
+                payload,
+                Process.clock_gettime(Process::CLOCK_MONOTONIC) - started_at
+              )
             end
           end
         end

lib/bigcommerce/prometheus/integrations/resque/job_payload.rb

@@ -41,6 +41,10 @@ class Resque
         module JobPayload
           class << self
             ##
+            # Never raises: instrumentation must not break job execution, so
+            # a payload object is always returned. Unexpected failures
+            # degrade to a vanilla payload labelled 'unknown'.
+            #
             # @param [Resque::Job] resque_job
             # @return [ActiveJobPayload, VanillaResquePayload]
             #
@@ -50,6 +54,8 @@ def for(resque_job)
 
               inner = activejob_inner(payload)
               inner ? ActiveJobPayload.new(inner) : VanillaResquePayload.new(payload)
+            rescue StandardError
+              VanillaResquePayload.new({})
             end
 
             private

spec/bigcommerce/prometheus/integrations/resque/job_payload_spec.rb

@@ -110,5 +110,15 @@ def active_job_payload(job_class: 'MyJob', enqueued_at: nil, scheduled_at: nil)
     it "normalizes a non-Hash payload so the label falls back to 'unknown'" do
       expect(described_class.for(resque_job_double(nil)).job_class).to eq('unknown')
     end
+
+    it "returns a VanillaResquePayload labelled 'unknown' when payload access raises" do
+      job = double('Resque::Job')
+      allow(job).to receive(:payload).and_raise(StandardError, 'boom')
+
+      payload = described_class.for(job)
+
+      expect(payload).to be_a(Bigcommerce::Prometheus::Integrations::Resque::VanillaResquePayload)
+      expect(payload.job_class).to eq('unknown')
+    end
   end
 end