From 568fe72ad66b1868d0787e781030d8e5a511e2be Mon Sep 17 00:00:00 2001 From: Derek Fitchett <135860892+dfitchett@users.noreply.github.com> Date: Mon, 30 Sep 2024 12:19:31 -0700 Subject: [PATCH] BGS: Fix bug returning wrong result when downstream BGS returns non-2xx response. (#3527) Fix bug where create_claim_notes and create_veteran_notes are returning result of submitting metrics. Fixed noisy unit tests by mocking classes where necessary. --- svc-bgs-api/src/lib/bgs_client.rb | 8 ++++++-- svc-bgs-api/src/lib/metric_logger.rb | 13 +++++++------ svc-bgs-api/src/main_consumer.rb | 11 ++++++----- svc-bgs-api/src/spec/bgs_client_spec.rb | 5 +++++ svc-bgs-api/src/spec/metric_logger_spec.rb | 10 ++++++++++ 5 files changed, 34 insertions(+), 13 deletions(-) diff --git a/svc-bgs-api/src/lib/bgs_client.rb b/svc-bgs-api/src/lib/bgs_client.rb index b2eb70c4d2..b5574e6e16 100644 --- a/svc-bgs-api/src/lib/bgs_client.rb +++ b/svc-bgs-api/src/lib/bgs_client.rb @@ -80,9 +80,11 @@ def create_claim_notes(claim_id:, notes:) start_time = Time.now metric_custom_tags = ['bgsNoteType:claim'] @metrics.submit_count_with_default_value(METRIC[:REQUEST_START], metric_custom_tags) - bgs.notes.create_notes(note_hashes) + response = bgs.notes.create_notes(note_hashes) @metrics.submit_request_duration(start_time, Time.now, metric_custom_tags) @metrics.submit_count_with_default_value(METRIC[:RESPONSE_COMPLETE], metric_custom_tags) + + response end def create_veteran_note(note:, claim_id: nil, participant_id: nil) @@ -91,8 +93,10 @@ def create_veteran_note(note:, claim_id: nil, participant_id: nil) start_time = Time.now metric_custom_tags = ['bgsNoteType:veteran'] @metrics.submit_count_with_default_value(METRIC[:REQUEST_START], metric_custom_tags) - bgs.notes.create_note(participant_id:, txt: note, user_id: vro_participant_id) + response = bgs.notes.create_note(participant_id:, txt: note, user_id: vro_participant_id) @metrics.submit_request_duration(start_time, Time.now, metric_custom_tags) @metrics.submit_count_with_default_value(METRIC[:RESPONSE_COMPLETE], metric_custom_tags) + + response end end diff --git a/svc-bgs-api/src/lib/metric_logger.rb b/svc-bgs-api/src/lib/metric_logger.rb index f8417d1460..d1d3d8c0ff 100644 --- a/svc-bgs-api/src/lib/metric_logger.rb +++ b/svc-bgs-api/src/lib/metric_logger.rb @@ -30,7 +30,6 @@ def initialize begin api_instance = DatadogAPIClient::V1::AuthenticationAPI.new api_instance.validate - $logger.info('Succeeded Datadog authentication check') rescue Exception => e $logger.warn("Failed Datadog authentication check: #{e.message}") end @@ -81,9 +80,11 @@ def submit_count(metric, value, custom_tags) rescue Exception => e $logger.warn("Error logging metric: #{metric} (count). Error: #{e.class}, Message: #{e.message}") end + + nil end - def submit_count_with_default_value(metric, custom_tags) + def submit_count_with_default_value(metric, custom_tags = nil) submit_count(metric, 1, custom_tags) end @@ -100,15 +101,15 @@ def submit_request_duration(start_time, end_time, custom_tags = nil) opts = { content_encoding: DatadogAPIClient::V1::DistributionPointsContentEncoding::DEFLATE } - payload_result = @distribution_metrics_api.submit_distribution_points(payload, opts) - $logger.info( - "submitted #{payload.series.first.metric} #{payload_result.status}" - ) + @distribution_metrics_api.submit_distribution_points(payload, opts) + $logger.info("submitted #{payload.series.first.metric}") rescue Exception => e $logger.warn( "exception submitting request duration #{e.message}" ) end + + nil end def generate_distribution_metric(metric, value, custom_tags = nil) diff --git a/svc-bgs-api/src/main_consumer.rb b/svc-bgs-api/src/main_consumer.rb index 483ed89dab..f1b58c66ad 100644 --- a/svc-bgs-api/src/main_consumer.rb +++ b/svc-bgs-api/src/main_consumer.rb @@ -42,6 +42,12 @@ def initialize_subscriber(bgs_client, metric_logger) begin bgs_client.handle_request(json) rescue => e + begin + metric_logger.submit_count_with_default_value(METRIC[:RESPONSE_ERROR]) + rescue => metric_e + $logger.error "Exception submitting metric RESPONSE_ERROR #{metric_e.message}" + end + status_code = e.is_a?(ArgumentError) ? 400 : 500 status_str = e.is_a?(ArgumentError) ? "BAD_REQUEST" : "INTERNAL_SERVER_ERROR" { @@ -58,11 +64,6 @@ def initialize_subscriber(bgs_client, metric_logger) } ] } - begin - metric_logger.submit_count_with_default_value(METRIC[:RESPONSE_ERROR], nil) - rescue => metric_e - $logger.error "Exception submitting metric RESPONSE_ERROR #{e.message}" - end else { statusCode: 200, diff --git a/svc-bgs-api/src/spec/bgs_client_spec.rb b/svc-bgs-api/src/spec/bgs_client_spec.rb index 930be9baf4..32909150ed 100644 --- a/svc-bgs-api/src/spec/bgs_client_spec.rb +++ b/svc-bgs-api/src/spec/bgs_client_spec.rb @@ -2,9 +2,14 @@ describe BgsClient do let(:client) { BgsClient.new } + let(:metrics) { double("metrics") } let(:notes_service) { double("notes_service") } before do + allow(MetricLogger).to receive(:new).and_return(metrics) + allow(metrics).to receive(:submit_count_with_default_value).and_return(202) + allow(metrics).to receive(:submit_request_duration).and_return(202) + allow(BGS::Services).to receive(:new).and_return(double("bgs_services", notes: notes_service)) allow(client).to receive(:vro_participant_id).and_return("12345") end diff --git a/svc-bgs-api/src/spec/metric_logger_spec.rb b/svc-bgs-api/src/spec/metric_logger_spec.rb index 77aac18897..ffb586031e 100644 --- a/svc-bgs-api/src/spec/metric_logger_spec.rb +++ b/svc-bgs-api/src/spec/metric_logger_spec.rb @@ -2,6 +2,16 @@ describe MetricLogger do let(:client) { MetricLogger.new } + let(:api_instance) { double("api_instance") } + let(:metrics) { double("metrics") } + let(:distributions) { double("distributions") } + + before do + allow(DatadogAPIClient::V1::AuthenticationAPI).to receive(:new).and_return(api_instance) + allow(api_instance).to receive(:validate).and_return(true) + allow(DatadogAPIClient::V1::MetricsAPI).to receive(:new).and_return(metrics) + allow(DatadogAPIClient::V2::MetricsAPI).to receive(:new).and_return(distributions) + end it 'generates standard tags when no custom tags are specified' do tags = client.generate_tags