Add back missing client_info metrics

Also adding simple tests to check those are passed through the Kotlin/Swift SDK.
mozilla · Jul 25, 2022 · 6554909 · 6554909
1 parent 27be02c
commit 6554909
Show file tree

Hide file tree

Showing 5 changed files with 69 additions and 0 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -2,6 +2,9 @@
 
 [Full changelog](https://github.com/mozilla/glean/compare/v51.0.0...main)
 
+* General
+  * BUGFIX: Set the following `client_info` fields correctly again: `android_sdk_version`, `device_manufacturer`, `device_model`, `locale`. These were never set in Glean v50.0.0 to v51.0.0 ([#2131](https://github.com/mozilla/glean/pull/2131))
+
 # v51.0.0 (2022-07-22)
 
 [Full changelog](https://github.com/mozilla/glean/compare/v50.1.2...v51.0.0)

diff --git a/glean-core/android/src/test/java/mozilla/telemetry/glean/private/PingTypeTest.kt b/glean-core/android/src/test/java/mozilla/telemetry/glean/private/PingTypeTest.kt
@@ -86,6 +86,12 @@ class PingTypeTest {
 
         val pingJson = JSONObject(request.getPlainBody())
         assertNotNull(pingJson.getJSONObject("client_info")["client_id"])
+
+        assertNotNull(pingJson.getJSONObject("client_info")["android_sdk_version"])
+        assertNotNull(pingJson.getJSONObject("client_info")["device_model"])
+        assertNotNull(pingJson.getJSONObject("client_info")["device_manufacturer"])
+        assertNotNull(pingJson.getJSONObject("client_info")["locale"])
+
         checkPingSchema(pingJson)
     }
 

diff --git a/glean-core/ios/GleanTests/Metrics/PingTests.swift b/glean-core/ios/GleanTests/Metrics/PingTests.swift
@@ -72,6 +72,9 @@ class PingTests: XCTestCase {
 
         let clientInfo = lastPingJson?["client_info"] as? [String: Any]
         XCTAssertNotNil(clientInfo?["client_id"] as? String)
+        XCTAssertNotNil(clientInfo?["device_model"] as? String)
+        XCTAssertNotNil(clientInfo?["device_manufacturer"] as? String)
+        XCTAssertNotNil(clientInfo?["locale"] as? String)
     }
 
     func testSendingOfCustomPingsWithoutClientId() {

diff --git a/glean-core/src/core_metrics.rs b/glean-core/src/core_metrics.rs
@@ -130,6 +130,50 @@ pub mod internal_metrics {
         })
     });
 
+    pub static android_sdk_version: Lazy<StringMetric> = Lazy::new(|| {
+        StringMetric::new(CommonMetricData {
+            name: "android_sdk_version".into(),
+            category: "".into(),
+            send_in_pings: vec!["glean_client_info".into()],
+            lifetime: Lifetime::Application,
+            disabled: false,
+            ..Default::default()
+        })
+    });
+
+    pub static device_manufacturer: Lazy<StringMetric> = Lazy::new(|| {
+        StringMetric::new(CommonMetricData {
+            name: "device_manufacturer".into(),
+            category: "".into(),
+            send_in_pings: vec!["glean_client_info".into()],
+            lifetime: Lifetime::Application,
+            disabled: false,
+            ..Default::default()
+        })
+    });
+
+    pub static device_model: Lazy<StringMetric> = Lazy::new(|| {
+        StringMetric::new(CommonMetricData {
+            name: "device_model".into(),
+            category: "".into(),
+            send_in_pings: vec!["glean_client_info".into()],
+            lifetime: Lifetime::Application,
+            disabled: false,
+            ..Default::default()
+        })
+    });
+
+    pub static locale: Lazy<StringMetric> = Lazy::new(|| {
+        StringMetric::new(CommonMetricData {
+            name: "locale".into(),
+            category: "".into(),
+            send_in_pings: vec!["glean_client_info".into()],
+            lifetime: Lifetime::Application,
+            disabled: false,
+            ..Default::default()
+        })
+    });
+
     pub static baseline_duration: Lazy<TimespanMetric> = Lazy::new(|| {
         TimespanMetric::new(
             CommonMetricData {

diff --git a/glean-core/src/lib.rs b/glean-core/src/lib.rs
@@ -498,6 +498,19 @@ fn initialize_core_metrics(glean: &Glean, client_info: &ClientInfoMetrics) {
     }
     core_metrics::internal_metrics::os_version.set_sync(glean, system::get_os_version());
     core_metrics::internal_metrics::architecture.set_sync(glean, system::ARCH.to_string());
+
+    if let Some(android_sdk_version) = client_info.android_sdk_version.as_ref() {
+        core_metrics::internal_metrics::android_sdk_version.set_sync(glean, android_sdk_version);
+    }
+    if let Some(device_manufacturer) = client_info.device_manufacturer.as_ref() {
+        core_metrics::internal_metrics::device_manufacturer.set_sync(glean, device_manufacturer);
+    }
+    if let Some(device_model) = client_info.device_model.as_ref() {
+        core_metrics::internal_metrics::device_model.set_sync(glean, device_model);
+    }
+    if let Some(locale) = client_info.locale.as_ref() {
+        core_metrics::internal_metrics::locale.set_sync(glean, locale);
+    }
 }
 
 /// Checks if [`initialize`] was ever called.