1409542 - Gather telemetry on why we decide whether or not to prefetch a resource in the predictor

Status: RESOLVED FIXED

(Core :: Networking, defect, P2)

Description

[u408661]

We want to be able to know what kind of hit we'll take for potential prefetches by putting limitations on the resources we mark eligible for prefetching (for example, because the URL has a query string, or because it has an appropriate Cache-Control header).

At the same time, we should probably gather telemetry on which resources are prefetchable based on the properties of the resource (eg, marked prefetchable in the cache) versus the number of those we disqualify from prefetching for other reasons (parent resource wasn't requested recently enough, for example).

This will help us evaluate other potential solutions to the double-fetch issue some ad networks have been complaining about whenever the predictor gets enabled on release.

Comment 1

[u408661]

Attachment: Bug 1409542 part 1 - telemetry on why a resource is marked prefetchable or not.

Review commit: https://reviewboard.mozilla.org/r/192064/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/192064/

Comment 2

[u408661]

Attachment: Bug 1409542 part 2 - telemetry on why a resource marked prefetchable is not prefetched.

Review commit: https://reviewboard.mozilla.org/r/192066/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/192066/

Comment 3

[u408661]

:francois - I'm looking for data-review here. According to my reading of things, these should be somewhere around level 1 or 2 data, but that's why you're the data peer, and I'm just the necko hacker :)

Comment 4

[François Marier [:francois]]

Comment on attachment 8921103 [details]
Bug 1409542 part 1 - telemetry on why a resource is marked prefetchable or not.

https://reviewboard.mozilla.org/r/192064/#review197348

This is Category 1 data.

datareview+

::: toolkit/components/telemetry/Histograms.json:3915
(Diff revision 1)
>    },
> +  "PREDICTOR_PREFETCH_DECISION_REASON": {
> +    "record_in_processes": ["main"],
> +    "expires_in_version": "60",
> +    "kind": "enumerated",
> +    "n_values": 7,

Are you planning on adding more values in the future? If so, you may want to set n_values to a higher number because it can't be increased later without renaming the probe (i.e. creating a new one).

Comment 5

[François Marier [:francois]]

Comment on attachment 8921104 [details]
Bug 1409542 part 2 - telemetry on why a resource marked prefetchable is not prefetched.

https://reviewboard.mozilla.org/r/192066/#review197350

::: toolkit/components/telemetry/Histograms.json:3924
(Diff revision 1)
>    },
> +  "PREDICTOR_PREFETCH_IGNORE_REASON": {
> +    "record_in_processes": ["main"],
> +    "expires_in_version": "60",
> +    "kind": "enumerated",
> +    "n_values": 7,

Again, you may want to add a bit of a buffer for future expansion.

::: toolkit/components/telemetry/Histograms.json:3927
(Diff revision 1)
> +    "expires_in_version": "60",
> +    "kind": "enumerated",
> +    "n_values": 7,
> +    "alert_emails": ["[e-mail]"],
> +    "bug_numbers": [1409542],
> +    "description": "Why the predictor determined a particular resource that was marked eligible for prefetch should not be prefetched"

You need to either include the values in the description, or link to the file where they are defined (like you did in the other probe).

Comment 6

[u408661]

Comment on attachment 8921103 [details]
Bug 1409542 part 1 - telemetry on why a resource is marked prefetchable or not.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/192064/diff/1-2/

Comment 7

[u408661]

Comment on attachment 8921104 [details]
Bug 1409542 part 2 - telemetry on why a resource marked prefetchable is not prefetched.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/192066/diff/1-2/

Comment 8

[u408661]

Fixed the comments from your previous look, thanks Francois!

Comment 9

[François Marier [:francois]]

Comment on attachment 8921104 [details]
Bug 1409542 part 2 - telemetry on why a resource marked prefetchable is not prefetched.

https://reviewboard.mozilla.org/r/192066/#review197398

Category 1 data.

datareview+

Comment 10

[Valentin Gosu [:valentin] (he/him)]

Comment on attachment 8921104 [details]
Bug 1409542 part 2 - telemetry on why a resource marked prefetchable is not prefetched.

https://reviewboard.mozilla.org/r/192066/#review197576

Comment 11

[Valentin Gosu [:valentin] (he/him)]

Comment on attachment 8921103 [details]
Bug 1409542 part 1 - telemetry on why a resource is marked prefetchable or not.

https://reviewboard.mozilla.org/r/192064/#review197578

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/a120a00b3f2e
part 1 - telemetry on why a resource is marked prefetchable or not. r=francois,valentin
https://hg.mozilla.org/integration/autoland/rev/7935635f721f
part 2 - telemetry on why a resource marked prefetchable is not prefetched. r=francois,valentin

Comment 13

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/a120a00b3f2e
https://hg.mozilla.org/mozilla-central/rev/7935635f721f