Bug 1767039 - Expand JOG to support event metrics r=TravisLong

Differential Revision: https://phabricator.services.mozilla.com/D158060
2022-10-12 16:35:28 +00:00 · 2022-10-12 16:35:28 +00:00 · daceb61587
--- a/toolkit/components/glean/api/src/ffi/event.rs
+++ b/toolkit/components/glean/api/src/ffi/event.rs
@ -20,6 +20,17 @@ pub extern "C" fn fog_event_record(
 ) {
    // If no extra keys are passed, we can shortcut here.
    if extra_keys.is_empty() {
+        if id & (1 << crate::factory::DYNAMIC_METRIC_BIT) > 0 {
+            let map = crate::factory::__jog_metric_maps::EVENT_MAP
+                .read()
+                .expect("Read lock for dynamic metric map was poisoned");
+            match map.get(&id.into()) {
+                Some(m) => m.record_raw(Default::default()),
+                None => panic!("No (dynamic) metric for event with id {}", id),
+            }
+            return;
+        }
+
        if metric_maps::record_event_by_id(id, Default::default()).is_err() {
            panic!("No event for id {}", id);
        }
@ -40,11 +51,20 @@ pub extern "C" fn fog_event_record(
        .zip(extra_values.iter())
        .map(|(k, v)| (k.to_string(), v.to_string()))
        .collect();
-    match metric_maps::record_event_by_id(id, extra) {
-        Ok(()) => {}
-        Err(EventRecordingError::InvalidId) => panic!("No event for id {}", id),
-        Err(EventRecordingError::InvalidExtraKey) => {
-            // TODO: Record an error. bug 1704504.
+    if id & (1 << crate::factory::DYNAMIC_METRIC_BIT) > 0 {
+        let map = crate::factory::__jog_metric_maps::EVENT_MAP
+            .read()
+            .expect("Read lock for dynamic metric map was poisoned");
+        match map.get(&id.into()) {
+            Some(m) => m.record_raw(extra),
+            None => panic!("No (dynamic) metric for event with id {}", id),
+        }
+        return;
+    } else {
+        match metric_maps::record_event_by_id(id, extra) {
+            Ok(()) => {}
+            Err(EventRecordingError::InvalidId) => panic!("No event for id {}", id),
+            Err(_) => panic!("Unpossible!"),
        }
    }
 }
@ -56,12 +76,32 @@ pub unsafe extern "C" fn fog_event_test_has_value(id: u32, ping_name: &nsACStrin
    } else {
        Some(ping_name.to_utf8().into_owned())
    };
-    metric_maps::event_test_get_value_wrapper(id, storage).is_some()
+    if id & (1 << crate::factory::DYNAMIC_METRIC_BIT) > 0 {
+        let map = crate::factory::__jog_metric_maps::EVENT_MAP
+            .read()
+            .expect("Read lock for dynamic metric map was poisoned");
+        match map.get(&id.into()) {
+            Some(m) => m.test_get_value(storage.as_deref()).is_some(),
+            None => panic!("No (dynamic) metric for event with id {}", id),
+        }
+    } else {
+        metric_maps::event_test_get_value_wrapper(id, storage).is_some()
+    }
 }

 #[no_mangle]
 pub extern "C" fn fog_event_test_get_error(id: u32, error_str: &mut nsACString) -> bool {
-    let err = metric_maps::event_test_get_error(id);
+    let err = if id & (1 << crate::factory::DYNAMIC_METRIC_BIT) > 0 {
+        let map = crate::factory::__jog_metric_maps::EVENT_MAP
+            .read()
+            .expect("Read lock for dynamic metric map was poisoned");
+        match map.get(&id.into()) {
+            Some(m) => test_get_errors!(m),
+            None => panic!("No (dynamic) metric for event with id {}", id),
+        }
+    } else {
+        metric_maps::event_test_get_error(id)
+    };
    err.map(|err_str| error_str.assign(&err_str)).is_some()
 }

@ -88,9 +128,23 @@ pub extern "C" fn fog_event_test_get_value(
        Some(ping_name.to_utf8().into_owned())
    };

-    let events = match metric_maps::event_test_get_value_wrapper(id, storage) {
-        Some(events) => events,
-        None => return,
+    let events = if id & (1 << crate::factory::DYNAMIC_METRIC_BIT) > 0 {
+        let map = crate::factory::__jog_metric_maps::EVENT_MAP
+            .read()
+            .expect("Read lock for dynamic metric map was poisoned");
+        let events = match map.get(&id.into()) {
+            Some(m) => m.test_get_value(storage.as_deref()),
+            None => return,
+        };
+        match events {
+            Some(events) => events,
+            None => return,
+        }
+    } else {
+        match metric_maps::event_test_get_value_wrapper(id, storage) {
+            Some(events) => events,
+            None => return,
+        }
    };

    for event in events {
--- a/toolkit/components/glean/api/src/ipc.rs
+++ b/toolkit/components/glean/api/src/ipc.rs
@ -248,9 +248,19 @@ pub fn replay_from_buf(buf: &[u8]) -> Result<(), ()> {
        }
    }
    for (id, records) in ipc_payload.events.into_iter() {
-        // TODO(bug XXX): Implement events in JOG
-        for (timestamp, extra) in records.into_iter() {
-            let _ = __glean_metric_maps::record_event_by_id_with_time(id, timestamp, extra);
+        if id.0 & (1 << crate::factory::DYNAMIC_METRIC_BIT) > 0 {
+            let map = crate::factory::__jog_metric_maps::EVENT_MAP
+                .read()
+                .expect("Read lock for dynamic event map was poisoned");
+            if let Some(metric) = map.get(&id) {
+                for (timestamp, extra) in records.into_iter() {
+                    metric.record_with_time(timestamp, extra);
+                }
+            }
+        } else {
+            for (timestamp, extra) in records.into_iter() {
+                let _ = __glean_metric_maps::record_event_by_id_with_time(id, timestamp, extra);
+            }
        }
    }
    for (id, labeled_counts) in ipc_payload.labeled_counters.into_iter() {
--- a/toolkit/components/glean/api/src/private/event.rs
+++ b/toolkit/components/glean/api/src/private/event.rs
@ -43,6 +43,20 @@ impl<K: 'static + ExtraKeys + Send + Sync> EventMetric<K> {
        }
    }

+    pub fn with_runtime_extra_keys(
+        id: MetricId,
+        meta: CommonMetricData,
+        allowed_extra_keys: Vec<String>,
+    ) -> Self {
+        if need_ipc() {
+            EventMetric::Child(EventMetricIpc(id))
+        } else {
+            let inner =
+                glean::private::EventMetric::with_runtime_extra_keys(meta, allowed_extra_keys);
+            EventMetric::Parent { id, inner }
+        }
+    }
+
    #[cfg(test)]
    pub(crate) fn child_metric(&self) -> Self {
        match self {
--- a/toolkit/components/glean/bindings/jog/src/lib.rs
+++ b/toolkit/components/glean/bindings/jog/src/lib.rs
@ -79,7 +79,7 @@ pub extern "C" fn jog_test_register_metric(
        disabled,
        extra_args.time_unit,
        extra_args.memory_unit,
-        extra_args.allowed_extra_keys,
+        extra_args.allowed_extra_keys.or_else(|| Some(Vec::new())),
        extra_args.range_min,
        extra_args.range_max,
        extra_args.bucket_count,
--- a/toolkit/components/glean/build_scripts/glean_parser_ext/templates/jog_factory.jinja2
+++ b/toolkit/components/glean/build_scripts/glean_parser_ext/templates/jog_factory.jinja2
@ -21,8 +21,7 @@ use crate::private::{
    TimeUnit,
    Ping,
    LabeledMetric,
-    {# No EventMetric for us yet because ExtraKeys are hard #}
-{% for metric_type_name in metric_types.keys() if metric_type_name != 'event' and not metric_type_name.startswith('labeled_') %}
+{% for metric_type_name in metric_types.keys() if not metric_type_name.startswith('labeled_') %}
    {{ metric_type_name|Camelize }}Metric,
 {% endfor %}};
 use crate::private::traits::HistogramType;
@ -40,8 +39,8 @@ pub(crate) mod __jog_metric_maps {
    use crate::private::{
        Ping,
        LabeledMetric,
-    {# No EventMetric for us yet because ExtraKeys are hard #}
-    {% for metric_type_name in metric_types.keys() if metric_type_name != "event"%}
+        NoExtraKeys,
+    {% for metric_type_name in metric_types.keys() %}
        {{ metric_type_name|Camelize }}Metric,
    {% endfor %}
    };
@ -49,20 +48,23 @@ pub(crate) mod __jog_metric_maps {
    use std::collections::HashMap;
    use std::sync::{Arc, RwLock};

-{# No EventMetric for us yet because ExtraKeys are hard #}
 {% for metric_type_name in metric_types.keys() if metric_type_name != "event" and not metric_type_name.startswith('labeled_') %}
    pub static {{ metric_type_name.upper() }}_MAP: Lazy<Arc<RwLock<HashMap<MetricId, {{ metric_type_name|Camelize }}Metric>>>> =
        Lazy::new(|| Arc::new(RwLock::new(HashMap::new())));

 {% endfor %}
 {# Labeled metrics are special because they're LabeledMetric<Labeled{Counter|Boolean|...}Metric> #}
-{% for metric_type_name in metric_types.keys() if metric_type_name != "event" and metric_type_name.startswith('labeled_') %}
+{% for metric_type_name in metric_types.keys() if metric_type_name.startswith('labeled_') %}
    pub static {{ metric_type_name.upper() }}_MAP: Lazy<Arc<RwLock<HashMap<MetricId, LabeledMetric<{{ metric_type_name|Camelize }}Metric>>>>> =
        Lazy::new(|| Arc::new(RwLock::new(HashMap::new())));

 {% endfor %}
    pub static PING_MAP: Lazy<Arc<RwLock<HashMap<u32, Ping>>>> =
        Lazy::new(|| Arc::new(RwLock::new(HashMap::new())));
+
+{# Event metrics are special because they're EventMetric<K> #}
+    pub static EVENT_MAP: Lazy<Arc<RwLock<HashMap<MetricId, EventMetric<NoExtraKeys>>>>> =
+        Lazy::new(|| Arc::new(RwLock::new(HashMap::new())));
 }

 #[derive(Debug)]
@ -87,8 +89,7 @@ map of argument name to argument type. I may regret this if I need it again. #}
    disabled: bool,
    time_unit: Option<TimeUnit>,
    memory_unit: Option<MemoryUnit>,
-{# No EventMetric for us yet because ExtraKeys are hard #}
-    _allowed_extra_keys: Option<Vec<String>>,
+    allowed_extra_keys: Option<Vec<String>>,
 {# Skipping reason_codes since that's a ping thing. #}
    range_min: Option<u64>,
    range_max: Option<u64>,
@ -100,10 +101,9 @@ map of argument name to argument type. I may regret this if I need it again. #}
 ) -> Result<u32, Box<dyn std::error::Error>> {
    let metric_id = NEXT_METRIC_ID.fetch_add(1, Ordering::SeqCst);
    let metric32 = match metric_type {
-{# No EventMetric for us yet because ExtraKeys are hard #}
-{% for metric_type_name, metric_type in metric_types.items() if metric_type_name != "event" %}
+{% for metric_type_name, metric_type in metric_types.items() %}
        "{{ metric_type_name }}" => {
-            let metric = {{ metric_type_name|Camelize if not metric_type_name.startswith('labeled_') else "Labeled"}}Metric::new(metric_id.into(), CommonMetricData {
+            let metric = {{ metric_type_name|Camelize if not metric_type_name.startswith('labeled_') else "Labeled"}}Metric::{% if metric_type_name == 'event' %}with_runtime_extra_keys{% else %}new{% endif %}(metric_id.into(), CommonMetricData {
                {% for arg_name in common_metric_data_args if arg_name in metric_type.args %}
                {{ arg_name }},
                {% endfor %}
--- a/toolkit/components/glean/build_scripts/glean_parser_ext/util.py
+++ b/toolkit/components/glean/build_scripts/glean_parser_ext/util.py
@ -95,7 +95,7 @@ def type_ids_and_categories(objs) -> Tuple[Dict[str, Tuple[int, List[str]]], Lis
                type_id = len(metric_type_ids) + 1
                args = util.common_metric_args.copy()
                for arg_name in util.extra_metric_args:
-                    if hasattr(metric, arg_name) and arg_name != "allowed_extra_keys":
+                    if hasattr(metric, arg_name):
                        args.append(arg_name)
                metric_type_ids[metric.type] = {"id": type_id, "args": args}

--- a/toolkit/components/glean/tests/test_metrics.yaml
+++ b/toolkit/components/glean/tests/test_metrics.yaml
@ -734,3 +734,47 @@ test_only.ipc:
    no_lint:
      - COMMON_PREFIX
    telemetry_mirror: TELEMETRY_TEST_MIRROR_FOR_URL
+
+test_only.jog:
+  a_counter:
+    type: counter
+    description: |
+      This is a test-only metric.
+      Just counting things.
+      Tied closely to test_jog_name_collision.
+    bugs:
+      - https://bugzilla.mozilla.org/show_bug.cgi?id=1767039
+    data_reviews:
+      - https://bugzilla.mozilla.org/show_bug.cgi?id=1767039
+    data_sensitivity:
+      - technical
+    notification_emails:
+      - glean-team@mozilla.com
+    expires: never
+    send_in_pings:
+      - store1
+
+  an_event:
+    type: event
+    extra_keys:
+      extra1:
+        type: string
+        description: "Some extra data"
+      extra2:
+        type: string
+        description: "Some extra data again"
+    description: |
+      This is a test-only metric.
+      Just recording some events.
+      Tied closely to test_jog_name_collision.
+    bugs:
+      - https://bugzilla.mozilla.org/show_bug.cgi?id=1767039
+    data_reviews:
+      - https://bugzilla.mozilla.org/show_bug.cgi?id=1767039
+    data_sensitivity:
+      - technical
+    notification_emails:
+      - glean-team@mozilla.com
+    expires: never
+    send_in_pings:
+      - store1
--- a/toolkit/components/glean/tests/xpcshell/test_JOG.js
+++ b/toolkit/components/glean/tests/xpcshell/test_JOG.js
@ -151,89 +151,85 @@ add_task(function test_jog_boolean_works() {
  Assert.equal(false, Glean.jogCat.jogBool.testGetValue());
 });

-add_task(
-  { skip_if: () => true /* bug XXX */ },
-  async function test_jog_event_works() {
-    Services.fog.testRegisterRuntimeMetric(
-      "event",
-      "jog_cat",
-      "jog_event_no_extra",
-      ["test-only"],
-      `"ping"`,
-      false
-    );
-    Glean.jogCat.jogEventNoExtra.record();
-    var events = Glean.testOnlyIpc.noExtraEvent.testGetValue();
-    Assert.equal(1, events.length);
-    Assert.equal("jog_cat", events[0].category);
-    Assert.equal("jog_event_no_extra", events[0].name);
+add_task(async function test_jog_event_works() {
+  Services.fog.testRegisterRuntimeMetric(
+    "event",
+    "jog_cat",
+    "jog_event_no_extra",
+    ["test-only"],
+    `"ping"`,
+    false
+  );
+  Glean.jogCat.jogEventNoExtra.record();
+  var events = Glean.jogCat.jogEventNoExtra.testGetValue();
+  Assert.equal(1, events.length);
+  Assert.equal("jog_cat", events[0].category);
+  Assert.equal("jog_event_no_extra", events[0].name);

-    Services.fog.testRegisterRuntimeMetric(
-      "event",
-      "jog_cat",
-      "jog_event",
-      ["test-only"],
-      `"ping"`,
-      false,
-      JSON.stringify({ extras: { extra1: "string", extra2: "string" } })
-    );
-    let extra = { extra1: "can set extras", extra2: "passing more data" };
-    Glean.jogCat.jogEvent.record(extra);
-    events = Glean.jogCat.jogEvent.testGetValue();
-    Assert.equal(1, events.length);
-    Assert.equal("jog_cat", events[0].category);
-    Assert.equal("jog_event", events[0].name);
-    Assert.deepEqual(extra, events[0].extra);
+  Services.fog.testRegisterRuntimeMetric(
+    "event",
+    "jog_cat",
+    "jog_event",
+    ["test-only"],
+    `"ping"`,
+    false,
+    JSON.stringify({ allowed_extra_keys: ["extra1", "extra2"] })
+  );
+  let extra = { extra1: "can set extras", extra2: "passing more data" };
+  Glean.jogCat.jogEvent.record(extra);
+  events = Glean.jogCat.jogEvent.testGetValue();
+  Assert.equal(1, events.length);
+  Assert.equal("jog_cat", events[0].category);
+  Assert.equal("jog_event", events[0].name);
+  Assert.deepEqual(extra, events[0].extra);

-    Services.fog.testRegisterRuntimeMetric(
-      "event",
-      "jog_cat",
-      "jog_event_with_extra",
-      ["test-only"],
-      `"ping"`,
-      false,
-      JSON.stringify({
-        extras: {
-          extra1: "string",
-          extra2: "quantity",
-          extra3_longer_name: "boolean",
-        },
-      })
-    );
-    let extra2 = {
-      extra1: "can set extras",
-      extra2: 37,
-      extra3_longer_name: false,
-    };
-    Glean.jogCat.jogEventWithExtra.record(extra2);
-    events = Glean.jogCat.jogEventWithExtra.testGetValue();
-    Assert.equal(1, events.length);
-    Assert.equal("jog_cat", events[0].category);
-    Assert.equal("jog_event_with_extra", events[0].name);
-    let expectedExtra = {
-      extra1: "can set extras",
-      extra2: "37",
-      extra3_longer_name: "false",
-    };
-    Assert.deepEqual(expectedExtra, events[0].extra);
+  Services.fog.testRegisterRuntimeMetric(
+    "event",
+    "jog_cat",
+    "jog_event_with_extra",
+    ["test-only"],
+    `"ping"`,
+    false,
+    JSON.stringify({
+      allowed_extra_keys: ["extra1", "extra2", "extra3_longer_name"],
+    })
+  );
+  let extra2 = {
+    extra1: "can set extras",
+    extra2: 37,
+    extra3_longer_name: false,
+  };
+  Glean.jogCat.jogEventWithExtra.record(extra2);
+  events = Glean.jogCat.jogEventWithExtra.testGetValue();
+  Assert.equal(1, events.length);
+  Assert.equal("jog_cat", events[0].category);
+  Assert.equal("jog_event_with_extra", events[0].name);
+  let expectedExtra = {
+    extra1: "can set extras",
+    extra2: "37",
+    extra3_longer_name: "false",
+  };
+  Assert.deepEqual(expectedExtra, events[0].extra);

-    // Invalid extra keys don't crash, the event is not recorded.
-    let extra3 = {
-      extra1_nonexistent_extra: "this does not crash",
-    };
-    Glean.jogCat.jogEventWithExtra.record(extra3);
-    events = Glean.jogCat.jogEventWithExtra.testGetValue();
-    Assert.equal(1, events.length, "Recorded one event too many.");
+  // Quantities need to be non-negative.
+  let extra4 = {
+    extra2: -1,
+  };
+  Glean.jogCat.jogEventWithExtra.record(extra4);
+  events = Glean.jogCat.jogEventWithExtra.testGetValue();
+  Assert.equal(1, events.length, "Recorded one event too many.");

-    // Quantities need to be non-negative.
-    let extra4 = {
-      extra2: -1,
-    };
-    Glean.jogCat.jogEventWithExtra.record(extra4);
-    events = Glean.jogCat.jogEventWithExtra.testGetValue();
-    Assert.equal(1, events.length, "Recorded one event too many.");
-  }
-);
+  // Invalid extra keys don't crash, the event is not recorded.
+  let extra3 = {
+    extra1_nonexistent_extra: "this does not crash",
+  };
+  Glean.jogCat.jogEventWithExtra.record(extra3);
+  // And test methods throw appropriately
+  Assert.throws(
+    () => Glean.jogCat.jogEventWithExtra.testGetValue(),
+    /NS_ERROR_LOSS_OF_SIGNIFICANT_DATA/
+  );
+});

 add_task(async function test_jog_memory_distribution_works() {
  Services.fog.testRegisterRuntimeMetric(
@ -294,9 +290,9 @@ add_task(async function test_jog_custom_distribution_works() {
 });

 add_task(
-  /* TODO(bug 1737520 and XXX): Enable custom ping support in JOG and on Android */
-  { skip_if: () => true /*|| AppConstants.platform == "android"*/ },
-  function test_jog_custom_pings() {
+  /* TODO: Enable custom ping support on Android */
+  { skip_if: () => AppConstants.platform == "android" },
+  async function test_jog_custom_pings() {
    Services.fog.testRegisterRuntimeMetric(
      "boolean",
      "jog_cat",
@ -305,15 +301,15 @@ add_task(
      `"ping"`,
      false
    );
-    //Services.fog.testRegisterRuntimePing("jog-ping");
+    Services.fog.testRegisterRuntimePing("jog-ping", true, true, []);
    Assert.ok("jogPing" in GleanPings);
    let submitted = false;
    Glean.jogCat.jogPingBool.set(false);
-    GleanPings.onePingOnly.testBeforeNextSubmit(reason => {
+    GleanPings.jogPing.testBeforeNextSubmit(reason => {
      submitted = true;
      Assert.equal(false, Glean.jogCat.jogPingBool.testGetValue());
    });
-    GleanPings.onePingOnly.submit();
+    GleanPings.jogPing.submit();
    Assert.ok(submitted, "Ping was submitted, callback was called.");
    // ping-lifetime value was cleared.
    Assert.equal(undefined, Glean.jogCat.jogPingBool.testGetValue());
@ -670,3 +666,56 @@ add_task(
    Assert.ok(submitted, "Ping must have been submitted");
  }
 );
+
+add_task(function test_jog_name_collision() {
+  Assert.ok("aCounter" in Glean.testOnlyJog);
+  Assert.equal(undefined, Glean.testOnlyJog.aCounter.testGetValue());
+  const kValue = 42;
+  Glean.testOnlyJog.aCounter.add(kValue);
+  Assert.equal(kValue, Glean.testOnlyJog.aCounter.testGetValue());
+
+  // Let's overwrite the test_only.jog.a_counter counter.
+  Services.fog.testRegisterRuntimeMetric(
+    "counter",
+    "test_only.jog",
+    "a_counter",
+    ["store1"],
+    `"ping"`,
+    true // changing the metric to disabled.
+  );
+
+  Assert.ok("aCounter" in Glean.testOnlyJog);
+  Assert.equal(kValue, Glean.testOnlyJog.aCounter.testGetValue());
+  Glean.testOnlyJog.aCounter.add(kValue);
+  Assert.equal(
+    kValue,
+    Glean.testOnlyJog.aCounter.testGetValue(),
+    "value of now-disabled metric remains unchanged."
+  );
+
+  // Now let's mess with events:
+  Assert.ok("anEvent" in Glean.testOnlyJog);
+  Assert.equal(undefined, Glean.testOnlyJog.anEvent.testGetValue());
+  const extra12 = {
+    extra1: "a value",
+    extra2: "another value",
+  };
+  Glean.testOnlyJog.anEvent.record(extra12);
+  Assert.deepEqual(extra12, Glean.testOnlyJog.anEvent.testGetValue()[0].extra);
+  Services.fog.testRegisterRuntimeMetric(
+    "event",
+    "test_only.jog",
+    "an_event",
+    ["store1"],
+    `"ping"`,
+    false,
+    JSON.stringify({ allowed_extra_keys: ["extra1", "extra2", "extra3"] }) // New extra key just dropped
+  );
+  const extra123 = {
+    extra1: "different value",
+    extra2: "another different value",
+    extra3: 42,
+  };
+  Glean.testOnlyJog.anEvent.record(extra123);
+  Assert.deepEqual(extra123, Glean.testOnlyJog.anEvent.testGetValue()[1].extra);
+});
--- a/toolkit/components/glean/tests/xpcshell/test_JOGIPC.js
+++ b/toolkit/components/glean/tests/xpcshell/test_JOGIPC.js
@ -28,9 +28,7 @@ add_task(
 const COUNT = 42;
 const STRING = "a string!";
 const ANOTHER_STRING = "another string!";
-/* TODO:(bugXXX) Support events in JOG
 const EVENT_EXTRA = { extra1: "so very extra" };
-*/
 const MEMORIES = [13, 31];
 const MEMORY_BUCKETS = ["13509772", "32131834"]; // buckets are strings : |
 const COUNTERS_1 = 3;
@ -42,10 +40,16 @@ const INVALID_COUNTERS = 7;
 const METRICS = [
  ["counter", "jog_ipc", "jog_counter", ["test-only"], `"ping"`, false],
  ["string_list", "jog_ipc", "jog_string_list", ["test-only"], `"ping"`, false],
-  /* TODO:(bugXXX) Support events in JOG
-["event", "jog_ipc", "jog_event_no_extra", ["test-only"], `"ping"`, false],
-["event", "jog_ipc", "jog_event", ["test-only"], `"ping"`, false, JSON.stringify({extras: {extra1: "string"})}],
-*/
+  ["event", "jog_ipc", "jog_event_no_extra", ["test-only"], `"ping"`, false],
+  [
+    "event",
+    "jog_ipc",
+    "jog_event",
+    ["test-only"],
+    `"ping"`,
+    false,
+    JSON.stringify({ allowed_extra_keys: ["extra1"] }),
+  ],
  [
    "memory_distribution",
    "jog_ipc",
@ -123,10 +127,8 @@ add_task({ skip_if: () => runningInParent }, async function run_child_stuff() {
  Glean.jogIpc.jogStringList.add(STRING);
  Glean.jogIpc.jogStringList.add(ANOTHER_STRING);

-  /* TODO:(bugXXX) Support events in JOG
  Glean.jogIpc.jogEventNoExtra.record();
  Glean.jogIpc.jogEvent.record(EVENT_EXTRA);
-  */

  for (let memory of MEMORIES) {
    Glean.jogIpc.jogMemoryDist.accumulate(memory);
@ -197,8 +199,7 @@ add_task(
      );
    }

-    /* TODO(bug XXX): Support events in JOG
-    var events = Glean.jogIpc.jogEventNoExtra.testGetValue();
+    let events = Glean.jogIpc.jogEventNoExtra.testGetValue();
    Assert.equal(1, events.length);
    Assert.equal("jog_ipc", events[0].category);
    Assert.equal("jog_event_no_extra", events[0].name);
@ -208,7 +209,6 @@ add_task(
    Assert.equal("jog_ipc", events[0].category);
    Assert.equal("jog_event", events[0].name);
    Assert.deepEqual(EVENT_EXTRA, events[0].extra);
-    */

    const NANOS_IN_MILLIS = 1e6;
    const EPSILON = 40000; // bug 1701949