Closed Bug 1275707 Opened 8 years ago Closed 8 years ago

Filter out numbers from message manager size telemetry

Categories

(Core :: DOM: Core & HTML, defect, P3)

defect

Tracking

()

RESOLVED FIXED
mozilla49
Tracking Status
e10s + ---
firefox47 --- wontfix
firefox48 --- fixed
firefox49 --- fixed

People

(Reporter: mccr8, Assigned: mccr8)

References

Details

Attachments

(3 files, 1 obsolete file)

There are a huge number of different message manager messages with names of the form "ublock0:sb:{N}", where {N} is some number from 1 to over 1000. Having so many different keys seems to cause problems for telemetry and makes it harder to tell how many messages of each type there really are, so this patch combines them by eliminating any digits. It will also help for the webdev tools that use channels with names like "debug:server1.conn5.child1:packet". This will create some ambiguity (eg there are some messages of the form "ublock:sb:{N}"), but that should be a minor issue.
This seems pretty low risk, but I'll do a try run before landing just in case.
https://treeherder.mozilla.org/#/jobs?repo=try&revision=3abc64c7e9fe
Attachment #8756529 - Flags: review?(amarchesini)
See Also: → 1275010
tracking-e10s: --- → ?
Priority: -- → P3
Comment on attachment 8756529 [details] [diff] [review]
part 1 - Factor out a common method for message manager size telemetry.

Maybe you could review these two patches, Bill? They are super simple and I expect baku is gone for the weekend already.
Attachment #8756529 - Flags: review?(amarchesini) → review?(wmccloskey)
Attachment #8756530 - Flags: review?(amarchesini) → review?(wmccloskey)
I'm also asking for the telemetry data review thing as part of this review.
Attachment #8756975 - Flags: review?(benjamin)
Attachment #8756529 - Flags: review?(wmccloskey) → review+
Attachment #8756530 - Flags: review?(wmccloskey) → review+
Comment on attachment 8756975 [details] [diff] [review]
part 3 - Rename MESSAGE_MANAGER_MESSAGE_SIZE.

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

::: toolkit/components/telemetry/Histograms.json
@@ +10731,4 @@
>      "bug_numbers": [1260908],
>      "expires_in_version": "55",
>      "kind": "exponential",
>      "high": 8000000,

One thing I regretted with these histograms is not specifying "low": 8192. Can you try doing that? It just makes it a little easier to read the graphs.
Set "low" to 8192.
Attachment #8757505 - Flags: review?(benjamin)
Attachment #8756975 - Attachment is obsolete: true
Attachment #8756975 - Flags: review?(benjamin)
Attachment #8757505 - Flags: review?(benjamin) → review+
Attachment #8757505 - Flags: feedback?(benjamin)
(f? for telemetry data approval)
Attachment #8757505 - Flags: feedback?(benjamin) → feedback+
Pushed by amccreight@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/6115c96a8701
part 1 - Factor out a common method for message manager size telemetry. r=billm
https://hg.mozilla.org/integration/mozilla-inbound/rev/ef70956cd95c
part 2 - Remove numbers from the message manager message names. r=billm
https://hg.mozilla.org/integration/mozilla-inbound/rev/5885ad0c0a1d
part 3 - Rename MESSAGE_MANAGER_MESSAGE_SIZE. r=baku
I'll nominate this for uplift in a few days after I've confirmed that data has started coming in and is non-spammy.
Flags: needinfo?(continuation)
I've confirmed that there are only around 20 categories now, and the ublock and Adblock stuff is all being merged together.
Flags: needinfo?(continuation)
Comment on attachment 8756529 [details] [diff] [review]
part 1 - Factor out a common method for message manager size telemetry.

This approval request is for all three patches.

Approval Request Comment
[Feature/regressing bug #]: n/a
[User impact if declined]: One kind of e10s OOM-related telemetry was disabled because it was causing telemetry server issues, this makes a small tweak and reenables it. 
[Describe test coverage new/current, TreeHerder]: this code runs frequently
[Risks and why]: low, it just makes a small tweak to telemetry (the first part just moves code around)
[String/UUID change made/needed]: none
Attachment #8756529 - Flags: approval-mozilla-beta?
Attachment #8757505 - Flags: approval-mozilla-beta?
Attachment #8756530 - Flags: approval-mozilla-beta?
Comment on attachment 8757505 [details] [diff] [review]
part 3 - Rename MESSAGE_MANAGER_MESSAGE_SIZE.

Improve telemetry, taking it.
Should be in 48 beta 2
Attachment #8757505 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Attachment #8756530 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Attachment #8756529 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: