Merge pull request #303 from brizental/1709787-set-raw

Bug 1709787 - Implement setRawNanos API for the TimespanMetricType

Author: Beatriz Rizental

CHANGELOG.md

@@ -4,7 +4,8 @@
 
 * [#279](https://github.com/mozilla/glean.js/pull/279): BUGFIX: Ensure only empty pings triggers logging of "empty ping" messages.
 * [#288](https://github.com/mozilla/glean.js/pull/288): Support collecting `PlatformInfo` from `Qt` applications. Only OS name and locale are supported.
-* [#281](https://github.com/mozilla/glean.js/pull/281): Add the QuantityMetricType
+* [#281](https://github.com/mozilla/glean.js/pull/281): Add the QuantityMetricType.
+* [#303](https://github.com/mozilla/glean.js/pull/303): Implement setRawNanos API for the TimespanMetricType.
 
 # v0.11.0 (2021-05-03)
 

glean/src/core/metrics/types/timespan.ts

@@ -69,6 +69,57 @@ class TimespanMetricType extends MetricType {
     this.timeUnit = timeUnit as TimeUnit;
   }
 
+  /**
+   * An internal implemention of `setRaw` that does not dispatch the recording task.
+   *
+   * # Important
+   *
+   * This is absolutely not meant to be used outside of Glean itself.
+   * It may cause multiple issues because it cannot guarantee
+   * that the recording of the metric will happen in order with other Glean API calls.
+   *
+   * @param instance The metric instance to record to.
+   * @param elapsed The elapsed time to record, in milliseconds.
+   */
+  static async _private_setRawUndispatched(instance: TimespanMetricType, elapsed: number): Promise<void> {
+    if (!instance.shouldRecord(Context.uploadEnabled)) {
+      return;
+    }
+
+    if (!isUndefined(instance.startTime)) {
+      // TODO: record error once Bug 1682574 is resolved.
+      console.error("Timespan already running. Raw value not recorded.");
+      return;
+    }
+
+    let reportValueExists = false;
+    const transformFn = ((elapsed) => {
+      return (old?: JSONValue): TimespanMetric => {
+        let metric: TimespanMetric;
+        try {
+          metric = new TimespanMetric(old);
+          // If creating the metric didn't error,
+          // there is a valid timespan already recorded for this metric.
+          reportValueExists = true;
+        } catch {
+          metric = new TimespanMetric({
+            timespan: elapsed,
+            timeUnit: instance.timeUnit,
+          });
+        }
+
+        return metric;
+      };
+    })(elapsed);
+
+    await Context.metricsDatabase.transform(instance, transformFn);
+
+    if (reportValueExists) {
+      // TODO: record error once Bug 1682574 is resolved.
+      console.error("Timespan value already recorded. New value discarded.");
+    }
+  }
+
   /**
    * Starts tracking time for the provided metric.
    *
@@ -131,32 +182,7 @@ class TimespanMetricType extends MetricType {
         return;
       }
 
-      let reportValueExists = false;
-      const transformFn = ((elapsed) => {
-        return (old?: JSONValue): TimespanMetric => {
-          let metric: TimespanMetric;
-          try {
-            metric = new TimespanMetric(old);
-            // If creating the metric didn't error,
-            // there is a valid timespan already recorded for this metric.
-            reportValueExists = true;
-          } catch {
-            metric = new TimespanMetric({
-              timespan: elapsed,
-              timeUnit: this.timeUnit,
-            });
-          }
-
-          return metric;
-        };
-      })(elapsed);
-
-      await Context.metricsDatabase.transform(this, transformFn);
-
-      if (reportValueExists) {
-        // TODO: record error once Bug 1682574 is resolved.
-        console.error("Timespan value already recorded. New value discarded.");
-      }
+      await TimespanMetricType._private_setRawUndispatched(this, elapsed);
     });
   }
 
@@ -172,6 +198,27 @@ class TimespanMetricType extends MetricType {
     });
   }
 
+  /**
+   * Explicitly sets the timespan value.
+   *
+   * This API should only be used if your library or application requires
+   * recording times in a way that can not make use of
+   * {@link TimespanMetricType#start}/{@link TimespanMetricType#stop}.
+   *
+   * Care should be taken using this if the ping lifetime might contain more
+   * than one timespan measurement. To be safe, this function should generally
+   * be followed by sending a custom ping containing the timespan.
+   *
+   * @param elapsed The elapsed time to record, in nanoseconds.
+   */
+  setRawNanos(elapsed: number): void {
+    Context.dispatcher.launch(async () => {
+      // `elapsed` is in nanoseconds in order to match the glean-core API.
+      const elapsedMillis = elapsed * 10**(-6);
+      await TimespanMetricType._private_setRawUndispatched(this, elapsedMillis);
+    });
+  }
+
   /**
    * **Test-only API.**
    *

glean/tests/core/metrics/timespan.spec.ts

@@ -297,4 +297,39 @@ describe("TimespanMetric", function() {
 
     assert.deepStrictEqual(consoleErrorSpy.callCount, 1);
   });
+
+  it("setting raw time works correctly", async function () {
+    const metric = new TimespanMetricType({
+      category: "aCategory",
+      name: "aTimespan",
+      sendInPings: ["aPing"],
+      lifetime: Lifetime.Ping,
+      disabled: false
+    }, "millisecond");
+
+    metric.setRawNanos(1000000); // 1ms
+    assert.strictEqual(await metric.testGetValue("aPing"), 1);
+  });
+
+  it("setting raw time does nothing when times is running", async function () {
+    fakeNow.onCall(0).callsFake(() => 0);
+    fakeNow.onCall(1).callsFake(() => 100);
+
+    const metric = new TimespanMetricType({
+      category: "aCategory",
+      name: "aTimespan",
+      sendInPings: ["aPing"],
+      lifetime: Lifetime.Ping,
+      disabled: false
+    }, "millisecond");
+
+    metric.start();
+    metric.setRawNanos(42);
+    metric.stop();
+
+    // We expect the start/stop value, not the raw value.
+    assert.strictEqual(await metric.testGetValue("aPing"), 100);
+
+    // TODO: check number of recorded errors instead once Bug 1682574 is resolved.
+  });
 });