Skip to content

Commit e97ba46

Browse files
authored
Merge pull request #227 from Dexterp37/fix_labeled
Fix labeled booleans and string
2 parents 9491671 + 4a3ac05 commit e97ba46

File tree

4 files changed

+64
-4
lines changed

4 files changed

+64
-4
lines changed

CHANGELOG.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,8 @@
22

33
[Full changelog](https://github.com/mozilla/glean.js/compare/v0.9.2...main)
44

5+
* [#227](https://github.com/mozilla/glean.js/pull/227): BUGFIX: Fix a bug that prevented using `labeled_string` and `labeled_boolean`.
6+
57
# v0.9.2 (2021-04-19)
68

79
[Full changelog](https://github.com/mozilla/glean.js/compare/v0.9.1...v0.9.2)

glean/src/cli.ts

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -141,8 +141,10 @@ async function createPythonVenv(venvPath: string): Promise<boolean> {
141141
exec.exec(cmd, (err, stdout, stderr) => resolve({err, stdout, stderr}));
142142
});
143143

144+
console.log(`${stdout}`);
145+
144146
if (err) {
145-
console.error(`${stdout}\n${stderr}`);
147+
console.error(`${stderr}`);
146148
return false;
147149
}
148150
}
@@ -182,8 +184,10 @@ async function runGlean(projectRoot: string, parserArgs: string[]) {
182184
exec.exec(cmd, (err, stdout, stderr) => resolve({err, stdout, stderr}));
183185
});
184186

187+
console.log(`${stdout}`);
188+
185189
if (err) {
186-
console.error(`${stdout}\n${stderr}`);
190+
console.error(`${stderr}`);
187191
}
188192
}
189193

glean/src/core/metrics/utils.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,9 @@ const METRIC_MAP: {
2121
"boolean": BooleanMetric,
2222
"counter": CounterMetric,
2323
"datetime": DatetimeMetric,
24+
"labeled_boolean": LabeledMetric,
2425
"labeled_counter": LabeledMetric,
26+
"labeled_string": LabeledMetric,
2527
"string": StringMetric,
2628
"uuid": UUIDMetric,
2729
});

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

Lines changed: 54 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -250,11 +250,17 @@ describe("LabeledMetric", function() {
250250
});
251251

252252
it("test labeled string metric type", async function() {
253+
const ping = new PingType({
254+
name: "test",
255+
includeClientId: true,
256+
sendIfEmpty: false,
257+
});
258+
253259
const labeledStringMetric = new LabeledMetricType(
254260
{
255261
category: "telemetry",
256262
name: "labeled_string_metric",
257-
sendInPings: ["metrics"],
263+
sendInPings: ["test"],
258264
lifetime: Lifetime.Application,
259265
disabled: false
260266
},
@@ -269,14 +275,40 @@ describe("LabeledMetric", function() {
269275
assert.strictEqual(await labeledStringMetric["label1"].testGetValue(), "foo");
270276
assert.strictEqual(await labeledStringMetric["label2"].testGetValue(), "bar");
271277
assert.strictEqual(await labeledStringMetric["__other__"].testGetValue(), undefined);
278+
279+
// TODO: bug 1691033 will allow us to change the code below this point,
280+
// once a custom uploader for testing will be available.
281+
ping.submit();
282+
await Context.dispatcher.testBlockOnQueue();
283+
284+
const storedPings = await Context.pingsDatabase["store"]._getWholeStore();
285+
assert.strictEqual(Object.keys(storedPings).length, 1);
286+
287+
// TODO: bug 1682282 will validate the payload schema.
288+
289+
// eslint-disable-next-line @typescript-eslint/no-unsafe-assignment
290+
const stored: JSONObject = storedPings[Object.keys(storedPings)[0]] as JSONObject;
291+
// eslint-disable-next-line @typescript-eslint/no-unsafe-member-access
292+
const payloadAsString = JSON.stringify(stored.payload);
293+
294+
// Do the same checks again on the JSON structure
295+
assert.strict(
296+
payloadAsString.includes("\"labeled_string\":{\"telemetry.labeled_string_metric\":{\"label1\":\"foo\",\"label2\":\"bar\"}}}")
297+
);
272298
});
273299

274300
it("test labeled boolean metric type", async function() {
301+
const ping = new PingType({
302+
name: "test",
303+
includeClientId: true,
304+
sendIfEmpty: false,
305+
});
306+
275307
const metric = new LabeledMetricType(
276308
{
277309
category: "telemetry",
278310
name: "labeled_bool",
279-
sendInPings: ["metrics"],
311+
sendInPings: ["test"],
280312
lifetime: Lifetime.Application,
281313
disabled: false
282314
},
@@ -291,6 +323,26 @@ describe("LabeledMetric", function() {
291323

292324
value = await metric["label2"].testGetValue();
293325
assert.strictEqual(value, true);
326+
327+
// TODO: bug 1691033 will allow us to change the code below this point,
328+
// once a custom uploader for testing will be available.
329+
ping.submit();
330+
await Context.dispatcher.testBlockOnQueue();
331+
332+
const storedPings = await Context.pingsDatabase["store"]._getWholeStore();
333+
assert.strictEqual(Object.keys(storedPings).length, 1);
334+
335+
// TODO: bug 1682282 will validate the payload schema.
336+
337+
// eslint-disable-next-line @typescript-eslint/no-unsafe-assignment
338+
const stored: JSONObject = storedPings[Object.keys(storedPings)[0]] as JSONObject;
339+
// eslint-disable-next-line @typescript-eslint/no-unsafe-member-access
340+
const payloadAsString = JSON.stringify(stored.payload);
341+
342+
// Do the same checks again on the JSON structure
343+
assert.strict(
344+
payloadAsString.includes("\"labeled_boolean\":{\"telemetry.labeled_bool\":{\"label1\":false,\"label2\":true}}}")
345+
);
294346
});
295347

296348
it("dynamic labels regex mismatch", async function() {

0 commit comments

Comments
 (0)