Skip to content

Commit d400397

Browse files
author
Beatriz Rizental
authored
Bug 1679379 - Implement the boolean metric type (#11)
1 parent 3b91cb0 commit d400397

File tree

14 files changed

+296
-54
lines changed

14 files changed

+296
-54
lines changed

‎.eslintrc‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,8 @@
2020
"no-debugger": ["error"],
2121
"jsdoc/no-types": "off",
2222
"jsdoc/require-param-type": "off",
23-
"jsdoc/require-returns-type": "off"
23+
"jsdoc/require-returns-type": "off",
24+
"jsdoc/check-param-names": ["error"]
2425
},
2526
"extends": [
2627
"plugin:jsdoc/recommended"

‎package.json‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,8 @@
44
"description": "An implementation of the Glean SDK, a modern cross-platform telemetry client, for Javascript environments.",
55
"main": "./dist/glean.js",
66
"scripts": {
7-
"test": "npm run build:test-webext && ts-mocha tests/**/*.spec.ts --paths -p ./tsconfig.json --timeout 0",
8-
"test:debug": "ts-mocha tests/**/*.spec.ts --paths -p ./tsconfig.json --inspect-brk",
7+
"test": "npm run build:test-webext && ts-mocha \"tests/**/*.spec.ts\" --paths -p ./tsconfig.json --recursive --timeout 0",
8+
"test:debug": "ts-mocha \"tests/**/*.spec.ts\" --paths -p ./tsconfig.json --recursive --inspect-brk",
99
"lint": "eslint . --ext .ts,.js,.json",
1010
"fix": "eslint . --ext .ts,.js,.json --fix",
1111
"build:webext": "webpack --config webpack.config.webext.js --mode production",

‎src/database.ts‎

Lines changed: 27 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -5,11 +5,12 @@
55
import { StorageValue, Store } from "storage";
66
import PersistentStore from "storage/persistent";
77
import Metric, { Lifetime } from "metrics";
8-
import { isObject, isString, isUndefined } from "utils";
8+
import { MetricPayload, isMetricPayload } from "metrics/payload";
9+
import { isObject, isUndefined } from "utils";
910

1011
export interface PingPayload {
1112
[aMetricType: string]: {
12-
[aMetricIdentifier: string]: string
13+
[aMetricIdentifier: string]: MetricPayload
1314
}
1415
}
1516

@@ -20,14 +21,14 @@ export interface PingPayload {
2021
*
2122
* @returns Whether or not `v` is a valid PingPayload.
2223
*/
23-
export function isValidPingPayload(v: StorageValue): v is PingPayload {
24+
export function isValidPingPayload(v: unknown): v is PingPayload {
2425
if (isObject(v)) {
2526
// The root keys should all be metric types.
2627
for (const metricType in v) {
2728
const metrics = v[metricType];
2829
if (isObject(metrics)) {
2930
for (const metricIdentifier in metrics) {
30-
if (!isString(metrics[metricIdentifier])) {
31+
if (!isMetricPayload(metricType, metrics[metricIdentifier])) {
3132
return false;
3233
}
3334
}
@@ -98,16 +99,8 @@ class Database {
9899
* @param metric The metric to record to.
99100
* @param value The value we want to record to the given metric.
100101
*/
101-
async record(metric: Metric, value: string): Promise<void> {
102-
if (metric.disabled) {
103-
return;
104-
}
105-
106-
const store = this._chooseStore(metric.lifetime);
107-
const storageKey = metric.identifier;
108-
for (const ping of metric.sendInPings) {
109-
await store.update([ping, metric.type, storageKey], () => value);
110-
}
102+
async record(metric: Metric, value: MetricPayload): Promise<void> {
103+
await this.transform(metric, () => value);
111104
}
112105

113106
/**
@@ -117,17 +110,17 @@ class Database {
117110
* @param metric The metric to record to.
118111
* @param transformFn The transformation function to apply to the currently persisted value.
119112
*/
120-
async transform(metric: Metric, transformFn: (v?: string) => string): Promise<void> {
113+
async transform(metric: Metric, transformFn: (v?: MetricPayload) => MetricPayload): Promise<void> {
121114
if (metric.disabled) {
122115
return;
123116
}
124117

125118
const store = this._chooseStore(metric.lifetime);
126119
const storageKey = metric.identifier;
127120
for (const ping of metric.sendInPings) {
128-
const finalTransformFn = (v: StorageValue): string => {
129-
if (isObject(v)) {
130-
throw new Error(`Unexpected value found for metric ${metric}: ${JSON.stringify(v)}.`);
121+
const finalTransformFn = (v: StorageValue): Exclude<StorageValue, undefined> => {
122+
if (!isUndefined(v) && !isMetricPayload(metric.type, v)) {
123+
throw new Error(`Unexpected value found for metric ${metric.identifier}: ${JSON.stringify(v)}.`);
131124
}
132125
return transformFn(v);
133126
};
@@ -139,17 +132,29 @@ class Database {
139132
* Gets the persisted payload of a given metric in a given ping.
140133
*
141134
* @param ping The ping from which we want to retrieve the given metric.
135+
* @param validateFn A validation function to verify if persisted payload is of the correct type.
142136
* @param metric An object containing the information about the metric to retrieve.
143137
*
144-
* @returns The string encoded payload persisted for the given metric,
138+
* @returns The payload persisted for the given metric,
145139
* `undefined` in case the metric has not been recorded yet.
146140
*/
147-
async getMetric(ping: string, metric: Metric): Promise<string | undefined> {
141+
async getMetric<T>(
142+
ping: string,
143+
validateFn: (v: unknown) => v is T,
144+
metric: Metric
145+
): Promise<T | undefined> {
148146
const store = this._chooseStore(metric.lifetime);
149147
const storageKey = metric.identifier;
150148
const value = await store.get([ping, metric.type, storageKey]);
151-
if (isObject(value)) {
152-
console.error(`Unexpected value found for metric ${metric}: ${JSON.stringify(value)}. Clearing.`);
149+
if (!isUndefined(value) && !validateFn(value)) {
150+
// The following behaviour is not consistent with what the Glean SDK does, but this is on purpose.
151+
// On the Glean SDK we panic when we can't serialize the given,
152+
// that is because this is a extremely unlikely situation for that environment.
153+
//
154+
// Since Glean.js will run on the browser, it is easy for a user to mess with the persisted data
155+
// which makes this sort of errors plausible. That is why we choose to not panic and
156+
// simply delete the corrupted data here.
157+
console.error(`Unexpected value found for metric ${metric.identifier}: ${JSON.stringify(value)}. Clearing.`);
153158
await store.delete([ping, metric.type, storageKey]);
154159
return;
155160
} else {

‎src/glean.ts‎

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
/* This Source Code Form is subject to the terms of the Mozilla Public
2+
* License, v. 2.0. If a copy of the MPL was not distributed with this
3+
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */
4+
5+
import Database from "database";
6+
import { isUndefined } from "utils";
7+
8+
class Glean {
9+
// The Glean singleton.
10+
private static _instance?: Glean;
11+
12+
// The metrics database.
13+
private _db: Database;
14+
// Whether or not to record metrics.
15+
private _uploadEnabled: boolean;
16+
17+
private constructor() {
18+
if (!isUndefined(Glean._instance)) {
19+
throw new Error(
20+
`Tried to instantiate Glean through \`new\`.
21+
Use Glean.instance instead to access the Glean singleton.`);
22+
}
23+
24+
this._db = new Database();
25+
// Temporarily setting this to true always, until Bug 1677444 is resolved.
26+
this._uploadEnabled = true;
27+
}
28+
29+
private static get instance(): Glean {
30+
if (!Glean._instance) {
31+
Glean._instance = new Glean();
32+
}
33+
34+
return Glean._instance;
35+
}
36+
37+
38+
static get db(): Database {
39+
return Glean.instance._db;
40+
}
41+
42+
// TODO: Make the following functions `private` once Bug 1682769 is resolved.
43+
static get uploadEnabled(): boolean {
44+
return Glean.instance._uploadEnabled;
45+
}
46+
47+
static set uploadEnabled(value: boolean) {
48+
Glean.instance._uploadEnabled = value;
49+
}
50+
}
51+
52+
export default Glean;

‎src/index.ts‎

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -2,16 +2,6 @@
22
* License, v. 2.0. If a copy of the MPL was not distributed with this
33
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */
44

5-
// Importing this here just so the size increase will show on the PR comments,
6-
// once everything is implemented we remove it.
7-
import StorageWeak from "storage/weak";
8-
import StoragePersistent from "storage/persistent";
9-
// If we leave the above imports unused they will not be added to the final webpack bundle.
10-
console.log(
11-
StorageWeak,
12-
StoragePersistent
13-
);
14-
155
export = {
166
/**
177
* Initializes Glean.

‎src/metrics/boolean.ts‎

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,58 @@
1+
/* This Source Code Form is subject to the terms of the Mozilla Public
2+
* License, v. 2.0. If a copy of the MPL was not distributed with this
3+
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */
4+
5+
import Metric, { CommonMetricData } from "metrics";
6+
import Glean from "glean";
7+
import { isBoolean } from "utils";
8+
9+
export type BooleanMetricPayload = boolean;
10+
/**
11+
* Checks whether or not `v` is a valid boolean metric payload.
12+
*
13+
* @param v The value to verify.
14+
*
15+
* @returns A special Typescript value (which compiles down to a boolean)
16+
* stating wether `v` is a valid boolean metric payload.
17+
*/
18+
export function isBooleanMetricPayload(v: unknown): v is BooleanMetricPayload {
19+
return isBoolean(v);
20+
}
21+
22+
class BooleanMetric extends Metric {
23+
constructor(meta: CommonMetricData) {
24+
super("boolean", meta);
25+
}
26+
27+
/**
28+
* Sets to the specified boolean value.
29+
*
30+
* @param value the value to set.
31+
*/
32+
async set(value: BooleanMetricPayload): Promise<void> {
33+
if (!this.shouldRecord()) {
34+
return;
35+
}
36+
37+
await Glean.db.record(this, value);
38+
}
39+
40+
/**
41+
* **Test-only API (exported for FFI purposes).**
42+
*
43+
* Gets the currently stored value as a boolean.
44+
*
45+
* This doesn't clear the stored value.
46+
*
47+
* TODO: Only allow this function to be called on test mode (depends on Bug 1682771).
48+
*
49+
* @param ping the ping from which we want to retrieve this metrics value from.
50+
*
51+
* @returns The value found in storage or `undefined` if nothing was found.
52+
*/
53+
async testGetValue(ping: string): Promise<BooleanMetricPayload | undefined> {
54+
return Glean.db.getMetric(ping, isBooleanMetricPayload, this);
55+
}
56+
}
57+
58+
export default BooleanMetric;

‎src/metrics/index.ts‎

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,8 @@
22
* License, v. 2.0. If a copy of the MPL was not distributed with this
33
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */
44

5+
import Glean from "glean";
6+
57
/**
68
* An enum representing the possible metric lifetimes.
79
*/
@@ -17,7 +19,7 @@ export const enum Lifetime {
1719
/**
1820
* The common set of data shared across all different metric types.
1921
*/
20-
interface CommonMetricData {
22+
export interface CommonMetricData {
2123
// The metric's name.
2224
readonly name: string,
2325
// The metric's category.
@@ -65,6 +67,15 @@ class Metric implements CommonMetricData {
6567
return this.name;
6668
}
6769
}
70+
71+
/**
72+
* Verify if whether or not this metric instance should be recorded to a given Glean instance.
73+
*
74+
* @returns Whether or not this metric instance should be recorded.
75+
*/
76+
shouldRecord(): boolean {
77+
return (Glean.uploadEnabled && !this.disabled);
78+
}
6879
}
6980

7081
export default Metric;

‎src/metrics/payload.ts‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
/* This Source Code Form is subject to the terms of the Mozilla Public
2+
* License, v. 2.0. If a copy of the MPL was not distributed with this
3+
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */
4+
5+
import { BooleanMetricPayload, isBooleanMetricPayload } from "metrics/boolean";
6+
import { isString } from "utils";
7+
8+
/**
9+
* Validates that a given value is the correct type of payload for a metric of a given type.
10+
*
11+
* @param type The type of the metric to validate
12+
* @param v The value to verify
13+
*
14+
* @returns Whether or not `v` is of the correct type.
15+
*/
16+
export function isMetricPayload(type: string, v: unknown): v is MetricPayload {
17+
switch (type) {
18+
case "boolean":
19+
return isBooleanMetricPayload(v);
20+
default:
21+
return isString(v);
22+
}
23+
}
24+
25+
// Leaving the `string` as a valid metric payload here so that tests keep working for now.
26+
export type MetricPayload = BooleanMetricPayload | string;
27+

‎src/storage/index.ts‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
* License, v. 2.0. If a copy of the MPL was not distributed with this
33
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */
44

5-
import { isString, isUndefined, isObject } from "utils";
5+
import { isString, isUndefined, isObject, isBoolean } from "utils";
66

77
/**
88
* The storage index in the ordered list of keys to navigate on the store
@@ -28,7 +28,7 @@ export type StorageIndex = string[];
2828
/**
2929
* The possible values to be retrievd from storage.
3030
*/
31-
export type StorageValue = undefined | string | StorageObject;
31+
export type StorageValue = undefined | string | boolean | StorageObject;
3232
export interface StorageObject {
3333
[key: string]: StorageValue;
3434
}
@@ -42,10 +42,10 @@ export interface StorageObject {
4242
* stating wether `v` is a valid StorageValue.
4343
*/
4444
export function isStorageValue(v: unknown): v is StorageValue {
45-
if (isUndefined(v) || isString(v)) {
45+
if (isUndefined(v) || isString(v) || isBoolean(v)) {
4646
return true;
4747
}
48-
48+
4949
if (isObject(v)) {
5050
if (Object.keys(v).length === 0) {
5151
return true;

‎src/storage/persistent/webext.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44

55
import { Store, StorageIndex, StorageValue, StorageObject, isStorageValue } from "storage";
66
import { updateNestedObject, getValueFromNestedObject, deleteKeyFromNestedObject } from "storage/utils";
7-
import { isString, isUndefined } from "utils";
7+
import { isObject } from "utils";
88

99
type WebExtStoreQuery = { [x: string]: { [x: string]: null; } | null; };
1010

@@ -75,7 +75,7 @@ class WebExtStore implements Store {
7575
}
7676

7777
if (isStorageValue(response)) {
78-
if (!isUndefined(response) && !isString(response)) {
78+
if (isObject(response)) {
7979
return getValueFromNestedObject(response, [ this.rootKey, ...index ]);
8080
} else {
8181
return response;

0 commit comments

Comments
 (0)