Closed Bug 1333797 Opened 5 years ago Closed 5 years ago

Making the process choice explicit/required for Scalars

Categories

(Toolkit :: Telemetry, defect, P1)

defect
Points:
1

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.
Blocks: 1278556
Points: --- → 1
Priority: -- → P2
Whiteboard: [measurement:client]
Assignee: nobody → chutten
Status: NEW → ASSIGNED
Priority: P2 → P1
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 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
https://hg.mozilla.org/mozilla-central/rev/43c2c4c0324d
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla54
You need to log in before you can comment on or make changes to this bug.