From 0d8ef6570e06409c4bea549817f333c4b6f180d9 Mon Sep 17 00:00:00 2001
From: Arpan Kaphle <akaphle@google.com>
Date: Tue, 25 Apr 2023 20:17:26 +0000
Subject: [PATCH] Cleaning up TODOs in Metric Classes

This removes fixed or finished TODOs, and ensures they are logically
conistent with work left over. In the future, the TODOs will be removed
alongside the work completion.

Bug: 279611480
Test: Build Test (comment only removal)
Change-Id: I75a6662cdd84f603eb4779e22e8acfdd4b847f7b
---
 .../com/android/server/credentials/MetricUtilities.java     | 1 +
 .../server/credentials/ProviderRegistryGetSession.java      | 2 +-
 .../server/credentials/metrics/CandidatePhaseMetric.java    | 6 +-----
 .../credentials/metrics/ChosenProviderFinalPhaseMetric.java | 3 ---
 .../server/credentials/metrics/InitialPhaseMetric.java      | 2 --
 .../server/credentials/metrics/RequestSessionMetric.java    | 4 ----
 6 files changed, 3 insertions(+), 15 deletions(-)

diff --git a/services/credentials/java/com/android/server/credentials/MetricUtilities.java b/services/credentials/java/com/android/server/credentials/MetricUtilities.java
index 50e5163cea55c..4e82ee71ac1b4 100644
--- a/services/credentials/java/com/android/server/credentials/MetricUtilities.java
+++ b/services/credentials/java/com/android/server/credentials/MetricUtilities.java
@@ -35,6 +35,7 @@ import java.util.Map;
 /**
  * For all future metric additions, this will contain their names for local usage after importing
  * from {@link com.android.internal.util.FrameworkStatsLog}.
+ * TODO(b/271135048) - Emit all atoms, including all V4 atoms (specifically the rest of track 1).
  */
 public class MetricUtilities {
     private static final boolean LOG_FLAG = true;
diff --git a/services/credentials/java/com/android/server/credentials/ProviderRegistryGetSession.java b/services/credentials/java/com/android/server/credentials/ProviderRegistryGetSession.java
index c10f5640c4667..ead86cefc5d41 100644
--- a/services/credentials/java/com/android/server/credentials/ProviderRegistryGetSession.java
+++ b/services/credentials/java/com/android/server/credentials/ProviderRegistryGetSession.java
@@ -266,7 +266,7 @@ public class ProviderRegistryGetSession extends ProviderSession<CredentialOption
                 .collect(Collectors.toList());
         updateStatusAndInvokeCallback(Status.CREDENTIALS_RECEIVED,
                 /*source=*/ CredentialsSource.REGISTRY);
-        // TODO(use metric later)
+        // TODO(b/273353677) : metric should be emitted similarly to sibling classes
     }
 
     @Nullable
diff --git a/services/credentials/java/com/android/server/credentials/metrics/CandidatePhaseMetric.java b/services/credentials/java/com/android/server/credentials/metrics/CandidatePhaseMetric.java
index 721d3d7826530..b212606b8271a 100644
--- a/services/credentials/java/com/android/server/credentials/metrics/CandidatePhaseMetric.java
+++ b/services/credentials/java/com/android/server/credentials/metrics/CandidatePhaseMetric.java
@@ -30,7 +30,6 @@ import java.util.stream.Collectors;
  * Some types are redundant across these metric collectors, but that has debug use-cases as
  * these data-types are available at different moments of the flow (and typically, one can feed
  * into the next).
- * TODO(b/270403549) - iterate on this in V3+
  */
 public class CandidatePhaseMetric {
 
@@ -56,10 +55,7 @@ public class CandidatePhaseMetric {
     private int mProviderQueryStatus = -1;
     // Indicates if an exception was thrown by this provider, false by default
     private boolean mHasException = false;
-    // Indicates the number of total entries available. We can also locally store the entries, but
-    // cannot emit them in the current split form. TODO(b/271135048) - possibly readjust candidate
-    // entries. Also, it may be okay to remove this and instead aggregate from inner counts.
-    // Defaults to -1
+    // Indicates the number of total entries available, defaults to -1
     private int mNumEntriesTotal = -1;
     // The count of action entries from this provider, defaults to -1
     private int mActionEntryCount = -1;
diff --git a/services/credentials/java/com/android/server/credentials/metrics/ChosenProviderFinalPhaseMetric.java b/services/credentials/java/com/android/server/credentials/metrics/ChosenProviderFinalPhaseMetric.java
index c80cc24fa455b..8f08bb02dfd0c 100644
--- a/services/credentials/java/com/android/server/credentials/metrics/ChosenProviderFinalPhaseMetric.java
+++ b/services/credentials/java/com/android/server/credentials/metrics/ChosenProviderFinalPhaseMetric.java
@@ -29,11 +29,8 @@ import java.util.List;
  * Some types are redundant across these metric collectors, but that has debug use-cases as
  * these data-types are available at different moments of the flow (and typically, one can feed
  * into the next).
- * TODO(b/270403549) - iterate on this in V3+
  */
 public class ChosenProviderFinalPhaseMetric {
-
-    // TODO(b/270403549) - applies elsewhere, likely removed or replaced w/ some hashed/count index
     private static final String TAG = "ChosenFinalPhaseMetric";
     // The session id associated with this API call, used to unite split emits
     private int mSessionId = -1;
diff --git a/services/credentials/java/com/android/server/credentials/metrics/InitialPhaseMetric.java b/services/credentials/java/com/android/server/credentials/metrics/InitialPhaseMetric.java
index 0ecd9cc79e486..5cfb0e7d375b0 100644
--- a/services/credentials/java/com/android/server/credentials/metrics/InitialPhaseMetric.java
+++ b/services/credentials/java/com/android/server/credentials/metrics/InitialPhaseMetric.java
@@ -26,7 +26,6 @@ import java.util.Map;
  * Some types are redundant across these metric collectors, but that has debug use-cases as
  * these data-types are available at different moments of the flow (and typically, one can feed
  * into the next).
- * TODO(b/270403549) - iterate on this in V3+
  */
 public class InitialPhaseMetric {
     private static final String TAG = "InitialPhaseMetric";
@@ -47,7 +46,6 @@ public class InitialPhaseMetric {
     private long mCredentialServiceBeginQueryTimeNanoseconds = -1;
 
     // Indicates if the origin was specified when making this API request
-    // TODO(b/271135048) - Emit once metrics approved
     private boolean mOriginSpecified = false;
 
     // Stores the deduped request information, particularly {"req":5}.
diff --git a/services/credentials/java/com/android/server/credentials/metrics/RequestSessionMetric.java b/services/credentials/java/com/android/server/credentials/metrics/RequestSessionMetric.java
index 547c09a625f65..4ecdfef401b6e 100644
--- a/services/credentials/java/com/android/server/credentials/metrics/RequestSessionMetric.java
+++ b/services/credentials/java/com/android/server/credentials/metrics/RequestSessionMetric.java
@@ -337,10 +337,6 @@ public class RequestSessionMetric {
      */
     public void logApiCalledAtFinish(int apiStatus) {
         try {
-            // TODO (b/270403549) - this browsing phase object is fine but also have a new emit
-            // For the returned types by authentication entries - i.e. a CandidatePhase During
-            // Browse
-            // Possibly think of adding in more atoms for other APIs as well.
             logApiCalledFinalPhase(mChosenProviderFinalPhaseMetric, mCandidateBrowsingPhaseMetric,
                     apiStatus,
                     ++mSequenceCounter);