Merge "Added impression logging for AnnotatedCallLog rebuild operations."

3 files changed, 65 insertions, 13 deletions

diff --git a/java/com/android/dialer/calllog/RefreshAnnotatedCallLogReceiver.java b/java/com/android/dialer/calllog/RefreshAnnotatedCallLogReceiver.java
index 3019f5e3a..4a7ce0bac 100644
--- a/java/com/android/dialer/calllog/RefreshAnnotatedCallLogReceiver.java
+++ b/java/com/android/dialer/calllog/RefreshAnnotatedCallLogReceiver.java
@@ -24,12 +24,15 @@ import android.support.annotation.Nullable;
 import com.android.dialer.calllog.RefreshAnnotatedCallLogWorker.RefreshResult;
 import com.android.dialer.calllog.constants.IntentNames;
 import com.android.dialer.common.LogUtil;
-import com.android.dialer.common.concurrent.DefaultFutureCallback;
 import com.android.dialer.common.concurrent.ThreadUtil;
+import com.android.dialer.logging.DialerImpression;
+import com.android.dialer.logging.Logger;
+import com.android.dialer.logging.LoggingBindings;
 import com.android.dialer.metrics.FutureTimer;
 import com.android.dialer.metrics.Metrics;
 import com.android.dialer.metrics.MetricsComponent;
 import com.google.common.base.Function;
+import com.google.common.util.concurrent.FutureCallback;
 import com.google.common.util.concurrent.Futures;
 import com.google.common.util.concurrent.ListenableFuture;
 import com.google.common.util.concurrent.MoreExecutors;
@@ -49,6 +52,7 @@ public final class RefreshAnnotatedCallLogReceiver extends BroadcastReceiver {
 
   private final RefreshAnnotatedCallLogWorker refreshAnnotatedCallLogWorker;
   private final FutureTimer futureTimer;
+  private final LoggingBindings logger;
 
   @Nullable private Runnable refreshAnnotatedCallLogRunnable;
 
@@ -64,6 +68,7 @@ public final class RefreshAnnotatedCallLogReceiver extends BroadcastReceiver {
     refreshAnnotatedCallLogWorker =
         CallLogComponent.get(context).getRefreshAnnotatedCallLogWorker();
     futureTimer = MetricsComponent.get(context).futureTimer();
+    logger = Logger.get(context);
   }
 
   @Override
@@ -109,9 +114,24 @@ public final class RefreshAnnotatedCallLogReceiver extends BroadcastReceiver {
                   ? refreshAnnotatedCallLogWorker.refreshWithDirtyCheck()
                   : refreshAnnotatedCallLogWorker.refreshWithoutDirtyCheck();
           Futures.addCallback(
-              future, new DefaultFutureCallback<>(), MoreExecutors.directExecutor());
+              future,
+              new FutureCallback<RefreshResult>() {
+                @Override
+                public void onSuccess(RefreshResult refreshResult) {
+                  logger.logImpression(getImpressionType(checkDirty, refreshResult));
+                }
+
+                @Override
+                public void onFailure(Throwable throwable) {
+                  ThreadUtil.getUiThreadHandler()
+                      .post(
+                          () -> {
+                            throw new RuntimeException(throwable);
+                          });
+                }
+              },
+              MoreExecutors.directExecutor());
           futureTimer.applyTiming(future, new EventNameFromResultFunction(checkDirty));
-          // TODO(zachh): Should also log impression counts of RefreshResults.
         };
 
     ThreadUtil.getUiThreadHandler()
@@ -140,16 +160,36 @@ public final class RefreshAnnotatedCallLogReceiver extends BroadcastReceiver {
     public String apply(RefreshResult refreshResult) {
       switch (refreshResult) {
         case NOT_DIRTY:
-          return Metrics.REFRESH_NOT_DIRTY; // NOT_DIRTY implies forceRefresh is false
+          return Metrics.ANNOTATED_CALL_LOG_NOT_DIRTY; // NOT_DIRTY implies forceRefresh is false
         case REBUILT_BUT_NO_CHANGES_NEEDED:
           return checkDirty
-              ? Metrics.REFRESH_NO_CHANGES_NEEDED
-              : Metrics.FORCE_REFRESH_NO_CHANGES_NEEDED;
+              ? Metrics.ANNOTATED_LOG_NO_CHANGES_NEEDED
+              : Metrics.NEW_CALL_LOG_FORCE_REFRESH_NO_CHANGES_NEEDED;
         case REBUILT_AND_CHANGES_NEEDED:
-          return checkDirty ? Metrics.REFRESH_CHANGES_NEEDED : Metrics.FORCE_REFRESH_CHANGES_NEEDED;
+          return checkDirty
+              ? Metrics.ANNOTATED_CALL_LOG_CHANGES_NEEDED
+              : Metrics.ANNOTATED_CALL_LOG_FORCE_REFRESH_CHANGES_NEEDED;
         default:
           throw new IllegalStateException("Unsupported result: " + refreshResult);
       }
     }
   }
+
+  private static DialerImpression.Type getImpressionType(
+      boolean checkDirty, RefreshResult refreshResult) {
+    switch (refreshResult) {
+      case NOT_DIRTY:
+        return DialerImpression.Type.ANNOTATED_CALL_LOG_NOT_DIRTY;
+      case REBUILT_BUT_NO_CHANGES_NEEDED:
+        return checkDirty
+            ? DialerImpression.Type.ANNOTATED_CALL_LOG_NO_CHANGES_NEEDED
+            : DialerImpression.Type.ANNOTATED_CALL_LOG_FORCE_REFRESH_NO_CHANGES_NEEDED;
+      case REBUILT_AND_CHANGES_NEEDED:
+        return checkDirty
+            ? DialerImpression.Type.ANNOTATED_CALL_LOG_CHANGES_NEEDED
+            : DialerImpression.Type.ANNOTATED_CALL_LOG_FORCE_REFRESH_CHANGES_NEEDED;
+      default:
+        throw new IllegalStateException("Unsupported result: " + refreshResult);
+    }
+  }
 }
diff --git a/java/com/android/dialer/logging/dialer_impression.proto b/java/com/android/dialer/logging/dialer_impression.proto
index 1b0d62650..80249f464 100644
--- a/java/com/android/dialer/logging/dialer_impression.proto
+++ b/java/com/android/dialer/logging/dialer_impression.proto
@@ -12,7 +12,7 @@ message DialerImpression {
   // Event enums to be used for Impression Logging in Dialer.
   // It's perfectly acceptable for this enum to be large
   // Values should be from 1000 to 100000.
-  // Next Tag: 1341
+  // Next Tag: 1346
   enum Type {
     UNKNOWN_AOSP_EVENT_TYPE = 1000;
 
@@ -675,5 +675,16 @@ message DialerImpression {
     MAIN_CLICK_SEARCH_BAR_VOICE_BUTTON = 1339;
     // NUI FAB
     MAIN_CLICK_FAB_TO_OPEN_DIALPAD = 1340;
+
+    // The call log was not dirty.
+    ANNOTATED_CALL_LOG_NOT_DIRTY = 1341;
+    // The call log was dirty but no changes were needed.
+    ANNOTATED_CALL_LOG_NO_CHANGES_NEEDED = 1342;
+    // The call log was force refreshed but no changes were needed.
+    ANNOTATED_CALL_LOG_FORCE_REFRESH_NO_CHANGES_NEEDED = 1343;
+    // The call log was dirty and changes were needed.
+    ANNOTATED_CALL_LOG_CHANGES_NEEDED = 1344;
+    // The call log was force refreshed and changes were needed.
+    ANNOTATED_CALL_LOG_FORCE_REFRESH_CHANGES_NEEDED = 1345;
   }
 }
diff --git a/java/com/android/dialer/metrics/Metrics.java b/java/com/android/dialer/metrics/Metrics.java
index e9e22c353..55726586a 100644
--- a/java/com/android/dialer/metrics/Metrics.java
+++ b/java/com/android/dialer/metrics/Metrics.java
@@ -38,11 +38,12 @@ public interface Metrics {
 
   // Events related to refreshing the annotated call log.
   String NEW_CALL_LOG_COALESCE = "NewCallLog.Coalesce";
-  String REFRESH_NOT_DIRTY = "RefreshAnnotatedCallLogReceiver.NotDirty";
-  String REFRESH_CHANGES_NEEDED = "RefreshAnnotatedCallLogReceiver.ChangesNeeded";
-  String REFRESH_NO_CHANGES_NEEDED = "RefreshAnnotatedCallLogReceiver.NoChangesNeeded";
-  String FORCE_REFRESH_CHANGES_NEEDED = "RefreshAnnotatedCallLogReceiver.ForceRefreshChangesNeeded";
-  String FORCE_REFRESH_NO_CHANGES_NEEDED =
+  String ANNOTATED_CALL_LOG_NOT_DIRTY = "RefreshAnnotatedCallLogReceiver.NotDirty";
+  String ANNOTATED_CALL_LOG_CHANGES_NEEDED = "RefreshAnnotatedCallLogReceiver.ChangesNeeded";
+  String ANNOTATED_LOG_NO_CHANGES_NEEDED = "RefreshAnnotatedCallLogReceiver.NoChangesNeeded";
+  String ANNOTATED_CALL_LOG_FORCE_REFRESH_CHANGES_NEEDED =
+      "RefreshAnnotatedCallLogReceiver.ForceRefreshChangesNeeded";
+  String NEW_CALL_LOG_FORCE_REFRESH_NO_CHANGES_NEEDED =
       "RefreshAnnotatedCallLogReceiver.ForceRefreshNoChangesNeeded";
 
   String INITIAL_FILL_EVENT_NAME = "RefreshAnnotatedCallLog.Initial.Fill";