Closed Bug 1328230 Opened 4 years ago Closed 3 years ago

Storing valid process types in a parser-friendly file

Categories

(Toolkit :: Telemetry, defect, P1)

defect
Points:
1

Tracking

()

RESOLVED FIXED
mozilla55
Tracking Status
firefox53 --- wontfix
firefox54 --- wontfix
firefox55 --- fixed

People

(Reporter: Dexter, Assigned: gfritzsche)

References

Details

(Whiteboard: [measurement:client])

Attachments

(1 file)

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.
Points: --- → 1
Priority: -- → P2
Whiteboard: [measurement:client]
Bug 1278556 comment 34 has some pointers about what to do for this bug.
Priority: P2 → P1
Assignee: nobody → alessio.placitelli
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
What do you think of the design in comment 2?
Flags: needinfo?(gfritzsche)
Priority: P1 → P2
Assignee: alessio.placitelli → nobody
Priority: P2 → P3
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
(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.
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
Blocks: 1364898
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)
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+
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+
Keywords: checkin-needed
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
https://hg.mozilla.org/mozilla-central/rev/2a2750d4a4e9
Status: NEW → RESOLVED
Closed: 3 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)
This does not require uplift.
Flags: needinfo?(gfritzsche)
You need to log in before you can comment on or make changes to this bug.