1297865 - Update Safe Browsing histograms

Status: RESOLVED FIXED

(Toolkit :: Safe Browsing, defect, P2)

Description

[François Marier [:francois]]

Many of the Safe Browsing histograms don't include alert email addresses.

Also, the description of some of them is lacking (e.g. https://bugzilla.mozilla.org/show_bug.cgi?id=1291472#c23).

Comment 1

[François Marier [:francois]]

I submitted a Service Now ticket (RITM0046933) to create a [e-mail] list. I'll use that in the alert field so that we don't need to update histograms.json whenever someone new wants to get added.

Comment 2

[François Marier [:francois]]

I could not find any reports for URLCLASSIFIER_PS_FAILURE in all of the releases (Nightlies, Releases and Betas) I looked.

It's only used in this goto: http://searchfox.org/mozilla-central/rev/44f6964ba95b8ddd8ebf70c55b34cd2323afeef4/toolkit/components/url-classifier/LookupCache.cpp#492

I suggest we take it out.

Comment 3

[François Marier [:francois]]

Attachment: Bug 1297865 - Remove unused URLCLASSIFIER_PS_FAILURE probe.

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

Comment 4

[François Marier [:francois]]

Attachment: Bug 1297865 - Extend Safe Browsing telemetry probes we are still using.

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

Comment 5

[François Marier [:francois]]

Attachment: Bug 1297865 - Add an email address to all Safe Browsing telemetry probes.

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

Comment 6

[François Marier [:francois]]

Attachment: Bug 1297865 - Improve the description of Application Reputation telemetry probes.

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

Comment 7

[Gian-Carlo Pascutto [:gcp]]

Comment on attachment 8785089 [details]
Bug 1297865 - Remove unused URLCLASSIFIER_PS_FAILURE probe.

https://reviewboard.mozilla.org/r/74408/#review73236

Comment 8

[Gian-Carlo Pascutto [:gcp]]

Comment on attachment 8785090 [details]
Bug 1297865 - Extend Safe Browsing telemetry probes we are still using.

https://reviewboard.mozilla.org/r/74410/#review73244

::: toolkit/components/telemetry/Histograms.json:3674
(Diff revision 1)
>      "bug_numbers": [1150921],
>      "description": "Server HTTP status code from remote SafeBrowsing gethash lookups. (0=1xx, 1=200, 2=2xx, 3=204, 4=3xx, 5=400, 6=4xx, 7=403, 8=404, 9=408, 10=413, 11=5xx, 12=502|504|511, 13=503, 14=505, 15=Other)"
>    },
>    "URLCLASSIFIER_COMPLETE_TIMEOUT": {
>      "alert_emails": ["[e-mail]", "[e-mail]"],
> -    "expires_in_version": "52",
> +    "expires_in_version": "56",

Maybe make this never? It's opt-in for release anyway, and we'll want to know when completes fail. I guess if those are no longer really a thing for SBv4 we won't need it after that switch, though.

Comment 9

[Gian-Carlo Pascutto [:gcp]]

Comment on attachment 8785091 [details]
Bug 1297865 - Add an email address to all Safe Browsing telemetry probes.

https://reviewboard.mozilla.org/r/74412/#review73256

Comment 10

[Gian-Carlo Pascutto [:gcp]]

Comment on attachment 8785092 [details]
Bug 1297865 - Improve the description of Application Reputation telemetry probes.

https://reviewboard.mozilla.org/r/74414/#review73258

Comment 11

[Benjamin Smedberg]

Comment on attachment 8785089 [details]
Bug 1297865 - Remove unused URLCLASSIFIER_PS_FAILURE probe.

https://reviewboard.mozilla.org/r/74408/#review73718

Comment 12

[Benjamin Smedberg]

Comment on attachment 8785090 [details]
Bug 1297865 - Extend Safe Browsing telemetry probes we are still using.

https://reviewboard.mozilla.org/r/74410/#review73722

::: toolkit/components/telemetry/Histograms.json:80
(Diff revision 1)
>      "kind": "enumerated",
>      "n_values": 3,
>      "description": "Application reputation remote status (0=OK, 1=FAIL, 2=INVALID)"
>    },
>    "APPLICATION_REPUTATION_SERVER_VERDICT": {
> -    "expires_in_version": "52",
> +    "expires_in_version": "56",

I presume that you've already proved that these are valuable metrics. Do you have a link to the report that you're using to monitor these?

::: toolkit/components/telemetry/Histograms.json:3629
(Diff revision 1)
>      "high": 1000,
>      "n_buckets": 10,
>      "description": "Time spent loading PrefixSet from file (ms)"
>    },
>    "URLCLASSIFIER_PS_FALLOCATE_TIME": {
> -    "expires_in_version": "default",
> +    "expires_in_version": "never",

What is your monitoring plan for this? Do you have a dashboard or some kind of alerting to notify you if this gets worse?

Comment 13

[Benjamin Smedberg]

Comment on attachment 8785091 [details]
Bug 1297865 - Add an email address to all Safe Browsing telemetry probes.

https://reviewboard.mozilla.org/r/74412/#review73726

Comment 14

[Benjamin Smedberg]

Comment on attachment 8785092 [details]
Bug 1297865 - Improve the description of Application Reputation telemetry probes.

https://reviewboard.mozilla.org/r/74414/#review73728

Comment 15

[François Marier [:francois]]

(In reply to Benjamin Smedberg [:bsmedberg] from comment #12)
> ::: toolkit/components/telemetry/Histograms.json:80
> (Diff revision 1)
> >      "kind": "enumerated",
> >      "n_values": 3,
> >      "description": "Application reputation remote status (0=OK, 1=FAIL, 2=INVALID)"
> >    },
> >    "APPLICATION_REPUTATION_SERVER_VERDICT": {
> > -    "expires_in_version": "52",
> > +    "expires_in_version": "56",
> 
> I presume that you've already proved that these are valuable metrics. Do you
> have a link to the report that you're using to monitor these?

I'm looking at these manually and discussing them with Google whenever they change.

> ::: toolkit/components/telemetry/Histograms.json:3629
> (Diff revision 1)
> >      "high": 1000,
> >      "n_buckets": 10,
> >      "description": "Time spent loading PrefixSet from file (ms)"
> >    },
> >    "URLCLASSIFIER_PS_FALLOCATE_TIME": {
> > -    "expires_in_version": "default",
> > +    "expires_in_version": "never",
> 
> What is your monitoring plan for this? Do you have a dashboard or some kind
> of alerting to notify you if this gets worse?

One of the other patches adds an alert email so that we can get notified if this gets worse.

I'm planning to work on better monitoring for Safe Browsing telemetry next quarter. This patch is just to clean up and preserve what we have at the moment.

Comment 16

[François Marier [:francois]]

Benjamin, do you need more information for the remaining review?

Comment 17

[Benjamin Smedberg]

Comment on attachment 8785090 [details]
Bug 1297865 - Extend Safe Browsing telemetry probes we are still using.

https://reviewboard.mozilla.org/r/74410/#review77060

ok. Be aware that current alerting only really works properly for scalar values (performance data). But it seems that you're using it effectively. I'd love to see a good public dashboard out of the work you're planning for next quarter.

Comment 18

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/073c18f01cc1
Remove unused URLCLASSIFIER_PS_FAILURE probe. r=bsmedberg,gcp
https://hg.mozilla.org/integration/autoland/rev/b73338e635ca
Extend Safe Browsing telemetry probes we are still using. r=bsmedberg,gcp
https://hg.mozilla.org/integration/autoland/rev/144ae7fb5144
Add an email address to all Safe Browsing telemetry probes. r=bsmedberg,gcp
https://hg.mozilla.org/integration/autoland/rev/be483544504d
Improve the description of Application Reputation telemetry probes. r=bsmedberg,gcp

Comment 19

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/073c18f01cc1
https://hg.mozilla.org/mozilla-central/rev/b73338e635ca
https://hg.mozilla.org/mozilla-central/rev/144ae7fb5144
https://hg.mozilla.org/mozilla-central/rev/be483544504d