Bug 1246209 - Add profile creation date to core ping. r=mfinkle draft
authorMichael Comella <michael.l.comella@gmail.com>
Mon, 08 Feb 2016 17:17:14 -0800
changeset 329740 3eb383b940891a868b2cfe32837411ab965fcc90
parent 329739 4480a9c3e27f1973b2f8ae8fbc2edff642563c3d
child 514016 5de838b8472552297fbc3cb5e419cd9406d85bc7
push id10587
push usermichael.l.comella@gmail.com
push dateTue, 09 Feb 2016 01:21:17 +0000
reviewersmfinkle
bugs1246209, 1246816
milestone47.0a1
Bug 1246209 - Add profile creation date to core ping. r=mfinkle This patch adds 2 workarounds for the fact that getProfileCreationDate returns -1 when it can't find a creation date. Returning -1 turned out to be not particularly robust but I did it this way to avoid adding too many additional versions of methods in order to have optional parameters such as profileCreationDate. The workarounds are added as TODOs w/ bug #'s in the code and mentioned in the comments of bug 1246816 itself. A future implementation should probably add a Builder to pass a single Object as the argument to TelemetryPingGenerator.createCorePing to prevent the argument list from growing unreasonably large and to properly operate on optional parameters. I didn't do this in this patch in order to simplify the uplifted code.
mobile/android/base/java/org/mozilla/gecko/telemetry/TelemetryConstants.java
mobile/android/base/java/org/mozilla/gecko/telemetry/TelemetryPingGenerator.java
mobile/android/base/java/org/mozilla/gecko/telemetry/TelemetryUploadService.java
--- a/mobile/android/base/java/org/mozilla/gecko/telemetry/TelemetryConstants.java
+++ b/mobile/android/base/java/org/mozilla/gecko/telemetry/TelemetryConstants.java
@@ -33,12 +33,13 @@ public class TelemetryConstants {
 
         public static final String ARCHITECTURE = "arch";
         public static final String CLIENT_ID = "clientId";
         public static final String DEVICE = "device";
         public static final String EXPERIMENTS = "experiments";
         public static final String LOCALE = "locale";
         public static final String OS_ATTR = "os";
         public static final String OS_VERSION = "osversion";
+        public static final String PROFILE_CREATION_DATE = "profileDate";
         public static final String SEQ = "seq";
         public static final String VERSION_ATTR = "v";
     }
 }
--- a/mobile/android/base/java/org/mozilla/gecko/telemetry/TelemetryPingGenerator.java
+++ b/mobile/android/base/java/org/mozilla/gecko/telemetry/TelemetryPingGenerator.java
@@ -53,27 +53,28 @@ public class TelemetryPingGenerator {
                 appUpdateChannel + '/' +
                 appBuildId;
     }
 
     /**
      * @param docId A unique document ID for the ping associated with the upload to this server
      * @param clientId The client ID of this profile (from Gecko)
      * @param serverURLSchemeHostPort The server url with the scheme, host, and port (e.g. "http://mozilla.org:80")
+     * @param profileCreationDateDays The profile creation date in days to the UNIX epoch, NOT MILLIS.
      * @throws IOException when client ID could not be created
      */
     public static TelemetryPing createCorePing(final Context context, final String docId, final String clientId,
-            final String serverURLSchemeHostPort, final int seq) {
+            final String serverURLSchemeHostPort, final int seq, final long profileCreationDateDays) {
         final String serverURL = getTelemetryServerURL(docId, serverURLSchemeHostPort, CorePing.NAME);
-        final ExtendedJSONObject payload = createCorePingPayload(context, clientId, seq);
+        final ExtendedJSONObject payload = createCorePingPayload(context, clientId, seq, profileCreationDateDays);
         return new TelemetryPing(serverURL, payload);
     }
 
     private static ExtendedJSONObject createCorePingPayload(final Context context, final String clientId,
-            final int seq) {
+            final int seq, final long profileCreationDate) {
         final ExtendedJSONObject ping = new ExtendedJSONObject();
         ping.put(CorePing.VERSION_ATTR, CorePing.VERSION_VALUE);
         ping.put(CorePing.OS_ATTR, CorePing.OS_VALUE);
 
         // We limit the device descriptor to 32 characters because it can get long. We give fewer characters to the
         // manufacturer because we're less likely to have manufacturers with similar names than we are for a
         // manufacturer to have two devices with the similar names (e.g. Galaxy S6 vs. Galaxy Note 6).
         final String deviceDescriptor =
@@ -83,16 +84,22 @@ public class TelemetryPingGenerator {
         ping.put(CorePing.CLIENT_ID, clientId);
         ping.put(CorePing.DEVICE, deviceDescriptor);
         ping.put(CorePing.LOCALE, Locales.getLanguageTag(Locale.getDefault()));
         ping.put(CorePing.OS_VERSION, Integer.toString(Build.VERSION.SDK_INT)); // A String for cross-platform reasons.
         ping.put(CorePing.SEQ, seq);
         if (AppConstants.MOZ_SWITCHBOARD) {
             ping.put(CorePing.EXPERIMENTS, getActiveExperiments(context));
         }
+        // TODO (bug 1246816): Remove this "optional" parameter work-around when
+        // GeckoProfile.getAndPersistProfileCreationDateFromFilesystem is implemented. That method returns -1
+        // while it's not implemented so we don't include the parameter in the ping if that's the case.
+        if (profileCreationDate >= 0) {
+            ping.put(CorePing.PROFILE_CREATION_DATE, profileCreationDate);
+        }
         return ping;
     }
 
     private static JSONArray getActiveExperiments(final Context context) {
         if (!AppConstants.MOZ_SWITCHBOARD) {
             throw new IllegalStateException("This method should not be called with switchboard disabled");
         }
         return new JSONArray(SwitchBoard.getActiveExperiments(context));
--- a/mobile/android/base/java/org/mozilla/gecko/telemetry/TelemetryUploadService.java
+++ b/mobile/android/base/java/org/mozilla/gecko/telemetry/TelemetryUploadService.java
@@ -3,16 +3,17 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 package org.mozilla.gecko.telemetry;
 
 import android.content.Context;
 import android.content.Intent;
 import android.content.SharedPreferences;
 import android.support.annotation.NonNull;
+import android.support.annotation.WorkerThread;
 import android.util.Log;
 import ch.boye.httpclientandroidlib.HttpResponse;
 import ch.boye.httpclientandroidlib.client.ClientProtocolException;
 import org.mozilla.gecko.GeckoProfile;
 import org.mozilla.gecko.GeckoSharedPrefs;
 import org.mozilla.gecko.background.BackgroundService;
 import org.mozilla.gecko.preferences.GeckoPreferences;
 import org.mozilla.gecko.sync.net.BaseResource;
@@ -29,16 +30,18 @@ import java.security.GeneralSecurityExce
  *
  * Note that we'll fail to upload if the network is off or background uploads are disabled but the caller is still
  * expected to increment the sequence number.
  */
 public class TelemetryUploadService extends BackgroundService {
     private static final String LOGTAG = StringUtils.safeSubstring("Gecko" + TelemetryUploadService.class.getSimpleName(), 0, 23);
     private static final String WORKER_THREAD_NAME = LOGTAG + "Worker";
 
+    private static final int MILLIS_IN_DAY = 1000 * 60 * 60 * 24;
+
     public TelemetryUploadService() {
         super(WORKER_THREAD_NAME);
 
         // Intent redelivery can fail hard (e.g. we OOM as we try to upload, the Intent gets redelivered, repeat) so for
         // simplicity, we avoid it for now. In the unlikely event that Android kills our upload service, we'll thus fail
         // to upload the document with a specific sequence number. Furthermore, we never attempt to re-upload it.
         //
         // We'll fix this issue in bug 1243585.
@@ -155,37 +158,38 @@ public class TelemetryUploadService exte
         if (intent.getStringExtra(TelemetryConstants.EXTRA_PROFILE_PATH) == null) {
             Log.d(LOGTAG, "Received invalid profile path in Intent");
             return false;
         }
 
         return true;
     }
 
+    @WorkerThread
     private void uploadCorePing(@NonNull final String docId, final int seq, @NonNull final String profileName,
                 @NonNull final String profilePath) {
         final GeckoProfile profile = GeckoProfile.get(this, profileName, profilePath);
-
+        final long profileCreationDate = getProfileCreationDate(profile);
         final String clientId;
         try {
             clientId = profile.getClientId();
         } catch (final IOException e) {
             // Don't log the exception to avoid leaking the profile path.
             Log.w(LOGTAG, "Unable to get client ID to generate core ping: returning.");
             return;
         }
 
         // Each profile can have different telemetry data so we intentionally grab the shared prefs for the profile.
         final SharedPreferences sharedPrefs = GeckoSharedPrefs.forProfileName(this, profileName);
         // TODO (bug 1241685): Sync this preference with the gecko preference.
         final String serverURLSchemeHostPort =
                 sharedPrefs.getString(TelemetryConstants.PREF_SERVER_URL, TelemetryConstants.DEFAULT_SERVER_URL);
 
-        final TelemetryPing corePing =
-                TelemetryPingGenerator.createCorePing(this, docId, clientId, serverURLSchemeHostPort, seq);
+        final TelemetryPing corePing = TelemetryPingGenerator.createCorePing(this, docId, clientId,
+                serverURLSchemeHostPort, seq, profileCreationDate);
         final CorePingResultDelegate resultDelegate = new CorePingResultDelegate();
         uploadPing(corePing, resultDelegate);
     }
 
     private void uploadPing(final TelemetryPing ping, final ResultDelegate delegate) {
         final BaseResource resource;
         try {
             resource = new BaseResource(ping.getURL());
@@ -197,16 +201,30 @@ public class TelemetryUploadService exte
         delegate.setResource(resource);
         resource.delegate = delegate;
 
         // We're in a background thread so we don't have any reason to do this asynchronously.
         // If we tried, onStartCommand would return and IntentService might stop itself before we finish.
         resource.postBlocking(ping.getPayload());
     }
 
+    /**
+     * @return the profile creation date in the format expected by TelemetryPingGenerator.
+     */
+    @WorkerThread
+    private long getProfileCreationDate(final GeckoProfile profile) {
+        final long profileMillis = profile.getProfileCreationDate();
+        // TODO (bug 1246816): Remove this work-around when finishing bug. getProfileCreationDate can return -1,
+        // and we don't want to truncate (-1 / MILLIS) to 0.
+        if (profileMillis < 0) {
+            return profileMillis;
+        }
+        return profileMillis / MILLIS_IN_DAY; // Intentional integer division to truncate value.
+    }
+
     private static class CorePingResultDelegate extends ResultDelegate {
         public CorePingResultDelegate() {
             super();
         }
 
         @Override
         public String getUserAgent() {
             return TelemetryConstants.USER_AGENT;