Bug 1308337 - Part 3: Handle sync restarts during telemetry collection r=nalexander
authorGrigory Kruglov <gkruglov@mozilla.com>
Tue, 30 May 2017 19:46:31 -0400
changeset 412579 2597d0701648726433d3c46a1236684fe220c706
parent 412578 846114208d7b988d2ce5864f59056cb84c0d5821
child 412580 99d1101ae492edd972dcad4a4831be71c780b3c0
push id1490
push usermtabara@mozilla.com
push dateMon, 31 Jul 2017 14:08:16 +0000
treeherdermozilla-release@70e32e6bf15e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersnalexander
bugs1308337
milestone55.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1308337 - Part 3: Handle sync restarts during telemetry collection r=nalexander The approach here is to simply mark current TelemetryCollector as having restarted. The downside of this approach is that two technically separate syncs are combined into one telemetry ping. However, the two syncs are logically connected to each other, and combining their telemetry will make it easier to figure out why a restart occurred, as well as what happened after the restart. MozReview-Commit-ID: AtJbge2ulMz
mobile/android/services/src/main/java/org/mozilla/gecko/sync/GlobalSession.java
mobile/android/services/src/main/java/org/mozilla/gecko/sync/telemetry/TelemetryCollector.java
mobile/android/services/src/main/java/org/mozilla/gecko/sync/telemetry/TelemetryContract.java
mobile/android/tests/background/junit4/src/org/mozilla/gecko/sync/telemetry/TelemetryCollectorTest.java
--- a/mobile/android/services/src/main/java/org/mozilla/gecko/sync/GlobalSession.java
+++ b/mobile/android/services/src/main/java/org/mozilla/gecko/sync/GlobalSession.java
@@ -340,16 +340,17 @@ public class GlobalSession implements Ht
     this.advance();
   }
 
   /**
    * Stop this sync and start again.
    * @throws AlreadySyncingException
    */
   protected void restart() throws AlreadySyncingException {
+    telemetryCollector.setRestarted();
     this.currentState = GlobalSyncStage.Stage.idle;
     if (callback.shouldBackOffStorage()) {
       this.callback.handleAborted(this, "Told to back off.");
       return;
     }
     // Restart with the same deadline as before.
     this.start(syncDeadline);
   }
--- a/mobile/android/services/src/main/java/org/mozilla/gecko/sync/telemetry/TelemetryCollector.java
+++ b/mobile/android/services/src/main/java/org/mozilla/gecko/sync/telemetry/TelemetryCollector.java
@@ -59,16 +59,20 @@ public class TelemetryCollector {
             return stageCollectors.get(stageName);
         }
 
         final TelemetryStageCollector collector = new TelemetryStageCollector(this);
         stageCollectors.put(stageName, collector);
         return collector;
     }
 
+    public void setRestarted() {
+        this.didRestart = true;
+    }
+
     public void setIDs(@NonNull String uid, @NonNull String deviceID) {
         // We use hashed_fxa_uid from the token server as our UID.
         this.hashedUID = uid;
         try {
             this.hashedDeviceID = Utils.byte2Hex(Utils.sha256(
                             deviceID.concat(uid).getBytes("UTF-8")
                     ));
         } catch (UnsupportedEncodingException | NoSuchAlgorithmException e) {
@@ -128,16 +132,19 @@ public class TelemetryCollector {
 
         final Bundle telemetry = new Bundle();
         telemetry.putString(TelemetryContract.KEY_LOCAL_UID, this.hashedUID);
         telemetry.putString(TelemetryContract.KEY_LOCAL_DEVICE_ID, this.hashedDeviceID);
         telemetry.putParcelableArrayList(TelemetryContract.KEY_DEVICES, this.devices);
         telemetry.putLong(TelemetryContract.KEY_TOOK, took);
         telemetry.putSerializable(TelemetryContract.KEY_ERROR, (Serializable) this.error);
         telemetry.putSerializable(TelemetryContract.KEY_STAGES, this.stageCollectors);
+        if (this.didRestart) {
+            telemetry.putBoolean(TelemetryContract.KEY_RESTARTED, true);
+        }
         return telemetry;
     }
 
     /**
      * Builder class which is responsible for mapping instances of exceptions thrown during sync
      * stages into a JSON structure that may be submitted as part of a sync ping.
      */
     public static class StageErrorBuilder {
--- a/mobile/android/services/src/main/java/org/mozilla/gecko/sync/telemetry/TelemetryContract.java
+++ b/mobile/android/services/src/main/java/org/mozilla/gecko/sync/telemetry/TelemetryContract.java
@@ -11,16 +11,17 @@ package org.mozilla.gecko.sync.telemetry
 public class TelemetryContract {
   public final static String KEY_TELEMETRY = "telemetry";
   public final static String KEY_STAGES = "stages";
   public final static String KEY_ERROR = "error";
   public final static String KEY_LOCAL_UID = "uid";
   public final static String KEY_LOCAL_DEVICE_ID = "deviceID";
   public final static String KEY_DEVICES = "devices";
   public final static String KEY_TOOK = "took";
+  public final static String KEY_RESTARTED = "restarted";
 
   public static final String KEY_TYPE = "type";
   public static final String KEY_TYPE_SYNC = "sync";
   public static final String KEY_TYPE_EVENT = "event";
 
   public static final String KEY_DEVICE_OS = "os";
   public static final String KEY_DEVICE_VERSION = "version";
   public static final String KEY_DEVICE_ID = "id";
--- a/mobile/android/tests/background/junit4/src/org/mozilla/gecko/sync/telemetry/TelemetryCollectorTest.java
+++ b/mobile/android/tests/background/junit4/src/org/mozilla/gecko/sync/telemetry/TelemetryCollectorTest.java
@@ -132,16 +132,30 @@ public class TelemetryCollectorTest {
     @Test
     public void testError() throws Exception {
         collector.setError("testError", "unexpectedStuff");
         assertEquals("testError", collector.error.getString("name"));
         assertEquals("unexpectedStuff", collector.error.getString("error"));
     }
 
     @Test
+    public void testRestarted() throws Exception {
+        collector.setStarted(5L);
+        collector.setFinished(10L);
+
+        Bundle data = collector.build();
+        assertFalse(data.getBoolean("restarted"));
+        assertFalse(data.containsKey("restarted"));
+
+        collector.setRestarted();
+
+        assertTrue(collector.build().getBoolean("restarted"));
+    }
+
+    @Test
     public void testCollectorFor() throws Exception {
         // Test that we'll get the same stage collector for the same stage name
         TelemetryStageCollector stageCollector = collector.collectorFor("test");
         TelemetryStageCollector stageCollector2 = collector.collectorFor("test");
         assertEquals(stageCollector, stageCollector2);
 
         // Test that we won't just keep getting the same stage collector for different stages
         TelemetryStageCollector stageCollector3 = collector.collectorFor("another");