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

p2p/discover: add traffic metrics #27008

Merged
merged 3 commits into from
Apr 25, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
50 changes: 50 additions & 0 deletions p2p/discover/metrics.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
package discover
holiman marked this conversation as resolved.
Show resolved Hide resolved

import (
"net"

"github.com/ethereum/go-ethereum/metrics"
)

const (
moduleName = "discover"
// ingressMeterName is the prefix of the per-packet inbound metrics.
ingressMeterName = moduleName + "/ingress"

// egressMeterName is the prefix of the per-packet outbound metrics.
egressMeterName = moduleName + "/egress"
)

var (
ingressTrafficMeter = metrics.NewRegisteredMeter(ingressMeterName, nil)
egressTrafficMeter = metrics.NewRegisteredMeter(egressMeterName, nil)
)

// meteredConn is a wrapper around a net.UDPConn that meters both the
// inbound and outbound network traffic.
type meteredUdpConn struct {
UDPConn
}

func newMeteredConn(conn UDPConn) UDPConn {
// Short circuit if metrics are disabled
if !metrics.Enabled {
Copy link
Member

Choose a reason for hiding this comment

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

I am not fully sure that we should use metrics.Enabled or metrics.EnabledExpensive.

The former one will be enabled once the metric system is turned on. The latter one is more for debug purpose which might impact runtime performance.

Copy link
Contributor

Choose a reason for hiding this comment

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

It's OK to use metrics.Enabled for discovery. The packet rate is not so high.

return conn
}

return &meteredUdpConn{UDPConn: conn}
}

// Read delegates a network read to the underlying connection, bumping the udp ingress traffic meter along the way.
func (c *meteredUdpConn) ReadFromUDP(b []byte) (n int, addr *net.UDPAddr, err error) {
n, addr, err = c.UDPConn.ReadFromUDP(b)
ingressTrafficMeter.Mark(int64(n))
return n, addr, err
}

// Write delegates a network write to the underlying connection, bumping the udp egress traffic meter along the way.
func (c *meteredUdpConn) WriteToUDP(b []byte, addr *net.UDPAddr) (n int, err error) {
n, err = c.UDPConn.WriteToUDP(b, addr)
egressTrafficMeter.Mark(int64(n))
return n, err
}
2 changes: 1 addition & 1 deletion p2p/discover/v4_udp.go
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ func ListenV4(c UDPConn, ln *enode.LocalNode, cfg Config) (*UDPv4, error) {
cfg = cfg.withDefaults()
closeCtx, cancel := context.WithCancel(context.Background())
t := &UDPv4{
conn: c,
conn: newMeteredConn(c),
priv: cfg.PrivateKey,
netrestrict: cfg.NetRestrict,
localNode: ln,
Expand Down
2 changes: 1 addition & 1 deletion p2p/discover/v5_udp.go
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,7 @@ func newUDPv5(conn UDPConn, ln *enode.LocalNode, cfg Config) (*UDPv5, error) {
cfg = cfg.withDefaults()
t := &UDPv5{
// static fields
conn: conn,
conn: newMeteredConn(conn),
localNode: ln,
db: ln.Database(),
netrestrict: cfg.NetRestrict,
Expand Down