Closed Bug 1425345 Opened 2 years ago Closed 2 years ago

Restrict UPDATE_* histograms to parent process

Categories

(Toolkit :: Telemetry, enhancement, P4)

enhancement

Tracking

()

RESOLVED FIXED
mozilla59
Tracking Status
firefox59 --- fixed

People

(Reporter: gfritzsche, Assigned: hrdktg, Mentored)

References

(Blocks 1 open bug)

Details

(Whiteboard: [good first bug] [lang=js])

Attachments

(1 file, 1 obsolete file)

There is a multitude of UPDATE_* histograms that are declared to record in "parent" and "content":
https://dxr.mozilla.org/mozilla-central/rev/22d2831cc1f41e1b3e1ebac9be5a7aff33684843/toolkit/components/telemetry/Histograms.json#5523

The update code should only be used in the parent process. We should restrict these histograms to record only in the parent, i.e.:
> "record_in_processes": ["main"],
Robert, does that sound right to you?
If so we can set it up as a mentored bug.
Flags: needinfo?(robert.strong.bugs)
Blocks: 1423446
That sounds right
Flags: needinfo?(robert.strong.bugs)
This will require changing various UPDATE_* histogram definitions in Histograms.json:
https://dxr.mozilla.org/mozilla-central/rev/22d2831cc1f41e1b3e1ebac9be5a7aff33684843/toolkit/components/telemetry/Histograms.json#5523

We should restrict these scalars to record only in the parent process, i.e.:
> "record_in_processes": ["main"]

After the change, make sure that building still works properly using:
> mach build
Mentor: chutten, alessio.placitelli
Whiteboard: [good first bug] [lang=js]
Changed all
>"UPDATE_*": {
>     "record_in_processes": ["main", "content"],
to 
>"UPDATE_*": {
>     "record_in_processes": ["main"],

I hope the commit message is relevant.
Attachment #8937337 - Flags: review?(chutten)
Comment on attachment 8937337 [details] [diff] [review]
UPDATE_* histograms should only record in parent.

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

Patch looks good! The commit message is relevant as well, but it is missing one piece. At the beginning of the first line it should have "bug 1425345 - " so that our tools and future contributors know what bug this commit was fixing.

Put that bug info and hyphen into the commit message and r? me again and we'll get this fixed and in the tree!
Attachment #8937337 - Flags: review?(chutten)
Changed the commit message.
Attachment #8937337 - Attachment is obsolete: true
Attachment #8937652 - Flags: review?(chutten)
Assignee: nobody → hrdktg
Status: NEW → ASSIGNED
Attachment #8937652 - Flags: review?(chutten) → review+
I have marked this bug as checkin-needed which means its patch will be pulled in on the next merge. Well done, and thank you for your contribution, :hrdktg!

Would you like any help finding another bug to work on, or do you have one in mind?
Keywords: checkin-needed
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/39f70372b66c
UPDATE_* histograms should record only in the parent. r=chutten
Keywords: checkin-needed
(In reply to Chris H-C :chutten from comment #7)
> I have marked this bug as checkin-needed which means its patch will be
> pulled in on the next merge. Well done, and thank you for your contribution,
> :hrdktg!
> 
> Would you like any help finding another bug to work on, or do you have one
> in mind?

Yes I do need help finding another bug to work. Please suggest.
Flags: needinfo?(chutten)
https://hg.mozilla.org/mozilla-central/rev/39f70372b66c
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla59
Let's see, I suppose it depends on your skillset and interests. If you are interested in a bit of everything, here's a list of bugs marked as "good second bug": https://mzl.la/2CLSgtt

Otherwise, here's Bugs Ahoy, which has a list of mentored bugs from throughout bugzilla: https://www.joshmatthews.net/bugsahoy/

For specific bugs I've run across lately:
If you want to discuss nonsensical English grammar rules and then do a little C++ or doc work, there's bug 1363725
If you want to help us modernize our Telemetry reporting you can migrate a very important measurement to a Scalar so it's easier to analyze in bug 1362474

If none of this works, or if you need some more help deciding, or if you just want to say hi, you can find me on IRC on the  #telemetry channel: https://wiki.mozilla.org/Irc
Flags: needinfo?(chutten)
You need to log in before you can comment on or make changes to this bug.