-
Notifications
You must be signed in to change notification settings - Fork 269
Prometheus Metrics #832
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
Prometheus Metrics #832
Conversation
metricsAddrFlag = &cli.StringFlag{ | ||
Name: "metrics-addr", | ||
Sources: cli.EnvVars("METRICS_ADDR"), | ||
Value: "localhost:18551", | ||
Usage: "listening address for the metrics server", | ||
Category: Metrics, | ||
} |
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.
minor nit: it would be nice to separate it out to metrics-addr and metrics-port into separate flags.
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.
This actually mirrors how we do it elsewhere. I'd like to keep it like this for consistency.
Lines 42 to 48 in 7e47dd2
addrFlag = &cli.StringFlag{ | |
Name: "addr", | |
Sources: cli.EnvVars("BOOST_LISTEN_ADDR"), | |
Value: "localhost:18550", | |
Usage: "listen-address for mev-boost server", | |
Category: GeneralCategory, | |
} |
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.
alright sounds good
server/get_header.go
Outdated
if RelayLatency != nil { | ||
RelayLatency.WithLabelValues(params.PathGetHeader, relay.String()).Observe(float64(time.Since(start).Milliseconds())) | ||
} |
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.
Let's keep it in microseconds? We can always divide the metric by 1000 to get the value in millisecond. Lets have more precision.
We're missing the README updates from: |
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.
Pull Request Overview
This PR adds comprehensive Prometheus metrics instrumentation to mev-boost, enabling monitoring of relay performance, bid tracking, and service health metrics.
- Implements a metrics server with various histograms, counters, and gauges for monitoring relay operations
- Adds latency tracking and status code recording for all relay interactions
- Introduces command-line flags to enable and configure the metrics server
Reviewed Changes
Copilot reviewed 9 out of 10 changed files in this pull request and generated 2 comments.
Show a summary per file
File | Description |
---|---|
server/metrics.go | New file defining Prometheus metrics definitions and helper functions |
server/service.go | Adds metrics server startup, beacon node status tracking, and relay monitoring |
server/register_validator.go | Instruments validator registration with latency and status code metrics |
server/get_header.go | Adds bid value tracking, winning bid recording, and slot timing metrics |
server/get_payload.go | Instruments payload operations with latency tracking and slot timing |
cli/main.go | Integrates metrics server startup with command-line configuration |
cli/flags.go | Defines new CLI flags for enabling and configuring metrics |
go.mod | Adds Prometheus client library dependency |
README.md | Documents new metrics functionality and CLI flags |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
9205495
to
75ef195
Compare
-metrics | ||
enables a metrics server (default: false) | ||
-metrics-addr string | ||
listening address for the metrics server (default: "localhost:18551") |
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.
are both flags necessary, or should we only use -metrics-addr
and when set then enable, and by default empty string disable?
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.
📝 Summary
Adds prometheus metrics related to
⛱ Motivation and Context
📚 References
✅ I have run these commands
make lint
make test-race
go mod tidy