Skip to content

Commit cc00636

Browse files
author
Beatriz Rizental
authored
Merge pull request #614 from ChinYing-Li/bug_1716956
Bug 1716956: Initial attempt to implement metric type "StringList"
2 parents 3165219 + 3091685 commit cc00636

File tree

7 files changed

+365
-0
lines changed

7 files changed

+365
-0
lines changed

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
* [#647](https://github.com/mozilla/glean.js/pull/647): Implement the Text metric type.
1515
* [#681](https://github.com/mozilla/glean.js/pull/681): BUGFIX: Fix error in scanning events database upon initialization on Qt/QML.
1616
* This bug prevents the changes introduced in [#526](https://github.com/mozilla/glean.js/pull/526) from working properly.
17+
* [#614](https://github.com/mozilla/glean.js/pull/614): Implement the String List metric type.
1718

1819
# v0.18.1 (2021-07-22)
1920

Lines changed: 166 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,166 @@
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 type { CommonMetricData } from "../index.js";
6+
import { MetricType } from "../index.js";
7+
import { Context } from "../../context.js";
8+
import { Metric } from "../metric.js";
9+
import { isString, truncateStringAtBoundaryWithError } from "../../utils.js";
10+
import type { JSONValue } from "../../utils.js";
11+
import { ErrorType } from "../../error/error_type.js";
12+
13+
export const MAX_LIST_LENGTH = 20;
14+
export const MAX_STRING_LENGTH = 50;
15+
16+
export class StringListMetric extends Metric<string[], string[]> {
17+
constructor(v: unknown) {
18+
super(v);
19+
}
20+
21+
validate(v: unknown): v is string[] {
22+
if (!Array.isArray(v)) {
23+
return false;
24+
}
25+
26+
if (v.length > MAX_LIST_LENGTH) {
27+
return false;
28+
}
29+
30+
for (const s of v) {
31+
if (!isString(s) || s.length > MAX_STRING_LENGTH) {
32+
return false;
33+
}
34+
}
35+
36+
return true;
37+
}
38+
39+
payload(): string[] {
40+
return this._inner;
41+
}
42+
}
43+
44+
/**
45+
* A string list metric.
46+
*
47+
* This allows appending a string value with arbitrary content to a list.
48+
* The list is length-limited to `MAX_LIST_LENGTH`.
49+
* Strings are length-limited to `MAX_STRING_LENGTH` characters.
50+
*/
51+
class StringListMetricType extends MetricType {
52+
constructor(meta: CommonMetricData) {
53+
super("string_list", meta);
54+
}
55+
56+
/**
57+
* Sets to the specified string list value.
58+
*
59+
* # Note
60+
*
61+
* Truncates the list if it is longer than `MAX_LIST_LENGTH` and records an error.
62+
*
63+
* Truncates the value if it is longer than `MAX_STRING_LENGTH` characters
64+
* and records an error.
65+
*
66+
* @param value The list of strings to set the metric to.
67+
*/
68+
set(value: string[]): void {
69+
Context.dispatcher.launch(async () => {
70+
if (!this.shouldRecord(Context.uploadEnabled)) {
71+
return;
72+
}
73+
74+
const truncatedList: string[] = [];
75+
if (value.length > MAX_LIST_LENGTH) {
76+
await Context.errorManager.record(
77+
this,
78+
ErrorType.InvalidValue,
79+
`String list length of ${value.length} exceeds maximum of ${MAX_LIST_LENGTH}.`
80+
);
81+
}
82+
83+
for (let i = 0; i < Math.min(value.length, MAX_LIST_LENGTH); ++i) {
84+
const truncatedString = await truncateStringAtBoundaryWithError(this, value[i], MAX_STRING_LENGTH);
85+
truncatedList.push(truncatedString);
86+
}
87+
const metric = new StringListMetric(truncatedList);
88+
await Context.metricsDatabase.record(this, metric);
89+
});
90+
}
91+
92+
/**
93+
* Adds a new string `value` to the list.
94+
*
95+
* # Note
96+
*
97+
* - If the list is already of length `MAX_LIST_LENGTH`, record an error.
98+
* - Truncates the value if it is longer than `MAX_STRING_LENGTH` characters
99+
* and records an error.
100+
*
101+
* @param value The string to add.
102+
*/
103+
add(value: string): void {
104+
Context.dispatcher.launch(async () => {
105+
if (!this.shouldRecord(Context.uploadEnabled)) {
106+
return;
107+
}
108+
109+
const truncatedValue = await truncateStringAtBoundaryWithError(this, value, MAX_STRING_LENGTH);
110+
let currentLen = 0;
111+
112+
const transformFn = ((value) => {
113+
return (v?: JSONValue): StringListMetric => {
114+
let metric: StringListMetric;
115+
let result: string[];
116+
try {
117+
metric = new StringListMetric(v);
118+
result = metric.get();
119+
currentLen = result.length;
120+
if (result.length < MAX_LIST_LENGTH) {
121+
result.push(value);
122+
}
123+
} catch {
124+
metric = new StringListMetric([value]);
125+
result = [value];
126+
}
127+
metric.set(result);
128+
return metric;
129+
};
130+
})(truncatedValue);
131+
132+
await Context.metricsDatabase.transform(this, transformFn);
133+
134+
if (currentLen >= MAX_LIST_LENGTH) {
135+
await Context.errorManager.record(
136+
this,
137+
ErrorType.InvalidValue,
138+
`String list length of ${currentLen+1} exceeds maximum of ${MAX_LIST_LENGTH}.`
139+
);
140+
}
141+
});
142+
}
143+
144+
/**
145+
* Test-only API**
146+
*
147+
* Gets the currently stored value as a string array.
148+
*
149+
* This doesn't clear the stored value.
150+
*
151+
* TODO: Only allow this function to be called on test mode (depends on Bug 1682771).
152+
*
153+
* @param ping the ping from which we want to retrieve this metrics value from.
154+
* Defaults to the first value in `sendInPings`.
155+
* @returns The value found in storage or `undefined` if nothing was found.
156+
*/
157+
async testGetValue(ping: string = this.sendInPings[0]): Promise<string[] | undefined> {
158+
let metric: string[] | undefined;
159+
await Context.dispatcher.testLaunch(async () => {
160+
metric = await Context.metricsDatabase.getMetric<string[]>(ping, this);
161+
});
162+
return metric;
163+
}
164+
}
165+
166+
export default StringListMetricType;

glean/src/core/metrics/utils.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import { CounterMetric } from "./types/counter.js";
1212
import { DatetimeMetric } from "./types/datetime.js";
1313
import { QuantityMetric } from "./types/quantity.js";
1414
import { StringMetric } from "./types/string.js";
15+
import { StringListMetric } from "./types/string_list.js";
1516
import { TextMetric } from "./types/text.js";
1617
import { TimespanMetric } from "./types/timespan.js";
1718
import { UrlMetric } from "./types/url.js";
@@ -31,6 +32,7 @@ const METRIC_MAP: {
3132
"labeled_string": LabeledMetric,
3233
"quantity": QuantityMetric,
3334
"string": StringMetric,
35+
"string_list": StringListMetric,
3436
"text": TextMetric,
3537
"timespan": TimespanMetric,
3638
"url": UrlMetric,

glean/src/index/qt.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import EventMetricType from "../core/metrics/types/event.js";
1717
import LabeledMetricType from "../core/metrics/types/labeled.js";
1818
import QuantityMetricType from "../core/metrics/types/quantity.js";
1919
import StringMetricType from "../core/metrics/types/string.js";
20+
import StringListMetricType from "../core/metrics/types/string_list.js";
2021
import TextMetricType from "../core/metrics/types/text.js";
2122
import TimespanMetricType from "../core/metrics/types/timespan.js";
2223
import UUIDMetricType from "../core/metrics/types/uuid.js";
@@ -124,6 +125,7 @@ export default {
124125
LabeledMetricType,
125126
QuantityMetricType,
126127
StringMetricType,
128+
StringListMetricType,
127129
TimespanMetricType,
128130
TextMetricType,
129131
UUIDMetricType,

glean/tests/integration/schema/metrics.yaml

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,19 @@ for_testing:
8484
expires: never
8585
send_in_pings:
8686
- testing
87+
string_list:
88+
type: string_list
89+
description: |
90+
Sample string list metric.
91+
bugs:
92+
- https://bugzilla.mozilla.org/000000
93+
data_reviews:
94+
- https://bugzilla.mozilla.org/show_bug.cgi?id=000000#c3
95+
notification_emails:
96+
- me@mozilla.com
97+
expires: never
98+
send_in_pings:
99+
- testing
87100
timespan:
88101
type: timespan
89102
description: |

glean/tests/integration/schema/schema.spec.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,7 @@ describe("schema", function() {
7070
metrics.labeledString["a_label"].set("ho");
7171
metrics.quantity.set(42);
7272
metrics.string.set("let's go");
73+
metrics.stringList.set(["let's go"]);
7374
metrics.text.set("let's gooooooooo");
7475
metrics.timespan.setRawNanos(10 * 10**6);
7576
metrics.uuid.generateAndSet();

0 commit comments

Comments
 (0)