Add verbose mode to ipfs swarm connect

[whyrusleeping]

It would be nice to get a log of what the dialer is trying and how each thing succeeds or fails:

$ ipfs swarm connect /foo/bar
Trying /ip4/1.2.3.4/tcp/12312
Trying /ip4/127.0.0.1/tcp/4444
From /ip4/1.2.3.4/tcp/12312: Error, connection refused
...
...
etc

[kevina]

This should be easy. @whyrusleeping do you want me to do this?

[whyrusleeping]

@kevina Sure, go ahead. It may require digging into some libp2p stuff, but it shouldnt be too difficult

[kevina]

I think the best way to do this would be to use WithValue/Value in context to set the value to either a channel or a writer where Connect can send debug output to if it is defined. This should be sufficient to allow passing the information you want but I am not that familiar with that part of the code.

@whyrusleeping thoughts.

[whyrusleeping]

@kevina I like the context approach. Its probably important here to clearly define what the interfaces should look like. Other than that, go for it.

[kevina]

As far as the API goes the output is now:

{"Strings":["connect QmaCpDMGvV2BGHeYERUEnRQAwe3N8SzbUtfsmvsqQLuvuJ success"]}

Since the doesn't have any structure already, if the --verbose option is given there will simply be more values in that list. This will unfortunately not stream the information as it happens, I can do that but then the output will need to be different, maybe something like

{"Message": "Trying /ip4/1.2.3.4/tcp/12312"}
{"Message": ...}
{"Strings":["connect QmaCpDMGvV2BGHeYERUEnRQAwe3N8SzbUtfsmvsqQLuvuJ success"]}

That is each debug output will be send as a separate json value with the final value being the normal output.

For the value added to the context I think I will call it "DebugChannel" and have the value be a string channel. (Or I could make it a generic interface{} channel but I hate all that extra casting.) If the value exists then debug strings will be sent to it.

[kevina]

@whyrusleeping could I get some feedback on this

[whyrusleeping]

I dislike the current api. Its really not a good format, and isnt machine parseable at all. I think we can get away with changing it here.

A better format moving forward could look like repeated messages of type:

{
  "address": "addr dialed",
  "event": "begin|success|error",
  "message": "error if error",
  "extra": "maybe details such as 'reuseport' or other dial options here?",
}

What do you think? cc @Kubuxu @lgierth

[Kubuxu]

SGWM, we should make a note that more events might be added

[kevina]

@whyrusleeping

1. This took some digging to figure out where to emit the status messages, I found two likely candidates: (1) go-libp2p-swarm/swarm_dial.go:dialAddr and (2) go-libp2p-conn/dial.go:Dial which do you think would be better.

2. Also I think the message should also include the peer ID so the message type should be:

   {
    "peer": "peer id",
    "address": "addr dialed",
     "event": "begin|success|error",
     "message": "error if error",
     "extra": "maybe details such as 'reuseport' or other dial options here?",
   }

   agreed? the other alternative is to report a fuller address that included the peer id.

3. what exactly should we report if there is no dial attempt because we already have a connection maybe something like: {peer: peer1, event: success, message: "already have connection"} or do we what something more structured. RoutedHost.Connect() won't will just return if there is already a connection without calling the dialer so we won't have an ip address readily available.

[ghost]

This took some digging to figure out where to emit the status messages, I found two likely candidates: (1) go-libp2p-swarm/swarm_dial.go:dialAddr and (2) go-libp2p-conn/dial.go:Dial which do you think would be better.

Better put it in go-libp2p-swarm -- it's further up the dependency tree

"peer": "peer id",

Let's call this peerID or peer_id

"extra": "maybe details such as 'reuseport' or other dial options here?",

Let's add fields when we need them, opaque unspecced values == bad :)

[ghost]

Better put it in go-libp2p-swarm -- it's further up the dependency tree

Actually it would be best if this could happen as close to the respective command code as possible

[kevina]

@lgierth sorry, I completely missed your comments. But I am a little confused. What are you trying to tell me when you say "Actually it would be best if this could happen as close to the respective command code as possible."?

[kevina]

"extra": "maybe details such as 'reuseport' or other dial options here?",

Let's add fields when we need them, opaque unspecced values == bad :)

Okay, that was @whyrusleeping idea.

[whyrusleeping]

hrm... yeah. in retrospect maybe that wasnt the best idea on my part.

[whyrusleeping]

@kevina any update here?