Bug 1329139 - Default event recording to disabled. r=dexter
authorGeorg Fritzsche <georg.fritzsche@googlemail.com>
Fri, 13 Jan 2017 14:21:56 +0700
changeset 374300 78d54111d9be3065681e0b21395a12e3c4485401
parent 374299 bf6a6ad3296eb520a318fec018ad3e11f861e189
child 374301 b28ef89ebda20992f764456aaf6eebd5421eb56f
push id6996
push userjlorenzo@mozilla.com
push dateMon, 06 Mar 2017 20:48:21 +0000
treeherdermozilla-beta@d89512dab048 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdexter
bugs1329139
milestone53.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 1329139 - Default event recording to disabled. r=dexter
browser/modules/test/browser_UsageTelemetry_content.js
browser/modules/test/browser_UsageTelemetry_content_aboutHome.js
browser/modules/test/browser_UsageTelemetry_searchbar.js
browser/modules/test/browser_UsageTelemetry_urlbar.js
toolkit/components/telemetry/Telemetry.cpp
toolkit/components/telemetry/TelemetryEvent.cpp
toolkit/components/telemetry/TelemetryEvent.h
toolkit/components/telemetry/nsITelemetry.idl
toolkit/components/telemetry/tests/unit/test_TelemetryEvents.js
toolkit/components/telemetry/tests/unit/test_TelemetrySession.js
--- a/browser/modules/test/browser_UsageTelemetry_content.js
+++ b/browser/modules/test/browser_UsageTelemetry_content.js
@@ -23,22 +23,26 @@ add_task(function* setup() {
   let engineOneOff = Services.search.getEngineByName("MozSearch2");
   Services.search.moveEngine(engineOneOff, 0);
 
   yield SpecialPowers.pushPrefEnv({"set": [
     ["dom.select_events.enabled", true], // We want select events to be fired.
     ["toolkit.telemetry.enabled", true]  // And Extended Telemetry to be enabled.
   ]});
 
+  // Enable event recording for the events tested here.
+  Services.telemetry.setEventRecordingEnabled("navigation", true);
+
   // Make sure to restore the engine once we're done.
   registerCleanupFunction(function* () {
     Services.search.currentEngine = originalEngine;
     Services.search.removeEngine(engineDefault);
     Services.search.removeEngine(engineOneOff);
     yield PlacesTestUtils.clearHistory();
+    Services.telemetry.setEventRecordingEnabled("navigation", false);
   });
 });
 
 add_task(function* test_context_menu() {
   // Let's reset the Telemetry data.
   Services.telemetry.clearScalars();
   Services.telemetry.clearEvents();
   let search_hist = getSearchCountsHistogram();
--- a/browser/modules/test/browser_UsageTelemetry_content_aboutHome.js
+++ b/browser/modules/test/browser_UsageTelemetry_content_aboutHome.js
@@ -25,22 +25,26 @@ add_task(function* setup() {
 
   // Move the second engine at the beginning of the one-off list.
   let engineOneOff = Services.search.getEngineByName("MozSearch2");
   Services.search.moveEngine(engineOneOff, 0);
 
   // Enable Extended Telemetry.
   yield SpecialPowers.pushPrefEnv({"set": [["toolkit.telemetry.enabled", true]]});
 
+  // Enable event recording for the events tested here.
+  Services.telemetry.setEventRecordingEnabled("navigation", true);
+
   // Make sure to restore the engine once we're done.
   registerCleanupFunction(function* () {
     Services.search.currentEngine = originalEngine;
     Services.search.removeEngine(engineDefault);
     Services.search.removeEngine(engineOneOff);
     yield PlacesTestUtils.clearHistory();
+    Services.telemetry.setEventRecordingEnabled("navigation", false);
   });
 });
 
 add_task(function* test_abouthome_simpleQuery() {
   // Let's reset the counts.
   Services.telemetry.clearScalars();
   Services.telemetry.clearEvents();
   let search_hist = getSearchCountsHistogram();
--- a/browser/modules/test/browser_UsageTelemetry_searchbar.js
+++ b/browser/modules/test/browser_UsageTelemetry_searchbar.js
@@ -65,21 +65,25 @@ add_task(function* setup() {
 
   // Move the second engine at the beginning of the one-off list.
   let engineOneOff = Services.search.getEngineByName("MozSearch2");
   Services.search.moveEngine(engineOneOff, 0);
 
   // Enable Extended Telemetry.
   yield SpecialPowers.pushPrefEnv({"set": [["toolkit.telemetry.enabled", true]]});
 
+  // Enable event recording for the events tested here.
+  Services.telemetry.setEventRecordingEnabled("navigation", true);
+
   // Make sure to restore the engine once we're done.
   registerCleanupFunction(function* () {
     Services.search.currentEngine = originalEngine;
     Services.search.removeEngine(engineDefault);
     Services.search.removeEngine(engineOneOff);
+    Services.telemetry.setEventRecordingEnabled("navigation", false);
   });
 });
 
 add_task(function* test_plainQuery() {
   // Let's reset the counts.
   Services.telemetry.clearScalars();
   Services.telemetry.clearEvents();
   let search_hist = getSearchCountsHistogram();
--- a/browser/modules/test/browser_UsageTelemetry_urlbar.js
+++ b/browser/modules/test/browser_UsageTelemetry_urlbar.js
@@ -76,24 +76,28 @@ add_task(function* setup() {
 
   // Enable Extended Telemetry.
   yield SpecialPowers.pushPrefEnv({"set": [["toolkit.telemetry.enabled", true]]});
 
   // Enable local telemetry recording for the duration of the tests.
   let oldCanRecord = Services.telemetry.canRecordExtended;
   Services.telemetry.canRecordExtended = true;
 
+  // Enable event recording for the events tested here.
+  Services.telemetry.setEventRecordingEnabled("navigation", true);
+
   // Make sure to restore the engine once we're done.
   registerCleanupFunction(function* () {
     Services.telemetry.canRecordExtended = oldCanRecord;
     Services.search.currentEngine = originalEngine;
     Services.search.removeEngine(engine);
     Services.prefs.clearUserPref(SUGGEST_URLBAR_PREF, true);
     Services.prefs.clearUserPref(ONEOFF_URLBAR_PREF);
     yield PlacesTestUtils.clearHistory();
+    Services.telemetry.setEventRecordingEnabled("navigation", false);
   });
 });
 
 add_task(function* test_simpleQuery() {
   // Let's reset the counts.
   Services.telemetry.clearScalars();
   Services.telemetry.clearEvents();
   let resultIndexHist = Services.telemetry.getHistogramById("FX_URLBAR_SELECTED_RESULT_INDEX");
--- a/toolkit/components/telemetry/Telemetry.cpp
+++ b/toolkit/components/telemetry/Telemetry.cpp
@@ -2630,16 +2630,22 @@ TelemetryImpl::SnapshotBuiltinEvents(uin
 
 NS_IMETHODIMP
 TelemetryImpl::ClearEvents()
 {
   TelemetryEvent::ClearEvents();
   return NS_OK;
 }
 
+NS_IMETHODIMP
+TelemetryImpl::SetEventRecordingEnabled(const nsACString& aCategory, bool aEnabled)
+{
+  TelemetryEvent::SetEventRecordingEnabled(aCategory, aEnabled);
+  return NS_OK;
+}
 
 NS_IMETHODIMP
 TelemetryImpl::FlushBatchedChildTelemetry()
 {
   TelemetryHistogram::IPCTimerFired(nullptr, nullptr);
   return NS_OK;
 }
 
--- a/toolkit/components/telemetry/TelemetryEvent.cpp
+++ b/toolkit/components/telemetry/TelemetryEvent.cpp
@@ -88,17 +88,17 @@ static_assert(kEventCount < kExpiredEven
 // If we cross this limit, we will drop any further event recording until elements
 // are removed from storage.
 const uint32_t kMaxEventRecords = 1000;
 // Maximum length of any passed value string, in UTF8 byte sequence length.
 const uint32_t kMaxValueByteLength = 80;
 // Maximum length of any string value in the extra dictionary, in UTF8 byte sequence length.
 const uint32_t kMaxExtraValueByteLength = 80;
 
-typedef nsDataHashtable<nsCStringHashKey, uint32_t> EventMapType;
+typedef nsDataHashtable<nsCStringHashKey, uint32_t> StringUintMap;
 typedef nsClassHashtable<nsCStringHashKey, nsCString> StringMap;
 
 enum class RecordEventResult {
   Ok,
   UnknownEvent,
   InvalidExtraKey,
   StorageLimitReached,
 };
@@ -244,18 +244,24 @@ TruncateToByteLength(nsCString& str, uin
 namespace {
 
 // Set to true once this global state has been initialized.
 bool gInitDone = false;
 
 bool gCanRecordBase;
 bool gCanRecordExtended;
 
-// The Name -> ID cache map.
-EventMapType gEventNameIDMap(kEventCount);
+// The EventName -> EventID cache map.
+StringUintMap gEventNameIDMap(kEventCount);
+
+// The CategoryName -> CategoryID cache map.
+StringUintMap gCategoryNameIDMap;
+
+// This tracks the IDs of the categories for which recording is enabled.
+nsTHashtable<nsUint32HashKey> gEnabledCategories;
 
 // The main event storage. Events are inserted here in recording order.
 StaticAutoPtr<nsTArray<EventRecord>> gEventRecords;
 
 } // namespace
 
 ////////////////////////////////////////////////////////////////////////
 ////////////////////////////////////////////////////////////////////////
@@ -266,17 +272,21 @@ namespace {
 
 bool
 CanRecordEvent(const StaticMutexAutoLock& lock, const CommonEventInfo& info)
 {
   if (!gCanRecordBase) {
     return false;
   }
 
-  return CanRecordDataset(info.dataset, gCanRecordBase, gCanRecordExtended);
+  if (!CanRecordDataset(info.dataset, gCanRecordBase, gCanRecordExtended)) {
+    return false;
+  }
+
+  return gEnabledCategories.GetEntry(info.category_offset);
 }
 
 RecordEventResult
 RecordEvent(const StaticMutexAutoLock& lock, double timestamp,
             const nsACString& category, const nsACString& method,
             const nsACString& object, const Maybe<nsCString>& value,
             const ExtraArray& extra)
 {
@@ -364,34 +374,41 @@ TelemetryEvent::InitializeGlobalState(bo
     // If this event is expired, mark it with a special event id.
     // This avoids doing expensive expiry checks at runtime.
     if (IsExpiredVersion(info.common_info.expiration_version()) ||
         IsExpiredDate(info.common_info.expiration_day)) {
       eventId = kExpiredEventId;
     }
 
     gEventNameIDMap.Put(UniqueEventName(info), eventId);
+    if (!gCategoryNameIDMap.Contains(nsDependentCString(info.common_info.category()))) {
+      gCategoryNameIDMap.Put(nsDependentCString(info.common_info.category()),
+                             info.common_info.category_offset);
+    }
   }
 
 #ifdef DEBUG
   gEventNameIDMap.MarkImmutable();
+  gCategoryNameIDMap.MarkImmutable();
 #endif
   gInitDone = true;
 }
 
 void
 TelemetryEvent::DeInitializeGlobalState()
 {
   StaticMutexAutoLock locker(gTelemetryEventsMutex);
   MOZ_ASSERT(gInitDone);
 
   gCanRecordBase = false;
   gCanRecordExtended = false;
 
   gEventNameIDMap.Clear();
+  gCategoryNameIDMap.Clear();
+  gEnabledCategories.Clear();
   gEventRecords->Clear();
   gEventRecords = nullptr;
 
   gInitDone = false;
 }
 
 void
 TelemetryEvent::SetCanRecordBase(bool b)
@@ -662,26 +679,52 @@ TelemetryEvent::ClearEvents()
 
   if (!gInitDone) {
     return;
   }
 
   gEventRecords->Clear();
 }
 
+void
+TelemetryEvent::SetEventRecordingEnabled(const nsACString& category, bool enabled)
+{
+  StaticMutexAutoLock locker(gTelemetryEventsMutex);
+
+  uint32_t categoryId;
+  if (!gCategoryNameIDMap.Get(category, &categoryId)) {
+    LogToBrowserConsole(nsIScriptError::warningFlag,
+                        NS_LITERAL_STRING("Unkown category for SetEventRecordingEnabled."));
+    return;
+  }
+
+  if (enabled) {
+    gEnabledCategories.PutEntry(categoryId);
+  } else {
+    gEnabledCategories.RemoveEntry(categoryId);
+  }
+}
+
 size_t
 TelemetryEvent::SizeOfIncludingThis(mozilla::MallocSizeOf aMallocSizeOf)
 {
   StaticMutexAutoLock locker(gTelemetryEventsMutex);
   size_t n = 0;
 
   n += gEventRecords->ShallowSizeOfIncludingThis(aMallocSizeOf);
   for (uint32_t i = 0; i < gEventRecords->Length(); ++i) {
     n += (*gEventRecords)[i].SizeOfExcludingThis(aMallocSizeOf);
   }
 
   n += gEventNameIDMap.ShallowSizeOfExcludingThis(aMallocSizeOf);
   for (auto iter = gEventNameIDMap.ConstIter(); !iter.Done(); iter.Next()) {
     n += iter.Key().SizeOfExcludingThisIfUnshared(aMallocSizeOf);
   }
 
+  n += gCategoryNameIDMap.ShallowSizeOfExcludingThis(aMallocSizeOf);
+  for (auto iter = gCategoryNameIDMap.ConstIter(); !iter.Done(); iter.Next()) {
+    n += iter.Key().SizeOfExcludingThisIfUnshared(aMallocSizeOf);
+  }
+
+  n += gEnabledCategories.ShallowSizeOfExcludingThis(aMallocSizeOf);
+
   return n;
 }
--- a/toolkit/components/telemetry/TelemetryEvent.h
+++ b/toolkit/components/telemetry/TelemetryEvent.h
@@ -21,16 +21,17 @@ void DeInitializeGlobalState();
 void SetCanRecordBase(bool b);
 void SetCanRecordExtended(bool b);
 
 // JS API Endpoints.
 nsresult RecordEvent(const nsACString& aCategory, const nsACString& aMethod,
                      const nsACString& aObject, JS::HandleValue aValue,
                      JS::HandleValue aExtra, JSContext* aCx,
                      uint8_t optional_argc);
+void SetEventRecordingEnabled(const nsACString& aCategory, bool aEnabled);
 nsresult CreateSnapshots(uint32_t aDataset, bool aClear, JSContext* aCx,
                          uint8_t optional_argc, JS::MutableHandleValue aResult);
 
 // Only to be used for testing.
 void ClearEvents();
 
 size_t SizeOfIncludingThis(mozilla::MallocSizeOf aMallocSizeOf);
 
--- a/toolkit/components/telemetry/nsITelemetry.idl
+++ b/toolkit/components/telemetry/nsITelemetry.idl
@@ -483,16 +483,26 @@ interface nsITelemetry : nsISupports
    *               It should only contain registered extra keys.
    *
    * @throws NS_ERROR_INVALID_ARG When trying to record an unknown event.
    */
   [implicit_jscontext, optional_argc]
   void recordEvent(in ACString aCategory, in ACString aMethod, in ACString aObject, [optional] in jsval aValue, [optional] in jsval extra);
 
   /**
+   * Enable recording of events in a category.
+   * Events default to recording disabled. This allows to toggle recording for all events
+   * in the specified category.
+   *
+   * @param aCategory The category name.
+   * @param aEnabled Whether recording is enabled for events in that category.
+   */
+  void setEventRecordingEnabled(in ACString aCategory, in boolean aEnabled);
+
+  /**
    * Serializes the recorded events to a JSON-appropriate array and optionally resets them.
    * The returned structure looks like this:
    *   [
    *     // [timestamp, category, method, object, stringValue, extraValues]
    *     [43245, "category1", "method1", "object1", "string value", null],
    *     [43258, "category1", "method2", "object1", null, {"key1": "string value"}],
    *     ...
    *   ]
--- a/toolkit/components/telemetry/tests/unit/test_TelemetryEvents.js
+++ b/toolkit/components/telemetry/tests/unit/test_TelemetryEvents.js
@@ -31,16 +31,57 @@ function checkEventFormat(events) {
       Assert.ok(Object.keys(extra).every(k => typeof(k) == "string"),
                 "All extra keys should be strings.");
       Assert.ok(Object.values(extra).every(v => typeof(v) == "string"),
                 "All extra values should be strings.");
     }
   }
 }
 
+add_task(function* test_recording_state() {
+  const events = [
+    ["telemetry.test", "test1", "object1"],
+    ["telemetry.test.second", "test", "object1"],
+  ];
+
+  // Both test categories should be off by default.
+  events.forEach(e => Telemetry.recordEvent(...e));
+  let snapshot = Telemetry.snapshotBuiltinEvents(OPTIN, true);
+  Assert.deepEqual(snapshot, [], "Should not have recorded any events.");
+
+  // Enable one test category and see that we record correctly.
+  Telemetry.setEventRecordingEnabled("telemetry.test", true);
+  events.forEach(e => Telemetry.recordEvent(...e));
+  snapshot = Telemetry.snapshotBuiltinEvents(OPTIN, true);
+  Assert.equal(snapshot.length, 1, "Should have recorded one event.");
+  Assert.equal(snapshot[0][1], "telemetry.test", "Should have recorded one event in telemetry.test");
+
+  // Also enable the other test category and see that we record correctly.
+  Telemetry.setEventRecordingEnabled("telemetry.test.second", true);
+  events.forEach(e => Telemetry.recordEvent(...e));
+  snapshot = Telemetry.snapshotBuiltinEvents(OPTIN, true);
+  Assert.equal(snapshot.length, 2, "Should have recorded two events.");
+  Assert.equal(snapshot[0][1], "telemetry.test", "Should have recorded one event in telemetry.test");
+  Assert.equal(snapshot[1][1], "telemetry.test.second", "Should have recorded one event in telemetry.test.second");
+
+  // Now turn of one category again and check that this works as expected.
+  Telemetry.setEventRecordingEnabled("telemetry.test", false);
+  events.forEach(e => Telemetry.recordEvent(...e));
+  snapshot = Telemetry.snapshotBuiltinEvents(OPTIN, true);
+  Assert.equal(snapshot.length, 1, "Should have recorded one event.");
+  Assert.equal(snapshot[0][1], "telemetry.test.second", "Should have recorded one event in telemetry.test.second");
+});
+
+add_task(function* recording_setup() {
+  // Make sure both test categories are enabled for the remaining tests.
+  // Otherwise their event recording won't work.
+  Telemetry.setEventRecordingEnabled("telemetry.test", true);
+  Telemetry.setEventRecordingEnabled("telemetry.test.second", true);
+});
+
 add_task(function* test_recording() {
   Telemetry.clearEvents();
 
   // Record some events.
   let expected = [
     {optout: false, event: ["telemetry.test", "test1", "object1"]},
     {optout: false, event: ["telemetry.test", "test2", "object2"]},
 
--- a/toolkit/components/telemetry/tests/unit/test_TelemetrySession.js
+++ b/toolkit/components/telemetry/tests/unit/test_TelemetrySession.js
@@ -711,16 +711,19 @@ add_task(function* test_checkSubsessionE
     // We don't support subsessions yet on Android.
     return;
   }
 
   // Clear the events.
   Telemetry.clearEvents();
   yield TelemetryController.testReset();
 
+  // Enable recording for the test events.
+  Telemetry.setEventRecordingEnabled("telemetry.test", true);
+
   // Record some events.
   let expected = [
     ["telemetry.test", "test1", "object1", "a", null],
     ["telemetry.test", "test1", "object1", null, {key1: "value"}],
   ];
   for (let event of expected) {
     Telemetry.recordEvent(...event);
   }