Telemetry's Histograms.json is missing the MEMORY_RESIDENT histogram

VERIFIED FIXED in Firefox 17

Status

()

Toolkit
Telemetry
VERIFIED FIXED
6 years ago
6 years ago

People

(Reporter: Justin Lebar (not reading bugmail), Assigned: Justin Lebar (not reading bugmail))

Tracking

({regression})

Trunk
mozilla18
regression
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox17- verified)

Details

Attachments

(3 attachments, 1 obsolete attachment)

(Assignee)

Description

6 years ago
See http://bit.ly/memorytelemetry

I don't know why the data is missing, but I don't see memory_resident in my about:telemetry, which is quite bad.
(Assignee)

Updated

6 years ago
Depends on: 790621
(Assignee)

Comment 1

6 years ago
Um...MEMORY_RESIDENT isn't even in Histograms.json.
(Assignee)

Updated

6 years ago
Blocks: 781531
(Assignee)

Comment 2

6 years ago
When bug 781531 converted us from TelemetryHistograms.h to Histograms.json, it lost MEMORY_RESIDENT.
(Assignee)

Updated

6 years ago
Keywords: regression
(Assignee)

Comment 3

6 years ago
Created attachment 660479 [details]
All histograms from TelemetryHistograms.h

Retrieved by running TelemetryHistograms.h through the pre-processor and filtering the results.
(Assignee)

Comment 4

6 years ago
Created attachment 660482 [details]
Histograms in original Histograms.json

Retrieved from https://hg.mozilla.org/mozilla-central/rev/b4316e1c474d.
(Assignee)

Comment 5

6 years ago
Comment on attachment 660479 [details]
All histograms from TelemetryHistograms.h

Oops; I forgot to remove the idfef's from the header file.
Attachment #660479 - Attachment is obsolete: true
(Assignee)

Comment 6

6 years ago
Created attachment 660485 [details]
Full list of histograms from TelemetryHistograms.h
(Assignee)

Comment 7

6 years ago
$ diff <(curl -L https://bugzilla.mozilla.org/attachment.cgi?id=660485) <(curl -L https://bugzilla.mozilla.org/attachment.cgi?id=660482) 
221d220
< MEMORY_RESIDENT

Looks like MEMORY_RESIDENT was the only one we lost.
(Assignee)

Updated

6 years ago
Assignee: nobody → justin.lebar+bug
(Assignee)

Updated

6 years ago
Summary: No idle-daily memory_resident data since 8/23 in telemetry → Telemetry's Histograms.json is missing the MEMORY_RESIDENT histogram
(Assignee)

Comment 8

6 years ago
Created attachment 660488 [details] [diff] [review]
Patch, v1
Attachment #660488 - Flags: review?(taras.mozilla)
(Assignee)

Comment 9

6 years ago
This affects FF17; we need to patch it there as well.
status-firefox17: --- → affected
tracking-firefox17: --- → ?
How did that happen?  Good catch, jlebar.
Affects 17 and we'll take a patch for uplift but it wouldn't block release of 17 so we don't need to track.
tracking-firefox17: ? → -
Comment on attachment 660488 [details] [diff] [review]
Patch, v1

Whoops!  Nice catch.
Attachment #660488 - Flags: review?(taras.mozilla) → review+
(Assignee)

Comment 13

6 years ago
Comment on attachment 660488 [details] [diff] [review]
Patch, v1

[Approval Request Comment]
Bug caused by (feature/regressing bug #): bug 781531
User impact if declined: No impact on users; this is a telemetry-only change.  If declined, we'll have no insight into the most important memory-related telemetry metric.
Testing completed (on m-c, etc.): I'll push to m-c and we can check a few days after to verify that the telemetry info appears.
Risk to taking this patch (and alternatives if risky): Shouldn't be risky.
String or UUID changes made by this patch: None.
Attachment #660488 - Flags: approval-mozilla-aurora?
(Assignee)

Comment 14

6 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/509a54583009

> How did that happen?

I am also curious about this.
(Assignee)

Comment 15

6 years ago
Nathan, for good measure, you might want to check that all the histogram parameters were converted correctly when you went from the header to the JSON file.
(In reply to Justin Lebar [:jlebar] from comment #14)
> https://hg.mozilla.org/integration/mozilla-inbound/rev/509a54583009
> 
> > How did that happen?
> 
> I am also curious about this.

I had a bad regexp in the conversion script.  I assumed that nobody would have general C expressions, merely [0-9]+, for the lower bound of a histogram.  I should have had it print out more information and/or examined the output a little more closely.
https://hg.mozilla.org/mozilla-central/rev/509a54583009
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla18

Updated

6 years ago
Attachment #660488 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
(Assignee)

Comment 18

6 years ago
We're getting memory_resident numbers from nightly again.  Hooray.
Status: RESOLVED → VERIFIED
(Assignee)

Comment 19

6 years ago
Thanks for the approval!

https://hg.mozilla.org/releases/mozilla-aurora/rev/7676a9a06403
status-firefox17: affected → fixed
Keywords: verifyme
(Assignee)

Comment 20

6 years ago
I really don't think QA needs to spend time verifying this fix.  I have already done so, and indicated as such in the bug status.
Does this not need verification against Firefox 17? I assume given by the target milestone that this has been verified fixed against Firefox 18 only.
(Assignee)

Comment 22

6 years ago
Ah, excellent point!

This should be quite simple to verify on Aurora, except Telemetry Evolution isn't loading for me.
(Assignee)

Comment 23

6 years ago
Okay, I finally managed to get TE to show Aurora, and indeed we're getting data there.

Thanks for pushing on that one, Anthony.
status-firefox17: fixed → verified
Keywords: verifyme
Thank you very much, Justin.
You need to log in before you can comment on or make changes to this bug.