-
Notifications
You must be signed in to change notification settings - Fork 20.1k
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
p2p, p2p/discover: add dial metrics #27621
Conversation
Okay I ended up realizing I could just make some measurements from the eth handshake and subtract those from the p2p dial "successes" to determine the number of peers we dial that turn into useful peers. |
de7e5d7
to
2465a30
Compare
2465a30
to
111fa80
Compare
This PR sucks. It should be closed immediately. |
Don't be impolite @anti-fjl! The PR is decent, just needs a bit of work to be fully correct. |
I think there is some confusion about the term 'dial' here. In package p2p, when we say 'dial' we exclusively mean 'outbound connections created by us'. So the terms 'ingress' 'egress' cannot be applied to a dialed connection. |
5bf3938
to
18c0cc7
Compare
Good point on the confusion of "dial" and ingress/egress. I've updated it in the PR and it should be fixed now. |
This PR adds metrics for p2p dialing, which gives us visibility into the quality of the dial candidates returned by our discovery methods.
This reverts commit 463ad60.
This reverts commit 463ad60.
This PR adds metrics for p2p dialing, which gives us visibility into the quality of the dial candidates returned by our discovery methods.
This PR adds metrics for the p2p dialer, which gives us visibility into the quality of the dial candidates given to us by our discovery methods. This isn't quite a full picture though, because often once the dial completes, the subprotocols will then perform their handshakes. These frequently fail for a multitude of reasons: wrong chain, too many peers, etc. That isn't shown with these metrics.
This is the reason the "success" rate seems very high for a rather low peer count. Using discv4, around 28% we can connect to on RLPx, but many of them immediately disconnect during(fixed by tracking errors during handshake). With DNS discovery, the number is much lower because (presumably) they're already maxed out on peers due to the constant barrage of ingress connection requests they receive. So it takes longer to find a node with a free peer slot.eth
handshakingThe one notable change here other than the new metrics is moving
p2p/dials
andp2p/serves
outside ofmeteredConn
. This seems to be nice because it reduces the noise of dialing on the metrics. Connections are only counted now if they reach the "peer added" checkpoint.Added Metrics
discover/bucket/{index}/count
- the number of nodes in bucketindex
eth/protocols/eth/{ingress|egress}/handshake/error/peer
- unexpected peer behavioreth/protocols/eth/{ingress|egress}/handshake/error/timeout
- handshake timeouteth/protocols/eth/{ingress|egress}/handshake/error/network
- wrong network ideth/protocols/eth/{ingress|egress}/handshake/error/version
- wrong protocol versioneth/protocols/eth/{ingress|egress}/handshake/error/genesis
- wrong genesiseth/protocols/eth/{ingress|egress}/handshake/error/forkid
- wrong forkideth/protocols/snap/{ingress|egress}/registration/error
- error registering snap protocol (usually snap, but no eth)p2p/dials/success
- the total number of dials that result in a peer being added to the peersetp2p/dials/error/connection
- unable to initiate TCP connection to targetp2p/dials/error/saturated
- local client is already connected to its maximum number of peersp2p/dials/error/known
- dialing an already connected peerp2p/dials/error/self
- dialing the local node's idp2p/dials/error/useless
- dialing a peer that doesn't share an capabilities with the local nodep2p/dials/error/id/unexpected
- dialed peer repsoned with different id than expectedp2p/dials/error/rlpx/enc
- error negotiating the rlpx encryption during dialp2p/dials/error/rlpx/proto
- error during rlpx protocol handshake`Mainnet (discv4 only)
Sepolia (discv4 only)
Dashboard
JSON