1312057 - Fix prefetch use telemetry

Status: RESOLVED FIXED

(Core :: Networking, defect, P2)

Description

[u408661]

Due to a brain fart (or bull-headed ignorance), our telemetry on whether or not a prefetch gets used is mostly (totally?) worthless right now. We should probably fix that, so we can tweak prefetch conditions properly if necessary :)

Comment 1

[u408661]

Honza - in trying to fix this, I'm working on a patch that adds multiple telemetry states per prefetch - one that says "we used this entry", and a few different "we didn't use this prefetched entry because ...". One of the reasons for "we didn't use this entry" should be "because we didn't ask for it until after its forced valid lifetime expired". It looks to me like the best places to keep track of that would be in CacheStorageService::IsForcedValidEntry, CacheStorageService::ForcedValidEntriesPrune, and CacheStorageService::RemoveEntryForceValid. I would have to add a bool to what we keep in the hash of forced valid entries to say "was this looked up while it was forced valid?" along the timestamp, and accumulate telemetry (or not) based on that. Does that seem like a reasonable course of action? (I can attach a proposed patch that probably won't even compile, if that makes my proposal easier to understand)

Comment 2

[Honza Bambas (:mayhemer)]

What you propose sounds reasonable.  Other option, since you can force valid a cache entry only when having CacheEntry (nsICacheEntry) in hands, is to keep this on the CacheEntry class.  If you force-valid an entry, set some flag on CacheEntry.  In AsyncOpen (means someone requested the entry) check if this entry entry is still still valid, if so, drop the flag.  In ~CacheEntry collect the telemetry when the flag is still set (no one requested the entry on time).  Makes sense?  Might be a bit cleaner, but up to you.

Comment 3

[Firefox Bug Husbandry Bot]

Bulk priority update: https://bugzilla.mozilla.org/show_bug.cgi?id=1399258

Comment 4

[Sylvestre Ledru [:Sylvestre]]

Moving to p3 because no activity for at least 1 year(s).
See https://github.com/mozilla/bug-handling/blob/master/policy/triage-bugzilla.md#how-do-you-triage for more information

Comment 5

[u408661]

Attachment: Bug 1312057 - Fix prefetch telmetry r=#necko

This does two things:
 - Properly track whether or not a prefetched resource is used (the previous version was broken)
 - Keep accumulating telemetry on why we decide to prefetch a resource (or not). This was scheduled to expire, but never gave data, as prefetch was never enabled

Comment 6

[Chris H-C :chutten]

A quick note: this will require Data Collection Review[1], and that the first batch of collection from new probes is recommended to be collected for a preliminary 6-month period to ensure it is useful, and more importantly, used.

You can of course go straight to a permanent collection, but then you will probably want some automated tests to ensure no one's future patches accidentally interfere with the collection.

Also, you may wish to consider using "categorical" histograms instead of "enumerated", as they are often the better of the two (the labels are included in the probe, new labels can be added without having to write a separate probe, and the Measurement dashboard displays the labels on the graph for you).

[1]: https://wiki.mozilla.org/Firefox/Data_Collection

Comment 7

[u408661]

Thanks for the reminder about data collection review - that's what happens when I type moz-phab submit and head to lunch :)

Comment 8

[u408661]

Attachment: prefetch-telemetry-review-request.txt

Comment 9

[François Marier [:francois]]

Comment on attachment 9024116 [details]
prefetch-telemetry-review-request.txt

1) Is there or will there be **documentation** that describes the schema for the ultimate data set available publicly, complete and accurate?

Yes, in Histograms.json.

2) Is there a control mechanism that allows the user to turn the data collection on and off?

Yes, telemetry setting.

3) If the request is for permanent data collection, is there someone who will monitor the data over time?**

Yes, Nicholas Hurley.

4) Using the **[category system of data types](https://wiki.mozilla.org/Firefox/Data_Collection)** on the Mozilla wiki, what collection type of data do the requested measurements fall under?  **

Category 1.

5) Is the data collection request for default-on or default-off?

Default ON, only in pre-release channels.

6) Does the instrumentation include the addition of **any *new* identifiers** (whether anonymous or otherwise; e.g., username, random IDs, etc.  See the appendix for more details)?

No.

7) Is the data collection covered by the existing Firefox privacy notice?

Yes.

8) Does there need to be a check-in in the future to determine whether to renew the data?

No, permanent.

Comment 10

[Dragana Damjanovic [:dragana]]

I will take this from Nick.

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/f8a9854f62e9
Fix prefetch telmetry r=mayhemer,necko-reviewers,dragana

Comment 12

[Noemi Erli[:noemi_erli]]

https://hg.mozilla.org/mozilla-central/rev/f8a9854f62e9