-
Notifications
You must be signed in to change notification settings - Fork 80
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
cmd/bootnode: enable prometheus #1534
Conversation
Codecov ReportBase: 54.37% // Head: 54.27% // Decreases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## main #1534 +/- ##
==========================================
- Coverage 54.37% 54.27% -0.11%
==========================================
Files 150 151 +1
Lines 19134 19144 +10
==========================================
- Hits 10405 10390 -15
- Misses 7319 7335 +16
- Partials 1410 1419 +9
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
if err != nil { | ||
p2pErr <- errors.Wrap(err, "new tcp node") | ||
// Start serving HTTP: ENR and monitoring. | ||
serverErr := make(chan error, 2) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why "2"?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
2 servers
) | ||
if config.HTTPAddr != "" { | ||
log.Info(ctx, "Runtime ENR available via http", | ||
z.Str("url", fmt.Sprintf("http://%s/enr", config.HTTPAddr)), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
config.HTTPAddr can be something like this http://abc.xyz
, this should be
fmt.Sprintf("%s/enr", config.HTTPAddr)
i think
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it shouldn't be:
flags.StringVar(&config.HTTPAddr, "bootnode-http-address", "127.0.0.1:3640", "Listening address (ip and port) for the bootnode http server serving runtime ENR.")
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can add a ticket to do validation of all listen address flags in the repo if you want.
limiter := rcmgr.DefaultLimits | ||
limiter.SystemBaseLimit.ConnsInbound = config.MaxConns | ||
limiter.SystemBaseLimit.FD = config.MaxConns | ||
limiter.TransientBaseLimit = limiter.SystemBaseLimit |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: add a comment for each of these limits
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just moved this. You are are welcome to add the comments if you want.
flags.BoolVar(&config.P2PRelay, "p2p-relay", true, "Enable libp2p tcp host providing circuit relay to charon clusters") | ||
flags.StringVar(&config.RelayLogLevel, "p2p-relay-loglevel", "", "Libp2p circuit relay log level. E.g., debug, info, warn, error") | ||
flags.StringVar(&config.HTTPAddr, "bootnode-http-address", "127.0.0.1:3640", "Listening address (ip and port) for the bootnode http server serving runtime ENR.") | ||
flags.StringVar(&config.MonitoringAddr, "bootnode-monitoring-address", "127.0.0.1:3620", "Listening address (ip and port) for the prometheus and pprof monitoring http server.") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
flags.StringVar(&config.MonitoringAddr, "bootnode-monitoring-address", "127.0.0.1:3620", "Listening address (ip and port) for the prometheus and pprof monitoring http server.") | |
flags.StringVar(&config.MonitoringAddr, "bootnode-monitoring-address", "127.0.0.1:3620", "Listening address (ip and port) for the prometheus and pprof monitoring http server for bootnode") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
bit redundant?
Enables prometheus metrics in the bootnode.
category: feature
ticket: #1506