Conversation
For Project {Phantom}Drift.
ghost
assigned
phantom-drift/introspection.proto
Outdated
| // streams within this connection by inlining. | ||
| repeated Stream streams = 13; | ||
|
|
||
| reserved 14, 15; |
There was a problem hiding this comment.
Could probably group some fields to spare more 1-byte ids.
There was a problem hiding this comment.
Well, we can change the EndpointPair to Endpoint and add transport there. We can also create a new message Traffic to handle the DataGauge traffic_in and traffic_out on both Connection and Stream. What do you think?
There was a problem hiding this comment.
I like the Traffic idea! About the transport inside Endpoint, it sounds weird.
vasco-santos
left a comment
vasco-santos
left a comment
There was a problem hiding this comment.
This is shaping well raul! Great work!
I provided some feedback that I would like your input
phantom-drift/introspection.proto
Outdated
| } | ||
|
|
||
| // PeerId represents a peer ID. | ||
| // TODO: At this point I'm unsure if we'll return the string or the bytes representation. |
There was a problem hiding this comment.
I think we can keep the id_bytes for now. However, as we intend to provide the necessary data to the UI with no computations in the libp2p context, id_bytes should not be necessary
There was a problem hiding this comment.
Yes, I agree it should be a string only.
phantom-drift/introspection.proto
Outdated
| // streams within this connection by inlining. | ||
| repeated Stream streams = 13; | ||
|
|
||
| reserved 14, 15; |
There was a problem hiding this comment.
Well, we can change the EndpointPair to Endpoint and add transport there. We can also create a new message Traffic to handle the DataGauge traffic_in and traffic_out on both Connection and Stream. What do you think?
phantom-drift/introspection.proto
Outdated
| // the status of this connection. | ||
| Status status = 3; | ||
| // the transport managing this connection. | ||
| Transport transport = 4; |
There was a problem hiding this comment.
shouldn't this be just the transport name? I think the Transport type has a lot of redundant information
There was a problem hiding this comment.
I changed this to a transport_id, and added a bytes id to Transport. How does that sound?
| // connection listing. | ||
| repeated Connection conns = 4; | ||
| // streams listing. | ||
| repeated Stream streams = 5; |
There was a problem hiding this comment.
We have the StreamList in each Connection. Shouldn't it be enough?
| } | ||
|
|
||
| // Host is a top-level object conveying a cross-cutting view of the entire system, including all subsystems. | ||
| // TODO: WIP right now we're only covering the Swarm subsystem. |
There was a problem hiding this comment.
move this to the Subsystems definition
Co-Authored-By: Vasco Santos <vasco.santos@ua.pt>
|
@raulk FYI I added the DHT subsystem to the protobuf definition |
|
|
||
| message Bucket { | ||
| // bucket peers. | ||
| repeated bytes peers = 1; |
There was a problem hiding this comment.
Things missing here:
- latency of each peer (we track this)
- average latency of the bucket
- distance of the bucket
- peer age in bucket
| // latency for provide operations. | ||
| repeated uint64 latency_provide_ns = 6; | ||
| // counter for provide operations. | ||
| ResultCounter operation_provide = 7; |
| repeated Bucket buckets = 2; | ||
| } | ||
|
|
||
| message Operations { |
There was a problem hiding this comment.
call this OperationStatistics?
| // type of the query. | ||
| Type type = 5; | ||
| // rpc message of the query. | ||
| bytes rpc = 6; |
| } | ||
|
|
||
| // id of the query. | ||
| bytes id = 1; |
There was a problem hiding this comment.
ID of the target of the query, right?
There was a problem hiding this comment.
Since the type is bytes and elsewhere peer Ids are string, I suspect this is intended for internal referencing, like connection and stream ids? In which case, we should add a string field like target_peer_id?
(related: should the peer lists in Query and Bucket in the DHT section be repeated string not repeated bytes?)
There was a problem hiding this comment.
This ID can represent a peer ID or a CID. I'm fine making these strings for now, so that they're presentable on UI immediately. If we make them bytes, the consumer would be burdened with converting to a string prior to presenting.
There was a problem hiding this comment.
Note that this object can represent three types of queries: peer, provider, key/value.
4 participants
For Project {Phantom}Drift (libp2p network visualizer). Some of these stats can be sourced from OpenCensus, others can't, and many elements are state, not stats.
Right now I'm only modelling the Swarm subsystem. Just getting some initial ideas across.
@vasco-santos – have a look.
This definition does not model the interaction between the tooling and the observed system. Right now it only tackles the data model. The protocol will be interactive and will likely make heavy use of gRPC messages and streams.