Allow timespans to track across upload toggles

This is a pre-requisite to allow the coming per-ping toggle:

At the `start` time we don't know _which_ pings to record for.

It's fine to do this because:
* Timespans, if at all, usually collect a span over a single action,
  with no chance of a upload toggle happening in between
* There's very few timespans in use right now
* It's often the wrong metric type anyway, and timing distributions
  should be used
* Timing Distributions already allow this

glean-core/src/metrics/timespan.rs

@@ -65,10 +65,6 @@ impl TimespanMetric {
     /// Set start time synchronously.
     #[doc(hidden)]
     pub fn set_start(&self, glean: &Glean, start_time: u64) {
-        if !self.should_record(glean) {
-            return;
-        }
-
         let mut lock = self
             .start_time
             .write()

glean-core/tests/timespan.rs

@@ -284,7 +284,7 @@ fn set_raw_time_does_nothing_when_timer_running() {
 }
 
 #[test]
-fn timespan_is_not_tracked_across_upload_toggle() {
+fn timespan_is_tracked_across_upload_toggle() {
     let (mut glean, _t) = new_glean(None);
 
     let metric = TimespanMetric::new(
@@ -307,23 +307,20 @@ fn timespan_is_not_tracked_across_upload_toggle() {
     // We should clear internal state as upload is disabled.
     metric.set_stop(&glean, 40);
 
+    assert_eq!(None, metric.get_value(&glean, "store1"));
+
     // App code eventually starts the timer again.
-    // Upload is disabled, so this should not have any effect.
     metric.set_start(&glean, 100);
     // User enables telemetry upload again.
     glean.set_upload_enabled(true);
     // App code eventually stops the timer.
-    // None should be running.
+    // The full timespan is recorded.
     metric.set_stop(&glean, 200);
 
-    // Nothing should have been recorded.
-    assert_eq!(None, metric.get_value(&glean, "store1"));
+    assert_eq!(Some(100), metric.get_value(&glean, "store1"));
 
-    // Make sure that the error has been recorded
-    assert_eq!(
-        Ok(1),
-        test_get_num_recorded_errors(&glean, metric.meta(), ErrorType::InvalidState)
-    );
+    // No errors have been recorded.
+    assert!(test_get_num_recorded_errors(&glean, metric.meta(), ErrorType::InvalidState).is_err());
 }
 
 #[test]

glean-core/tests/timing_distribution.rs

@@ -427,3 +427,49 @@ fn raw_samples_api_error_cases() {
         test_get_num_recorded_errors(&glean, metric.meta(), ErrorType::InvalidOverflow)
     );
 }
+
+#[test]
+fn timing_distribution_is_tracked_across_upload_toggle() {
+    let (mut glean, _t) = new_glean(None);
+
+    let metric = TimingDistributionMetric::new(
+        CommonMetricData {
+            name: "distribution".into(),
+            category: "telemetry".into(),
+            send_in_pings: vec!["store1".into()],
+            disabled: false,
+            lifetime: Lifetime::Ping,
+            ..Default::default()
+        },
+        TimeUnit::Nanosecond,
+    );
+
+    let id = 4u64.into();
+    let duration = 100;
+
+    // Timer is started.
+    metric.set_start(id, 0);
+    // User disables telemetry upload.
+    glean.set_upload_enabled(false);
+    // App code eventually stops the timer.
+    // We should clear internal state as upload is disabled.
+    metric.set_stop_and_accumulate(&glean, id, duration);
+
+    assert_eq!(None, metric.get_value(&glean, "store1"));
+
+    // App code eventually starts the timer again.
+    // Upload is disabled, so this should not have any effect.
+    metric.set_start(id, 100);
+    // User enables telemetry upload again.
+    glean.set_upload_enabled(true);
+    // App code eventually stops the timer.
+    // The full timespan is recorded.
+    metric.set_stop_and_accumulate(&glean, id, 100 + duration);
+
+    let data = metric.get_value(&glean, "store1").unwrap();
+    assert_eq!(1, data.count);
+    assert_eq!(100, data.sum);
+
+    // Make sure that the error has been recorded
+    assert!(test_get_num_recorded_errors(&glean, metric.meta(), ErrorType::InvalidState).is_err());
+}