Closed Bug 790615 Opened 8 years ago Closed 8 years ago

Telemetry's Histograms.json is missing the MEMORY_RESIDENT histogram

Categories

(Toolkit :: Telemetry, defect)

defect
Not set
normal

Tracking

()

VERIFIED FIXED
mozilla18
Tracking Status
firefox17 - verified

People

(Reporter: justin.lebar+bug, Assigned: justin.lebar+bug)

References

Details

(Keywords: regression)

Attachments

(3 files, 1 obsolete file)

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.
Depends on: 790621
Um...MEMORY_RESIDENT isn't even in Histograms.json.
Blocks: 781531
When bug 781531 converted us from TelemetryHistograms.h to Histograms.json, it lost MEMORY_RESIDENT.
Keywords: regression
Retrieved by running TelemetryHistograms.h through the pre-processor and filtering the results.
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
$ 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: nobody → justin.lebar+bug
Summary: No idle-daily memory_resident data since 8/23 in telemetry → Telemetry's Histograms.json is missing the MEMORY_RESIDENT histogram
Attached patch Patch, v1Splinter Review
Attachment #660488 - Flags: review?(taras.mozilla)
This affects FF17; we need to patch it there as well.
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.
Comment on attachment 660488 [details] [diff] [review]
Patch, v1

Whoops!  Nice catch.
Attachment #660488 - Flags: review?(taras.mozilla) → review+
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?
https://hg.mozilla.org/integration/mozilla-inbound/rev/509a54583009

> How did that happen?

I am also curious about this.
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
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla18
Attachment #660488 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
We're getting memory_resident numbers from nightly again.  Hooray.
Status: RESOLVED → VERIFIED
Keywords: verifyme
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.
Ah, excellent point!

This should be quite simple to verify on Aurora, except Telemetry Evolution isn't loading for me.
Okay, I finally managed to get TE to show Aurora, and indeed we're getting data there.

Thanks for pushing on that one, Anthony.
Thank you very much, Justin.
You need to log in before you can comment on or make changes to this bug.