1648004 - This telemetry probe looks wrong DNS_TRR_REQUEST_PER_CONN or do we have some bug in the request dispatching

Status: NEW

(Core :: Networking: DNS, task, P2)

Description

[Dragana Damjanovic [:dragana]]

https://telemetry.mozilla.org/new-pipeline/dist.html#!cumulative=0&end_date=2020-06-22&include_spill=0&keys=__none__!__none__!__none__&max_channel_version=nightly%252F79&measure=DNS_TRR_REQUEST_PER_CONN&min_channel_version=null&processType=*&product=Firefox&sanitize=1&sort_by_value=0&sort_keys=submissions&start_date=2020-06-01&table=0&trim=1&use_submission_date=0

Comment 1

[Kershaw Chang [:kershaw]]

Nhi, could you find someone to take a look?

Comment 2

[Valentin Gosu [:valentin]]

So, I looked into this locally, and it seemed to work fine.
What I did:

• Turned on TRR.
• Navigated a bit.
• Turned my network on/off
• checked about:telemetry for the probe
• It showed 68 requests.

I think the problem here is that we're only reporting this value in the destructor of ~Http2Session.
Since this connection is usually really long lived (probably never shuts down) I would expect that most people hit that part of the code at shutdown.
From what I can tell, it is usually destroyed before TelemetryShutdown, but according to telemetry people: if your recording happens before TelemetryControllerParent.jsm#914, the it might get recorded It does happen before that, but it's still not clear if it actually gets sent to the server.

So the question is do these values always get recorded? If the answer is no, then the probe is unreliable.

As to why we have a bunch of probes with 1-2, it's not clear to me why that happens. Also, these graphs look odd to me.

Note that if we don't trim small buckets or sanitize data, we also see some of the larger buckets. here

Comment 3

[Dragana Damjanovic [:dragana]]

Interesting, what happened on May 1st? :)

Comment 4

[Valentin Gosu [:valentin]]

chutten on #telemetry:mozilla.org:

You can set yourself up as an async shutdown blocker if you want to be certain of your accumulation counting: https://firefox-source-docs.mozilla.org/toolkit/modules/toolkit_modules/AsyncShutdown.html#list-of-phases
But in many cases Telemetry works better if you keep it up to date with your information over time and don't worry about when pings are sent.
Hm. Telemetry shutdown happens after net teardown, so you should normally be safe. But when you throw non-main-threads on it you can get weirdness
Might be worth a canary boolean Scalar that is just "shutdown telemetry metrics successfully reported" when it's true. Then you could see in the wild how often in a "main" ping with reason "shutdown" it is false.
Otherwise you'll have to add yourself as a blocker of a shutdown phase. Lucky you!