1509882 - Telemetry shouldn't attempt to send network requests during shutdown

Status: REOPENED

(Toolkit :: Telemetry, defect, P3)

Description

[Florian Quèze [:florian]]

This profile shows that 415ms after the profile-change-net-teardown notification, Telemetry code attempts to do 10 network requests (and they all receive an 'error' DOM event)
https://perfht.ml/2RenwsF

This https://perfht.ml/2RfgJPo shows the JS code involved here.

Comment 1

[Georg Fritzsche [:gfritzsche]]

chutten mentioned that things that stand out are:
- Prio is expensive. That is not used on the release channel though.
- We apparently still send pings / do network requests after network shutdown. We should have code preventing us from that, we need to check into that.

Comment 2

[Chris H-C :chutten]

Prio's perf is being tracked by bug 1509511

And we do have code that aims to prevent us from sending pings too late in shutdown (specifically, after profile-change-net-teardown) thanks to bug 1397293. It even sends us Telemetry, which has been relatively constant since its introduction: https://mzl.la/2PSRLst

(ignore the weird recent spikes. That happens for every new build due to the nature of the data being collected)

Comment 3

[Chris H-C :chutten]

The profile shows a session of length 6s which means delayed init likely didn't complete until after profile-change-net-teardown which means the observer was likely not added until after the topic was notified.

This is a common problem for Telemetry in short (under 2min) sessions and I don't think we have a plan for better handling that at this time.

Comment 4

[Florian Quèze [:florian]]

(In reply to Chris H-C :chutten from comment #3)

The profile shows a session of length 6s which means delayed init likely didn't complete until after profile-change-net-teardown which means the observer was likely not added until after the topic was notified.

I don't understand this. If the observer wasn't added, why are we sending network requests at all, rather than dropping everything on the floor?

This is a common problem for Telemetry in short (under 2min) sessions

If this is indeed a common problem, then it sounds like wontfix is really not the best option. Is there a way to address the common problem more generally so that Telemetry doesn't have unexpected behaviors for short lived sessions?

Comment 5

[Chris H-C :chutten]

... actually, maybe I'm wrong here. _sendingEnabled should be false until the observer has been registered, which means canSendNow is false which means any ping that is submitted should be saved to disk instead of triggered to be sent.

Maybe this requires more investigation.

Comment 6

[Florian Quèze [:florian]]

If you need a simple way to return early when some JS code is running during shutdown, you can use Services.startup.shuttingDown.