-
Notifications
You must be signed in to change notification settings - Fork 86
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: support HTTP metrics server #298
metrics: support HTTP metrics server #298
Conversation
c033bea
to
b9ef140
Compare
Please fix linting warnings. |
pkg/metrics/listener.go
Outdated
|
||
log.L.Infof("Start metrics HTTP server on %s", addr) | ||
|
||
listener, err := net.Listen("tcp4", addr) |
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.
No need to restrict it to "tcp4"
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.
Yes, fixed.
b9ef140
to
cc8507e
Compare
&cli.StringFlag{ | ||
Name: "metrics-address", | ||
Value: "", | ||
Usage: "metrics `ENDPOINT` does not listen by default", |
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.
Maybe Enable metrics server by setting to an address such as "localhost:8080", ":8080"
.
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.
Yes, It is more reasonable. Fixed.
cc8507e
to
63a90ab
Compare
Codecov ReportBase: 31.68% // Head: 31.81% // Increases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## main #298 +/- ##
==========================================
+ Coverage 31.68% 31.81% +0.13%
==========================================
Files 34 34
Lines 3576 3564 -12
==========================================
+ Hits 1133 1134 +1
+ Misses 2327 2314 -13
Partials 116 116
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. |
pkg/metrics/listener.go
Outdated
// listener. | ||
var DefaultBindAddress = ":8080" | ||
// Endpoint for prometheus metrics | ||
var endpointPromMetrics = "/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.
Should we change it to "/v1/metrics" like containerd ?
### Metrics API
The metrics API that outputs prometheus style metrics will be versioned independently,
prefixed with the API version. i.e. `/v1/metrics`, `/v2/metrics`.
The metrics API version will be incremented when breaking changes are made to the prometheus
output. New metrics can be added to the output in a backwards compatible manner without
bumping the API version.
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.
Yes, fixed it.
Otherwise, LGTM |
Signed-off-by: Bin Tang <tangbin.bin@bytedance.com>
63a90ab
to
b2c5b61
Compare
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.
LGTM
The common way to display prometheus format data is through the HTTP server. For instance, Grafana does not support collecting metrics from
unix domain socket
. Therefore, we should change theunix domain socket
server to a HTTP server.Here is an example:
Then, you will see the prometheus metrics.
Signed-off-by: Bin Tang tangbin.bin@bytedance.com