mozilla-mobile / mozilla-vpn-client

A fast, secure and easy to use VPN. Built by the makers of Firefox.
https://vpn.mozilla.org
Other
453 stars 108 forks source link

Implement Ping probe #5341

Closed data-sync-user closed 1 year ago

data-sync-user commented 1 year ago

Current implementation:

Currently we generate a ping every 1 second to a specific server and see if we get a response back. This happens in pinghelper.cpp. Connection health uses PingHelper to collect various data such as standard deviation, latency and max duration. The issue with this approach is that if/when we fail to receive a response from a ping, we can’t establish a connection without really understanding what went wrong. Is it an issue with the server we are pinging, or is it an issue with the network connectivity?

Expected implementation:

Rather than a single server ping, we want to ping all of the servers present in the VPN location the user has selected. For example, if a user is trying to connect to the VPN through Los Angeles, California, we should ping all 31 servers present in that location and see how many responses we receive back. Based on the number of responses received, we can somewhat confidently derive a conclusion. For example, if our of 31 servers, we receive responses back from 30 servers, we can confidently assume that a single server is most likely down and the network connection is stable. On the other end of the spectrum, if we don’t get any pings back, we can safely assume that there is an issue with the local network.

There are a couple of ways to go about where to implement this:

  1. Add this logic to connection health
  2. Create a new top level class that is only activated when a problem us detected, in which case we use this new case and ignore the connection health class. Depending on the complexity, this could be a superior approach as it allow us to move away from the code in ConnectionHealth and eventually get rid of it altogether.

We need to implement a probe tool (e.g. an inspector command) which would take all of the servers in a user’s chosen city and ping them all – this can be done in parallel to speed things up. The front end team can use this command to mock scenarios and implement the user facing UI.

Challenges:

┆Issue is synchronized with this Jira Task ┆Reporter: Santiago Andrigo

data-sync-user commented 1 year ago

➤ Gela Malek Pour commented:

Santiago Andrigo When you get a chance could you update the issue with more details such as a description and expectations? I’m currently unclear on what needs to be done

data-sync-user commented 1 year ago

➤ Santiago Andrigo commented:

The PRD / design doc is here: https://mozilla-hub.atlassian.net/wiki/spaces/SECPRV/pages/130809909/Improve+our+handling+of+network+connectivity+issues ( https://mozilla-hub.atlassian.net/wiki/spaces/SECPRV/pages/130809909/Improve+our+handling+of+network+connectivity+issues|smart-link )

(Since this is not a story, it doesn’t have acceptance criteria - but you can find it in the associated story)

If it’s not clear on the technical implementation side, then I think Naomi Kirby is the right person to consult.

data-sync-user commented 1 year ago

➤ Betty Fleming commented:

The results of this spike will feed into https://mozilla-hub.atlassian.net/browse/VPN-4918 ( https://mozilla-hub.atlassian.net/browse/VPN-4918|smart-link ), which is an Epic level tech doc for the overarching work. The pull request that was created as part of this spike will be repurposed as part of the implementation for that over-arching work. We will close this spike as ‘Done’.