VPN-6854: fix data throughput Glean issues #10296
Open
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
We record the data transfer telemetry from wireguard. This wireguard data only resets every time there is a new peer (VPN activation, silent server switch, location switch, etc.). We were sending it in every timer ping - which means we were counting data multiple times, as the subsequent timer/end ping would report the cumulative data since the user connected to the peer.
While there are probably ways to make fancy data science dashboards that only use the latest one reported, it seems easiest to fix this by only reporting data transfer when the peer is changed (or the VPN is turned off). (This also has a benefit of allowing adhoc metrics to be run more easily.)
Desktop - We just need to remove the data transfer metrics from the timer ping.
Android - This one was a bit harder, as there were some underlying data issues I noticed (also fixed in this PR):
wasTimerJustStarted
to fix this.shouldRecordTimerAndEndMetrics
was very, very wrong. On every activation in Android (from app, location switch, silent server switch from app, silent server switch from daemon) we run throughturnOn
. However,turnOff
is only hit when the actual VPN is coming down - not when doing a silent server switch or location switch. Thus, we always want to record ending metrics inturnOff
, and we want to skip start metrics inturnOn
if it's a silent server switch or location switch. Ultimately I removed theshouldRecordTimerAndEndMetrics
, as it was no longer useful.iOS - we do not record this data on iOS, so nothing to do here.
I've run through a bunch of tests on macOS and Android -
If there are any other scenarios to consider, please let me know.
Reference
VPN-6854
Checklist