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.
This commit is contained in:
Michael Comella 2016-02-08 17:17:14 -08:00
parent 59539bf626
commit 09035ff65f
3 changed files with 32 additions and 6 deletions

View File

@ -38,6 +38,7 @@ public class TelemetryConstants {
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";
}

View File

@ -58,17 +58,18 @@ public class TelemetryPingGenerator {
* @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);
@ -88,6 +89,12 @@ public class TelemetryPingGenerator {
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;
}

View File

@ -8,6 +8,7 @@ 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;
@ -34,6 +35,8 @@ 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);
@ -160,10 +163,11 @@ public class TelemetryUploadService extends BackgroundService {
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();
@ -179,8 +183,8 @@ public class TelemetryUploadService extends BackgroundService {
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);
}
@ -202,6 +206,20 @@ public class TelemetryUploadService extends BackgroundService {
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 (long) Math.floor((double) profileMillis / MILLIS_IN_DAY);
}
private static class CorePingResultDelegate extends ResultDelegate {
public CorePingResultDelegate() {
super();