fix bug in metric for failed events

Author: effron

app/models/journaled/outbox/event.rb

@@ -19,6 +19,7 @@ class Event < Journaled.outbox_base_class_name.constantize
       attribute :event_data, :json
 
       validates :event_type, :event_data, :partition_key, :stream_name, presence: true
+      validate :failed_at_and_failure_reason_must_be_consistent
 
       scope :ready_to_process, -> {
         where(failed_at: nil)
@@ -63,6 +64,14 @@ def requeue!
       def self.oldest_non_failed_timestamp
         ready_to_process.order(:id).limit(1).pick(:created_at)
       end
+
+      private
+
+      def failed_at_and_failure_reason_must_be_consistent
+        if failed_at.present? != failure_reason.present?
+          errors.add(:base, 'failed_at and failure_reason must both be present or both be absent')
+        end
+      end
     end
   end
 end

lib/journaled/outbox/metric_emitter.rb

@@ -73,7 +73,7 @@ def calculate_queue_metrics
             Event.select(
               'COUNT(*) AS total_count',
               'COUNT(*) FILTER (WHERE failed_at IS NULL) AS workable_count',
-              'COUNT(*) FILTER (WHERE failure_reason IS NOT NULL AND failed_at IS NULL) AS failed_count',
+              'COUNT(*) FILTER (WHERE failed_at IS NOT NULL) AS failed_count',
               'MIN(created_at) FILTER (WHERE failed_at IS NULL) AS oldest_non_failed_timestamp',
             ).to_sql,
           )

spec/lib/journaled/outbox/worker_spec.rb

@@ -341,6 +341,7 @@
           partition_key: 'key2',
           stream_name: 'test_stream',
           failure_reason: 'Some error',
+          failed_at: Time.current,
         )
 
         # Start worker, then travel forward in time
@@ -405,6 +406,7 @@
           event_data: { test: 'data' },
           partition_key: 'key2',
           stream_name: 'test_stream',
+          failure_reason: 'Some error',
           failed_at: Time.current,
         )
 
@@ -433,21 +435,20 @@
         expect(emitted['journaled.worker.queue_workable_count'][:value]).to eq(1) # Only non-failed events
       end
 
-      it 'counts erroring events correctly' do
+      it 'counts failed events correctly' do
         Journaled::Outbox::Event.create!(
           event_type: 'test_event',
           event_data: { test: 'data' },
           partition_key: 'key1',
           stream_name: 'test_stream',
-          failure_reason: 'Error but not failed',
         )
 
         Journaled::Outbox::Event.create!(
           event_type: 'test_event',
           event_data: { test: 'data' },
           partition_key: 'key2',
           stream_name: 'test_stream',
-          failure_reason: 'Error and failed',
+          failure_reason: 'Permanent failure',
           failed_at: Time.current,
         )
 
@@ -472,7 +473,7 @@
           sleep 0.1 until emitted.key?('journaled.worker.queue_failed_count') || Time.current > timeout
         end
 
-        expect(emitted['journaled.worker.queue_failed_count'][:value]).to eq(1) # Only events with error but not failed
+        expect(emitted['journaled.worker.queue_failed_count'][:value]).to eq(1)
       end
     end
   end