status-im / status-mobile

a free (libre) open source, mobile OS for Ethereum
https://status.app
Mozilla Public License 2.0
3.9k stars 988 forks source link

Show connection stats inside the app #6568

Closed vitvly closed 5 years ago

vitvly commented 5 years ago

Description

In order to debug some of the connection issues, it would be nice to enable connection stats logging on the client side. E.g., it can be hidden under a Profile->Advanced menu, and would display

Related to #6567

EugeOrtiz commented 5 years ago

This is very technical information to solve a specific issue. Why do we need to show it to people that are not developers? What value do we offer with this? Is there any other way to obtain this info somehow without showing it in the UI? @siphiuel @chadyj

vitvly commented 5 years ago

@EugeOrtiz This might be hidden under Developer-only settings. This would assist in debugging end-user connectivity issues - users would be able to see and send us their network stats/logs. Right now we can only get app crash dumps, and only when app crashed. Sometimes app seems to be healthy, but there are no data sent. So if an end-user experiences message delivery issues, they cannot really see what's going on.

All this info can be stored in logs, but a UI solution seems to be a nice option to have.

This is partially inspired by Apple Mail Connection Doctor (https://support.apple.com/en-us/HT204173)

Apple Support
Use Mail Connection Doctor
Mail Connection Doctor can help explain why the Mail app isn't sending or receiving email on your Mac.
chadyj commented 5 years ago

@siphiuel Is there an exact plan for this and how to implement? If it is too vague we ca remove from the release milestone?

adambabik commented 5 years ago

Please use these artifacts https://ci.status.im/job/status-go/job/parallel/141/ or enable-metrics-by-default branch in status-go.

Metrics should be available through debug_metrics JSON-RPC command. It takes one param (true or false) which modifies the output to raw or formatted numbers.

status-go » parallel #141 [Jenkins]
vitvly commented 5 years ago

UI update: for now we will show these stats in Profile->Advanced Settings.

adambabik commented 5 years ago

@siphiuel is there a build with the stats working?

rcullito commented 5 years ago

hey @adambabik not yet. I can ping here once there is a build with this information included. tackling this next 👍

rcullito commented 5 years ago

hey @adambabik and cc @siphiuel I was curious if either of you had input on what specifically within debug-metrics you would like to see in order to provide the high level summary of the general state of connections?

Some ideas given what's already been posted a.) discv5: inbound and outbound traffic, mean rate b.) p2p: also inbound and outbound traffic c.) les: packets in/out

Let me know if you guys have any other suggestions, thanks!

chadyj commented 5 years ago

@rcullito Just an idea....include whatever you have and we can see what's useful in daily use. A quick first pass is good for the milestone then we can revisit later with tweaks and better UI.

mandrigin commented 5 years ago

@rcullito some marker showing which services are running on a node will be useful: LES, ETH, Whisper, Mailserver

adambabik commented 5 years ago

I recommend running status-go locally with

{
  "HTTPEnabled": true,
  "APIModules": "admin,debug,eth,net"
}

and then curl -v -H "Content-Type: application/json" --data '{"jsonrpc":"2.0","method":"debug_metrics","params":[true],"id":1}' http://localhost:8545

It will print the whole output. result.{whisper,rendezvous,p2p,les,discv5} are the most interesting for us for the time being.

rcullito commented 5 years ago

Sounds good re: the individual status-go metrics @mandrigin and @adambabik, thanks for those! All set and running with a local status-go node now

EugeOrtiz commented 5 years ago

@rcullito Not sure if this is the right issue, but here is the design (Figma) for the connected label in Adv settings

oskarth commented 5 years ago

Is this a blocker for release?

vitvly commented 5 years ago

Yes, this has to be included, related PR is https://github.com/status-im/status-react/pull/6990

vkjr commented 5 years ago

@churik, @siphiuel, can we close this now, when #6990 is merged?