Closed
Bug 1333797
Opened 8 years ago
Closed 8 years ago
Making the process choice explicit/required for Scalars
Categories
(Toolkit :: Telemetry, defect, P1)
Toolkit
Telemetry
Tracking
()
RESOLVED
FIXED
mozilla54
Tracking | Status | |
---|---|---|
firefox54 | --- | fixed |
People
(Reporter: Dexter, Assigned: chutten)
References
Details
(Whiteboard: [measurement:client])
Attachments
(1 file)
In bug 1278556 comments 1 (and 3) we explain why it's important to restrict scalar collection to specific processes.
However, as shown by bug 1325536, making the "main" process the default if no 'record_in_processes' option is specified could be confusing.
We should make the process explicit or, even better, make the 'record_in_processes' property in Scalars.yaml mandatory for each definition.
Reporter | ||
Updated•8 years ago
|
Comment hidden (mozreview-request) |
Reporter | ||
Updated•8 years ago
|
Assignee: nobody → chutten
Status: NEW → ASSIGNED
Priority: P2 → P1
Reporter | ||
Comment 2•8 years ago
|
||
mozreview-review |
Comment on attachment 8830443 [details]
bug 1333797 - Require 'record_in_processes' values in Scalars.yaml
https://reviewboard.mozilla.org/r/107198/#review108656
Hey Chris, thanks for picking this up. I was waiting for the discussion to settle on FHR-dev before reviewing this. It looks ok with the below addressed.
Please also update the docs (scalars.rst).
::: toolkit/components/telemetry/parse_scalars.py:232
(Diff revision 1)
> return self._definition['notification_emails']
>
> @property
> def record_in_processes(self):
> """Get the non-empty list of processes to record data in"""
> return self._definition.get('record_in_processes', ['main'])
Let's drop the default value from here since we're requiring the field now.
Comment hidden (mozreview-request) |
Reporter | ||
Comment 4•8 years ago
|
||
mozreview-review |
Comment on attachment 8830443 [details]
bug 1333797 - Require 'record_in_processes' values in Scalars.yaml
https://reviewboard.mozilla.org/r/107198/#review109618
Thanks Chris, this looks good!
Attachment #8830443 -
Flags: review?(alessio.placitelli) → review+
Pushed by chutten@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/43c2c4c0324d
Require 'record_in_processes' values in Scalars.yaml r=Dexter
Comment 6•8 years ago
|
||
bugherder |
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla54
You need to log in
before you can comment on or make changes to this bug.
Description
•