From 732d9030af5170f6d7cbba4f57661d13eb0d7b07 Mon Sep 17 00:00:00 2001
From: Alessio Placitelli <alessio.placitelli@gmail.com>
Date: Fri, 4 Dec 2020 12:04:51 +0000
Subject: [PATCH] Bug 1679835 - Collect the OS version in FOG. r=janerik

Currently, the Glean RLB do not collect a meaningful value
for the OS version ("unknown"). Moreover, FOG is interested
in recording the OS version as detected by Gecko, which is
very specific. This adds the Gecko-detected OS version as
a metric in FOG.

Differential Revision: https://phabricator.services.mozilla.com/D98742
---
 toolkit/components/glean/metrics.yaml         | 20 +++++++
 toolkit/components/glean/src/lib.rs           |  5 +-
 .../components/glean/xpcshell/test_Glean.js   | 60 +++++++++++++++++++
 3 files changed, 83 insertions(+), 2 deletions(-)

diff --git a/toolkit/components/glean/metrics.yaml b/toolkit/components/glean/metrics.yaml
index 0dadb1efbf3a..fe59faf4597f 100644
--- a/toolkit/components/glean/metrics.yaml
+++ b/toolkit/components/glean/metrics.yaml
@@ -68,3 +68,23 @@ fog_validation:
     expires: "89"
     send_in_pings:
       - fog-validation
+
+  os_version:
+    type: string
+    description:
+      The version of the OS running Firefox, as detected by Gecko.
+      To be sent only in the "fog-validation" ping.
+    bugs:
+      - https://bugzilla.mozilla.org/show_bug.cgi?id=1679835
+    data_reviews:
+      - https://bugzilla.mozilla.org/show_bug.cgi?id=1679835#c3
+    data_sensitivity:
+      - technical
+    lifetime: application
+    notification_emails:
+      - aplacitelli@mozilla.com
+      - chutten@mozilla.com
+      - glean-team@mozilla.com
+    expires: "89"
+    send_in_pings:
+      - fog-validation
diff --git a/toolkit/components/glean/src/lib.rs b/toolkit/components/glean/src/lib.rs
index 205f47bf2402..7a6752eccc60 100644
--- a/toolkit/components/glean/src/lib.rs
+++ b/toolkit/components/glean/src/lib.rs
@@ -62,12 +62,13 @@ pub unsafe extern "C" fn fog_init() -> nsresult {
         Err(e) => return e,
     };
 
-    // TODO: os_version will be sent as a new metric in bug 1679835.
-    let (_os_version, _architecture) = match get_system_info() {
+    let (os_version, _architecture) = match get_system_info() {
         Ok(si) => si,
         Err(e) => return e,
     };
 
+    fog::metrics::fog_validation::os_version.set(os_version);
+
     let client_info = ClientInfoMetrics {
         app_build,
         app_display_version,
diff --git a/toolkit/components/glean/xpcshell/test_Glean.js b/toolkit/components/glean/xpcshell/test_Glean.js
index 030905144f93..383f977cc25a 100644
--- a/toolkit/components/glean/xpcshell/test_Glean.js
+++ b/toolkit/components/glean/xpcshell/test_Glean.js
@@ -10,8 +10,54 @@
 "use strict";
 
 Cu.importGlobalProperties(["Glean"]);
+const { MockRegistrar } = ChromeUtils.import(
+  "resource://testing-common/MockRegistrar.jsm"
+);
 const { setTimeout } = ChromeUtils.import("resource://gre/modules/Timer.jsm");
 
+/**
+ * Mock the SysInfo object used to read System data in Gecko.
+ */
+var SysInfo = {
+  overrides: {},
+
+  /**
+   * Checks if overrides are present and return them.
+   *
+   * @returns the overridden value or undefined if not present.
+   */
+  _getOverridden(name) {
+    if (name in this.overrides) {
+      return this.overrides[name];
+    }
+
+    return undefined;
+  },
+
+  // To support nsIPropertyBag.
+  getProperty(name) {
+    let override = this._getOverridden(name);
+    return override !== undefined
+      ? override
+      : this._genuine.QueryInterface(Ci.nsIPropertyBag).getProperty(name);
+  },
+
+  // To support nsIPropertyBag2.
+  get(name) {
+    let override = this._getOverridden(name);
+    return override !== undefined
+      ? override
+      : this._genuine.QueryInterface(Ci.nsIPropertyBag2).get(name);
+  },
+
+  // To support nsIPropertyBag2.
+  getPropertyAsACString(name) {
+    return this.get(name);
+  },
+
+  QueryInterface: ChromeUtils.generateQI(["nsIPropertyBag2", "nsISystemInfo"]),
+};
+
 function sleep(ms) {
   /* eslint-disable mozilla/no-arbitrary-setTimeout */
   return new Promise(resolve => setTimeout(resolve, ms));
@@ -21,11 +67,25 @@ add_task(function test_setup() {
   // FOG needs a profile directory to put its data in.
   do_get_profile();
 
+  // Mock SysInfo.
+  SysInfo.overrides = {
+    version: "1.2.3",
+    arc: "x64",
+  };
+  MockRegistrar.register("@mozilla.org/system-info;1", SysInfo);
+
   // We need to initialize it once, otherwise operations will be stuck in the pre-init queue.
   let FOG = Cc["@mozilla.org/toolkit/glean;1"].createInstance(Ci.nsIFOG);
   FOG.initializeFOG();
 });
 
+add_task(function test_osversion_is_set() {
+  Assert.equal(
+    "1.2.3",
+    Glean.fog_validation.os_version.testGetValue("fog-validation")
+  );
+});
+
 add_task(function test_fog_counter_works() {
   Glean.test_only.bad_code.add(31);
   Assert.equal(31, Glean.test_only.bad_code.testGetValue("test-ping"));