Closed Bug 1409542 Opened 2 years ago Closed 2 years ago

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

Categories

(Core :: Networking, defect, P2)

defect

Tracking

()

RESOLVED FIXED
mozilla58
Tracking Status
firefox58 --- fixed

People

(Reporter: u408661, Assigned: u408661)

References

(Blocks 1 open bug)

Details

(Whiteboard: [necko-triaged])

Attachments

(2 files)

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.
Attachment #8921103 - Flags: review?(valentin.gosu)
Attachment #8921103 - Flags: review?(francois)
Attachment #8921104 - Flags: review?(valentin.gosu)
Attachment #8921104 - Flags: review?(francois)
: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 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).
Attachment #8921103 - Flags: review?(francois) → review+
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": ["hurley@mozilla.com"],
> +    "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).
Attachment #8921104 - Flags: review?(francois) → review-
Fixed the comments from your previous look, thanks 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+
Attachment #8921104 - Flags: review?(francois) → review+
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
Attachment #8921104 - Flags: review?(valentin.gosu) → review+
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
Attachment #8921103 - Flags: review?(valentin.gosu) → review+
Pushed by hurley@mozilla.com:
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
Blocks: 1430322
You need to log in before you can comment on or make changes to this bug.