Bug 1245544 - Telemetry experiments aren't expiring historical data properly. r=gfritzsche
authorAlessio Placitelli <alessio.placitelli@gmail.com>
Fri, 04 Mar 2016 09:55:00 +0100
changeset 286987 eb559f626161d47fb4c27cdbcddb212698b52d53
parent 286986 5ee60bdfe6b8ec27522ee6dfaf43dc6d6ba99a1d
child 286988 fe1b8cb74332185af638d0cfe0565726042e66ea
push id18031
push useralessio.placitelli@gmail.com
push dateMon, 07 Mar 2016 08:45:07 +0000
treeherderfx-team@fe1b8cb74332 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersgfritzsche
bugs1245544
milestone47.0a1
Bug 1245544 - Telemetry experiments aren't expiring historical data properly. r=gfritzsche
browser/experiments/Experiments.jsm
browser/experiments/test/xpcshell/test_cache.js
--- a/browser/experiments/Experiments.jsm
+++ b/browser/experiments/Experiments.jsm
@@ -1038,16 +1038,24 @@ Experiments.Experiments.prototype = {
     }
 
     let experiments = new Map();
     for (let item of data.data) {
       let entry = new Experiments.ExperimentEntry(this._policy);
       if (!entry.initFromCacheData(item)) {
         continue;
       }
+
+      // Discard old experiments if they ended more than 180 days ago.
+      if (entry.shouldDiscard()) {
+        // We discarded an experiment, the cache needs to be updated.
+        this._dirty = true;
+        continue;
+      }
+
       experiments.set(entry.id, entry);
     }
 
     this._experiments = experiments;
   },
 
   /*
    * Update the experiment entries from the experiments
@@ -1538,17 +1546,19 @@ Experiments.ExperimentEntry.prototype = 
     this.DATE_KEYS.forEach(key => {
       if (key in data) {
         let date = new Date();
         date.setTime(data[key]);
         this[key] = date;
       }
     });
 
-    this._lastChangedDate = this._policy.now();
+    // In order for the experiment's data expiration mechanism to work, use the experiment's
+    // |_endData| as the |_lastChangedDate| (if available).
+    this._lastChangedDate = !!this._endDate ? this._endDate : this._policy.now();
 
     return true;
   },
 
   /*
    * Returns a JSON representation of this object.
    */
   toJSON: function () {
--- a/browser/experiments/test/xpcshell/test_cache.js
+++ b/browser/experiments/test/xpcshell/test_cache.js
@@ -75,16 +75,25 @@ function checkExperimentSerializations(e
     let jsonStr = JSON.stringify(experiment.toJSON());
     Assert.ok(experiment2.initFromCacheData(JSON.parse(jsonStr)),
               "Should have initialized successfully from JSON serialization.");
     Assert.equal(JSON.stringify(experiment), JSON.stringify(experiment2),
                  "Object stringifications should match.");
   }
 }
 
+function validateCache(cachedExperiments, experimentIds) {
+  let cachedExperimentIds = new Set(cachedExperiments);
+  Assert.equal(cachedExperimentIds.size, experimentIds.length,
+               "The number of cached experiments does not match with the provided list");
+  for (let id of experimentIds) {
+    Assert.ok(cachedExperimentIds.has(id), "The cache must contain the experiment with id " + id);
+  }
+}
+
 // Set up an experiments instance and check if it is properly restored from cache.
 
 add_task(function* test_cache() {
   // The manifest data we test with.
 
   gManifestObject = {
     "version": 1,
     experiments: [
@@ -242,8 +251,152 @@ add_task(function* test_cache() {
   checkExperimentSerializations(experiments._experiments.values());
 
   // Cleanup.
 
   yield experiments._toggleExperimentsEnabled(false);
   yield promiseRestartManager();
   yield removeCacheFile();
 });
+
+add_task(function* test_expiration() {
+  // The manifest data we test with.
+  gManifestObject = {
+    "version": 1,
+    experiments: [
+      {
+        id:               EXPERIMENT1_ID,
+        xpiURL:           gDataRoot + EXPERIMENT1_XPI_NAME,
+        xpiHash:          EXPERIMENT1_XPI_SHA1,
+        maxActiveSeconds: 10 * SEC_IN_ONE_DAY,
+        appName:          ["XPCShell"],
+        channel:          ["nightly"],
+      },
+      {
+        id:               EXPERIMENT2_ID,
+        xpiURL:           gDataRoot + EXPERIMENT2_XPI_NAME,
+        xpiHash:          EXPERIMENT2_XPI_SHA1,
+        maxActiveSeconds: 50 * SEC_IN_ONE_DAY,
+        appName:          ["XPCShell"],
+        channel:          ["nightly"],
+      },
+      // The 3rd experiment will never run, so it's ok to use experiment's 2 data.
+      {
+        id:               EXPERIMENT3_ID,
+        xpiURL:           gDataRoot + EXPERIMENT2_XPI_NAME,
+        xpiHash:          EXPERIMENT2_XPI_SHA1,
+        maxActiveSeconds: 10 * SEC_IN_ONE_DAY,
+        appName:          ["XPCShell"],
+        channel:          ["nightly"],
+      }
+    ],
+  };
+
+  // Data to compare the result of Experiments.getExperiments() against.
+  let experimentListData = [
+    {
+      id: EXPERIMENT2_ID,
+      name: "Test experiment 2",
+      description: "And yet another experiment that experiments experimentally.",
+    },
+    {
+      id: EXPERIMENT1_ID,
+      name: EXPERIMENT1_NAME,
+      description: "Yet another experiment that experiments experimentally.",
+    },
+  ];
+
+  // Setup dates for the experiments.
+  let baseDate   = new Date(2014, 5, 1, 12);
+  let startDates = [];
+  let endDates   = [];
+
+  for (let i=0; i<gManifestObject.experiments.length; ++i) {
+    let experiment = gManifestObject.experiments[i];
+    // Spread out experiments in time so that one experiment can end and expire while
+    // the next is still running.
+    startDates.push(futureDate(baseDate, (50 + (200 * i)) * MS_IN_ONE_DAY));
+    endDates  .push(futureDate(startDates[i], 50 * MS_IN_ONE_DAY));
+    experiment.startTime = dateToSeconds(startDates[i]);
+    experiment.endTime   = dateToSeconds(endDates[i]);
+  }
+
+  let now = null;
+  let experiments = null;
+
+  let setDateAndRestartExperiments = new Task.async(function* (newDate) {
+    now = newDate;
+    defineNow(gPolicy, now);
+
+    yield promiseRestartManager();
+    experiments = new Experiments.Experiments(gPolicy);
+    yield experiments._run();
+  });
+
+  // Trigger update & re-init, clock set to before any activation.
+  now = baseDate;
+  defineNow(gPolicy, now);
+
+  experiments = new Experiments.Experiments(gPolicy);
+  yield experiments.updateManifest();
+  let list = yield experiments.getExperiments();
+  Assert.equal(list.length, 0, "Experiment list should be empty.");
+
+  // Re-init, clock set for experiment 1 to start...
+  yield setDateAndRestartExperiments(startDates[0]);
+  list = yield experiments.getExperiments();
+  Assert.equal(list.length, 1, "The first experiment should have started.");
+
+  // ... init again, and set the clock so that the first experiment ends.
+  yield setDateAndRestartExperiments(endDates[0]);
+
+  // The experiment just ended, it should still be in the cache, but marked
+  // as finished.
+  list = yield experiments.getExperiments();
+  Assert.equal(list.length, 1, "Experiment list should have 1 entry.");
+
+  experimentListData[1].active = false;
+  experimentListData[1].endDate = now.getTime();
+  checkExperimentListsEqual(experimentListData.slice(1), list);
+  validateCache([...experiments._experiments.keys()], [EXPERIMENT1_ID, EXPERIMENT2_ID, EXPERIMENT3_ID]);
+
+  // Start the second experiment.
+  yield setDateAndRestartExperiments(startDates[1]);
+
+  // The experiments cache should contain the finished experiment and the
+  // one that's still running.
+  list = yield experiments.getExperiments();
+  Assert.equal(list.length, 2, "Experiment list should have 2 entries.");
+
+  experimentListData[0].active = true;
+  experimentListData[0].endDate = now.getTime() + 50 * MS_IN_ONE_DAY;
+  checkExperimentListsEqual(experimentListData, list);
+
+  // Move the clock in the future, just 31 days after the start date of the second experiment,
+  // so that the cache for the first experiment expires and the second experiment is still running.
+  yield setDateAndRestartExperiments(futureDate(startDates[1], 31 * MS_IN_ONE_DAY));
+  validateCache([...experiments._experiments.keys()], [EXPERIMENT2_ID, EXPERIMENT3_ID]);
+
+  // Make sure that the expired experiment is not reported anymore.
+  let history = yield experiments.getExperiments();
+  Assert.equal(history.length, 1, "Experiments older than 180 days must be removed from the cache.");
+
+  // Test that we don't write expired experiments in the cache.
+  yield setDateAndRestartExperiments(now);
+  validateCache([...experiments._experiments.keys()], [EXPERIMENT2_ID, EXPERIMENT3_ID]);
+
+  // The first experiment should be expired and not in the cache, it ended more than
+  // 180 days ago. We should see the one still running in the cache.
+  history = yield experiments.getExperiments();
+  Assert.equal(history.length, 1, "Expired experiments must not be saved to cache.");
+  checkExperimentListsEqual(experimentListData.slice(0, 1), history);
+
+  // Test that experiments that are cached locally but never ran are removed from cache
+  // when they are removed from the manifest (this is cached data, not really history).
+  gManifestObject["experiments"] = gManifestObject["experiments"].slice(1, 1);
+  yield experiments.updateManifest();
+  validateCache([...experiments._experiments.keys()], [EXPERIMENT2_ID]);
+
+  // Cleanup.
+  yield experiments._toggleExperimentsEnabled(false);
+  yield promiseRestartManager();
+  yield removeCacheFile();
+});