Bug 1370656 - Don't re-use the same instance of TelemetryCollector between syncs r=nalexander
authorGrigory Kruglov <gkruglov@mozilla.com>
Tue, 06 Jun 2017 17:42:28 -0400
changeset 410887 09c76ee950d007a1f6eb7ac875d7f506edd65ae7
parent 410886 b92c5e65face5f8f9e687c388e246c2969b6483e
child 410888 215afe84f16e8a1d191a85ce3d9e5ceea5038bbe
push id7391
push usermtabara@mozilla.com
push dateMon, 12 Jun 2017 13:08:53 +0000
treeherdermozilla-beta@2191d7f87e2e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersnalexander
bugs1370656
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 1370656 - Don't re-use the same instance of TelemetryCollector between syncs r=nalexander Parent class (FxAccountSyncAdapter) is essentially a singleton, and so we'd end up re-using class fields between syncs, among them the collected telemetry data. It's cleaner and safer to move ownership of TelemetryCollector into IntrumentedSessionCallback. With this change, telemetry data is contained within and eventually emitted from a single owner object. MozReview-Commit-ID: Abx13VmILcE
mobile/android/services/src/main/java/org/mozilla/gecko/fxa/sync/FxAccountSyncAdapter.java
--- a/mobile/android/services/src/main/java/org/mozilla/gecko/fxa/sync/FxAccountSyncAdapter.java
+++ b/mobile/android/services/src/main/java/org/mozilla/gecko/fxa/sync/FxAccountSyncAdapter.java
@@ -81,18 +81,16 @@ public class FxAccountSyncAdapter extend
   // Non-user initiated sync can't take longer than 30 minutes.
   // To ensure we're not churning through device's battery/resources, we limit sync to 10 minutes,
   // and request a re-sync if we hit that deadline.
   private static final long SYNC_DEADLINE_DELTA_MILLIS = TimeUnit.MINUTES.toMillis(10);
 
   protected final ExecutorService executor;
   protected final FxAccountNotificationManager notificationManager;
 
-  private final TelemetryCollector telemetryCollector = new TelemetryCollector();
-
   public FxAccountSyncAdapter(Context context, boolean autoInitialize) {
     super(context, autoInitialize);
     this.executor = Executors.newSingleThreadExecutor();
     this.notificationManager = new FxAccountNotificationManager(NOTIFICATION_ID);
   }
 
   protected static class SyncDelegate extends FxAccountSyncDelegate {
     @Override
@@ -152,20 +150,24 @@ public class FxAccountSyncAdapter extend
    * Due to how packages are built, we can not call into it directly from *.sync.
    */
   private static class InstrumentedSessionCallback extends SessionCallback {
     private static final String ACTION_BACKGROUND_TELEMETRY = "org.mozilla.gecko.telemetry.BACKGROUND";
 
     private final LocalBroadcastManager localBroadcastManager;
     private final TelemetryCollector telemetryCollector;
 
-    InstrumentedSessionCallback(TelemetryCollector telemetryCollector, LocalBroadcastManager localBroadcastManager, SyncDelegate syncDelegate, SchedulePolicy schedulePolicy) {
+    InstrumentedSessionCallback(LocalBroadcastManager localBroadcastManager, SyncDelegate syncDelegate, SchedulePolicy schedulePolicy) {
       super(syncDelegate, schedulePolicy);
-      this.telemetryCollector = telemetryCollector;
       this.localBroadcastManager = localBroadcastManager;
+      this.telemetryCollector = new TelemetryCollector();
+    }
+
+    TelemetryCollector getCollector() {
+      return telemetryCollector;
     }
 
     @Override
     public void handleSuccess(GlobalSession globalSession) {
       super.handleSuccess(globalSession);
       recordTelemetry();
     }
 
@@ -334,17 +336,17 @@ public class FxAccountSyncAdapter extend
   }
 
   protected void syncWithAssertion(final String assertion,
                                    final URI tokenServerEndpointURI,
                                    final BackoffHandler tokenBackoffHandler,
                                    final SharedPreferences sharedPrefs,
                                    final KeyBundle syncKeyBundle,
                                    final String clientState,
-                                   final SessionCallback callback,
+                                   final InstrumentedSessionCallback callback,
                                    final Bundle extras,
                                    final AndroidFxAccount fxAccount,
                                    final long syncDeadline) {
     final TokenServerClientDelegate delegate = new TokenServerClientDelegate() {
       private boolean didReceiveBackoff = false;
 
       @Override
       public String getUserAgent() {
@@ -417,18 +419,18 @@ public class FxAccountSyncAdapter extend
 
           final Context context = getContext();
           final SyncConfiguration syncConfig = new SyncConfiguration(token.uid, authHeaderProvider, sharedPrefs, syncKeyBundle);
 
           Collection<String> knownStageNames = SyncConfiguration.validEngineNames();
           syncConfig.stagesToSync = Utils.getStagesToSyncFromBundle(knownStageNames, extras);
           syncConfig.setClusterURL(storageServerURI);
 
-          globalSession = new GlobalSession(syncConfig, callback, context, clientsDataDelegate, telemetryCollector);
-          telemetryCollector.setIDs(token.hashedFxaUid, clientsDataDelegate.getAccountGUID());
+          globalSession = new GlobalSession(syncConfig, callback, context, clientsDataDelegate, callback.getCollector());
+          callback.getCollector().setIDs(token.hashedFxaUid, clientsDataDelegate.getAccountGUID());
           globalSession.start(syncDeadline);
         } catch (Exception e) {
           callback.handleError(globalSession, e);
           return;
         }
       }
 
       @Override
@@ -439,28 +441,28 @@ public class FxAccountSyncAdapter extend
           State state = fxAccount.getState();
           if (state.getStateLabel() == StateLabel.Married) {
             Married married = (Married) state;
             fxAccount.setState(married.makeCohabitingState());
           }
         } finally {
           fxAccount.releaseSharedAccountStateLock();
         }
-        telemetryCollector.setError(
+        callback.getCollector().setError(
                 TelemetryCollector.KEY_ERROR_TOKEN,
                 e.getClass().getSimpleName()
         );
         callback.handleError(null, e);
       }
 
       @Override
       public void handleError(Exception e) {
         Logger.error(LOG_TAG, "Failed to get token.", e);
         fxAccount.releaseSharedAccountStateLock();
-        telemetryCollector.setError(
+        callback.getCollector().setError(
                 TelemetryCollector.KEY_ERROR_TOKEN,
                 e.getClass().getSimpleName()
         );
         callback.handleError(null, e);
       }
 
       @Override
       public void handleBackoff(int backoffSeconds) {
@@ -472,17 +474,17 @@ public class FxAccountSyncAdapter extend
         // value for token server scheduling.
         tokenBackoffHandler.setEarliestNextRequest(delay(backoffSeconds * 1000));
       }
 
       private long delay(long delay) {
         return System.currentTimeMillis() + delay;
       }
     };
-    telemetryCollector.setStarted(SystemClock.elapsedRealtime());
+    callback.getCollector().setStarted(SystemClock.elapsedRealtime());
     TokenServerClient tokenServerclient = new TokenServerClient(tokenServerEndpointURI, executor);
     tokenServerclient.getTokenFromBrowserIDAssertion(assertion, true, clientState, delegate);
   }
 
   public void maybeRegisterDevice(Context context, AndroidFxAccount fxAccount) {
     // Register the device if necessary (asynchronous, in another thread).
     // As part of device registration, we obtain a PushSubscription, register our push endpoint
     // with FxA, and update account data with fxaDeviceId, which is part of our synced
@@ -670,18 +672,17 @@ public class FxAccountSyncAdapter extend
             if (!shouldRequestToken(tokenBackoffHandler, extras)) {
               Logger.info(LOG_TAG, "Not syncing (token server).");
               syncDelegate.postponeSync(tokenBackoffHandler.delayMilliseconds());
               return;
             }
 
             onSessionTokenStateReached(context, fxAccount);
 
-            final SessionCallback sessionCallback = new InstrumentedSessionCallback(
-                    telemetryCollector,
+            final InstrumentedSessionCallback sessionCallback = new InstrumentedSessionCallback(
                     LocalBroadcastManager.getInstance(context),
                     syncDelegate,
                     schedulePolicy
             );
 
             final KeyBundle syncKeyBundle = married.getSyncKeyBundle();
             final String clientState = married.getClientState();
             syncWithAssertion(