Bug 1800620 - Remove glean.validation.first_run_hour

CHANGELOG.md

@@ -2,6 +2,9 @@
 
 [Full changelog](https://github.com/mozilla/glean/compare/v51.8.3...main)
 
+* General
+  * Remove the metric `glean.validation.first_run_hour`. Note that this will mean no `reason=upgrade` metrics pings from freshly installed clients anymore. ([#2271](https://github.com/mozilla/glean/pull/2271))
+
 # v51.8.3 (2022-11-25)
 
 [Full changelog](https://github.com/mozilla/glean/compare/v51.8.2...v51.8.3)

docs/dev/core/internal/clearing.md

@@ -4,13 +4,13 @@ When disabling upload (`Glean.setUploadEnabled(false)`), metrics are also cleare
 of accidentally sending them.
 There are a couple of exceptions to this:
 
-- `first_run_date` and `first_run_hour` are retained so they aren't reset if metrics are later re-enabled.
+- `first_run_date` is retained so it isn't reset if metrics are later re-enabled.
 
 - `client_id` is set to the special value `"c0ffeec0-ffee-c0ff-eec0-ffeec0ffeec0"`. This should make it possible to detect the error case when metrics are sent from a client that has been disabled.
 
 When re-enabling metrics:
 
-- `first_run_date` and `first_run_hour` are left as-is. (They should remain a correct time of first run of the application, unaffected by disabling/enabling the Glean SDK).
+- `first_run_date` is left as-is. (It should remain a correct time of first run of the application, unaffected by disabling/enabling the Glean SDK).
 
 - The `client_id` is set to a newly-generated random UUID. It has no connection to the `client_id` used prior to disabling the Glean SDK.
 

docs/user/reference/general/initializing.md

@@ -34,7 +34,7 @@ This always happens asynchronously.
 
 ### When upload is disabled
 
-If upload is disabled, any persisted metrics, events and pings (other than `first_run_date` and `first_run_hour`) are cleared.
+If upload is disabled, any persisted metrics, events and pings (other than `first_run_date`) are cleared.
 Pending `deletion-request` pings are sent.
 Subsequent calls to record metrics and submit pings will be no-ops.
 Because Glean does that as part of its initialization, users are _required_ to always initialize Glean.

docs/user/reference/general/toggling-upload-status.md

@@ -13,14 +13,14 @@ When upload is disabled, the Glean SDK will perform the following tasks:
 1. Submit a [`deletion-request`](../../user/pings/deletion-request.md) ping.
 2. Cancel scheduled ping uploads.
 3. Clear metrics and pings data from the client, except for the
-  [`first_run_date` and `first_run_hour`](../../user/pings/index.html#the-client_info-section) metrics.
+  [`first_run_date`](../../user/pings/index.html#the-client_info-section) metric.
 
 While upload is disabled, metrics aren't recorded and no data is uploaded.
 
 ## Enabling upload
 
 When upload is enabled, the Glean SDK will re-initialize its [core metrics](../../user/collected-metrics/metrics.md).
-The only core metrics that are not re-initialized are the [`first_run_date` and `first_run_hour`](../../user/pings/index.html#the-client_info-section) metrics.
+The only core metric that is not re-initialized is the [`first_run_date`](../../user/pings/index.html#the-client_info-section) metric.
 
 While upload is enabled all metrics are recorded as expected
 and pings are sent to the telemetry servers.

docs/user/user/collected-metrics/metrics.md

@@ -4,7 +4,7 @@
 
 > If you are looking for the metrics collected by Glean.js,
 > refer to the documentation over on the [`@mozilla/glean.js`](https://github.com/mozilla/glean.js/blob/main/docs/reference/metrics.md) repository.
-<!-- AUTOGENERATED BY glean_parser v6.1.1. DO NOT EDIT. -->
+<!-- AUTOGENERATED BY glean_parser v6.3.0. DO NOT EDIT. -->
 
 # Metrics
 
@@ -79,7 +79,6 @@ In addition to those built-in metrics, the following metrics are added to the pi
 | Name | Type | Description | Data reviews | Extras | Expiration | [Data Sensitivity](https://wiki.mozilla.org/Firefox/Data_Collection) |
 | --- | --- | --- | --- | --- | --- | --- |
 | glean.baseline.duration |[timespan](https://mozilla.github.io/glean/book/user/metrics/timespan.html) |The duration of the last foreground session.  |[Bug 1512938](https://bugzilla.mozilla.org/show_bug.cgi?id=1512938#c3)||never |1, 2 |
-| glean.validation.first_run_hour |[datetime](https://mozilla.github.io/glean/book/user/metrics/datetime.html) |The hour of the first run of the application.  |[Bug 1680783](https://bugzilla.mozilla.org/show_bug.cgi?id=1680783#c5)||never |1 |
 | glean.validation.pings_submitted |[labeled_counter](https://mozilla.github.io/glean/book/user/metrics/labeled_counters.html) |A count of the pings submitted, by ping type.  This metric appears in both the metrics and baseline pings.  - On the metrics ping, the counts include the number of pings sent since   the last metrics ping (including the last metrics ping) - On the baseline ping, the counts include the number of pings send since   the last baseline ping (including the last baseline ping)  |[Bug 1586764](https://bugzilla.mozilla.org/show_bug.cgi?id=1586764#c3)||never |1 |
 
 ## deletion-request
@@ -167,11 +166,10 @@ In addition to those built-in metrics, the following metrics are added to the pi
 | glean.upload.pending_pings |[counter](https://mozilla.github.io/glean/book/user/metrics/counter.html) |The total number of pending pings at startup. This does not include deletion-request pings.  |[Bug 1665041](https://bugzilla.mozilla.org/show_bug.cgi?id=1665041#c23)||never |1 |
 | glean.upload.pending_pings_directory_size |[memory_distribution](https://mozilla.github.io/glean/book/user/metrics/memory_distribution.html) |The size of the pending pings directory upon initialization of Glean. This does not include the size of the deletion request pings directory.  |[Bug 1601550](https://bugzilla.mozilla.org/show_bug.cgi?id=1601550#c3)||never |1 |
 | glean.upload.ping_upload_failure |[labeled_counter](https://mozilla.github.io/glean/book/user/metrics/labeled_counters.html) |Counts the number of ping upload failures, by type of failure. This includes failures for all ping types, though the counts appear in the next successfully sent `metrics` ping.  |[Bug 1589124](https://bugzilla.mozilla.org/show_bug.cgi?id=1589124#c1)|<ul><li>status_code_4xx</li><li>status_code_5xx</li><li>status_code_unknown</li><li>unrecoverable</li><li>recoverable</li></ul>|never |1 |
-| glean.validation.first_run_hour |[datetime](https://mozilla.github.io/glean/book/user/metrics/datetime.html) |The hour of the first run of the application.  |[Bug 1680783](https://bugzilla.mozilla.org/show_bug.cgi?id=1680783#c5)||never |1 |
 | glean.validation.foreground_count |[counter](https://mozilla.github.io/glean/book/user/metrics/counter.html) |On mobile, the number of times the application went to foreground.  |[Bug 1683707](https://bugzilla.mozilla.org/show_bug.cgi?id=1683707#c2)||never |1 |
 | glean.validation.pings_submitted |[labeled_counter](https://mozilla.github.io/glean/book/user/metrics/labeled_counters.html) |A count of the pings submitted, by ping type.  This metric appears in both the metrics and baseline pings.  - On the metrics ping, the counts include the number of pings sent since   the last metrics ping (including the last metrics ping) - On the baseline ping, the counts include the number of pings send since   the last baseline ping (including the last baseline ping)  |[Bug 1586764](https://bugzilla.mozilla.org/show_bug.cgi?id=1586764#c3)||never |1 |
 
 Data categories are [defined here](https://wiki.mozilla.org/Firefox/Data_Collection).
 
-<!-- AUTOGENERATED BY glean_parser v6.1.1. DO NOT EDIT. -->
+<!-- AUTOGENERATED BY glean_parser v6.3.0. DO NOT EDIT. -->
 

glean-core/android/src/main/java/mozilla/telemetry/glean/Glean.kt

@@ -143,7 +143,7 @@ open class GleanInternalAPI internal constructor() {
      * as shared preferences
      * @param uploadEnabled A [Boolean] that determines whether telemetry is enabled.
      *     If disabled, all persisted metrics, events and queued pings (except
-     *     first_run_date and first_run_hour) are cleared.
+     *     first_run_date) are cleared.
      * @param configuration A Glean [Configuration] object with global settings.
      * @param buildInfo A Glean [BuildInfo] object with build-time metadata. This
      *     object is generated at build time by glean_parser at the import path

glean-core/android/src/test/java/mozilla/telemetry/glean/GleanTest.kt

@@ -247,11 +247,11 @@ class GleanTest {
                     //   - seq: 0, reason: active, duration: null
                     //   - seq: 1, reason: inactive, duration: non-null
                     //   - seq: 2, reason: active, duration: null
-                    val baselineMetricsObject = json.getJSONObject("metrics")
                     if (seq == 0) {
                         assertEquals("active", json.getJSONObject("ping_info").getString("reason"))
                     }
                     if (seq == 1) {
+                        val baselineMetricsObject = json.getJSONObject("metrics")
                         assertEquals("inactive", json.getJSONObject("ping_info").getString("reason"))
                         val baselineTimespanMetrics = baselineMetricsObject.getJSONObject("timespan")
                         assertEquals(1, baselineTimespanMetrics.length())

glean-core/android/src/test/java/mozilla/telemetry/glean/scheduler/MetricsPingSchedulerTest.kt

@@ -275,16 +275,6 @@ class MetricsPingSchedulerTest {
             )
         )
 
-        // Catch the automatic metrics ping
-
-        // Trigger worker task to upload the pings in the background
-        triggerWorkManager(context)
-
-        // Fetch the ping from the server and decode its JSON body.
-        var request = server.takeRequest(20L, AndroidTimeUnit.SECONDS)!!
-        var docType = request.path!!.split("/")[3]
-        assertEquals("The received ping must be a 'metrics' ping", "metrics", docType)
-
         try {
             // Setup a testing metric and set it to some value.
             val testMetric = StringMetricType(
@@ -312,8 +302,8 @@ class MetricsPingSchedulerTest {
             triggerWorkManager(context)
 
             // Fetch the ping from the server and decode its JSON body.
-            request = server.takeRequest(20L, AndroidTimeUnit.SECONDS)!!
-            docType = request.path!!.split("/")[3]
+            val request = server.takeRequest(20L, AndroidTimeUnit.SECONDS)!!
+            val docType = request.path!!.split("/")[3]
             assertEquals("The received ping must be a 'metrics' ping", "metrics", docType)
 
             val metricsJsonData = request.getPlainBody()

glean-core/ios/Glean/Glean.swift

@@ -112,7 +112,7 @@ public class Glean {
     /// - parameters:
     ///     * uploadEnabled: A `Bool` that enables or disables telemetry.
     ///       If disabled, all persisted metrics, events and queued pings (except
-    ///       first_run_date and first_run_hour) are cleared.
+    ///       first_run_date) are cleared.
     ///     * configuration: A Glean `Configuration` object with global settings.
     public func initialize(uploadEnabled: Bool,
                            configuration: Configuration = Configuration(),

glean-core/ios/GleanTests/Net/BaselinePingTests.swift

@@ -31,8 +31,8 @@ final class BaselinePingTests: XCTestCase {
             let metrics = json?["metrics"] as? [String: Any]
             if metrics != nil {
                 // Since we are only expecting error metrics,
-                // let's check that this is all we got (plus the `validation.first_run_hour`).
-                XCTAssertEqual(metrics?.count, 2, "metrics has more keys than expected: \(JSONStringify(metrics!))")
+                // let's check that this is all we got.
+                XCTAssertEqual(metrics?.count, 1, "metrics has more keys than expected: \(JSONStringify(metrics!))")
                 let labeledCounters = metrics?["labeled_counter"] as? [String: Any]
                 labeledCounters!.forEach { key, _ in
                     XCTAssertTrue(
@@ -92,14 +92,8 @@ final class BaselinePingTests: XCTestCase {
             // No errors should be reported.
             let metrics = json!["metrics"] as? [String: Any]
             if metrics != nil {
-                let datetimes = metrics!["datetime"] as! [String: Any]
-                XCTAssertTrue(datetimes.keys.contains("glean.validation.first_run_hour"),
-                              "Datetime should have first_run_hour: \(datetimes)")
-
                 if metrics!.count > 1 {
-                    // Since we are only expecting error metrics,
-                    // let's check that this is all we got (plus the `validation.first_run_hour`).
-                    XCTAssertEqual(metrics?.count, 2, "metrics has more keys than expected: \(JSONStringify(metrics!))")
+                    XCTAssertEqual(metrics?.count, 1, "metrics has more keys than expected: \(JSONStringify(metrics!))")
                     let labeledCounters = metrics?["labeled_counter"] as? [String: Any]
                     labeledCounters!.forEach { key, _ in
                         XCTAssertTrue(

glean-core/metrics.yaml

@@ -602,27 +602,6 @@ glean.database:
     expires: never
 
 glean.validation:
-  first_run_hour:
-    no_lint:
-      - UNIT_IN_NAME
-    type: datetime
-    lifetime: user
-    send_in_pings:
-      - metrics
-      - baseline
-    time_unit: hour
-    description: |
-      The hour of the first run of the application.
-    bugs:
-      - https://bugzilla.mozilla.org/1680783
-    data_reviews:
-      - https://bugzilla.mozilla.org/show_bug.cgi?id=1680783#c5
-    data_sensitivity:
-      - technical
-    notification_emails:
-      - glean-team@mozilla.com
-    expires:
-      never
   foreground_count:
     type: counter
     description: |

glean-core/python/glean/glean.py

@@ -147,7 +147,7 @@ def initialize(
                 meaningful.
             upload_enabled (bool): Controls whether telemetry is enabled. If
                 disabled, all persisted metrics, events and queued pings
-                (except first_run_date and first_run_hour) are cleared.
+                (except first_run_date) are cleared.
             configuration (glean.config.Configuration): (optional) An object with
                 global settings.
             data_dir (pathlib.Path): The path to the Glean data directory.

glean-core/python/tests/test_glean.py

@@ -801,7 +801,8 @@ def test_client_activity_api(tmpdir, monkeypatch):
     url_path, payload = wait_for_ping(info_path, max_wait=20)
     assert "baseline" == url_path.split("/")[3]
     assert payload["ping_info"]["reason"] == "active"
-    assert "timespan" not in payload["metrics"]
+    # It's an empty ping.
+    assert "metrics" not in payload
 
     # The upload process is fast, but not fast enough to communicate its status.
     # We give it just a blink of an eye to wind down.

glean-core/src/core/mod.rs

@@ -322,7 +322,6 @@ impl Glean {
             .is_none()
         {
             self.core_metrics.first_run_date.set_sync(self, None);
-            self.core_metrics.first_run_hour.set_sync(self, None);
             // The `first_run_date` field is generated on the very first run
             // and persisted across upload toggling. We can assume that, the only
             // time it is set, that's indeed our "first run".
@@ -446,16 +445,14 @@ impl Glean {
         // so that it can't be accessed until this function is done.
         let _lock = self.upload_manager.clear_ping_queue();
 
-        // There are only two metrics that we want to survive after clearing all
-        // metrics: first_run_date and first_run_hour. Here, we store their values
-        // so we can restore them after clearing the metrics.
+        // There is only one metric that we want to survive after clearing all
+        // metrics: first_run_date. Here, we store its value so we can restore
+        // it after clearing the metrics.
         let existing_first_run_date = self
             .core_metrics
             .first_run_date
             .get_value(self, "glean_client_info");
 
-        let existing_first_run_hour = self.core_metrics.first_run_hour.get_value(self, "metrics");
-
         // Clear any pending pings.
         let ping_maker = PingMaker::new();
         if let Err(err) = ping_maker.clear_pending_pings(self.get_data_path()) {
@@ -500,13 +497,6 @@ impl Glean {
                     .set_sync_chrono(self, existing_first_run_date);
             }
 
-            // Restore the first_run_hour.
-            if let Some(existing_first_run_hour) = existing_first_run_hour {
-                self.core_metrics
-                    .first_run_hour
-                    .set_sync_chrono(self, existing_first_run_hour);
-            }
-
             self.upload_enabled = false;
         }
     }

glean-core/src/internal_metrics.rs

@@ -8,7 +8,6 @@ use super::{metrics::*, CommonMetricData, Lifetime};
 pub struct CoreMetrics {
     pub client_id: UuidMetric,
     pub first_run_date: DatetimeMetric,
-    pub first_run_hour: DatetimeMetric,
     pub os: StringMetric,
 }
 
@@ -46,18 +45,6 @@ impl CoreMetrics {
                 TimeUnit::Day,
             ),
 
-            first_run_hour: DatetimeMetric::new(
-                CommonMetricData {
-                    name: "first_run_hour".into(),
-                    category: "glean.validation".into(),
-                    send_in_pings: vec!["metrics".into(), "baseline".into()],
-                    lifetime: Lifetime::User,
-                    disabled: false,
-                    dynamic_label: None,
-                },
-                TimeUnit::Hour,
-            ),
-
             os: StringMetric::new(CommonMetricData {
                 name: "os".into(),
                 category: "".into(),

glean-core/src/lib_unit_tests.rs

@@ -175,7 +175,7 @@ fn experiments_status_is_correctly_toggled() {
 }
 
 #[test]
-fn client_id_and_first_run_date_and_first_run_hour_must_be_regenerated() {
+fn client_id_and_first_run_date_must_be_regenerated() {
     let dir = tempfile::tempdir().unwrap();
     let tmpname = dir.path().display().to_string();
     {
@@ -193,11 +193,6 @@ fn client_id_and_first_run_date_and_first_run_hour_must_be_regenerated() {
             .first_run_date
             .get_value(&glean, "glean_client_info")
             .is_none());
-        assert!(glean
-            .core_metrics
-            .first_run_hour
-            .get_value(&glean, "metrics")
-            .is_none());
     }
 
     {
@@ -212,11 +207,6 @@ fn client_id_and_first_run_date_and_first_run_hour_must_be_regenerated() {
             .first_run_date
             .get_value(&glean, "glean_client_info")
             .is_some());
-        assert!(glean
-            .core_metrics
-            .first_run_hour
-            .get_value(&glean, "metrics")
-            .is_some());
     }
 }
 
@@ -273,34 +263,6 @@ fn first_run_date_is_managed_correctly_when_toggling_uploading() {
     );
 }
 
-#[test]
-fn first_run_hour_is_managed_correctly_when_toggling_uploading() {
-    let (mut glean, _) = new_glean(None);
-
-    let original_first_run_hour = glean
-        .core_metrics
-        .first_run_hour
-        .get_value(&glean, "metrics");
-
-    glean.set_upload_enabled(false);
-    assert_eq!(
-        original_first_run_hour,
-        glean
-            .core_metrics
-            .first_run_hour
-            .get_value(&glean, "metrics")
-    );
-
-    glean.set_upload_enabled(true);
-    assert_eq!(
-        original_first_run_hour,
-        glean
-            .core_metrics
-            .first_run_hour
-            .get_value(&glean, "metrics")
-    );
-}
-
 #[test]
 fn client_id_is_managed_correctly_when_toggling_uploading() {
     let (mut glean, _) = new_glean(None);
@@ -906,7 +868,7 @@ fn records_io_errors() {
     fs::set_permissions(&pending_pings_dir, permissions).unwrap();
 
     // Writing the ping file should fail.
-    let submitted = glean.internal_pings.metrics.submit_sync(&glean, None);
+    let submitted = glean.internal_pings.baseline.submit_sync(&glean, None);
     // But the return value is still `true` because we enqueue the ping anyway.
     assert!(submitted);