Bug 1894530 - Augment the Glean Interface For Firefox Telemetry (GIFFT) to support the Legacy Telemetry event value parameter.r=chutten

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

toolkit/components/glean/bindings/private/Event.cpp

@@ -47,6 +47,7 @@ void GleanEvent::Record(
   nsTArray<nsCString> extraKeys;
   nsTArray<nsCString> extraValues;
   CopyableTArray<Telemetry::EventExtraEntry> telExtras;
+  nsCString telValue;
   for (const auto& entry : aExtra.Value().Entries()) {
     if (entry.mValue.IsVoid()) {
       // Someone passed undefined/null for this value.
@@ -58,14 +59,21 @@ void GleanEvent::Record(
 
     extraKeys.AppendElement(snakeKey);
     extraValues.AppendElement(entry.mValue);
-    telExtras.EmplaceBack(Telemetry::EventExtraEntry{entry.mKey, entry.mValue});
+
+    if (snakeKey.EqualsLiteral("value")) {
+      telValue = entry.mValue;
+    } else {
+      telExtras.EmplaceBack(
+          Telemetry::EventExtraEntry{entry.mKey, entry.mValue});
+    }
   }
 
   // Since this calls the implementation directly, we need to implement GIFFT
   // here as well as in EventMetric::Record.
   auto id = EventIdForMetric(mEvent.mId);
   if (id) {
-    Telemetry::RecordEvent(id.extract(), Nothing(),
+    Telemetry::RecordEvent(id.extract(),
+                           telValue.IsEmpty() ? Nothing() : Some(telValue),
                            telExtras.IsEmpty() ? Nothing() : Some(telExtras));
   }
 

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

@@ -45,7 +45,7 @@ This compatibility table explains which Telemetry probe types can be mirrors for
 | [uuid](https://mozilla.github.io/glean/book/reference/metrics/uuid.html) | [Scalar of kind: string](/toolkit/components/telemetry/collection/scalars.html). Value will be in canonical 8-4-4-4-12 format. Value is not guaranteed to be valid, and invalid values may be present in the mirrored scalar while the uuid metric remains empty. Calling `GenerateAndSet` on the uuid is not mirrored, and will log a warning. |
 | [url](https://mozilla.github.io/glean/book/reference/metrics/url.html) | [Scalar of kind: string](/toolkit/components/telemetry/collection/scalars.html). The stringified Url will be cropped to the maximum length allowed by the legacy type. |
 | [datetime](https://mozilla.github.io/glean/book/reference/metrics/datetime.html) | [Scalar of kind: string](/toolkit/components/telemetry/collection/scalars.html). Value will be in ISO8601 format. |
-| [events](https://mozilla.github.io/glean/book/reference/metrics/event.html) | [Events](/toolkit/components/telemetry/collection/events.html). The `value` field will be left empty.  |
+| [events](https://mozilla.github.io/glean/book/reference/metrics/event.html) | [Events](/toolkit/components/telemetry/collection/events.html). The `value` field will be filled by the Glean extra named `value` if defined and present. |
 | [quantity](https://mozilla.github.io/glean/book/reference/metrics/quantity.html) | [Scalar of kind: uint](/toolkit/components/telemetry/collection/scalars.html) |
 | [rate](https://mozilla.github.io/glean/book/reference/metrics/rate.html) | [Keyed Scalar of kind: uint](/toolkit/components/telemetry/collection/scalars.html). The keys are "numerator" and "denominator". Does not work for `rate` metrics with external denominators. |
 | [text](https://mozilla.github.io/glean/book/reference/metrics/text.html) | *No Supported Telemetry Type* |
@@ -165,6 +165,12 @@ See
 [the Telemetry Event docs](/toolkit/components/telemetry/collection/events.rst)
 for details on how disabled Telemetry Events behave.
 
+In order to make use of the `value` field in Telemetry Events, you must
+first define an event extra in the metrics.yaml file with the name "value".
+On recording the event with the Glean extra key for the "value" filled in,
+GIFFT will map this to the Telemetry Event `value` property and remove it from
+the list of extras so it is not duplicated.
+
 ### Numeric Values
 
 The arguments and storage formats for Glean's numeric types

toolkit/components/glean/tests/test_metrics.yaml

@@ -648,6 +648,9 @@ test_only.ipc:
   an_event:
     type: event
     extra_keys:
+      value:
+        type: string
+        description: Maps to Legacy Telemetry `value` property
       extra1:
         type: string
         description: "Some extra data"

toolkit/components/glean/tests/xpcshell/test_GIFFT.js

@@ -226,7 +226,15 @@ add_task(function test_gifft_events() {
   Assert.equal("test_only.ipc", events[0].category);
   Assert.equal("no_extra_event", events[0].name);
 
-  let extra = { extra1: "can set extras", extra2: "passing more data" };
+  let extra = {
+    value: "a value for Telemetry",
+    extra1: "can set extras",
+    extra2: "passing more data",
+  };
+  // Since `value` will be stripped and used for the value in the Legacy Telemetry API, we need
+  // a copy without it in order to test with `assertEvents` later.
+  let { extra1, extra2 } = extra;
+  let telExtra = { extra1, extra2 };
   Glean.testOnlyIpc.anEvent.record(extra);
   events = Glean.testOnlyIpc.anEvent.testGetValue();
   Assert.equal(1, events.length);
@@ -237,7 +245,7 @@ add_task(function test_gifft_events() {
   TelemetryTestUtils.assertEvents(
     [
       ["telemetry.test", "not_expired_optout", "object1", undefined, undefined],
-      ["telemetry.test", "mirror_with_extra", "object1", null, extra],
+      ["telemetry.test", "mirror_with_extra", "object1", extra.value, telExtra],
     ],
     { category: "telemetry.test" }
   );