From be21fdead181807370d35bdd019c2a6ddab40f4f Mon Sep 17 00:00:00 2001
From: Casey Williams <caseywilliams@users.noreply.github.com>
Date: Wed, 17 Apr 2024 13:33:53 -0700
Subject: [PATCH] API-34961 Trace appeals PII removal with Datadog (#16364)

---
 .../app/services/appeals_api/remove_pii.rb    | 29 +++++++--------
 .../services/appeals_api/remove_pii_spec.rb   | 35 ++++++++++++++-----
 2 files changed, 39 insertions(+), 25 deletions(-)

diff --git a/modules/appeals_api/app/services/appeals_api/remove_pii.rb b/modules/appeals_api/app/services/appeals_api/remove_pii.rb
index 39fd91d8673..c9626c78e1c 100644
--- a/modules/appeals_api/app/services/appeals_api/remove_pii.rb
+++ b/modules/appeals_api/app/services/appeals_api/remove_pii.rb
@@ -1,5 +1,7 @@
 # frozen_string_literal: true
 
+require 'ddtrace'
+
 module AppealsApi
   class RemovePii
     include SentryLogging
@@ -15,13 +17,20 @@ def initialize(form_type:)
     end
 
     def run!
-      validate_form_type!
+      Datadog::Tracing.trace("#{self.class.name} - #{form_type}") do
+        validate_form_type!
 
-      result = remove_pii!
+        result = remove_pii!
 
-      log_failure_to_sentry if records_were_not_cleared(result)
+        if result.blank? && records_to_be_expunged.present?
+          ids = records_to_be_expunged.pluck(:id)
+          msg = "Failed to remove expired #{form_type} PII from records"
+          Rails.logger.error(msg, ids)
+          AppealsApi::Slack::Messager.new({ msg:, ids: }).notify!
+        end
 
-      result
+        result
+      end
     end
 
     private
@@ -49,17 +58,5 @@ def records_to_be_expunged
                                              .pii_expunge_policy
                                   end
     end
-
-    def records_were_not_cleared(result)
-      result.blank? && records_to_be_expunged.present?
-    end
-
-    def log_failure_to_sentry
-      log_message_to_sentry(
-        "Failed to expunge PII from #{form_type} (modules/appeals_api)",
-        :error,
-        ids: records_to_be_expunged.pluck(:id)
-      )
-    end
   end
 end
diff --git a/modules/appeals_api/spec/services/appeals_api/remove_pii_spec.rb b/modules/appeals_api/spec/services/appeals_api/remove_pii_spec.rb
index 8599a4838f5..62d9785c9e6 100644
--- a/modules/appeals_api/spec/services/appeals_api/remove_pii_spec.rb
+++ b/modules/appeals_api/spec/services/appeals_api/remove_pii_spec.rb
@@ -1,7 +1,6 @@
 # frozen_string_literal: true
 
 require 'rails_helper'
-require AppealsApi::Engine.root.join('spec', 'spec_helper.rb')
 
 def update_appeal_status(appeal, status, code: nil, detail: nil)
   # At the time of writing, the `update_status` method for each appeal model involves kicking off a sidekiq job to
@@ -159,6 +158,32 @@ def create_misc_appeals = create_appeals + misc_appeal_types.map { |f| FactoryBo
       expect { AppealsApi::RemovePii.new(form_type: 'Invalid').run! }.to raise_error(ArgumentError)
     end
 
+    context 'when the removal fails' do
+      let!(:appeals) do
+        Timecop.freeze(100.days.ago) do
+          status = 'complete'
+          [create(:supplemental_claim, status:), create(:supplemental_claim_v0, status:)]
+        end
+      end
+
+      before do
+        instance = AppealsApi::RemovePii.new(form_type: AppealsApi::SupplementalClaim)
+        msg = 'Failed to remove expired AppealsApi::SupplementalClaim PII from records'
+        expect(Rails.logger).to receive(:error).with(msg, appeals.map(&:id))
+        expect_any_instance_of(AppealsApi::Slack::Messager).to receive(:notify!)
+        allow(instance).to receive(:remove_pii!).and_return []
+        instance.run!
+      end
+
+      it 'logs an error and the IDs of records whose PII failed to be removed' do
+        appeals.each do |appeal|
+          appeal.reload
+          expect(appeal.auth_headers).to be_present
+          expect(appeal.form_data).to be_present
+        end
+      end
+    end
+
     it 'removes PII from HLR records needing PII removal' do
       day_old_has_pii_v2 = create :higher_level_review_v2, status: 'complete'
       day_old_has_pii_v2.update updated_at: 1.day.ago
@@ -271,13 +296,5 @@ def create_misc_appeals = create_appeals + misc_appeal_types.map { |f| FactoryBo
         expect(week_old_has_pii_error.reload.form_data_ciphertext).to be_present
       end
     end
-
-    it 'sends a message to sentry if the removal failed.' do
-      allow_any_instance_of(AppealsApi::RemovePii).to receive(:records_were_not_cleared).and_return(true)
-      service = AppealsApi::RemovePii.new(form_type: AppealsApi::NoticeOfDisagreement)
-      expect(service).to receive(:log_failure_to_sentry)
-
-      service.run!
-    end
   end
 end