Note: There are a few cases of duplicates in user autocompletion which are being worked on.

Support scalar telemetry probe types in Telemetry.js

RESOLVED FIXED in Firefox 55

Status

()

Firefox
Developer Tools
P2
normal
RESOLVED FIXED
3 months ago
3 months ago

People

(Reporter: pbro, Assigned: miker)

Tracking

unspecified
Firefox 55
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox55 fixed)

Details

(Whiteboard: [todo-mr])

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(1 attachment)

(Reporter)

Description

3 months ago
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
(Assignee)

Updated

3 months ago
Blocks: 1357704
Comment hidden (mozreview-request)
(Reporter)

Comment 3

3 months 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)

Comment 5

3 months ago
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

3 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/91c19f2d71f3
Status: NEW → RESOLVED
Last Resolved: 3 months ago
status-firefox55: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 55
You need to log in before you can comment on or make changes to this bug.