Closed
Bug 1356223
Opened 8 years ago
Closed 8 years ago
Support scalar telemetry probe types in Telemetry.js
Categories
(DevTools :: General, enhancement, P2)
DevTools
General
Tracking
(firefox55 fixed)
RESOLVED
FIXED
Firefox 55
Tracking | Status | |
---|---|---|
firefox55 | --- | fixed |
People
(Reporter: pbro, Assigned: miker)
References
Details
(Whiteboard: [todo-mr])
Attachments
(1 file)
I recently tried to add 3 new "count" probes to Histograms.json and got backed out because probes "kind": "count" aren't allowed anymore for Firefox Desktop!
See bug 1352115 for context.
Here's the failure: https://treeherder.mozilla.org/logviewer.html#?job_id=90949702&repo=autoland&lineNumber=1125
And the error message from there:
New "count" histograms are not supported on Desktop, you should use scalars instead: https://gecko.readthedocs.io/en/latest/toolkit/components/telemetry/telemetry/collection/scalars.html
So, going forward we need to use "uint" probes for these things, but we have a custom wrapper class for telemetry in DevTools, and this wrapper won't work with these new types, because the JS API to set values is different.
So, this bug is about changing Telemetry.js to support scalar types.
Note that in this bug, we should also update the documentation in http://searchfox.org/mozilla-central/source/devtools/docs/frontend/telemetry.md to mention that count kind isn't supported anymore on Desktop.
Assignee | ||
Comment 1•8 years ago
|
||
We should use Services.telemetry.scalarSet(histId, true); inside of Telemetry.js
Assignee: nobody → mratcliffe
Priority: -- → P2
Comment hidden (mozreview-request) |
Reporter | ||
Comment 3•8 years ago
|
||
mozreview-review |
Comment on attachment 8859562 [details]
Bug 1356223 - Support scalar telemetry probe types in Telemetry.js
https://reviewboard.mozilla.org/r/131546/#review134776
::: devtools/client/shared/telemetry.js:286
(Diff revision 1)
> + * Scalar in which the data is to be stored.
> + * @param value
> + * Value to store.
> + */
> + logScalar: function (scalarId, value) {
> + if (scalarId) {
nit: Maybe invert this condition to make the function easier to read:
```
if (!scalarId) {
return;
}
try {
...
```
::: devtools/client/shared/telemetry.js:298
(Diff revision 1)
> + return;
> + }
> + Services.telemetry.scalarSet(scalarId, value);
> + } catch (e) {
> + dump(`Warning: An attempt was made to write to the ${scalarId} ` +
> + `histogram, which is not defined in Scalars.yaml\n`);
s/histogram/scalar
Attachment #8859562 -
Flags: review?(pbrosset) → review+
Comment hidden (mozreview-request) |
Pushed by mratcliffe@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/91c19f2d71f3
Support scalar telemetry probe types in Telemetry.js r=pbro
Comment 6•8 years ago
|
||
bugherder |
Status: NEW → RESOLVED
Closed: 8 years ago
status-firefox55:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 55
Updated•6 years ago
|
Product: Firefox → DevTools
You need to log in
before you can comment on or make changes to this bug.
Description
•