1357207 - Caching duration telemetry appears to be broken

Status: RESOLVED FIXED

(Toolkit :: Safe Browsing, defect, P2)

Description

[François Marier [:francois]]

The following telemtry probes have recently thrown alerts:

- URLCLASSIFIER_NEGATIVE_CACHE_DURATION: http://alerts.telemetry.mozilla.org/index.html#/detectors/1/metrics/2181/alerts/?from=2017-04-11&to=2017-04-11

- URLCLASSIFIER_POSITIVE_CACHE_DURATION: http://alerts.telemetry.mozilla.org/index.html#/detectors/1/metrics/2182/alerts/?from=2017-04-11&to=2017-04-11

Because we have started to see negative and postive cache durations of approx 300 ms when they used to be 300k ms (i.e. 300 seconds):

- https://telemetry.mozilla.org/new-pipeline/dist.html#!cumulative=0&end_date=2017-04-16&keys=__none__!__none__!__none__&max_channel_version=nightly%252F55&measure=URLCLASSIFIER_NEGATIVE_CACHE_DURATION&min_channel_version=null&processType=*&product=Firefox&sanitize=1&sort_keys=submissions&start_date=2017-03-23&table=1&trim=1&use_submission_date=0
- https://telemetry.mozilla.org/new-pipeline/dist.html#!cumulative=0&end_date=2017-04-16&keys=__none__!__none__!__none__&max_channel_version=nightly%252F55&measure=URLCLASSIFIER_POSITIVE_CACHE_DURATION&min_channel_version=null&processType=*&product=Firefox&sanitize=1&sort_keys=submissions&start_date=2017-03-23&table=1&trim=1&use_submission_date=0

Given that the caching work of bug 1311935 is in the pushlog for the above alerts:

https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=731639fccc709a4dd95fed7e9dda88efb2227906&tochange=f914d40a48009c5acd1093e9939cc0ec035696dd

we should investigate to see if we perhaps forgot to multiply the telemetry numbers by 1000 when we switched the cache duration to seconds internally.

Comment 1

[Dimi Lee [:dimi]]

Attachment: Bug 1357207 - Caching duration recorded to telemetry should be milli-second .

In Bug 1311935, We change positive/negative cache duration from milli-second to second.
But the value doesn't covert back to milli-second when store to telemetry(telemetry use
milli-second).

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

Comment 2

[François Marier [:francois]]

Comment on attachment 8859058 [details]
Bug 1357207 - Caching duration recorded to telemetry should be milli-second .

https://reviewboard.mozilla.org/r/131084/#review133954

Thanks for renaming the variables. It's much less likely we'll get confused about units in the future :)

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/2963aec1e67f
Caching duration recorded to telemetry should be milli-second . r=francois

Comment 4

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/2963aec1e67f