Add peer info route #1918

iulianpascalau · 2020-06-10T18:27:44Z

added peer info route that will output information for a provided peer (or part of a peer) like: known addresses, its hex public key, if is black listed or not.
Usage: http://127.0.0.1:8080/node/peerinfo?pid=[part of the pid]

sasurobert · 2020-06-10T21:08:45Z

api/node/routes.go

+	}
+
+	queryVals := c.Request.URL.Query()
+	pids := queryVals["pid"]


move magic strings to constants "pid", "info" "succesful" "internal error"

I guess they will be replaced anyway when @bogdan-rosianu's PR will enter development.

replaced magic sting "pid", the other remained as they will be replaced.

sasurobert · 2020-06-10T21:14:14Z

api/node/routes.go

@@ -20,9 +20,17 @@ type FacadeHandler interface {
 	TpsBenchmark() *statistics.TpsBenchmark
 	StatusMetrics() external.StatusMetricsHandler
 	GetQueryHandler(name string) (debug.QueryHandler, error)
+	GetPeerInfo(pid string) ([]interface{}, error)


will this be changed from []interface{} ?

+1 to use the explicit struct.

remained from the usage of map[string]interface{} . Will change to an array of DTO

done, tricky when testing because gin somehow transforms []core.QueryP2PPeerInfo into map[string]map[string]interface{} automatically when it interprets the response ⚔️

sasurobert · 2020-06-10T21:20:00Z

sharding/networksharding/peerShardMapper.go

@@ -180,6 +182,7 @@ func (psm *PeerShardMapper) getPeerInfoSearchingPidInFallbackCache(pid core.Peer
 		return &core.P2PPeerInfo{
 			PeerType: core.UnknownPeer,
 			ShardID:  0,
+			PkBytes:  nil,


no need for nil initialization. delete from every space

agree, removed 👍

sasurobert · 2020-06-10T21:23:13Z

node/node.go

@@ -60,6 +62,15 @@ var _ facade.NodeHandler = (*Node)(nil)
 //  over the None struct.
 type Option func(*Node) error

+// QueryP2PPeerInfo represents a DTO used in exporting p2p peer info after a query
+type QueryP2PPeerInfo struct {


maybe move to a DTO package.

done, moved in core

andreibancioiu · 2020-06-10T21:40:58Z

api/node/routes.go

 	IsInterfaceNil() bool
 }

+// TODO remove this struct, use shared.GenericAPIResponse


Perhaps add a task in JIRA to make the updates after Bogdan's PR?

done, EN-6756

why? it's just a merge which I have to do anyway

not quite, you will merge it into development, this will be merged in release-candidate which will go into master and then (through a merge that will be done automatically as there won't be conflicts) into dev. If this merge will be done after you merged into dev the generic api response, will miss working on TODOs. Task can be safely closed afterwords if you make the changes but we won't forget to do it.

andreibancioiu · 2020-06-10T21:42:13Z

api/node/routes.go

+	}
+
+	queryVals := c.Request.URL.Query()
+	pids := queryVals["pid"]


I guess they will be replaced anyway when @bogdan-rosianu's PR will enter development.

andreibancioiu · 2020-06-10T21:44:03Z

api/node/routes_test.go

+
+	facade := &mock.WrongFacade{}
+	ws := startNodeServerWithFacade(facade)
+	req, _ := http.NewRequest("GET", "/node/peerinfo?pid=asasdasd", nil)


Perhaps use a prettier dummy string?

andreibancioiu · 2020-06-10T21:52:37Z

node/node.go

+	}
+
+	sort.Slice(pidsFound, func(i, j int) bool {
+		return pidsFound[i].Pretty() < pidsFound[j].Pretty()


Finally a nice feature of go, this built-in string comparison operator :)

andreibancioiu · 2020-06-10T22:01:09Z

node/node.go

+		IsBlacklisted: n.peerBlackListHandler.Has(p),
+	}
+
+	pInfo := n.networkShardingCollector.GetPeerInfo(p)


Perhaps rename to peerInfo? This name triggers awful memories of lpSomethingSomething from Win32 API.

pInfo -> peerInfo

andreibancioiu · 2020-06-10T22:12:54Z

api/node/routes.go

@@ -20,9 +20,17 @@ type FacadeHandler interface {
 	TpsBenchmark() *statistics.TpsBenchmark
 	StatusMetrics() external.StatusMetricsHandler
 	GetQueryHandler(name string) (debug.QueryHandler, error)
+	GetPeerInfo(pid string) ([]interface{}, error)


+1 to use the explicit struct.

andreibancioiu · 2020-06-10T22:13:30Z

api/node/routes.go

@@ -60,6 +68,7 @@ func Routes(router *wrapper.RouterWrapper) {
 	router.RegisterHandler(http.MethodGet, "/status", StatusMetrics)
 	router.RegisterHandler(http.MethodGet, "/p2pstatus", P2pStatusMetrics)
 	router.RegisterHandler(http.MethodPost, "/debug", QueryDebug)
+	router.RegisterHandler(http.MethodGet, "/peerinfo", PeerInfo)


Always on? Or just for debugging purpose?

always on, it will be a neat feature of the node

bogdan-rosianu · 2020-06-11T06:55:06Z

node/node.go

+	Pid           string   `json:"pid"`
+	Addresses     []string `json:"addresses"`
+	Pk            string   `json:"pk"`
+	IsBlacklisted bool     `json:"isblacklisted"`


move the bool field to the top or the bottom (memory allign reasons)

…simplified peer shard mapper

- added peer info route

40adeda

iulianpascalau added the type:feature New feature or request label Jun 10, 2020

iulianpascalau self-assigned this Jun 10, 2020

Merge branch 'release-candidate' into EN-6752-add-peer-info-route

06dacd7

sasurobert self-requested a review June 10, 2020 18:37

andreibancioiu self-requested a review June 10, 2020 20:38

Merge branch 'release-candidate' into EN-6752-add-peer-info-route

03bd002

sasurobert requested changes Jun 10, 2020

View reviewed changes

andreibancioiu reviewed Jun 10, 2020

View reviewed changes

bogdan-rosianu reviewed Jun 11, 2020

View reviewed changes

- fixes after review: changed returned struct type in api, renaming, …

e2f5b21

…simplified peer shard mapper

sasurobert approved these changes Jun 11, 2020

View reviewed changes

bogdan-rosianu approved these changes Jun 11, 2020

View reviewed changes

andreibancioiu approved these changes Jun 11, 2020

View reviewed changes

LucianMincu merged commit aedd680 into release-candidate Jun 11, 2020

LucianMincu deleted the EN-6752-add-peer-info-route branch June 11, 2020 11:46

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add peer info route #1918

Add peer info route #1918

iulianpascalau commented Jun 10, 2020

sasurobert Jun 10, 2020

andreibancioiu Jun 10, 2020

iulianpascalau Jun 11, 2020

sasurobert Jun 10, 2020

andreibancioiu Jun 10, 2020

iulianpascalau Jun 11, 2020

iulianpascalau Jun 11, 2020 •

edited

sasurobert Jun 10, 2020

iulianpascalau Jun 11, 2020

sasurobert Jun 10, 2020

iulianpascalau Jun 11, 2020

andreibancioiu Jun 10, 2020

iulianpascalau Jun 11, 2020

bogdan-rosianu Jun 11, 2020

iulianpascalau Jun 11, 2020

andreibancioiu Jun 10, 2020

andreibancioiu Jun 10, 2020

iulianpascalau Jun 11, 2020

andreibancioiu Jun 10, 2020

iulianpascalau Jun 11, 2020

andreibancioiu Jun 10, 2020

iulianpascalau Jun 11, 2020

andreibancioiu Jun 10, 2020

andreibancioiu Jun 10, 2020

iulianpascalau Jun 11, 2020

bogdan-rosianu Jun 11, 2020

iulianpascalau Jun 11, 2020

Add peer info route #1918

Add peer info route #1918

Conversation

iulianpascalau commented Jun 10, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

iulianpascalau Jun 11, 2020 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

iulianpascalau Jun 11, 2020 •

edited