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
--- 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");