BGS: Fix bug returning wrong result when downstream BGS returns non-2…

…xx 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

@@ -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

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)

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,

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

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