Add UDP, Time, & Duration to Connectivity Report #327
Conversation
| @@ -80,7 +92,8 @@ type errorJSON struct { | |||
| // Posix error, when available | |||
| PosixError string `json:"posix_error,omitempty"` | |||
| // TODO: remove IP addresses | |||
| Msg string `json:"msg,omitempty"` | |||
| Msg string `json:"msg,omitempty"` | |||
There was a problem hiding this comment.
We need to note that these two fields may contain Personally Identifiable Information (PII). We need to figure out a way to not leak that before this is actually used in production with user probes.
We should probably allowlist specific errors as we learn about them. Though we need the full message to learn about them.
| @@ -120,7 +134,9 @@ func init() { | |||
| } | |||
| func newTCPTraceDialer( | |||
| onDNS func(ctx context.Context, domain string) func(di httptrace.DNSDoneInfo), | |||
| onDial func(ctx context.Context, network, addr string, connErr error)) transport.StreamDialer { | |||
| onDial func(ctx context.Context, network, addr string, connErr error), | |||
There was a problem hiding this comment.
This design puts the onus on the user to match dial start and end. That also complicates the testing code.
Please change the design so that we have a single onDial that is called on the beginning of the dial, and returns the onDialDone handler, like we do for DNS.
In the case of TCP/UDP, you will need a map (net, addr) -> done handler that you can create here.
There was a problem hiding this comment.
@fortuna I am going to iterate on the proposed design. Ideally the user should just run the test and get report with a clean API that is part of the SDK (/x) and test-connectivity code would just use it.
| @@ -134,6 +150,9 @@ func newTCPTraceDialer( | |||
| onDNSDone = nil | |||
| } | |||
| }, | |||
| ConnectStart: func(network, addr string) { | |||
| onDialStart(ctx, network, addr) | |||
| }, | |||
| ConnectDone: func(network, addr string, connErr error) { | |||
| onDial(ctx, network, addr, connErr) | |||
There was a problem hiding this comment.
This should lookup the table to retrieve and call the onDone handler.
| @@ -240,6 +268,8 @@ func main() { | |||
| var mu sync.Mutex | |||
| dnsReports := make([]dnsReport, 0) | |||
| tcpReports := make([]tcpReport, 0) | |||
| udpReports := make([]udpReport, 0) | |||
| var connectStart = make(map[string]time.Time) | |||
There was a problem hiding this comment.
Move to the newTraceDialers
| @@ -282,10 +314,46 @@ func main() { | |||
| tcpReports = append(tcpReports, report) | |||
| mu.Unlock() | |||
| } | |||
| return newTCPTraceDialer(onDNS, onDial).DialStream(ctx, addr) | |||
| onDialStart := func(ctx context.Context, network, addr string) { | |||
There was a problem hiding this comment.
This should save the start time, and return the onDone handler, which can refer to the start time directly.
This PR adds UDP report, Time, & Duration to Connectivity Report. I also adds
MsgFullwhich captures the wrapped error message.UDP report can be helpful in understanding connectivity issues for SOCK5 transport in particular since any read/write issues during UDP association command communicated over TCP can lead to failure in UDP connect. In that case, UDP connection won't be reached and the UDP report will be missing indicating the issue.
I also suggest we refactor this out of examples so that other apps can import connectivity reporting function and just call it to get the report. Maybe we can move this to the report
packageorconnectivitypackages.Example report output using socks5 proxy:
{ "test": { "resolver": "8.8.8.8:53", "proto": "udp", "transport": "socks5://[email protected]:5000|ss://[email protected]:443", "time": "2024-11-11T21:56:59Z", "duration_ms": 5001, "error": { "op": "receive", "posix_error": "ETIMEDOUT", "msg": " i/o timeout", "msg_full": "receive DNS message failed: read message failed: read udp 10.10.10.214:49811->x.x.x.x:5000: i/o timeout" } }, "dns_queries": [ { "query_name": "proxy.soax.com", "time": "2024-11-11T21:56:59Z", "duration_ms": 25, "answer_ips": [ "23.109.157.92", "23.109.148.44", "23.109.90.116" ], "error": "" } ], "tcp_connections": [ { "hostname": "proxy.soax.com", "ip": "23.109.157.92", "port": "5000", "error": "", "time": "2024-11-11T21:56:59Z", "duration_ms": 148 } ], "udp_connections": [ { "hostname": "23.109.130.244", "ip": "23.109.130.244", "port": "5000", "error": "", "time": "2024-11-11T21:56:59Z", "duration_ms": 0 } ] }