Address review comments

Author: Dexterp37

CHANGELOG.md

@@ -4,7 +4,7 @@
 
 * [#101](https://github.com/mozilla/glean.js/pull/101): BUGFIX: Only validate Debug View Tag and Source Tags when they are present.
 * [#102](https://github.com/mozilla/glean.js/pull/102): BUGFIX: Include a Glean User-Agent header in all pings.
-* [#97](https://github.com/mozilla/glean.js/pull/97): Add support for labelled metric types (string, boolean and counter).
+* [#97](https://github.com/mozilla/glean.js/pull/97): Add support for labeled metric types (string, boolean and counter).
 
 # v0.4.0 (2021-03-10)
 

glean/src/core/metrics/database.ts

@@ -133,7 +133,7 @@ class MetricsDatabase {
     }
 
     const store = this._chooseStore(metric.lifetime);
-    const storageKey = await metric.getAsyncIdentifier();
+    const storageKey = await metric.identifier();
     for (const ping of metric.sendInPings) {
       const finalTransformFn = (v?: JSONValue): JSONValue => transformFn(v).get();
       await store.update([ping, metric.type, storageKey], finalTransformFn);
@@ -205,7 +205,7 @@ class MetricsDatabase {
     metric: MetricType
   ): Promise<T | undefined> {
     const store = this._chooseStore(metric.lifetime);
-    const storageKey = await metric.getAsyncIdentifier();
+    const storageKey = await metric.identifier();
     const value = await store.get([ping, metric.type, storageKey]);
     if (!isUndefined(value) && !validateMetricInternalRepresentation<T>(metric.type, value)) {
       console.error(`Unexpected value found for metric ${storageKey}: ${JSON.stringify(value)}. Clearing.`);

glean/src/core/metrics/index.ts

@@ -155,11 +155,11 @@ export abstract class MetricType implements CommonMetricData {
    * @returns The generated identifier. If `category` is empty, it's ommitted. Otherwise,
    *          it's the combination of the metric's `category`, `name` and `label`.
    */
-  async getAsyncIdentifier(): Promise<string> {
+  async identifier(): Promise<string> {
     const baseIdentifier = this.baseIdentifier();
 
     // We need to use `isUndefined` and cannot use `(this.dynamicLabel)` because we want
-    // empty strings to propagate as a dynamic labels, so that erros are potentially recorded.
+    // empty strings to propagate as dynamic labels, so that erros are potentially recorded.
     if (!isUndefined(this.dynamicLabel)) {
       return await LabeledMetricType.getValidDynamicLabel(this);
     } else {
@@ -178,7 +178,7 @@ export abstract class MetricType implements CommonMetricData {
 }
 
 /**
- * This is no-op internal metric representation.
+ * This is a no-op internal metric representation.
  *
  * This can be used to instruct the validators to simply report
  * whatever is stored internally, without performing any specific

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

@@ -55,10 +55,10 @@ class LabeledMetricType<T extends SupportedLabeledTypes> {
       // eslint-disable-next-line @typescript-eslint/no-explicit-any
       get: (_target: LabeledMetricType<T>, label: string): any => {
         if (labels) {
-          return LabeledMetricType.createFromStaticLabel<typeof submetric>(meta, submetric, labels, label);
+          return LabeledMetricType.createFromStaticLabel(meta, submetric, labels, label);
         }
 
-        return LabeledMetricType.createFromDynamicLabel<typeof submetric>(meta, submetric, label);
+        return LabeledMetricType.createFromDynamicLabel(meta, submetric, label);
       }
     });
   }

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

@@ -34,7 +34,7 @@ describe("LabeledMetric", function() {
       sendIfEmpty: false,
     });
 
-    const labeledCounterMetric = new LabeledMetricType<CounterMetricType>(
+    const labeledCounterMetric = new LabeledMetricType(
       {
         category: "telemetry",
         name: "labeled_counter_metric",
@@ -79,7 +79,7 @@ describe("LabeledMetric", function() {
       sendIfEmpty: false,
     });
 
-    const labeledCounterMetric = new LabeledMetricType<CounterMetricType>(
+    const labeledCounterMetric = new LabeledMetricType(
       {
         category: "telemetry",
         name: "labeled_counter_metric",
@@ -126,7 +126,7 @@ describe("LabeledMetric", function() {
   });
 
   it("test __other__ label without predefined labels", async function() {
-    const labeledCounterMetric = new LabeledMetricType<CounterMetricType>(
+    const labeledCounterMetric = new LabeledMetricType(
       {
         category: "telemetry",
         name: "labeled_counter_metric",
@@ -151,7 +151,7 @@ describe("LabeledMetric", function() {
   });
 
   it("test __other__ label without predefined labels before Glean initialization", async function() {
-    const labeledCounterMetric = new LabeledMetricType<CounterMetricType>(
+    const labeledCounterMetric = new LabeledMetricType(
       {
         category: "telemetry",
         name: "labeled_counter_metric",
@@ -181,7 +181,7 @@ describe("LabeledMetric", function() {
   });
 
   it("Ensure invalid labels on labeled counter go to __other__", async function() {
-    const labeledCounterMetric = new LabeledMetricType<CounterMetricType>(
+    const labeledCounterMetric = new LabeledMetricType(
       {
         category: "telemetry",
         name: "labeled_counter_metric",
@@ -203,7 +203,7 @@ describe("LabeledMetric", function() {
   });
 
   it("Ensure invalid labels on labeled boolean go to __other__", async function() {
-    const labeledBooleanMetric = new LabeledMetricType<BooleanMetricType>(
+    const labeledBooleanMetric = new LabeledMetricType(
       {
         category: "telemetry",
         name: "labeled_boolean_metric",
@@ -225,7 +225,7 @@ describe("LabeledMetric", function() {
   });
 
   it("Ensure invalid labels on labeled string go to __other__", async function() {
-    const labeledStringMetric = new LabeledMetricType<StringMetricType>(
+    const labeledStringMetric = new LabeledMetricType(
       {
         category: "telemetry",
         name: "labeled_string_metric",
@@ -247,7 +247,7 @@ describe("LabeledMetric", function() {
   });
 
   it("test labeled string metric type", async function() {
-    const labeledStringMetric = new LabeledMetricType<StringMetricType>(
+    const labeledStringMetric = new LabeledMetricType(
       {
         category: "telemetry",
         name: "labeled_string_metric",
@@ -269,7 +269,7 @@ describe("LabeledMetric", function() {
   });
 
   it("test labeled boolean metric type", async function() {
-    const metric = new LabeledMetricType<BooleanMetricType>(
+    const metric = new LabeledMetricType(
       {
         category: "telemetry",
         name: "labeled_bool",
@@ -291,7 +291,7 @@ describe("LabeledMetric", function() {
   });
 
   it("dynamic labels regex mismatch", async function() {
-    const labeledCounterMetric = new LabeledMetricType<CounterMetricType>(
+    const labeledCounterMetric = new LabeledMetricType(
       {
         category: "telemetry",
         name: "labeled_counter_metric",
@@ -320,7 +320,7 @@ describe("LabeledMetric", function() {
   });
 
   it("dynamic labels regex allowed", async function() {
-    const labeledCounterMetric = new LabeledMetricType<CounterMetricType>(
+    const labeledCounterMetric = new LabeledMetricType(
       {
         category: "telemetry",
         name: "labeled_counter_metric",
@@ -350,7 +350,7 @@ describe("LabeledMetric", function() {
   });
 
   it("seen labels get reloaded across initializations", async function() {
-    const labeledCounterMetric = new LabeledMetricType<CounterMetricType>(
+    const labeledCounterMetric = new LabeledMetricType(
       {
         category: "telemetry",
         name: "labeled_metric",