feat: add DNS and TCP reports to test-connectivity #272
Conversation
|
I added timing for DNS, but not TCP yet. It requires a map of net-address to start time. |
|
@fortuna I like the proposed approach. It basically makes the context specific to a given dialer which can solve the concurrency issue and provide access to hostname. We can technically refactor this into a |
|
@fortuna I made a light refactor. I also added start time and duration to TCP connection. Also, it seems like your branch is on the top of an older version that does not have the recent udp support for socks. I can do a rebase. |
|
I've moved your changes to #275. I'm not touching any dependencies in my PR, so if it doesn't have UDP SOCKS5, it's because main doesn't have it. |
|
Oh, nevermind, I started in an old main |
|
Ok, my branch is not on top of main @ HEAD |
|
Now with colored slog! |
x/examples/test-connectivity/main.go
Outdated
| switch proto { | ||
| case "tcp": | ||
| configToDialer := config.NewDefaultConfigToDialer() | ||
| configToDialer.BaseStreamDialer = transport.FuncStreamDialer(func(ctx context.Context, addr string) (transport.StreamConn, error) { |
There was a problem hiding this comment.
@fortuna I believe we should move setting both udp and tcp base dialers outside of the switch statement. In other words, we should set both udp and tcp base dialer regardless of the selected protocol since for example doing udp resolution over socks5 will do both udp and tcp operations.
In the current implementation, we cannot collect dns report if protocol is set to udp (-proto udp) and if the transport is socks5 since it would use plain tcp dialer which does not have the tracer setup.
You can checkout the changes I made on this branch: https://github.com/Jigsaw-Code/outline-sdk/blob/amir-tc-3/x/examples/test-connectivity/main.go
There was a problem hiding this comment.
If you try the cli with -proto udp over socks5 transport you can see that dns and tcp connect report are empty.
x/examples/test-connectivity/main.go
Outdated
| for _, ip := range di.Addrs { | ||
| report.AnswerIPs = append(report.AnswerIPs, ip.IP.String()) | ||
| } | ||
| // TODO(fortuna): Use a Mutex. |
There was a problem hiding this comment.
| // TODO(fortuna): Use a Mutex. | |
| mu.Lock() |
x/examples/test-connectivity/main.go
Outdated
| } | ||
| // TODO(fortuna): Use a Mutex. | ||
| dnsReports = append(dnsReports, report) | ||
| }, |
There was a problem hiding this comment.
| }, | |
| mu.Unlock() | |
| }, |
@amircybersec Check out this implementation.
I am able to get both TCP and DNS reports, while keeping the main test agnostic to the transport. (Thanks to your httptrace trick!)
Thoughts?