From cc566045df5decc8e3f1c25394f5a6f71efd41ab Mon Sep 17 00:00:00 2001 From: Bryan Alexander Date: Mon, 23 Sep 2024 17:09:30 -0400 Subject: [PATCH] 91824: Create Burials::Monitor and modify controller (#18537) --- .github/CODEOWNERS | 2 + .../v0/burial_claims_controller.rb | 91 ++++++++++++----- lib/burials/monitor.rb | 89 +++++++++++++++++ .../v0/burial_claims_controller_spec.rb | 35 +++++++ spec/lib/burials/monitor_spec.rb | 99 +++++++++++++++++++ 5 files changed, 289 insertions(+), 27 deletions(-) create mode 100644 lib/burials/monitor.rb create mode 100644 spec/lib/burials/monitor_spec.rb diff --git a/.github/CODEOWNERS b/.github/CODEOWNERS index a174d05eda8..5f3c1446369 100644 --- a/.github/CODEOWNERS +++ b/.github/CODEOWNERS @@ -841,6 +841,7 @@ lib/benefits_intake_service @department-of-veterans-affairs/benefits-dependents- lib/bgs @department-of-veterans-affairs/va-api-engineers @department-of-veterans-affairs/backend-review-group lib/bid @department-of-veterans-affairs/Disability-Experience @department-of-veterans-affairs/va-api-engineers @department-of-veterans-affairs/backend-review-group lib/bip_claims @department-of-veterans-affairs/va-api-engineers @department-of-veterans-affairs/backend-review-group +lib/burials @department-of-veterans-affairs/va-api-engineers @department-of-veterans-affairs/backend-review-group lib/carma @department-of-veterans-affairs/vfs-10-10 @department-of-veterans-affairs/va-api-engineers @department-of-veterans-affairs/backend-review-group lib/caseflow @department-of-veterans-affairs/lighthouse-banana-peels @department-of-veterans-affairs/va-api-engineers @department-of-veterans-affairs/backend-review-group lib/central_mail @department-of-veterans-affairs/va-api-engineers @department-of-veterans-affairs/backend-review-group @@ -1382,6 +1383,7 @@ spec/lib/bgs @department-of-veterans-affairs/benefits-dependents-management @dep spec/lib/bid @department-of-veterans-affairs/Disability-Experience @department-of-veterans-affairs/va-api-engineers @department-of-veterans-affairs/backend-review-group spec/lib/bip_claims @department-of-veterans-affairs/va-api-engineers @department-of-veterans-affairs/backend-review-group spec/lib/breakers @department-of-veterans-affairs/va-api-engineers @department-of-veterans-affairs/backend-review-group +spec/lib/burials @department-of-veterans-affairs/va-api-engineers @department-of-veterans-affairs/backend-review-group spec/lib/carma @department-of-veterans-affairs/vfs-10-10 @department-of-veterans-affairs/va-api-engineers @department-of-veterans-affairs/backend-review-group spec/lib/caseflow @department-of-veterans-affairs/lighthouse-banana-peels @department-of-veterans-affairs/va-api-engineers @department-of-veterans-affairs/backend-review-group spec/lib/central_mail @department-of-veterans-affairs/va-api-engineers @department-of-veterans-affairs/backend-review-group diff --git a/app/controllers/v0/burial_claims_controller.rb b/app/controllers/v0/burial_claims_controller.rb index 5e3b70df986..132bddd4b01 100644 --- a/app/controllers/v0/burial_claims_controller.rb +++ b/app/controllers/v0/burial_claims_controller.rb @@ -1,52 +1,61 @@ # frozen_string_literal: true require 'pension_burial/tag_sentry' +require 'burials/monitor' module V0 class BurialClaimsController < ClaimsBaseController service_tag 'burial-application' def show - submission_attempt = determine_submission_attempt + claim = claim_class.find_by!(guid: params[:id]) + form_submission = claim&.form_submissions&.last + submission_attempt = form_submission&.form_submission_attempts&.last if submission_attempt state = submission_attempt.aasm_state == 'failure' ? 'failure' : 'success' render(json: { data: { attributes: { state: } } }) elsif central_mail_submission render json: CentralMailSubmissionSerializer.new(central_mail_submission) - else - Rails.logger.error("ActiveRecord::RecordNotFound: Claim submission not found for claim_id: #{params[:id]}") - render(json: { data: { attributes: { state: 'not found' } } }, status: :not_found) end + rescue ActiveRecord::RecordNotFound => e + monitor.track_show404(params[:id], current_user, e) + render(json: { data: { attributes: { state: 'not found' } } }, status: :not_found) rescue => e - Rails.logger.error(e.to_s) + monitor.track_show_error(params[:id], current_user, e) render(json: { data: { attributes: { state: 'error processing request' } } }, status: :unprocessable_entity) end def create PensionBurial::TagSentry.tag_sentry - claim = if Flipper.enabled?(:va_burial_v2) - # cannot parse a nil form, to pass unit tests do a check for form presence - form = filtered_params[:form] - claim_class.new(form:, formV2: form.present? ? JSON.parse(form)['formV2'] : nil) - else - claim_class.new(form: filtered_params[:form]) - end - - unless claim.save - StatsD.increment("#{stats_key}.failure") - Sentry.set_tags(team: 'benefits-memorial-1') # tag sentry logs with team name - Rails.logger.error('Burial claim was not saved', { error_messages: claim.errors, - user_uuid: current_user&.uuid, - in_progress_form_id: in_progress_form&.id }) - raise Common::Exceptions::ValidationErrors, claim - end + claim = create_claim + monitor.track_create_attempt(claim, current_user) + + track_claim_save_failure(claim) unless claim.save + # this method also calls claim.process_attachments! claim.submit_to_structured_data_services! Rails.logger.info "ClaimID=#{claim.confirmation_number} Form=#{claim.form_id}" + + in_progress_form = current_user ? InProgressForm.form_for_user(claim.form_id, current_user) : nil + claim.form_start_date = in_progress_form.created_at if in_progress_form + monitor.track_create_success(in_progress_form, claim, current_user) + clear_saved_form(claim.form_id) render json: SavedClaimSerializer.new(claim) + rescue => e + monitor.track_create_error(in_progress_form, claim, current_user, e) + raise e + end + + def create_claim + if Flipper.enabled?(:va_burial_v2) + form = filtered_params[:form] + claim_class.new(form:, formV2: form.present? ? JSON.parse(form)['formV2'] : nil) + else + claim_class.new(form: filtered_params[:form]) + end end def short_name @@ -59,12 +68,6 @@ def claim_class private - def determine_submission_attempt - claim = claim_class.find_by(guid: params[:id]) - form_submission = claim&.form_submissions&.last - form_submission&.form_submission_attempts&.last - end - def central_mail_submission CentralMailSubmission.joins(:central_mail_claim).find_by(saved_claims: { guid: params[:id] }) end @@ -72,5 +75,39 @@ def central_mail_submission def in_progress_form current_user ? InProgressForm.form_for_user(claim.form_id, current_user) : nil end + + def track_claim_save_failure(claim) + StatsD.increment("#{stats_key}.failure") + Sentry.set_tags(team: 'benefits-memorial-1') # tag sentry logs with team name + Rails.logger.error('Burial claim was not saved', { error_messages: claim.errors, + user_uuid: current_user&.uuid, + in_progress_form_id: in_progress_form&.id }) + log_validation_error_to_metadata(in_progress_form, claim) + raise Common::Exceptions::ValidationErrors, claim + end + + ## + # include validation error on in_progress_form metadata. + # `noop` if in_progress_form is `blank?` + # + # @param in_progress_form [InProgressForm] + # @param claim [Pensions::SavedClaim] + # + def log_validation_error_to_metadata(in_progress_form, claim) + return if in_progress_form.blank? + + metadata = in_progress_form.metadata + metadata['submission']['error_message'] = claim&.errors&.errors&.to_s + in_progress_form.update(metadata:) + end + + ## + # retreive a monitor for tracking + # + # @return [Burials::Monitor] + # + def monitor + @monitor ||= Burials::Monitor.new + end end end diff --git a/lib/burials/monitor.rb b/lib/burials/monitor.rb new file mode 100644 index 00000000000..66d1436b735 --- /dev/null +++ b/lib/burials/monitor.rb @@ -0,0 +1,89 @@ +# frozen_string_literal: true + +module Burials + ## + # Monitor functions for Rails logging and StatsD + # + class Monitor + CLAIM_STATS_KEY = 'api.burial_claim' + + ## + # log GET 404 from controller + # @see BurialClaimsController + # + # @param confirmation_number [UUID] saved_claim guid + # @param current_user [User] + # @param e [ActiveRecord::RecordNotFound] + # + def track_show404(confirmation_number, current_user, e) + Rails.logger.error('21P-530EZ submission not found', + { confirmation_number:, user_uuid: current_user&.uuid, message: e&.message }) + end + + ## + # log GET 500 from controller + # @see BurialClaimsController + # + # @param confirmation_number [UUID] saved_claim guid + # @param current_user [User] + # @param e [Error] + # + def track_show_error(confirmation_number, current_user, e) + Rails.logger.error('21P-530EZ fetching submission failed', + { confirmation_number:, user_uuid: current_user&.uuid, message: e&.message }) + end + + ## + # log POST processing started + # @see BurialClaimsController + # + # @param claim [Pension::SavedClaim] + # @param current_user [User] + # + def track_create_attempt(claim, current_user) + StatsD.increment("#{CLAIM_STATS_KEY}.attempt") + Rails.logger.info('21P-530EZ submission to Sidekiq begun', + { confirmation_number: claim&.confirmation_number, user_uuid: current_user&.uuid }) + end + + ## + # log POST processing failure + # @see BurialClaimsController + # + # @param in_progress_form [InProgressForm] + # @param claim [SavedClaim::Burial] + # @param current_user [User] + # @param e [Error] + # + def track_create_error(in_progress_form, claim, current_user, e = nil) + StatsD.increment("#{CLAIM_STATS_KEY}.failure") + Rails.logger.error('21P-530EZ submission to Sidekiq failed', + { confirmation_number: claim&.confirmation_number, user_uuid: current_user&.uuid, + in_progress_form_id: in_progress_form&.id, errors: claim&.errors&.errors, + message: e&.message }) + end + + ## + # log POST processing success + # @see BurialClaimsController + # + # @param in_progress_form [InProgressForm] + # @param claim [SavedClaim::Burial] + # @param current_user [User] + # + def track_create_success(in_progress_form, claim, current_user) + StatsD.increment("#{CLAIM_STATS_KEY}.success") + if claim.form_start_date + claim_duration = claim.created_at - claim.form_start_date + tags = ["form_id:#{claim.form_id}"] + StatsD.measure('saved_claim.time-to-file', claim_duration, tags:) + end + context = { + confirmation_number: claim&.confirmation_number, + user_uuid: current_user&.uuid, + in_progress_form_id: in_progress_form&.id + } + Rails.logger.info('21P-530EZ submission to Sidekiq success', context) + end + end +end diff --git a/spec/controllers/v0/burial_claims_controller_spec.rb b/spec/controllers/v0/burial_claims_controller_spec.rb index bac833a160e..da007e1540d 100644 --- a/spec/controllers/v0/burial_claims_controller_spec.rb +++ b/spec/controllers/v0/burial_claims_controller_spec.rb @@ -2,10 +2,16 @@ require 'rails_helper' require 'support/controller_spec_helper' +require_relative '../../../lib/burials/monitor' RSpec.describe V0::BurialClaimsController, type: :controller do + let(:monitor) { double('Burials::Monitor') } + before do Flipper.enable(:va_burial_v2) + allow(Burials::Monitor).to receive(:new).and_return(monitor) + allow(monitor).to receive_messages(track_show404: nil, track_show_error: nil, track_create_attempt: nil, + track_create_error: nil, track_create_success: nil) end describe 'with a user' do @@ -26,11 +32,28 @@ def send_create VCR::MATCH_EVERYTHING ) do create(:in_progress_form, user_uuid: user.uuid, form_id:) + expect(monitor).to receive(:track_create_attempt).once + expect(monitor).to receive(:track_create_success).once expect(controller).to receive(:clear_saved_form).with(form_id).and_call_original sign_in_as(user) expect { send_create }.to change(InProgressForm, :count).by(-1) end end + + it 'logs validation errors' do + allow(SavedClaim::Burial).to receive(:new).and_return(form) + allow(form).to receive(:save).and_raise(StandardError, 'mock error') + + VCR.use_cassette( + 'mvi/find_candidate/find_profile_with_attributes', + VCR::MATCH_EVERYTHING + ) do + expect(monitor).to receive(:track_create_attempt).once + expect(monitor).to receive(:track_create_error).once + response = send_create + expect(response.status).to eq(500) + end + end end describe '#show' do @@ -51,8 +74,20 @@ def send_create end it 'returns an error if the claim is not found' do + expect(monitor).to receive(:track_show404).once + get(:show, params: { id: '12345' }) + expect(response).to have_http_status(:not_found) end + + it 'logs validation errors' do + allow(SavedClaim::Burial).to receive(:find_by).and_raise(StandardError, 'mock error') + expect(monitor).to receive(:track_show_error).once + + get(:show, params: { id: '12345' }) + + expect(response).to have_http_status(:unprocessable_entity) + end end end diff --git a/spec/lib/burials/monitor_spec.rb b/spec/lib/burials/monitor_spec.rb new file mode 100644 index 00000000000..89cdaf7b8ac --- /dev/null +++ b/spec/lib/burials/monitor_spec.rb @@ -0,0 +1,99 @@ +# frozen_string_literal: true + +require 'rails_helper' +require_relative '../../../lib/burials/monitor' + +RSpec.describe Burials::Monitor do + let(:monitor) { described_class.new } + let(:claim_stats_key) { described_class::CLAIM_STATS_KEY } + let(:claim) { create(:burial_claim_v2) } + let(:ipf) { create(:in_progress_form) } + + context 'with all params supplied' do + let(:current_user) { create(:user) } + let(:monitor_error) { create(:monitor_error) } + let(:lh_service) { OpenStruct.new(uuid: 'uuid') } + + describe '#track_show404' do + it 'logs a not found error' do + log = '21P-530EZ submission not found' + payload = { + confirmation_number: claim.confirmation_number, + user_uuid: current_user.uuid, + message: monitor_error.message + } + + expect(Rails.logger).to receive(:error).with(log, payload) + + monitor.track_show404(claim.confirmation_number, current_user, monitor_error) + end + end + + describe '#track_show_error' do + it 'logs a submission failed error' do + log = '21P-530EZ fetching submission failed' + payload = { + confirmation_number: claim.confirmation_number, + user_uuid: current_user.uuid, + message: monitor_error.message + } + + expect(Rails.logger).to receive(:error).with(log, payload) + + monitor.track_show_error(claim.confirmation_number, current_user, monitor_error) + end + end + + describe '#track_create_attempt' do + it 'logs sidekiq started' do + log = '21P-530EZ submission to Sidekiq begun' + payload = { + confirmation_number: claim.confirmation_number, + user_uuid: current_user.uuid + } + + expect(StatsD).to receive(:increment).with("#{claim_stats_key}.attempt") + expect(Rails.logger).to receive(:info).with(log, payload) + + monitor.track_create_attempt(claim, current_user) + end + end + + describe '#track_create_error' do + it 'logs sidekiq failed' do + log = '21P-530EZ submission to Sidekiq failed' + payload = { + confirmation_number: claim.confirmation_number, + user_uuid: current_user.uuid, + in_progress_form_id: ipf.id, + errors: [], # mock claim does not have `errors` + message: monitor_error.message + } + + expect(StatsD).to receive(:increment).with("#{claim_stats_key}.failure") + expect(Rails.logger).to receive(:error).with(log, payload) + + monitor.track_create_error(ipf, claim, current_user, monitor_error) + end + end + + describe '#track_create_success' do + it 'logs sidekiq success' do + log = '21P-530EZ submission to Sidekiq success' + payload = { + confirmation_number: claim.confirmation_number, + user_uuid: current_user.uuid, + in_progress_form_id: ipf.id + } + claim.form_start_date = Time.zone.now + + expect(StatsD).to receive(:increment).with("#{claim_stats_key}.success") + expect(StatsD).to receive(:measure).with('saved_claim.time-to-file', claim.created_at - claim.form_start_date, + tags: ["form_id:#{claim.form_id}"]) + expect(Rails.logger).to receive(:info).with(log, payload) + + monitor.track_create_success(ipf, claim, current_user) + end + end + end +end