mozilla · Dexterp37 · Apr 13, 2021 · Apr 12, 2021
diff --git a/glean/src/core/metrics/database.ts b/glean/src/core/metrics/database.ts
@@ -131,7 +131,7 @@ class MetricsDatabase {
     }
 
     const store = this._chooseStore(metric.lifetime);
-    const storageKey = await metric.identifier();
+    const storageKey = await metric.identifier(this);
     for (const ping of metric.sendInPings) {
       const finalTransformFn = (v?: JSONValue): JSONValue => transformFn(v).get();
       await store.update([ping, metric.type, storageKey], finalTransformFn);
@@ -203,7 +203,7 @@ class MetricsDatabase {
     metric: MetricType
   ): Promise<T | undefined> {
     const store = this._chooseStore(metric.lifetime);
-    const storageKey = await metric.identifier();
+    const storageKey = await metric.identifier(this);
     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.`);

diff --git a/glean/src/core/metrics/index.ts b/glean/src/core/metrics/index.ts
@@ -4,9 +4,10 @@
 
 import type { JSONValue } from "../utils.js";
 import { isUndefined } from "../utils.js";
-import LabeledMetricType from "./types/labeled.js";
 import { Metric } from "./metric.js";
 import type { Lifetime } from "./lifetime.js";
+import type MetricsDatabase from "./database.js";
+import { getValidDynamicLabel } from "./types/labeled_utils.js";
 
 /**
  * The common set of data shared across all different metric types.
@@ -73,16 +74,18 @@ export abstract class MetricType implements CommonMetricData {
   /**
    * The metric's unique identifier, including the category, name and label.
    *
+   * @param metricsDatabase The metrics database.
+   *
    * @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 identifier(): Promise<string> {
+  async identifier(metricsDatabase: MetricsDatabase): Promise<string> {
     const baseIdentifier = this.baseIdentifier();
 
     // We need to use `isUndefined` and cannot use `(this.dynamicLabel)` because we want
     // empty strings to propagate as dynamic labels, so that erros are potentially recorded.
     if (!isUndefined(this.dynamicLabel)) {
-      return await LabeledMetricType.getValidDynamicLabel(this);
+      return await getValidDynamicLabel(metricsDatabase, this);
     } else {
       return baseIdentifier;
     }

diff --git a/glean/src/core/metrics/types/labeled.ts b/glean/src/core/metrics/types/labeled.ts
@@ -2,40 +2,11 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
-import type { CommonMetricData, MetricType } from "../index.js";
-import Glean from "../../glean.js";
+import type { CommonMetricData } from "../index.js";
 import type CounterMetricType from "./counter.js";
 import type BooleanMetricType from "./boolean.js";
 import type StringMetricType from "./string.js";
-
-const MAX_LABELS = 16;
-const OTHER_LABEL = "__other__";
-const MAX_LABEL_LENGTH = 61;
-
-// ** IMPORTANT **
-// When changing this documentation or the regex, be sure to change the same code
-// in the Glean SDK repository as well.
-//
-// This regex is used for matching against labels and should allow for dots,
-// underscores, and/or hyphens. Labels are also limited to starting with either
-// a letter or an underscore character.
-//
-// Some examples of good and bad labels:
-//
-// Good:
-// * `this.is.fine`
-// * `this_is_fine_too`
-// * `this.is_still_fine`
-// * `thisisfine`
-// * `_.is_fine`
-// * `this.is-fine`
-// * `this-is-fine`
-// Bad:
-// * `this.is.not_fine_due_tu_the_length_being_too_long_i_thing.i.guess`
-// * `1.not_fine`
-// * `this.$isnotfine`
-// * `-.not_fine`
-const LABEL_REGEX = /^[a-z_][a-z0-9_-]{0,29}(\.[a-z_][a-z0-9_-]{0,29})*$/;
+import { combineIdentifierAndLabel, OTHER_LABEL } from "./labeled_utils.js";
 
 type SupportedLabeledTypes = CounterMetricType | BooleanMetricType | StringMetricType;
 
@@ -62,21 +33,6 @@ class LabeledMetricType<T extends SupportedLabeledTypes> {
     });
   }
 
-  /**
-   * Combines a metric's base identifier and label.
-   *
-   * @param metricName the metric base identifier
-   * @param label the label
-   *
-   * @returns a string representing the complete metric id including the label.
-   */
-  private static combineIdentifierAndLabel(
-    metricName: string,
-    label: string
-  ): string {
-    return `${metricName}/${label}`;
-  }
-
   /**
    * Create an instance of the submetric type for the provided static label.
    *
@@ -99,7 +55,7 @@ class LabeledMetricType<T extends SupportedLabeledTypes> {
     const adjustedLabel = allowedLabels.includes(label) ? label : OTHER_LABEL;
     const newMeta: CommonMetricData = {
       ...meta,
-      name: LabeledMetricType.combineIdentifierAndLabel(meta.name, adjustedLabel)
+      name: combineIdentifierAndLabel(meta.name, adjustedLabel)
     };
     return new submetricClass(newMeta);
   }
@@ -125,58 +81,6 @@ class LabeledMetricType<T extends SupportedLabeledTypes> {
     };
     return new submetricClass(newMeta);
   }
-
-  /**
-   * Checks if the dynamic label stored in the metric data is
-   * valid. If not, record an error and store data in the "__other__"
-   * label.
-   *
-   * @param metric the metric metadata.
-   *
-   * @returns a valid label that can be used to store data.
-   */
-  static async getValidDynamicLabel(metric: MetricType): Promise<string> {
-    // Note that we assume `metric.dynamicLabel` to always be available within this function.
-    // This is a safe assumptions because we should only call `getValidDynamicLabel` if we have
-    // a dynamic label.
-    if (metric.dynamicLabel === undefined) {
-      throw new Error("This point should never be reached.");
-    }
-
-    const key = LabeledMetricType.combineIdentifierAndLabel(metric.baseIdentifier(), metric.dynamicLabel);
-
-    for (const ping of metric.sendInPings) {
-      if (await Glean.metricsDatabase.hasMetric(metric.lifetime, ping, metric.type, key)) {
-        return key;
-      }
-    }
-
-    let numUsedKeys = 0;
-    for (const ping of metric.sendInPings) {
-      numUsedKeys += await Glean.metricsDatabase.countByBaseIdentifier(
-        metric.lifetime,
-        ping,
-        metric.type,
-        metric.baseIdentifier());
-    }
-
-    let hitError = false;
-    if (numUsedKeys >= MAX_LABELS) {
-      hitError = true;
-    } else if (metric.dynamicLabel.length > MAX_LABEL_LENGTH) {
-      console.error(`label length ${metric.dynamicLabel.length} exceeds maximum of ${MAX_LABEL_LENGTH}`);
-      hitError = true;
-      // TODO: record error in bug 1682574
-    } else if (!LABEL_REGEX.test(metric.dynamicLabel)) {
-      console.error(`label must be snake_case, got '${metric.dynamicLabel}'`);
-      hitError = true;
-      // TODO: record error in bug 1682574
-    }
-
-    return (hitError)
-      ? LabeledMetricType.combineIdentifierAndLabel(metric.baseIdentifier(), OTHER_LABEL)
-      : key;
-  }
 }
 
 export default LabeledMetricType;
diff --git a/glean/src/core/metrics/types/labeled_utils.ts b/glean/src/core/metrics/types/labeled_utils.ts
@@ -0,0 +1,104 @@
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+import type { MetricType } from "..";
+import type MetricsDatabase from "../database";
+
+const MAX_LABELS = 16;
+const MAX_LABEL_LENGTH = 61;
+export const OTHER_LABEL = "__other__";
+
+// ** IMPORTANT **
+// When changing this documentation or the regex, be sure to change the same code
+// in the Glean SDK repository as well.
+//
+// This regex is used for matching against labels and should allow for dots,
+// underscores, and/or hyphens. Labels are also limited to starting with either
+// a letter or an underscore character.
+//
+// Some examples of good and bad labels:
+//
+// Good:
+// * `this.is.fine`
+// * `this_is_fine_too`
+// * `this.is_still_fine`
+// * `thisisfine`
+// * `_.is_fine`
+// * `this.is-fine`
+// * `this-is-fine`
+// Bad:
+// * `this.is.not_fine_due_tu_the_length_being_too_long_i_thing.i.guess`
+// * `1.not_fine`
+// * `this.$isnotfine`
+// * `-.not_fine`
+const LABEL_REGEX = /^[a-z_][a-z0-9_-]{0,29}(\.[a-z_][a-z0-9_-]{0,29})*$/;
+
+/**
+ * Combines a metric's base identifier and label.
+ *
+ * @param metricName the metric base identifier
+ * @param label the label
+ *
+ * @returns a string representing the complete metric id including the label.
+ */
+export function combineIdentifierAndLabel(
+  metricName: string,
+  label: string
+): string {
+  return `${metricName}/${label}`;
+}
+
+/**
+ * Checks if the dynamic label stored in the metric data is
+ * valid. If not, record an error and store data in the "__other__"
+ * label.
+ *
+ * @param metricsDatabase the metrics database.
+ * @param metric the metric metadata.
+ *
+ * @returns a valid label that can be used to store data.
+ */
+export async function getValidDynamicLabel(metricsDatabase: MetricsDatabase, metric: MetricType): Promise<string> {
+  // Note that we assume `metric.dynamicLabel` to always be available within this function.
+  // This is a safe assumptions because we should only call `getValidDynamicLabel` if we have
+  // a dynamic label.
+  if (metric.dynamicLabel === undefined) {
+    throw new Error("This point should never be reached.");
+  }
+
+  const key = combineIdentifierAndLabel(metric.baseIdentifier(), metric.dynamicLabel);
+
+  for (const ping of metric.sendInPings) {
+    if (await metricsDatabase.hasMetric(metric.lifetime, ping, metric.type, key)) {
+      return key;
+    }
+  }
+
+  let numUsedKeys = 0;
+  for (const ping of metric.sendInPings) {
+    numUsedKeys += await metricsDatabase.countByBaseIdentifier(
+      metric.lifetime,
+      ping,
+      metric.type,
+      metric.baseIdentifier());
+  }
+
+  let hitError = false;
+  if (numUsedKeys >= MAX_LABELS) {
+    hitError = true;
+  } else if (metric.dynamicLabel.length > MAX_LABEL_LENGTH) {
+    console.error(`label length ${metric.dynamicLabel.length} exceeds maximum of ${MAX_LABEL_LENGTH}`);
+    hitError = true;
+    // TODO: record error in bug 1682574
+  } else if (!LABEL_REGEX.test(metric.dynamicLabel)) {
+    console.error(`label must be snake_case, got '${metric.dynamicLabel}'`);
+    hitError = true;
+    // TODO: record error in bug 1682574
+  }
+
+  return (hitError)
+    ? combineIdentifierAndLabel(metric.baseIdentifier(), OTHER_LABEL)
+    : key;
+}
+