Bug 1920562 - Switch all legacy events to default to enabled r=florian,janerik,devtools-reviewers,nchevobbe

Differential Revision: https://phabricator.services.mozilla.com/D223831

devtools/docs/contributor/frontend/telemetry.md

@@ -239,9 +239,6 @@ this._telemetry = new Telemetry();
 And use the instance to report e.g. tool opening...
 
 ```js
-// Event telemetry is disabled by default so enable it for your category.
-this._telemetry.setEventRecordingEnabled(true);
-
 // If you already have all the properties for the event you can send the
 // telemetry event using:
 // this._telemetry.recordEvent(method, object, value, extra) e.g.

image/decoders/nsAVIFDecoder.cpp

@@ -673,9 +673,6 @@ class Dav1dDecoder final : AVIFDecoderInterface {
     // the easiest way to see if we're getting unexpected behavior to
     // investigate.
     if (aShouldSendTelemetry && r != 0) {
-      // Uncomment once bug 1691156 is fixed
-      // mozilla::Telemetry::SetEventRecordingEnabled("avif"_ns, true);
-
       mozilla::glean::avif::Dav1dGetPictureReturnValueExtra extra = {
           .value = Some(nsPrintfCString("%d", r)),
       };

toolkit/components/extensions/schemas/telemetry.json

@@ -364,7 +364,7 @@
       {
         "name": "setEventRecordingEnabled",
         "type": "function",
-        "description": "Enable recording of events in a category. Events default to recording disabled. This allows to toggle recording for all events in the specified category.",
+        "description": "Enable recording of events in a category. Events default to recording enabled. This allows to toggle recording for all events in the specified category.",
         "async": true,
         "parameters": [
           {

toolkit/components/glean/docs/user/gifft.md

@@ -156,9 +156,8 @@ Assert.equal(true, snapshot["telemetry.test.mirror_for_labeled_bool"]["1".repeat
 ### Telemetry Events
 
 A Glean event can be mirrored to a Telemetry Event.
-Telemetry Events must be enabled before they can be recorded to via the API
-`Telemetry.setEventRecordingEnabled(category, enable);`.
-If the Telemetry Event isn't enabled,
+If the Telemetry Event is disabled
+(by calling `Services.telemetry.setEventRecordingEnabled("event.category", false);`),
 recording to the Glean event will still work,
 and the event will be Summarized in Telemetry as all disabled events are.
 

toolkit/components/glean/docs/user/glean_for_legacy_events.md

@@ -24,12 +24,6 @@ To record your new event, use [the Glean `record(...)` API][glean-event-api].
 
 To test your new event, use [the Glean `testGetValue()` API][glean-test-api].
 
-```{admonition} Don't Forget!
-Though you're using Glean, there's still a Legacy Telemetry event underneath.
-You must call `Services.telemetry.setEventRecordingEnabled("myCategory", true);`
-in order for the Legacy Telemetry event to be recorded.
-```
-
 Your Legacy Telemetry event will appear in `about:telemetry`
 when your code is triggered as confirmation this is all working as you expect.
 

toolkit/components/telemetry/core/Telemetry.h

@@ -551,7 +551,7 @@ class MOZ_RAII AutoScalarTimer {
 
 /**
  * Enables recording of events in a category.
- * Events default to recording disabled.
+ * Events default to recording enabled.
  * This toggles recording for all events in the specified category.
  *
  * @param aCategory The category name.

toolkit/components/telemetry/core/TelemetryEvent.cpp

@@ -334,8 +334,8 @@ nsTHashMap<nsCStringHashKey, EventKey> gEventNameIDMap(kEventCount);
 // The CategoryName set.
 nsTHashSet<nsCString> gCategoryNames;
 
-// This tracks the IDs of the categories for which recording is enabled.
-nsTHashSet<nsCString> gEnabledCategories;
+// This tracks the IDs of the categories for which recording is disabled.
+nsTHashSet<nsCString> gDisabledCategories;
 
 // The main event storage. Events are inserted here, keyed by process id and
 // in recording order.
@@ -503,7 +503,7 @@ RecordEventResult RecordEvent(const StaticMutexAutoLock& lock,
                                   processType, dynamicNonBuiltin);
 
   // Check whether this event's category has recording enabled
-  if (!gEnabledCategories.Contains(GetCategory(lock, eventKey))) {
+  if (gDisabledCategories.Contains(GetCategory(lock, eventKey))) {
     return RecordEventResult::Ok;
   }
 
@@ -582,12 +582,6 @@ void RegisterEvents(const StaticMutexAutoLock& lock, const nsACString& category,
   if (aBuiltin) {
     gCategoryNames.Insert(category);
   }
-
-  if (!aBuiltin) {
-    // Now after successful registration enable recording for this category
-    // (if not a dynamic builtin).
-    gEnabledCategories.Insert(category);
-  }
 }
 
 }  // anonymous namespace
@@ -750,9 +744,6 @@ void TelemetryEvent::InitializeGlobalState(bool aCanRecordBase,
     gCategoryNames.Insert(info.common_info.category());
   }
 
-  // A hack until bug 1691156 is fixed
-  gEnabledCategories.Insert("avif"_ns);
-
   gInitDone = true;
 }
 
@@ -765,7 +756,7 @@ void TelemetryEvent::DeInitializeGlobalState() {
 
   gEventNameIDMap.Clear();
   gCategoryNames.Clear();
-  gEnabledCategories.Clear();
+  gDisabledCategories.Clear();
   gEventRecords.Clear();
 
   gDynamicEventInfo = nullptr;
@@ -1394,10 +1385,10 @@ void TelemetryEvent::SetEventRecordingEnabled(const nsACString& category,
     return;
   }
 
-  if (enabled) {
-    gEnabledCategories.Insert(category);
+  if (!enabled) {
+    gDisabledCategories.Insert(category);
   } else {
-    gEnabledCategories.Remove(category);
+    gDisabledCategories.Remove(category);
   }
 }
 
@@ -1427,7 +1418,7 @@ size_t TelemetryEvent::SizeOfIncludingThis(
   }
 
   n += gCategoryNames.ShallowSizeOfExcludingThis(aMallocSizeOf);
-  n += gEnabledCategories.ShallowSizeOfExcludingThis(aMallocSizeOf);
+  n += gDisabledCategories.ShallowSizeOfExcludingThis(aMallocSizeOf);
 
   if (gDynamicEventInfo) {
     n += gDynamicEventInfo->ShallowSizeOfIncludingThis(aMallocSizeOf);

toolkit/components/telemetry/core/nsITelemetry.idl

@@ -531,7 +531,7 @@ interface nsITelemetry : nsISupports
 
   /**
    * Enable recording of events in a category.
-   * Events default to recording disabled. This allows to toggle recording for all events
+   * Events default to recording enabled. This allows to toggle recording for all events
    * in the specified category.
    *
    * @param aCategory The category name.

toolkit/components/telemetry/docs/collection/events.rst

@@ -164,16 +164,14 @@ events in Firefox Desktop are
 
   Services.telemetry.setEventRecordingEnabled(category, enabled);
 
-Event recording is currently disabled by default for events registered in Events.yaml.
-Dynamically-registered events (those registered using ``registerEvents()``) are enabled by default, and cannot be disabled.
+Event recording is currently enabled by default for events registered in Events.yaml.
+Dynamically-registered events (those registered using ``registerEvents()``) cannot be disabled.
 Privileged add-ons and Firefox code can enable & disable recording events for specific categories using this function.
 
 Example:
 
 .. code-block:: js
 
-  Services.telemetry.setEventRecordingEnabled("ui", true);
-  // ... now events in the "ui" category will be recorded.
   Services.telemetry.setEventRecordingEnabled("ui", false);
   // ... now "ui" events will not be recorded anymore.
 
@@ -182,6 +180,11 @@ Example:
   Even if your event category isn't enabled, counts of events that attempted to be recorded will
   be :ref:`summarized <events.event-summary>`.
 
+.. note::
+  Events can be expensive to store, submit, and query.
+  You are responsible for ensuring that you don't submit too many events.
+  When your new events land in Nightly, consult with the Data Org about whether they are too "chatty".
+
 Internal API
 ------------
 
@@ -240,14 +243,12 @@ the dynamic-process scalar ``telemetry.dynamic_event_counts`` would have a key
 Testing
 =======
 
-Tests involving Event Telemetry often follow this four-step form:
+Tests involving Event Telemetry often follow this three-step form:
 
 1. ``Services.telemetry.clearEvents();`` To minimize the effects of prior code and tests.
-2. ``Services.telemetry.setEventRecordingEnabled(myCategory, true);`` To enable the collection of
-   your events. (May or may not be relevant in your case)
-3. ``runTheCode();`` This is part of the test where you call the code that's supposed to collect
+2. ``runTheCode();`` This is part of the test where you call the code that's supposed to collect
    Event Telemetry.
-4. ``TelemetryTestUtils.assertEvents(expected, filter, options);`` This will check the
+3. ``TelemetryTestUtils.assertEvents(expected, filter, options);`` This will check the
    events recorded by Event Telemetry against your provided list of expected events.
    If you only need to check the number of events recorded, you can use
    ``TelemetryTestUtils.assertNumberOfEvents(expectedNum, filter, options);``.

toolkit/components/telemetry/docs/collection/webextension-api.rst

@@ -139,7 +139,7 @@ Instead, use :doc:`Glean event definitions <../../glean/user/glean_for_legacy_ev
 
   browser.telemetry.setEventRecordingEnabled(category, enabled);
 
-Enable recording of events in a category. Events default to recording disabled. This allows to toggle recording for all events in the specified category.
+Enable recording of events in a category. Events default to recording enabled. This allows to toggle recording for all events in the specified category.
 
 * ``category`` - *(string)* The category name.
 * ``enabled`` - *(boolean)* Whether recording is enabled for events in that category.

toolkit/components/telemetry/tests/gtest/TestEvents.cpp

@@ -40,6 +40,9 @@ TEST_F(TelemetryTestFixture, RecordEventNative) {
       "this extra value is much too long and must be truncated to fit in the "
       "limit which at time of writing was 80 bytes.");
 
+  // Ensure "telemetry.test" is disabled
+  Telemetry::SetEventRecordingEnabled(category, false);
+
   // Try recording before category's enabled.
   TelemetryEvent::RecordEventNative(
       Telemetry::EventID::TelemetryTest_Test1_Object1, Nothing(), Nothing());
@@ -133,7 +136,6 @@ TEST_F(TelemetryTestFixture, GIFFTValue) {
   const nsCString category("telemetry.test");
   const nsCString method("mirror_with_extra");
   const nsCString object("object1");
-  Telemetry::SetEventRecordingEnabled(category, true);
 
   // Record in Glean.
   // We include an extra extra key (extra1, here) to ensure there's always six

toolkit/components/telemetry/tests/integration/tests/conftest.py

@@ -105,20 +105,6 @@ class Browser(object):
         )
         self.marionette.set_pref("datareporting.healthreport.uploadEnabled", False)
 
-    def enable_search_events(self):
-        """
-        Event Telemetry categories are disabled by default.
-        Search events are in the "navigation" category and are not enabled by
-        default in builds of Firefox, so we enable them here.
-        """
-
-        script = """\
-        Services.telemetry.setEventRecordingEnabled("navigation", true);
-        """
-
-        with self.marionette.using_context(self.marionette.CONTEXT_CHROME):
-            self.marionette.execute_script(textwrap.dedent(script))
-
     def enable_telemetry(self):
         self.marionette.instance.profile.set_persistent_preferences(
             {"datareporting.healthreport.uploadEnabled": True}

toolkit/components/telemetry/tests/integration/tests/test_event_ping.py

@@ -10,7 +10,6 @@ def test_event_ping(browser, helpers):
     Barebones test for "event" ping:
     Search, close Firefox, check "event" ping for search events.
     """
-    browser.enable_search_events()
     browser.wait_for_search_service_init()
     browser.search("mozilla firefox")
 

toolkit/components/telemetry/tests/unit/test_TelemetryEvents.js

@@ -170,7 +170,10 @@ add_task(async function test_recording_state() {
     ["telemetry.test.second", "test", "object1"],
   ];
 
-  // Both test categories should be off by default.
+  Telemetry.setEventRecordingEnabled("telemetry.test", false);
+  Telemetry.setEventRecordingEnabled("telemetry.test.second", false);
+
+  // Both test categories should now be off.
   events.forEach(e => Telemetry.recordEvent(...e));
   TelemetryTestUtils.assertEvents([]);
   checkEventSummary(

toolkit/components/telemetry/tests/unit/test_TelemetryEvents_buildFaster.js

@@ -244,7 +244,8 @@ add_task(async function test_dynamicBuiltinEventsDisabledByDefault() {
   });
 
   // Record some events.
-  // Explicitely _don't_ enable the category
+  // Explicitely disable the category
+  Telemetry.setEventRecordingEnabled(TEST_EVENT_NAME, false);
   Telemetry.recordEvent(TEST_EVENT_NAME, "test1", "object1");
 
   // Now check that the snapshot contains the expected data.