From f9533e016fd6b6c74f703021f6ee0bdc801c361d Mon Sep 17 00:00:00 2001
From: Ravi Khadiwala <ravi@signal.org>
Date: Mon, 11 Mar 2024 11:03:24 -0500
Subject: [PATCH] Adjust metrics in RemoveExpiredUsernameHoldsCommand

---
 .../RemoveExpiredUsernameHoldsCommand.java    | 21 +++++++++++--------
 1 file changed, 12 insertions(+), 9 deletions(-)

diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/workers/RemoveExpiredUsernameHoldsCommand.java b/service/src/main/java/org/whispersystems/textsecuregcm/workers/RemoveExpiredUsernameHoldsCommand.java
index 8dcb880d0..09345ba9c 100644
--- a/service/src/main/java/org/whispersystems/textsecuregcm/workers/RemoveExpiredUsernameHoldsCommand.java
+++ b/service/src/main/java/org/whispersystems/textsecuregcm/workers/RemoveExpiredUsernameHoldsCommand.java
@@ -22,6 +22,7 @@
 import java.util.ArrayList;
 import java.util.Iterator;
 import java.util.List;
+import java.util.concurrent.atomic.AtomicLong;
 
 import static org.whispersystems.textsecuregcm.metrics.MetricsUtil.name;
 
@@ -39,8 +40,8 @@ public class RemoveExpiredUsernameHoldsCommand extends AbstractSinglePassCrawlAc
   private static final String DELETED_HOLDS_COUNTER_NAME =
       name(RemoveExpiredUsernameHoldsCommand.class, "expiredHolds");
 
-  private static final String UPDATED_ACCOUNTS_COUNTER_NAME =
-      name(RemoveExpiredUsernameHoldsCommand.class, "accountsWithExpiredHolds");
+  private static final String INSPECTED_ACCOUNTS_COUNTER_NAME =
+      name(RemoveExpiredUsernameHoldsCommand.class, "inspectedAccounts");
 
   private static final Logger log = LoggerFactory.getLogger(RemoveExpiredUsernameHoldsCommand.class);
 
@@ -74,28 +75,30 @@ protected void crawlAccounts(final Flux<Account> accounts) {
 
     final Counter deletedHoldsCounter =
         Metrics.counter(DELETED_HOLDS_COUNTER_NAME, "dryRun", String.valueOf(isDryRun));
-    final Counter updatedAccountsCounter =
-        Metrics.counter(UPDATED_ACCOUNTS_COUNTER_NAME, "dryRun", String.valueOf(isDryRun));
 
     final AccountsManager accountManager = getCommandDependencies().accountsManager();
+    final AtomicLong accountsInspected = new AtomicLong();
     accounts.flatMap(account -> {
+          accountsInspected.incrementAndGet();
           final List<Account.UsernameHold> holds = new ArrayList<>(account.getUsernameHolds());
-          int holdsToRemove = removeExpired(holds);
+          final int holdsToRemove = removeExpired(holds);
           final Mono<Void> purgeMono = isDryRun || holdsToRemove == 0
               ? Mono.empty()
               : Mono.fromFuture(() ->
                   accountManager.updateAsync(account, a -> a.setUsernameHolds(holds)).thenRun(Util.NOOP));
+          Metrics.counter(INSPECTED_ACCOUNTS_COUNTER_NAME,
+                  "dryRun", String.valueOf(isDryRun),
+                  "expiredHolds", String.valueOf(holdsToRemove > 0))
+              .increment();
           return purgeMono
-              .doOnSuccess(ignored -> {
-                deletedHoldsCounter.increment(holdsToRemove);
-                updatedAccountsCounter.increment();
-              })
+              .doOnSuccess(ignored -> deletedHoldsCounter.increment(holdsToRemove))
               .onErrorResume(throwable -> {
                 log.warn("Failed to purge {} expired holds on account {}", holdsToRemove, account.getUuid());
                 return Mono.empty();
               });
         }, maxConcurrency)
         .then().block();
+    log.info("Finished crawl of {} accounts", accountsInspected.get());
   }
 
   @VisibleForTesting