Storing valid process types in a parser-friendly file

RESOLVED FIXED in Firefox 55

Status

()

defect
P1
normal
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: Dexter, Assigned: gfritzsche)

Tracking

Trunk
mozilla55
Points:
1
Dependency tree / graph

Firefox Tracking Flags

(firefox53 wontfix, firefox54 wontfix, firefox55 fixed)

Details

(Whiteboard: [measurement:client])

Attachments

(1 attachment)

Reporter

Description

2 years ago
Now that we have child processes measurements in place, it would be good to store valid process types in a YAML or JSON file so that other services (aggregator? batch views?) can easily look up the valid processes types.
Reporter

Updated

2 years ago
Points: --- → 1
Priority: -- → P2
Whiteboard: [measurement:client]
Reporter

Comment 1

2 years ago
Bug 1278556 comment 34 has some pointers about what to do for this bug.
Reporter

Updated

2 years ago
Priority: P2 → P1
Reporter

Updated

2 years ago
Assignee: nobody → alessio.placitelli
Reporter

Comment 2

2 years ago
After discussing this with Georg, we figured that we could drive this a bit further. Instead of simply moving the processes flags definition out of the python file [1], we could introduce a new YAML "processes registry":

- To list all the supported processes
- Provide a description for each process (for discoverability)

The registry could be used to generate the ScalarInfo.h processes enumeration as well [2].

The registry format could be something along the line of

> Main:
>  description: The main process, also known as "parent" process.
>  is_child: false
>  
> Content:
>  description: The content process, used to render DOM and stuff.
>  is_child: true 

The registry file could be used by other systems as well to find out what data to expect in Telemetry pings.

[1] - http://searchfox.org/mozilla-central/rev/790b2cb423ea1ecb5746722d51633caec9bab95a/toolkit/components/telemetry/parse_scalars.py#20
[2] - http://searchfox.org/mozilla-central/rev/790b2cb423ea1ecb5746722d51633caec9bab95a/toolkit/components/telemetry/ScalarInfo.h#19
Blocks: 1332117
Reporter

Comment 3

2 years ago
What do you think of the design in comment 2?
Flags: needinfo?(gfritzsche)
Reporter

Updated

2 years ago
Priority: P1 → P2
Reporter

Updated

2 years ago
Assignee: alessio.placitelli → nobody
Reporter

Updated

2 years ago
Priority: P2 → P3
Assignee

Comment 4

2 years ago
Late reply... but comment 2 looks good.
I will probably do this as part of a refactor in bug 1361661.
Flags: needinfo?(gfritzsche)
Priority: P3 → P2
Assignee

Comment 5

2 years ago
(In reply to Georg Fritzsche [:gfritzsche] from comment #4)
> Late reply... but comment 2 looks good.

However, i don't think we need "is_child" flags - we have only one "main" process, so we can just hard-code that as "parent" where needed.
Assignee

Comment 6

2 years ago
The best thing to do would be to use the parser file to generate all the enums and strings we use in the build.
From taking a stab at it, this becomes a bit more involved, so i'm moving this to a separate bug.
This bug can just start to introduce the file so it can start to be used by the pipeline after bug 1361661.
Assignee: nobody → gfritzsche
Priority: P2 → P1
Assignee

Updated

2 years ago
Blocks: 1364898
Assignee

Comment 7

2 years ago
Roberto, does that cover what you are looking for? Or can you redirect to someone else?
Attachment #8867700 - Flags: review?(alessio.placitelli)
Attachment #8867700 - Flags: feedback?(rvitillo)
Reporter

Comment 8

2 years ago
Comment on attachment 8867700 [details] [diff] [review]
Store valid process types in a parser-friendly file

Review of attachment 8867700 [details] [diff] [review]:
-----------------------------------------------------------------

This looks good to me, thanks! Let's wait for Roberto's feedback before landing it
Attachment #8867700 - Flags: review?(alessio.placitelli) → review+
Assignee

Comment 9

2 years ago
Comment on attachment 8867700 [details] [diff] [review]
Store valid process types in a parser-friendly file

Redirecting per Roberto.
Attachment #8867700 - Flags: feedback?(rvitillo) → feedback?(rharter)
Thanks, Georg. That looks sufficient to fix Bug 1332117.
Attachment #8867700 - Flags: feedback?(rharter) → feedback+
Assignee

Updated

2 years ago
Keywords: checkin-needed

Comment 11

2 years ago
Pushed by cbook@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/2a2750d4a4e9
Store valid process types in a parser-friendly file. r=dexter
Keywords: checkin-needed

Comment 12

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/2a2750d4a4e9
Status: NEW → RESOLVED
Last Resolved: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
Is this something you are aiming to uplift to beta 54? Or can we let it ride to release with 55?
Flags: needinfo?(gfritzsche)
Assignee

Comment 14

2 years ago
This does not require uplift.
Flags: needinfo?(gfritzsche)
You need to log in before you can comment on or make changes to this bug.