DNS_TRR_COMPARE telemetry counter shows slow TRR as "NativeWorked"

RESOLVED FIXED in Firefox 62

Status

()

enhancement
P2
normal
RESOLVED FIXED
Last year
Last year

People

(Reporter: bagder, Assigned: bagder)

Tracking

Trunk
mozilla62
Points:
---

Firefox Tracking Flags

(firefox62 fixed)

Details

(Whiteboard: [necko-triaged][trr])

Attachments

(1 attachment)

Assignee

Description

Last year
The existing logic assumes that both mNativeSuccess and mTRRSuccess are true when both resolves worked, but if the native resolver is faster than the first TRR response then the mTRRSuccess counter is still at zero when this code hits and DNS_TRR_COMPARE then counts that as a "NativeWorked".

It also works the same in the other direction, if TRR gets a result before mNativeSuccess is set, it gets counted as "TRRWorked".

So, the labels are slightly misleading, but I'm not sure we can do much about the logic since when one of the responses is much faster than the other, we can't tell yet if the other resolve works or will work.
Assignee

Comment 1

Last year
Strike that.

The real problem is that TRR is counted as "attempted" even when the resolve is actually TRR blacklisted or failed in other ways before the request is sent off to the DOH server. This flaw makes "TRR attampted but failed" seem higher than expected in telemetry data.
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)

Comment 4

Last year
mozreview-review
Comment on attachment 8986422 [details]
bug 1463356 do not count "not started" TRR resolves as failures

https://reviewboard.mozilla.org/r/251786/#review258208

::: netwerk/dns/TRR.h:150
(Diff revision 2)
>    nsresult DohEncode(nsCString &target);
>    nsresult PassQName(unsigned int &index);
>    nsresult GetQname(nsAutoCString &aQname, unsigned int &aIndex);
>    nsresult DohDecode(nsCString &aHost);
>    nsresult ReturnData();
> -  nsresult FailData();
> +  nsresult FailData(nsresult error);

brief documentation here about special error code value
Attachment #8986422 - Flags: review?(mcmanus) → review+
Comment hidden (mozreview-request)

Comment 6

Last year
Pushed by daniel@haxx.se:
https://hg.mozilla.org/integration/autoland/rev/08b8d3da8635
do not count "not started" TRR resolves as failures r=mcmanus

Comment 7

Last year
bugherder
https://hg.mozilla.org/mozilla-central/rev/08b8d3da8635
Status: NEW → RESOLVED
Closed: Last year
Resolution: --- → FIXED
Target Milestone: --- → mozilla62
You need to log in before you can comment on or make changes to this bug.