Closed Bug 1356223 Opened 4 years ago Closed 4 years ago

Support scalar telemetry probe types in Telemetry.js

Categories

(DevTools :: General, enhancement, P2)

enhancement

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.
We should use Services.telemetry.scalarSet(histId, true); inside of Telemetry.js
Assignee: nobody → mratcliffe
Priority: -- → P2
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+
Pushed by mratcliffe@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/91c19f2d71f3
Support scalar telemetry probe types in Telemetry.js r=pbro
https://hg.mozilla.org/mozilla-central/rev/91c19f2d71f3
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 55
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.