1470813 - TRR: add telemetry probe to enhance DNS_TRR_RACE

Status: RESOLVED FIXED

(Core :: Networking: DNS, enhancement, P1)

Description

[Daniel Stenberg [:bagder]]

The replacement probe, should add two "faster than 100" categories as well compared to the old version.

Comment 1

[Daniel Stenberg [:bagder]]

Attachment: bug 1470813 - Telemetry::DNS_TRR_RACE2 adds two new categories

... compared to DNS_TRR_RACE, to also count when either of the resolves
were more than 100 milliseconds faster.

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

Comment 2

[Patrick McManus [:mcmanus]]

Comment on attachment 8987438 [details]
bug 1470813 - Telemetry::DNS_TRR_RACE2 adds two new categories

https://reviewboard.mozilla.org/r/252678/#review259624

data collection review too

Comment 3

[Daniel Stenberg [:bagder]]

Attachment: telemetry-dns_trr_race2.txt

Request for data collection review: Telemetry::DNS_TRR_RACE2

Comment 4

[François Marier [:francois]]

Comment on attachment 8987438 [details]
bug 1470813 - Telemetry::DNS_TRR_RACE2 adds two new categories

https://reviewboard.mozilla.org/r/252678/#review261410

::: toolkit/components/telemetry/Histograms.json:3265
(Diff revision 1)
>      "alert_emails": ["[e-mail]", "[e-mail]"],
>      "expires_in_version": "never",
>      "kind": "categorical",
> -    "labels": ["TRRFasterBy50", "TRRFaster", "NativeFaster", "NativeFasterBy50"],
> -    "bug_numbers": [1434852],
> +    "labels": ["TRRFasterBy100", "TRRFasterBy50", "TRRFaster", "NativeFaster", "NativeFasterBy50",
> +               "NativeFasterBy100"],
> +    "bug_numbers": [1470813],

I'd suggest you keep the original bug number in this array so that it's easier to extract the history of this probe.

Comment 5

[François Marier [:francois]]

Comment on attachment 8987851 [details]
telemetry-dns_trr_race2.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, Daniel Stenberg.

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 in pre-release channels only.

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 6

[Daniel Stenberg [:bagder]]

(In reply to François Marier [:francois] from comment #4)

> I'd suggest you keep the original bug number in this array so that it's
> easier to extract the history of this probe.

Thanks will do! I wasn't sure what's considered the right thing there.

Comment 7

[Daniel Stenberg [:bagder]]

Comment on attachment 8987438 [details]
bug 1470813 - Telemetry::DNS_TRR_RACE2 adds two new categories

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

Comment 8

[Mozilla Autoland]

We're sorry, Autoland could not rebase your commits for you automatically. Please manually rebase your commits and try again.

hg error in cmd: hg rebase -s f2fc11a02538de25ca4da6fa2a0cb72206ae191b -d 2fafbc23c259: rebasing 471125:f2fc11a02538 "bug 1470813 - Telemetry::DNS_TRR_RACE2 adds two new categories r=mcmanus" (tip)
merging netwerk/dns/nsHostResolver.cpp
merging toolkit/components/telemetry/Histograms.json
warning: conflicts while merging toolkit/components/telemetry/Histograms.json! (edit, then use 'hg resolve --mark')
unresolved conflicts (see hg resolve, then hg rebase --continue)

Comment 9

[Daniel Stenberg [:bagder]]

Comment on attachment 8987438 [details]
bug 1470813 - Telemetry::DNS_TRR_RACE2 adds two new categories

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

Comment 10

[Mozilla Autoland]

We're sorry, Autoland could not rebase your commits for you automatically. Please manually rebase your commits and try again.

hg error in cmd: hg rebase -s 0dca4aec3e1eb519a0345f26900641b860a547ad -d 3928fa2e88ee: rebasing 471126:0dca4aec3e1e "bug 1470813 - Telemetry::DNS_TRR_RACE2 adds two new categories r=mcmanus" (tip)
merging netwerk/dns/nsHostResolver.cpp
merging toolkit/components/telemetry/Histograms.json
warning: conflicts while merging toolkit/components/telemetry/Histograms.json! (edit, then use 'hg resolve --mark')
unresolved conflicts (see hg resolve, then hg rebase --continue)

Comment 11

[Daniel Stenberg [:bagder]]

Attachment: 0001-bug-1470813-Telemetry-DNS_TRR_RACE2-adds-two-new-cat.patch

My mozreview skills fail me, so I'm attaching the rebased replacement patch here

Comment 12

[Cristina Coroiu [:ccoroiu]]

While try to apply this patch I get this error:

ccoroiu@ubuntu:~/mozilla-unified$ hg qimport bz://1470813
Fetching... done
Parsing... done
adding 1470813 to series file
renamed 1470813 -> 0001-bug-1470813-Telemetry-DNS_TRR_RACE2-adds-two-new-cat.patch
ccoroiu@ubuntu:~/mozilla-unified$ hg qpush 
applying 0001-bug-1470813-Telemetry-DNS_TRR_RACE2-adds-two-new-cat.patch
patching file toolkit/components/telemetry/Histograms.json
Hunk #1 FAILED at 3244
1 out of 1 hunks FAILED -- saving rejects to file toolkit/components/telemetry/Histograms.json.rej
patch failed, unable to continue (try -v)
patch failed, rejects left in working directory
errors during apply, please fix and qrefresh 0001-bug-1470813-Telemetry-DNS_TRR_RACE2-adds-two-new-cat.patch

Could you please take a look?

Comment 13

[Daniel Stenberg [:bagder]]

Attachment: v2-0001-bug-1470813-Telemetry-DNS_TRR_RACE2-adds-two-new-.patch

Rebased. Clearly this patch was in a race with another one...

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/f0c99191daf9
Telemetry::DNS_TRR_RACE2 adds two new categories r=francois

Comment 15

[Andreea Pavel [:apavel]]

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