Addon histograms shouldn't be really used anymore by any active projects. We should verify if that is the case. Depending on the results we can either: - completely remove the code for them in current versions or - make the APIs print console warnings only and inform any project that still uses them

As of some back-of-napkin analysis I did yesterday, pings with addonHistograms in their payloads are vanishingly rare. About a hundredth of a percent of main pings on release, for instance. I'll look today to see what data's actually in those few pings, and to clean up the analysis for review. Preliminary results: nothing but pdfjs, shumway, and firebug.

Analysis up for review: https://github.com/mozilla/mozilla-reports/pull/50

For those: - Firebug is abandoned. - Shumway still exists but seems effectively dead. - pdf.js is the only one still seeing some development. I put up a PR for pdf.js: https://github.com/mozilla/pdf.js/pull/8239 With that we should be able to just remove the addon histogram APIs from our code. @Yury: Does that sound fine to you? Or do you think we still need to do a PR for Shumway as well?

Quick drive-by: - we still need the about:telemetry code for addon histograms for one more cycle (to allow inspecting archived ping data) - we should send a heads-up e-mail to fhr-dev after this lands

Comment on attachment 8857143 [details] bug 1353295 - Remove addonHistograms from Telemetry https://reviewboard.mozilla.org/r/129076/#review131896 r+wc, this looks good but the following need to be addressed before landing, especially the about:telemetry part. ::: toolkit/components/telemetry/Telemetry.cpp:2929 (Diff revision 1) > size_t > TelemetryImpl::SizeOfIncludingThis(mozilla::MallocSizeOf aMallocSizeOf) > { > size_t n = aMallocSizeOf(this); > > // Ignore the hashtables in mAddonMap; they are not significant. nit: drop this comment, as it's no longer relevant ::: toolkit/components/telemetry/docs/data/main-ping.rst:150 (Diff revision 1) > > The recorded events are defined in the `Events.yaml <https://dxr.mozilla.org/mozilla-central/source/toolkit/components/telemetry/Events.yaml>`_. The ``info.revision`` field indicates the revision of the file that describes the reported events. > > childPayloads > ------------- > The Telemetry payloads sent by child processes, recorded on child process shutdown (event ``content-child-shutdown`` observed). They are reduced session payloads, only available with e10s. Among some other things, they don't contain histograms, keyed histograms, addon details, addon histograms, or UI Telemetry. nit: remove "addon histograms," from this line. ::: toolkit/components/telemetry/nsITelemetry.idl:318 (Diff revision 1) > * data). This is true on official, non-debug builds with built in support for Mozilla > * Telemetry reporting. > */ > readonly attribute boolean isOfficialTelemetry; > > /** Addon telemetry hooks */ nit: I think you can get rid of this line too. ::: toolkit/content/aboutTelemetry.js (Diff revision 1) > setHasData("keyed-histograms-section", hasData || keyedHgramsSelect.options.length); > } > > // Show event data. > Events.render(payload); > - As Georg mentioned, let's keep the stuff in about:telemetry for at least a cycle. Let's file a follow-up bug about removing stuff from about:telemetry so we don't forget. ::: toolkit/content/aboutTelemetry.xhtml (Diff revision 1) > <span class="empty-caption">&aboutTelemetry.emptySection;</span> > <div id="addon-details" class="data"> > </div> > </section> > > - <section id="addon-histograms-section" class="data-section"> Let's keep this (about:telemetry) ::: toolkit/locales/en-US/chrome/global/aboutTelemetry.dtd (Diff revision 1) > > <!ENTITY aboutTelemetry.sessionInfoSection " > Session Information > "> > > -<!ENTITY aboutTelemetry.addonHistogramsSection " Let's keep this (about:telemetry)

Pushed by chutten@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/cfe47bace342 Remove addonHistograms from Telemetry r=Dexter

had to back this out for assertion failures that happened on m-c after this merged like https://treeherder.mozilla.org/logviewer.html#?job_id=91115044&repo=mozilla-central

Backout by cbook@mozilla.com: https://hg.mozilla.org/mozilla-central/rev/1f1c921f172c Backed out changeset cfe47bace342 for assertion failures

Hrm. Running the changes locally on my Win10 box (wow, Debug builds take a long time on Windows) results in no problems. A rerun of try results in no problems (well, an intermittent orange in websockets)[1]. I wonder if this was somehow multifactorial with other changes, or the result of some temporary nonsense. I'm going to try landing this again. [1]: https://treeherder.mozilla.org/#/jobs?repo=try&revision=948d2447699c7a02fd418e83041a7f470d8eed44

Pushed by chutten@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/890a29e0bd94 Remove addonHistograms from Telemetry r=Dexter