dialdbg initial implementation #57

benjirewis · 2023-06-30T20:54:35Z

cc @edaniels , @cheukt , @zaporter-work , @mattjperez

Implements a basic CLI tool, dialdbg, to better understand the behavior of rust-utils' dial functionality (both through gRPC and WebRTC).

dialdbg --help output:

dialdbg gives information on how rust-utils' dial function makes connections

Usage: viam-dialdbg [OPTIONS] --uri <URI>

Options:
  -u, --uri <URI>
          URI to dial. Must be provided
      --nogrpc
          Whether direct gRPC connection should not be examined. If not provided, gRPC connection will be examined
      --nowebrtc
          Whether WebRTC connection should not be examined. If not provided, WebRTC connection will be examined
  -o, --output <OUTPUT>
          Filepath for output of dialdbg (file will be overwritten). If not provided, dialdbg will output to STDOUT
  -c, --credential <CREDENTIAL>
          Credential payload with which to connect to the URI. If not provided, dialdbg will dial without credentials
  -t, --credential-type <CREDENTIAL_TYPE>
          Type of credential with which to connect to the URI. Can only be provided with "--credential". If "--credential" is provided but "--credential-type" is not, credential type will default to "robot-location-secret"
  -h, --help
          Print help
  -V, --version
          Print version

dialdbg --uri URI --credential ROBOTSECRET output where URI is a robot on the same subnet (connectable to through mDNS) and ROBOTSECRET is its robot location secret.

Debugging dial with basic gRPC...

mDNS address 10.1.5.217:35321 was used for connection
mDNS queried in 339ms
authentication successful in 56ms
gRPC connection establishment successful in 397ms

Done debugging dial with basic gRPC.

Debugging dial with WebRTC...

mDNS address 10.1.5.217:35321 was used for connection
mDNS queried in 176ms
authentication successful in 98ms
WebRTC connection establishment successful in 345ms
selected ICE candidate pair was:
	(local) udp host e6e2f09b-bb6c-4ae4-9c5a-9f1fea22bb13.local:54432 <-> (remote) udp host 10.1.5.217:35130

nominated ICE candidates:

	remote ICE candidate:
		IP address: 127.0.0.1
		port: 49866
		nominated 1.094708ms ago
		relay protocol: udp
		network type: udp4
	local ICE candidate:
		IP address: e6e2f09b-bb6c-4ae4-9c5a-9f1fea22bb13.local
		port: 53121
		nominated 1.105ms ago
		relay protocol: udp
		network type: udp6
	remote ICE candidate:
		IP address: 100.104.116.117
		port: 54473
		nominated 1.093708ms ago
		relay protocol: udp
		network type: udp4
	local ICE candidate:
		IP address: e6e2f09b-bb6c-4ae4-9c5a-9f1fea22bb13.local
		port: 54432
		nominated 1.104291ms ago
		relay protocol: udp
		network type: udp4
	remote ICE candidate:
		IP address: 10.1.5.217
		port: 35130
		nominated 1.094375ms ago
		relay protocol: udp
		network type: udp4
	local ICE candidate:
		IP address: 71.167.222.76
		port: 60829
		nominated 1.103916ms ago
		relay protocol: udp
		network type: udp4
	remote ICE candidate:
		IP address: fd7a:115c:a1e0:ab12:4843:cd96:6268:7475
		port: 56270
		nominated 1.09525ms ago
		relay protocol: udp
		network type: udp6

Done debugging dial with WebRTC.

dialdbg --uri BAD_URI --secret ROBOTSECRET output where bad_rover.json is an incorrect viam.cloud URI.

Debugging dial with basic gRPC...

mDNS could not be used to connect
authentication failed
gRPC connection establishment failed

unexpected dial connect error: status: PermissionDenied, message: "failed to authenticate: no location found", details: [], metadata: MetadataMap { headers: {"content-security-policy": "frame-ancestors 'self'; form-action 'self';", "content-type": "application/grpc", "strict-transport-security": "max-age=63072000; includeSubDomains; preload", "trailer": "Grpc-Status", "trailer": "Grpc-Message", "trailer": "Grpc-Status-Details-Bin", "vary": "Origin", "x-content-type-options": "nosniff", "x-frame-options": "SAMEORIGIN", "date": "Fri, 30 Jun 2023 20:50:04 GMT", "server": "Google Frontend", "x-cloud-trace-context": "012f431cf2557357c3024279dff20029/2554969079733192940", "traceparent": "00-012f431cf2557357c3024279dff20029-237512d7599a88ec-00", "content-length": "0"} }

Done debugging dial with basic gRPC.

Debugging dial with WebRTC...

mDNS could not be used to connect
authentication failed
WebRTC connection establishment failed

unexpected dial connect error: status: PermissionDenied, message: "failed to authenticate: no location found", details: [], metadata: MetadataMap { headers: {"content-security-policy": "frame-ancestors 'self'; form-action 'self';", "content-type": "application/grpc", "strict-transport-security": "max-age=63072000; includeSubDomains; preload", "trailer": "Grpc-Status", "trailer": "Grpc-Message", "trailer": "Grpc-Status-Details-Bin", "vary": "Origin", "x-content-type-options": "nosniff", "x-frame-options": "SAMEORIGIN", "date": "Fri, 30 Jun 2023 20:50:10 GMT", "server": "Google Frontend", "traceparent": "00-196a03bfe2f8f54a3e98cb162e75396e-bc5a856d927ccbd8-00", "x-cloud-trace-context": "196a03bfe2f8f54a3e98cb162e75396e/13572307132689992664", "content-length": "0"} }

Done debugging dial with WebRTC.

Similar errors are output for invalid secret and invalid non-viam URI respectively:

unexpected dial connect error: status: Unauthenticated, message: "invalid credentials", details: [], metadata: MetadataMap { headers: {"content-type": "application/grpc"} }

unexpected dial connect error: status: Unknown, message: "transport error", details: [], metadata: MetadataMap { headers: {} }

edaniels · 2023-07-05T13:52:40Z

Will take a look today!

edaniels

Done with a first pass. nice!

Cargo.toml

src/rpc/base_channel.rs

dialdbg/src/main.rs

zaporter-work

Mostly optional language comments. Logic looks reasonable

dialdbg/src/main.rs

dialdbg/src/parse.rs

npmenard

Done with first pass. Tricky to use log file to pass stats around (tracing could be better here) but for a POC i think its fine. I would definitely add a test to make sure that parsing works when the rust-lib gets updated.
Your parsing fails on the first error encountered, so where it makes sense I would continue parsing even in case of errors.

dialdbg/src/main.rs

src/rpc/dial.rs

stuqdog

First pass, looks pretty good!

src/rpc/dial.rs

dialdbg/src/main.rs

dialdbg/src/parse.rs

dialdbg/src/strings.rs

stuqdog · 2023-07-07T14:21:05Z

(removed @jckras as a reviewer to avoid a "too many cooks" situation, Julie feel free to look of course if you're interested!)

edaniels

LGTM from my side but I am yet a lowly rust newbie.

dialdbg/src/parse.rs

npmenard

LGTM

zaporter-work · 2023-07-11T16:09:13Z

Don't have time for in-depth review, but overall LGTM!

benjirewis · 2023-07-11T20:36:58Z

src/rpc/dial.rs

+        // NOTE(benjirewis): Use a duration of 1500ms for getting the mDNS URI. I've anecdotally
+        // seen times as great as 922ms to fetch a non-loopback mDNS URI. With an
+        // interface_with_loopback query interval of 250ms, 1500ms here should give us time for ~6
+        // queries.


@cheukt / @stuqdog , I lowered both the interface_with_loopback query interval and the get_mdns_uri timeout to allow for faster-completing mDNS discovery (@erh hopefully this fixes your issue with excessively long dials when mDNS is enabled but not used).

stuqdog

LGTM!

src/rpc/dial.rs

dialdbg/src/main.rs

stuqdog · 2023-07-12T18:33:39Z

dialdbg/src/parse.rs

+pub(crate) const DIAL_ERROR_PREFIX: &'static str = "unexpected dial connect error";
+
+#[derive(Debug, Default)]
+pub(crate) struct GRPCResult {


Probably over-engineering right now, but it would be neat if we could dedup some of the shared code across GRPCResult and WebRTCResult

That's a good point; I probably won't in this PR, but I can imagine deduping more in the future as the code around these structs increases (potentially as part of RSDK-3444).

dialdbg/src/parse.rs

benjirewis requested review from npmenard and stuqdog June 30, 2023 21:02

benjirewis marked this pull request as ready for review June 30, 2023 21:02

benjirewis requested a review from a team as a code owner June 30, 2023 21:02

benjirewis requested review from jckras and removed request for a team June 30, 2023 21:02

edaniels suggested changes Jul 5, 2023

View reviewed changes

zaporter-work reviewed Jul 5, 2023

View reviewed changes

npmenard requested changes Jul 5, 2023

View reviewed changes

dialdbg/src/main.rs Outdated Show resolved Hide resolved

dialdbg/src/main.rs Outdated Show resolved Hide resolved

src/rpc/dial.rs Outdated Show resolved Hide resolved

stuqdog requested changes Jul 7, 2023

View reviewed changes

stuqdog removed the request for review from jckras July 7, 2023 14:20

benjirewis force-pushed the dialdbg branch from 010c63f to 46bdeab Compare July 7, 2023 20:26

edaniels approved these changes Jul 7, 2023

View reviewed changes

benjirewis requested review from npmenard, stuqdog and zaporter-work July 10, 2023 23:54

npmenard reviewed Jul 11, 2023

View reviewed changes

dialdbg/src/parse.rs Outdated Show resolved Hide resolved

npmenard approved these changes Jul 11, 2023

View reviewed changes

zaporter-work removed their request for review July 11, 2023 16:08

benjirewis force-pushed the dialdbg branch from 6d3572a to f69e837 Compare July 11, 2023 20:33

benjirewis commented Jul 11, 2023

View reviewed changes

benjirewis added 6 commits July 12, 2023 11:32

initial commit

c22b3a6

eric comment

35a36a4

remove references to robots and secrets

96eb743

zack comments

fb52eac

nicolas comments

624f39f

ethan comments

32a418b

benjirewis added 7 commits July 12, 2023 11:32

share consts

30e77ae

grpcresult mdns -> mdns_query

39a5b1d

mdns fixes

7e05cb7

move dial attempt log earlier

ecd15bd

lower query interval

8abfae2

no unwrap

9292f17

lock changes

9632966

benjirewis force-pushed the dialdbg branch from f69e837 to 9632966 Compare July 12, 2023 15:37

update dep decisions

fdf3710

stuqdog approved these changes Jul 12, 2023

View reviewed changes

ethan final comments

25d1136

benjirewis merged commit 3468c50 into viamrobotics:main Jul 12, 2023

benjirewis deleted the dialdbg branch July 12, 2023 19:56

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

dialdbg initial implementation #57

dialdbg initial implementation #57

benjirewis commented Jun 30, 2023 •

edited

Loading

edaniels commented Jul 5, 2023

edaniels left a comment

zaporter-work left a comment

npmenard left a comment •

edited

Loading

stuqdog left a comment

stuqdog commented Jul 7, 2023

edaniels left a comment

npmenard left a comment

zaporter-work commented Jul 11, 2023 •

edited

Loading

benjirewis Jul 11, 2023

stuqdog left a comment

stuqdog Jul 12, 2023

benjirewis Jul 12, 2023

dialdbg initial implementation #57

dialdbg initial implementation #57

Conversation

benjirewis commented Jun 30, 2023 • edited Loading

edaniels commented Jul 5, 2023

edaniels left a comment

Choose a reason for hiding this comment

zaporter-work left a comment

Choose a reason for hiding this comment

npmenard left a comment • edited Loading

Choose a reason for hiding this comment

stuqdog left a comment

Choose a reason for hiding this comment

stuqdog commented Jul 7, 2023

edaniels left a comment

Choose a reason for hiding this comment

npmenard left a comment

Choose a reason for hiding this comment

zaporter-work commented Jul 11, 2023 • edited Loading

benjirewis Jul 11, 2023

Choose a reason for hiding this comment

stuqdog left a comment

Choose a reason for hiding this comment

stuqdog Jul 12, 2023

Choose a reason for hiding this comment

benjirewis Jul 12, 2023

Choose a reason for hiding this comment

benjirewis commented Jun 30, 2023 •

edited

Loading

npmenard left a comment •

edited

Loading

zaporter-work commented Jul 11, 2023 •

edited

Loading