Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

metrics: add transport label to p2p_peers_total #2728

Merged
merged 1 commit into from
May 18, 2016

Conversation

ghost
Copy link

@ghost ghost commented May 18, 2016

Gives us per-transport peers counts:

ipfs_p2p_peers_total{transport="/ip4/tcp"} 25
ipfs_p2p_peers_total{transport="/ip6/tcp"} 13
ipfs_p2p_peers_total{transport="/ip6/udp/utp"} 17

License: MIT
Signed-off-by: Lars Gierth larsg@systemli.org

@ghost ghost added the topic/tools Topic tools label May 18, 2016
for _, conn := range c.Node.PeerHost.Network().Conns() {
tr := ""
for _, proto := range conn.RemoteMultiaddr().Protocols() {
tr = tr + "/" + proto.Name
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe this would be good to have in go-multiaddr, Multiaddr.ProtocolsString()

@ghost
Copy link
Author

ghost commented May 18, 2016

Hold on, I'll also add a version label, so that we can also graph peer counts by remote-version.

@Kubuxu Kubuxu added the need/author-input Needs input from the original author label May 18, 2016
@ghost
Copy link
Author

ghost commented May 18, 2016

Mh does anything store the remote version? The peerstore maybe? I don't wanna fire any requests, just read existing data.

@ghost
Copy link
Author

ghost commented May 18, 2016

Ok let's go ahead with this one, without remote-version.

17:17+0200 <@lgierth> daviddias: ideally the agent version -- e.g. that peer is "foo-ipfs/1.2.3"
17:18+0200 <@lgierth> i just thought i'd add that dimension to the peers count metric too while i'm at it -- i got the transport dimension yesterday, e.g. transport="/ip6/udp/utp"
17:21+0200 <@daviddias> lgierth: we don't really store or send that
17:21+0200 <@daviddias> we treat each proto as a standalone thing
17:21+0200 <@daviddias> and the handshake happens per stream
17:22+0200 <@daviddias> for all that is worth, the ipfs version doesn't matter between two peers, as long as they have compatible wire protocols
17:22+0200 <@daviddias> that being said, it would be just one more thing to add to the Identify protocol
17:22+0200 <@daviddias> it can be added
17:24+0200 <@lgierth> ok sounds good
17:24+0200 <@lgierth> i'll leave it be for now and add it to my metrics wishlist

@ghost ghost added RFCR and removed need/author-input Needs input from the original author labels May 18, 2016
@Kubuxu Kubuxu added need/review Needs a review and removed RFCR labels May 18, 2016
func (c IpfsNodeCollector) PeersTotalValue() float64 {
return float64(len(c.Node.PeerHost.Network().Conns()))
func (c IpfsNodeCollector) PeersTotalValues() map[string]float64 {
vals := map[string]float64{}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i prefer using make for this

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Made it make

@whyrusleeping
Copy link
Member

I think we store the remote version in the peerstore, although we can add that in later.

@whyrusleeping
Copy link
Member

aside from one nitpick, this LGTM

@Kubuxu Kubuxu removed the need/review Needs a review label May 18, 2016
Gives us per-transport peers counts:

	ipfs_p2p_peers_total{transport="/ip4/tcp"} 25
	ipfs_p2p_peers_total{transport="/ip6/tcp"} 13
	ipfs_p2p_peers_total{transport="/ip6/udp/utp"} 17

License: MIT
Signed-off-by: Lars Gierth <larsg@systemli.org>
@ghost ghost force-pushed the peers-total-by-transport branch from 91e141f to 05cb7a1 Compare May 18, 2016 20:31
@Kubuxu Kubuxu added the RFM label May 18, 2016
@whyrusleeping whyrusleeping merged commit f014610 into master May 18, 2016
@whyrusleeping whyrusleeping deleted the peers-total-by-transport branch May 18, 2016 21:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants