From 6a19448e1d50477afd27316f840c031752ecae21 Mon Sep 17 00:00:00 2001 From: Joshua Drumm Date: Fri, 6 Sep 2024 13:11:20 -0400 Subject: [PATCH 1/9] Updated submission failure notification code and wrote some tests --- app/sidekiq/hca/ezr_submission_job.rb | 11 ++- lib/form1010_ezr/service.rb | 32 ++++++--- spec/lib/form1010_ezr/service_spec.rb | 75 +++++++++++++++++++-- spec/sidekiq/hca/ezr_submission_job_spec.rb | 25 +++++-- 4 files changed, 115 insertions(+), 28 deletions(-) diff --git a/app/sidekiq/hca/ezr_submission_job.rb b/app/sidekiq/hca/ezr_submission_job.rb index 1b2eccf35e4..42e2ae51aa9 100644 --- a/app/sidekiq/hca/ezr_submission_job.rb +++ b/app/sidekiq/hca/ezr_submission_job.rb @@ -12,11 +12,10 @@ class EzrSubmissionJob sidekiq_options retry: 14 sidekiq_retries_exhausted do |msg, _e| - log_submission_failure(decrypt_form(msg['args'][0])) - end - - def self.log_submission_failure(parsed_form) - Form1010Ezr::Service.new(nil).log_submission_failure(parsed_form) + Form1010Ezr::Service.new(nil).log_submission_failure( + decrypt_form(msg['args'][0]), + retries_exhausted: true + ) end def self.decrypt_form(encrypted_form) @@ -29,7 +28,7 @@ def perform(encrypted_form, user_uuid) Form1010Ezr::Service.new(user).submit_sync(parsed_form) rescue VALIDATION_ERROR => e - self.class.log_submission_failure(parsed_form) + Form1010Ezr::Service.new(nil).log_submission_failure(parsed_form) log_exception_to_sentry(e) rescue StatsD.increment("#{Form1010Ezr::Service::STATSD_KEY_PREFIX}.async.retries") diff --git a/lib/form1010_ezr/service.rb b/lib/form1010_ezr/service.rb index c87e9cabda4..49d9233c7eb 100644 --- a/lib/form1010_ezr/service.rb +++ b/lib/form1010_ezr/service.rb @@ -41,7 +41,11 @@ def submit_sync(parsed_form) end # Log the 'formSubmissionId' for successful submissions - Rails.logger.info("SubmissionID=#{res[:formSubmissionId]}") + Rails.logger.info( + '1010EZR successfully submitted', + submission_id: res[:formSubmissionId], + veteran_initials: veteran_initials(parsed_form) + ) res rescue => e @@ -60,24 +64,22 @@ def submit_form(parsed_form) log_and_raise_error(e, parsed_form) end - def log_submission_failure(parsed_form) - StatsD.increment("#{Form1010Ezr::Service::STATSD_KEY_PREFIX}.failed_wont_retry") + def log_submission_failure(parsed_form, retries_exhausted: false) + StatsD.increment("#{Form1010Ezr::Service::STATSD_KEY_PREFIX}.failed#{'_wont_retry' if retries_exhausted}") if parsed_form.present? PersonalInformationLog.create!( data: parsed_form, - error_class: 'Form1010Ezr FailedWontRetry' + error_class: "Form1010Ezr Failed#{'WontRetry' if retries_exhausted}" ) + failure_message = "1010EZR #{'total ' if retries_exhausted}failure" + log_message_to_sentry( - '1010EZR total failure', + failure_message, :error, - { - first_initial: parsed_form.dig('veteranFullName', 'first')&.chr || 'no initial provided', - middle_initial: parsed_form.dig('veteranFullName', 'middle')&.chr || 'no initial provided', - last_initial: parsed_form.dig('veteranFullName', 'last')&.chr || 'no initial provided' - }, - ezr: :total_failure + veteran_initials(parsed_form), + ezr: retries_exhausted ? :total_failure : :failure ) end end @@ -174,5 +176,13 @@ def log_and_raise_error(error, form) Rails.logger.error "10-10EZR form submission failed: #{error.message}" raise error end + + def veteran_initials(parsed_form) + { + first_initial: parsed_form.dig('veteranFullName', 'first')&.chr || 'no initial provided', + middle_initial: parsed_form.dig('veteranFullName', 'middle')&.chr || 'no initial provided', + last_initial: parsed_form.dig('veteranFullName', 'last')&.chr || 'no initial provided' + } + end end end diff --git a/spec/lib/form1010_ezr/service_spec.rb b/spec/lib/form1010_ezr/service_spec.rb index 61a1159dc89..9c6a3f2b02f 100644 --- a/spec/lib/form1010_ezr/service_spec.rb +++ b/spec/lib/form1010_ezr/service_spec.rb @@ -62,6 +62,13 @@ def ezr_form_with_attachments ) end + def expect_personal_info_log(message) + pii_log = PersonalInformationLog.last + + expect(pii_log.error_class).to eq(message) + expect(pii_log.data).to eq(form) + end + describe '#add_financial_flag' do context 'when the form has veteran gross income' do let(:parsed_form) do @@ -86,17 +93,18 @@ def ezr_form_with_attachments describe '#post_fill_required_fields' do it 'Adds required fields in the Enrollment System API to the form object', - run_at: 'Fri, 08 Feb 2019 02:50:45 GMT' do + run_at: 'Fri, 08 Feb 2019 02:50:45 GMT' do VCR.use_cassette( 'hca/ee/lookup_user', VCR::MATCH_EVERYTHING.merge(erb: true) ) do - expect(form['isEssentialAcaCoverage']).to eq(nil) - expect(form['vaMedicalFacility']).to eq(nil) + expect(form.keys).to_not include('isEssentialAcaCoverage', 'vaMedicalFacility') service.send(:post_fill_required_fields, form) expect(form.keys).to include('isEssentialAcaCoverage', 'vaMedicalFacility') + expect(form['isEssentialAcaCoverage']).to eq(false) + expect(form['vaMedicalFacility']).to eq('988') end end end @@ -156,6 +164,54 @@ def ezr_form_with_attachments end end + describe '#log_submission_failure' do + context "when 'parsed_form' is present" do + context "when 'retries_exhausted' is false" do + it "increments StatsD, creates a 'PersonalInformationLog' record, and logs a failure message to sentry" do + # The names for these errors will not include 'total' or 'wont_retry' as they are not for total failures + allow(StatsD).to receive(:increment) + expect(StatsD).to receive(:increment).with('api.1010ezr.failed') + expect_any_instance_of(SentryLogging).to receive(:log_message_to_sentry).with( + '1010EZR failure', + :error, + { + first_initial: 'F', + middle_initial: 'M', + last_initial: 'Z' + }, + ezr: :failure + ) + + described_class.new(nil).log_submission_failure(form) + + expect_personal_info_log('Form1010Ezr Failed') + end + end + + context "when 'retries_exhausted' is true" do + it "increments StatsD, creates a 'PersonalInformationLog' record, and logs a TOTAL failure message to sentry" do + allow(StatsD).to receive(:increment) + + expect(StatsD).to receive(:increment).with('api.1010ezr.failed_wont_retry') + expect_any_instance_of(SentryLogging).to receive(:log_message_to_sentry).with( + '1010EZR total failure', + :error, + { + first_initial: 'F', + middle_initial: 'M', + last_initial: 'Z' + }, + ezr: :total_failure + ) + + described_class.new(nil).log_submission_failure(form, retries_exhausted: true) + + expect_personal_info_log('Form1010Ezr FailedWontRetry') + end + end + end + end + describe '#submit_form' do it 'submits the ezr with a background job', run_at: 'Tue, 21 Nov 2023 20:42:44 GMT' do VCR.use_cassette( @@ -310,7 +366,8 @@ def ezr_form_with_attachments end end - it 'logs the submission id, payload size, and individual attachment sizes in descending order (if applicable)', + it "logs the submission id, user's initials, payload size, and individual attachment sizes in descending "\ + 'order (if applicable)', run_at: 'Wed, 17 Jul 2024 18:17:30 GMT' do VCR.use_cassette( 'form1010_ezr/authorized_submit_with_attachments', @@ -318,7 +375,15 @@ def ezr_form_with_attachments ) do submission_response = service.submit_sync(ezr_form_with_attachments) - expect(Rails.logger).to have_received(:info).with("SubmissionID=#{submission_response[:formSubmissionId]}") + expect(Rails.logger).to have_received(:info).with( + '1010EZR successfully submitted', + submission_id: submission_response[:formSubmissionId], + veteran_initials: { + first_initial: 'F', + middle_initial: 'M', + last_initial: 'Z' + } + ) expect(Rails.logger).to have_received(:info).with('Payload for submitted 1010EZR: ' \ 'Body size of 362 KB with 2 attachment(s)') expect(Rails.logger).to have_received(:info).with( diff --git a/spec/sidekiq/hca/ezr_submission_job_spec.rb b/spec/sidekiq/hca/ezr_submission_job_spec.rb index b86465558de..a65715a2a33 100644 --- a/spec/sidekiq/hca/ezr_submission_job_spec.rb +++ b/spec/sidekiq/hca/ezr_submission_job_spec.rb @@ -12,18 +12,31 @@ end let(:ezr_service) { double } - describe 'when job has failed' do + describe 'when retries are exhausted' do let(:msg) do { 'args' => [encrypted_form, nil] } end - it 'passes unencrypted form to 1010ezr service' do - expect_any_instance_of(Form1010Ezr::Service).to receive(:log_submission_failure).with( - form - ) - described_class.new.sidekiq_retries_exhausted_block.call(msg) + it "increments StatsD, creates a 'PersonalInformationLog' record, and logs a failure message to sentry" do + described_class.within_sidekiq_retries_exhausted_block(msg) do + expect(StatsD).to receive(:increment).with('api.1010ezr.failed_wont_retry') + expect_any_instance_of(SentryLogging).to receive(:log_message_to_sentry).with( + '1010EZR total failure', + :error, + { + first_initial: 'F', + middle_initial: 'M', + last_initial: 'Z' + }, + ezr: :total_failure + ) + end + + pii_log = PersonalInformationLog.last + expect(pii_log.error_class).to eq('Form1010Ezr FailedWontRetry') + expect(pii_log.data).to eq(form) end end From 163582a160c24f0169138c96e17e26fcb3678d01 Mon Sep 17 00:00:00 2001 From: Joshua Drumm Date: Fri, 6 Sep 2024 14:49:03 -0400 Subject: [PATCH 2/9] Fixed a failing test --- spec/lib/form1010_ezr/service_spec.rb | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/spec/lib/form1010_ezr/service_spec.rb b/spec/lib/form1010_ezr/service_spec.rb index 9c6a3f2b02f..46380c70520 100644 --- a/spec/lib/form1010_ezr/service_spec.rb +++ b/spec/lib/form1010_ezr/service_spec.rb @@ -165,6 +165,15 @@ def expect_personal_info_log(message) end describe '#log_submission_failure' do + context "when 'parsed_form' is not present" do + it 'only increments StatsD' do + allow(StatsD).to receive(:increment) + expect(StatsD).to receive(:increment).with('api.1010ezr.failed') + + described_class.new(nil).log_submission_failure(nil) + end + end + context "when 'parsed_form' is present" do context "when 'retries_exhausted' is false" do it "increments StatsD, creates a 'PersonalInformationLog' record, and logs a failure message to sentry" do @@ -316,7 +325,7 @@ def expect_personal_info_log(message) it 'increments StatsD as well as logs and raises the error' do allow(StatsD).to receive(:increment) - expect(StatsD).to receive(:increment).with('api.1010ezr.failed_wont_retry') + expect(StatsD).to receive(:increment).with('api.1010ezr.failed') expect { submit_form(form) }.to raise_error( StandardError, 'Uh oh. Some bad error occurred.' ) From 0ec91d80ec5902b530862a563dd0bac16b4b761f Mon Sep 17 00:00:00 2001 From: Joshua Drumm Date: Fri, 6 Sep 2024 15:27:19 -0400 Subject: [PATCH 3/9] Fixed linting issue --- spec/lib/form1010_ezr/service_spec.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/spec/lib/form1010_ezr/service_spec.rb b/spec/lib/form1010_ezr/service_spec.rb index 46380c70520..aefb3bc4c9f 100644 --- a/spec/lib/form1010_ezr/service_spec.rb +++ b/spec/lib/form1010_ezr/service_spec.rb @@ -93,12 +93,12 @@ def expect_personal_info_log(message) describe '#post_fill_required_fields' do it 'Adds required fields in the Enrollment System API to the form object', - run_at: 'Fri, 08 Feb 2019 02:50:45 GMT' do + run_at: 'Fri, 08 Feb 2019 02:50:45 GMT' do VCR.use_cassette( 'hca/ee/lookup_user', VCR::MATCH_EVERYTHING.merge(erb: true) ) do - expect(form.keys).to_not include('isEssentialAcaCoverage', 'vaMedicalFacility') + expect(form.keys).not_to include('isEssentialAcaCoverage', 'vaMedicalFacility') service.send(:post_fill_required_fields, form) @@ -375,7 +375,7 @@ def expect_personal_info_log(message) end end - it "logs the submission id, user's initials, payload size, and individual attachment sizes in descending "\ + it "logs the submission id, user's initials, payload size, and individual attachment sizes in descending " \ 'order (if applicable)', run_at: 'Wed, 17 Jul 2024 18:17:30 GMT' do VCR.use_cassette( From 878153c1389897851ef9a48551eb75ebfde89209 Mon Sep 17 00:00:00 2001 From: Joshua Drumm Date: Fri, 6 Sep 2024 16:17:50 -0400 Subject: [PATCH 4/9] Created two separate methods for failures and exhausted failures --- app/sidekiq/hca/ezr_submission_job.rb | 5 +- lib/form1010_ezr/service.rb | 31 ++++++++--- spec/lib/form1010_ezr/service_spec.rb | 78 +++++++++++++++------------ 3 files changed, 70 insertions(+), 44 deletions(-) diff --git a/app/sidekiq/hca/ezr_submission_job.rb b/app/sidekiq/hca/ezr_submission_job.rb index 42e2ae51aa9..649c2e423a1 100644 --- a/app/sidekiq/hca/ezr_submission_job.rb +++ b/app/sidekiq/hca/ezr_submission_job.rb @@ -12,9 +12,8 @@ class EzrSubmissionJob sidekiq_options retry: 14 sidekiq_retries_exhausted do |msg, _e| - Form1010Ezr::Service.new(nil).log_submission_failure( - decrypt_form(msg['args'][0]), - retries_exhausted: true + Form1010Ezr::Service.new(nil).log_exhausted_submission_failure( + decrypt_form(msg['args'][0]) ) end diff --git a/lib/form1010_ezr/service.rb b/lib/form1010_ezr/service.rb index 49d9233c7eb..837775326f3 100644 --- a/lib/form1010_ezr/service.rb +++ b/lib/form1010_ezr/service.rb @@ -64,22 +64,41 @@ def submit_form(parsed_form) log_and_raise_error(e, parsed_form) end - def log_submission_failure(parsed_form, retries_exhausted: false) - StatsD.increment("#{Form1010Ezr::Service::STATSD_KEY_PREFIX}.failed#{'_wont_retry' if retries_exhausted}") + def log_submission_failure(parsed_form) + StatsD.increment("#{Form1010Ezr::Service::STATSD_KEY_PREFIX}.failed") if parsed_form.present? PersonalInformationLog.create!( data: parsed_form, - error_class: "Form1010Ezr Failed#{'WontRetry' if retries_exhausted}" + error_class: 'Form1010Ezr Failed' ) - failure_message = "1010EZR #{'total ' if retries_exhausted}failure" + failure_message = '1010EZR failure' log_message_to_sentry( - failure_message, + '1010EZR failure', :error, veteran_initials(parsed_form), - ezr: retries_exhausted ? :total_failure : :failure + ezr: :failure + ) + end + end + + # When a submission runs out of retry attempts + def log_exhausted_submission_failure(parsed_form) + StatsD.increment("#{Form1010Ezr::Service::STATSD_KEY_PREFIX}.failed_wont_retry") + + if parsed_form.present? + PersonalInformationLog.create!( + data: parsed_form, + error_class: 'Form1010Ezr FailedWontRetry' + ) + + log_message_to_sentry( + '1010EZR total failure', + :error, + veteran_initials(parsed_form), + ezr: :total_failure ) end end diff --git a/spec/lib/form1010_ezr/service_spec.rb b/spec/lib/form1010_ezr/service_spec.rb index aefb3bc4c9f..de2d526a59d 100644 --- a/spec/lib/form1010_ezr/service_spec.rb +++ b/spec/lib/form1010_ezr/service_spec.rb @@ -175,48 +175,56 @@ def expect_personal_info_log(message) end context "when 'parsed_form' is present" do - context "when 'retries_exhausted' is false" do - it "increments StatsD, creates a 'PersonalInformationLog' record, and logs a failure message to sentry" do - # The names for these errors will not include 'total' or 'wont_retry' as they are not for total failures - allow(StatsD).to receive(:increment) - expect(StatsD).to receive(:increment).with('api.1010ezr.failed') - expect_any_instance_of(SentryLogging).to receive(:log_message_to_sentry).with( - '1010EZR failure', - :error, - { - first_initial: 'F', - middle_initial: 'M', - last_initial: 'Z' - }, - ezr: :failure - ) + it "increments StatsD, creates a 'PersonalInformationLog' record, and logs a failure message to sentry" do + allow(StatsD).to receive(:increment) + expect(StatsD).to receive(:increment).with('api.1010ezr.failed') + expect_any_instance_of(SentryLogging).to receive(:log_message_to_sentry).with( + '1010EZR failure', + :error, + { + first_initial: 'F', + middle_initial: 'M', + last_initial: 'Z' + }, + ezr: :failure + ) - described_class.new(nil).log_submission_failure(form) + described_class.new(nil).log_submission_failure(form) - expect_personal_info_log('Form1010Ezr Failed') - end + expect_personal_info_log('Form1010Ezr Failed') end + end + end - context "when 'retries_exhausted' is true" do - it "increments StatsD, creates a 'PersonalInformationLog' record, and logs a TOTAL failure message to sentry" do - allow(StatsD).to receive(:increment) + describe '#log_exhausted_submission_failure' do + context "when 'parsed_form' is not present" do + it 'only increments StatsD' do + allow(StatsD).to receive(:increment) + expect(StatsD).to receive(:increment).with('api.1010ezr.failed_wont_retry') - expect(StatsD).to receive(:increment).with('api.1010ezr.failed_wont_retry') - expect_any_instance_of(SentryLogging).to receive(:log_message_to_sentry).with( - '1010EZR total failure', - :error, - { - first_initial: 'F', - middle_initial: 'M', - last_initial: 'Z' - }, - ezr: :total_failure - ) + described_class.new(nil).log_exhausted_submission_failure(nil) + end + end - described_class.new(nil).log_submission_failure(form, retries_exhausted: true) + context "when 'parsed_form' is present" do + it "increments StatsD, creates a 'PersonalInformationLog' record, and logs a failure message to sentry" do + allow(StatsD).to receive(:increment) - expect_personal_info_log('Form1010Ezr FailedWontRetry') - end + expect(StatsD).to receive(:increment).with('api.1010ezr.failed_wont_retry') + expect_any_instance_of(SentryLogging).to receive(:log_message_to_sentry).with( + '1010EZR total failure', + :error, + { + first_initial: 'F', + middle_initial: 'M', + last_initial: 'Z' + }, + ezr: :total_failure + ) + + described_class.new(nil).log_exhausted_submission_failure(form) + + expect_personal_info_log('Form1010Ezr FailedWontRetry') end end end From a8b574810b16190f0f7c145194864cf1c6689eec Mon Sep 17 00:00:00 2001 From: Joshua Drumm Date: Fri, 6 Sep 2024 16:22:02 -0400 Subject: [PATCH 5/9] Removed unnecessary code --- lib/form1010_ezr/service.rb | 2 -- 1 file changed, 2 deletions(-) diff --git a/lib/form1010_ezr/service.rb b/lib/form1010_ezr/service.rb index 837775326f3..14fbe48f1f6 100644 --- a/lib/form1010_ezr/service.rb +++ b/lib/form1010_ezr/service.rb @@ -73,8 +73,6 @@ def log_submission_failure(parsed_form) error_class: 'Form1010Ezr Failed' ) - failure_message = '1010EZR failure' - log_message_to_sentry( '1010EZR failure', :error, From 7652af664803453abf67fc9022ddc87e4e1f7e55 Mon Sep 17 00:00:00 2001 From: Joshua Drumm Date: Fri, 6 Sep 2024 18:22:36 -0400 Subject: [PATCH 6/9] Moved the total failure code into the job class --- app/sidekiq/hca/ezr_submission_job.rb | 21 ++++++-- lib/form1010_ezr/service.rb | 31 +++--------- spec/lib/form1010_ezr/service_spec.rb | 33 ------------- spec/sidekiq/hca/ezr_submission_job_spec.rb | 55 +++++++++++++-------- 4 files changed, 58 insertions(+), 82 deletions(-) diff --git a/app/sidekiq/hca/ezr_submission_job.rb b/app/sidekiq/hca/ezr_submission_job.rb index 649c2e423a1..da3d0a0b583 100644 --- a/app/sidekiq/hca/ezr_submission_job.rb +++ b/app/sidekiq/hca/ezr_submission_job.rb @@ -7,14 +7,29 @@ module HCA class EzrSubmissionJob include Sidekiq::Job include SentryLogging + include Common::Client::Concerns::Monitoring VALIDATION_ERROR = HCA::SOAPParser::ValidationError sidekiq_options retry: 14 sidekiq_retries_exhausted do |msg, _e| - Form1010Ezr::Service.new(nil).log_exhausted_submission_failure( - decrypt_form(msg['args'][0]) - ) + parsed_form = decrypt_form(msg['args'][0]) + + StatsD.increment("#{Form1010Ezr::Service::STATSD_KEY_PREFIX}.failed_wont_retry") + + if parsed_form.present? + PersonalInformationLog.create!( + data: parsed_form, + error_class: 'Form1010Ezr FailedWontRetry' + ) + + new.log_message_to_sentry( + '1010EZR total failure', + :error, + Form1010Ezr::Service.new(nil).veteran_initials(parsed_form), + ezr: :total_failure + ) + end end def self.decrypt_form(encrypted_form) diff --git a/lib/form1010_ezr/service.rb b/lib/form1010_ezr/service.rb index 14fbe48f1f6..2f6b459aeb3 100644 --- a/lib/form1010_ezr/service.rb +++ b/lib/form1010_ezr/service.rb @@ -82,23 +82,12 @@ def log_submission_failure(parsed_form) end end - # When a submission runs out of retry attempts - def log_exhausted_submission_failure(parsed_form) - StatsD.increment("#{Form1010Ezr::Service::STATSD_KEY_PREFIX}.failed_wont_retry") - - if parsed_form.present? - PersonalInformationLog.create!( - data: parsed_form, - error_class: 'Form1010Ezr FailedWontRetry' - ) - - log_message_to_sentry( - '1010EZR total failure', - :error, - veteran_initials(parsed_form), - ezr: :total_failure - ) - end + def veteran_initials(parsed_form) + { + first_initial: parsed_form.dig('veteranFullName', 'first')&.chr || 'no initial provided', + middle_initial: parsed_form.dig('veteranFullName', 'middle')&.chr || 'no initial provided', + last_initial: parsed_form.dig('veteranFullName', 'last')&.chr || 'no initial provided' + } end private @@ -193,13 +182,5 @@ def log_and_raise_error(error, form) Rails.logger.error "10-10EZR form submission failed: #{error.message}" raise error end - - def veteran_initials(parsed_form) - { - first_initial: parsed_form.dig('veteranFullName', 'first')&.chr || 'no initial provided', - middle_initial: parsed_form.dig('veteranFullName', 'middle')&.chr || 'no initial provided', - last_initial: parsed_form.dig('veteranFullName', 'last')&.chr || 'no initial provided' - } - end end end diff --git a/spec/lib/form1010_ezr/service_spec.rb b/spec/lib/form1010_ezr/service_spec.rb index de2d526a59d..0b36a41c372 100644 --- a/spec/lib/form1010_ezr/service_spec.rb +++ b/spec/lib/form1010_ezr/service_spec.rb @@ -196,39 +196,6 @@ def expect_personal_info_log(message) end end - describe '#log_exhausted_submission_failure' do - context "when 'parsed_form' is not present" do - it 'only increments StatsD' do - allow(StatsD).to receive(:increment) - expect(StatsD).to receive(:increment).with('api.1010ezr.failed_wont_retry') - - described_class.new(nil).log_exhausted_submission_failure(nil) - end - end - - context "when 'parsed_form' is present" do - it "increments StatsD, creates a 'PersonalInformationLog' record, and logs a failure message to sentry" do - allow(StatsD).to receive(:increment) - - expect(StatsD).to receive(:increment).with('api.1010ezr.failed_wont_retry') - expect_any_instance_of(SentryLogging).to receive(:log_message_to_sentry).with( - '1010EZR total failure', - :error, - { - first_initial: 'F', - middle_initial: 'M', - last_initial: 'Z' - }, - ezr: :total_failure - ) - - described_class.new(nil).log_exhausted_submission_failure(form) - - expect_personal_info_log('Form1010Ezr FailedWontRetry') - end - end - end - describe '#submit_form' do it 'submits the ezr with a background job', run_at: 'Tue, 21 Nov 2023 20:42:44 GMT' do VCR.use_cassette( diff --git a/spec/sidekiq/hca/ezr_submission_job_spec.rb b/spec/sidekiq/hca/ezr_submission_job_spec.rb index a65715a2a33..03b93b78dce 100644 --- a/spec/sidekiq/hca/ezr_submission_job_spec.rb +++ b/spec/sidekiq/hca/ezr_submission_job_spec.rb @@ -13,30 +13,43 @@ let(:ezr_service) { double } describe 'when retries are exhausted' do - let(:msg) do - { - 'args' => [encrypted_form, nil] - } + context 'when the parsed form is not present' do + it 'only increments StatsD' do + msg = { + 'args' => [HealthCareApplication::LOCKBOX.encrypt({}.to_json), nil] + } + + described_class.within_sidekiq_retries_exhausted_block(msg) do + allow(StatsD).to receive(:increment) + expect(StatsD).to receive(:increment).with('api.1010ezr.failed_wont_retry') + end + end end - it "increments StatsD, creates a 'PersonalInformationLog' record, and logs a failure message to sentry" do - described_class.within_sidekiq_retries_exhausted_block(msg) do - expect(StatsD).to receive(:increment).with('api.1010ezr.failed_wont_retry') - expect_any_instance_of(SentryLogging).to receive(:log_message_to_sentry).with( - '1010EZR total failure', - :error, - { - first_initial: 'F', - middle_initial: 'M', - last_initial: 'Z' - }, - ezr: :total_failure - ) - end + context 'when the parsed form is present' do + it "increments StatsD, creates a 'PersonalInformationLog' record, and logs a failure message to sentry" do + msg = { + 'args' => [encrypted_form, nil] + } + + described_class.within_sidekiq_retries_exhausted_block(msg) do + expect(StatsD).to receive(:increment).with('api.1010ezr.failed_wont_retry') + expect_any_instance_of(SentryLogging).to receive(:log_message_to_sentry).with( + '1010EZR total failure', + :error, + { + first_initial: 'F', + middle_initial: 'M', + last_initial: 'Z' + }, + ezr: :total_failure + ) + end - pii_log = PersonalInformationLog.last - expect(pii_log.error_class).to eq('Form1010Ezr FailedWontRetry') - expect(pii_log.data).to eq(form) + pii_log = PersonalInformationLog.last + expect(pii_log.error_class).to eq('Form1010Ezr FailedWontRetry') + expect(pii_log.data).to eq(form) + end end end From 1ae7d576d0604b892414ee3e467487b40c93a5dc Mon Sep 17 00:00:00 2001 From: Joshua Drumm Date: Fri, 6 Sep 2024 18:26:00 -0400 Subject: [PATCH 7/9] Removed unnecessary code --- app/sidekiq/hca/ezr_submission_job.rb | 1 - 1 file changed, 1 deletion(-) diff --git a/app/sidekiq/hca/ezr_submission_job.rb b/app/sidekiq/hca/ezr_submission_job.rb index da3d0a0b583..3dd92ee1ec5 100644 --- a/app/sidekiq/hca/ezr_submission_job.rb +++ b/app/sidekiq/hca/ezr_submission_job.rb @@ -7,7 +7,6 @@ module HCA class EzrSubmissionJob include Sidekiq::Job include SentryLogging - include Common::Client::Concerns::Monitoring VALIDATION_ERROR = HCA::SOAPParser::ValidationError sidekiq_options retry: 14 From 9162c50927b99d9dfaf615cc660c248f2f84330e Mon Sep 17 00:00:00 2001 From: Joshua Drumm Date: Fri, 6 Sep 2024 18:29:27 -0400 Subject: [PATCH 8/9] Improved code quality --- app/sidekiq/hca/ezr_submission_job.rb | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/app/sidekiq/hca/ezr_submission_job.rb b/app/sidekiq/hca/ezr_submission_job.rb index 3dd92ee1ec5..5ded4bd4482 100644 --- a/app/sidekiq/hca/ezr_submission_job.rb +++ b/app/sidekiq/hca/ezr_submission_job.rb @@ -8,13 +8,14 @@ class EzrSubmissionJob include Sidekiq::Job include SentryLogging VALIDATION_ERROR = HCA::SOAPParser::ValidationError + STATSD_KEY_PREFIX = 'api.1010ezr' sidekiq_options retry: 14 sidekiq_retries_exhausted do |msg, _e| parsed_form = decrypt_form(msg['args'][0]) - StatsD.increment("#{Form1010Ezr::Service::STATSD_KEY_PREFIX}.failed_wont_retry") + StatsD.increment("#{STATSD_KEY_PREFIX}.failed_wont_retry") if parsed_form.present? PersonalInformationLog.create!( @@ -44,7 +45,7 @@ def perform(encrypted_form, user_uuid) Form1010Ezr::Service.new(nil).log_submission_failure(parsed_form) log_exception_to_sentry(e) rescue - StatsD.increment("#{Form1010Ezr::Service::STATSD_KEY_PREFIX}.async.retries") + StatsD.increment("#{STATSD_KEY_PREFIX}.async.retries") raise end end From 75985254bd5a1ceba0e107f7242b51c01f7d6239 Mon Sep 17 00:00:00 2001 From: Joshua Drumm Date: Mon, 9 Sep 2024 13:45:13 -0400 Subject: [PATCH 9/9] Refactored code to extend SentryLogging instead of including it --- app/sidekiq/hca/ezr_submission_job.rb | 8 +++++--- spec/sidekiq/hca/ezr_submission_job_spec.rb | 4 ++-- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/app/sidekiq/hca/ezr_submission_job.rb b/app/sidekiq/hca/ezr_submission_job.rb index 5ded4bd4482..41010cff4e3 100644 --- a/app/sidekiq/hca/ezr_submission_job.rb +++ b/app/sidekiq/hca/ezr_submission_job.rb @@ -6,10 +6,12 @@ module HCA class EzrSubmissionJob include Sidekiq::Job - include SentryLogging + extend SentryLogging VALIDATION_ERROR = HCA::SOAPParser::ValidationError STATSD_KEY_PREFIX = 'api.1010ezr' + # 14 retries was decided on because it's the closest to a 24-hour time span based on + # Sidekiq's backoff formula: https://github.com/sidekiq/sidekiq/wiki/Error-Handling#automatic-job-retry sidekiq_options retry: 14 sidekiq_retries_exhausted do |msg, _e| @@ -23,7 +25,7 @@ class EzrSubmissionJob error_class: 'Form1010Ezr FailedWontRetry' ) - new.log_message_to_sentry( + log_message_to_sentry( '1010EZR total failure', :error, Form1010Ezr::Service.new(nil).veteran_initials(parsed_form), @@ -43,7 +45,7 @@ def perform(encrypted_form, user_uuid) Form1010Ezr::Service.new(user).submit_sync(parsed_form) rescue VALIDATION_ERROR => e Form1010Ezr::Service.new(nil).log_submission_failure(parsed_form) - log_exception_to_sentry(e) + self.class.log_exception_to_sentry(e) rescue StatsD.increment("#{STATSD_KEY_PREFIX}.async.retries") raise diff --git a/spec/sidekiq/hca/ezr_submission_job_spec.rb b/spec/sidekiq/hca/ezr_submission_job_spec.rb index 03b93b78dce..ae93a87c7f6 100644 --- a/spec/sidekiq/hca/ezr_submission_job_spec.rb +++ b/spec/sidekiq/hca/ezr_submission_job_spec.rb @@ -34,7 +34,7 @@ described_class.within_sidekiq_retries_exhausted_block(msg) do expect(StatsD).to receive(:increment).with('api.1010ezr.failed_wont_retry') - expect_any_instance_of(SentryLogging).to receive(:log_message_to_sentry).with( + expect(described_class).to receive(:log_message_to_sentry).with( '1010EZR total failure', :error, { @@ -73,7 +73,7 @@ # of the 'Form1010Ezr::Service', we need to stub out a new instance of the service allow(Form1010Ezr::Service).to receive(:new).with(nil).once.and_return(ezr_service) - expect_any_instance_of(HCA::EzrSubmissionJob).to receive(:log_exception_to_sentry).with(error) + expect(HCA::EzrSubmissionJob).to receive(:log_exception_to_sentry).with(error) expect(ezr_service).to receive(:log_submission_failure).with( form )