[feat] add prometheus support for proxy & httpserver. #877
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Codecov ReportBase: 76.03% // Head: 76.32% // Increases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## main #877 +/- ##
==========================================
+ Coverage 76.03% 76.32% +0.29%
==========================================
Files 110 110
Lines 12741 12867 +126
==========================================
+ Hits 9687 9821 +134
+ Misses 2507 2501 -6
+ Partials 547 545 -2
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. |
localvar
reviewed
Dec 19, 2022
doc/reference/metrics.md
Outdated
| |--------| ---- | ----------- |-------------------------------------------------------------------------| | ||
| | httpserver_health | gauge | show the status for the http server: 1 for ready, 0 for down | clusterName, clusterRole, instanceName, name, kind | | ||
| | httpserver_total_requests | counter | the total count of http requests | clusterName, clusterRole, instanceName, name, kind, routerKind, backend | | ||
| | httpserver_total_response | counter | the total count of http resposne | clusterName, clusterRole, instanceName, name, kind, routerKind, backend | |
There was a problem hiding this comment.
Suggested change
| | httpserver_total_response | counter | the total count of http resposne | clusterName, clusterRole, instanceName, name, kind, routerKind, backend | | |
| | httpserver_total_responses | counter | the total count of http responses | clusterName, clusterRole, instanceName, name, kind, routerKind, backend | |
doc/reference/metrics.md
Outdated
| | httpserver_total_error_requests | counter | the total count of http error requests | clusterName, clusterRole, instanceName, name, kind, routerKind, backend | | ||
| | httpserver_requests_duration | histogram | request processing duration histogram | clusterName, clusterRole, instanceName, name, kind, routerKind, backend | | ||
| | httpserver_requests_size_bytes | histogram | a histogram of the total size of the request. Includes body | clusterName, clusterRole, instanceName, name, kind, routerKind, backend | | ||
| | httpserver_response_size_bytes | histogram | a histogram of the total size of the returned response body | clusterName, clusterRole, instanceName, name, kind, routerKind, backend | |
There was a problem hiding this comment.
Suggested change
| | httpserver_response_size_bytes | histogram | a histogram of the total size of the returned response body | clusterName, clusterRole, instanceName, name, kind, routerKind, backend | | |
| | httpserver_responses_size_bytes | histogram | a histogram of the total size of the returned response body | clusterName, clusterRole, instanceName, name, kind, routerKind, backend | |
and the description is different from httpserver_requests_size_bytes, is it correct?
pkg/filters/proxy/metrics.go
Outdated
| ) | ||
|
|
||
| // newMetrics create the ProxyMetrics. | ||
| func (p *Proxy) newMetrics(name string) *metrics { |
There was a problem hiding this comment.
propose to put all the content of this file into pool.go, and change the receiver of this function to ServerPool.
pkg/object/httpserver/mux.go
Outdated
| @@ -47,6 +47,7 @@ type ( | |||
| mux struct { | |||
| httpStat *httpstat.HTTPStat | |||
| topN *httpstat.TopN | |||
| metrics *metrics | |||
There was a problem hiding this comment.
I haven't found any usage of this field, if this is true, please remove it.
pkg/object/httpserver/metrics.go
Outdated
| ) | ||
|
|
||
| // newMetrics create the HttpServerMetrics. | ||
| func (r *runtime) newMetrics(name string) *metrics { |
There was a problem hiding this comment.
please put the content of this file to runtime.go and mux.go and remove this file.
pkg/util/prometheushelper/helper.go
Outdated
| return summaryMap[metricName] | ||
| } | ||
|
|
||
| func getAndValid(metricName string, labels []string) (string, error) { |
There was a problem hiding this comment.
valid is an adjective.
Suggested change
| func getAndValid(metricName string, labels []string) (string, error) { | |
| func getAndValidate(metricName string, labels []string) (string, error) { |
pkg/util/prometheushelper/helper.go
Outdated
| } | ||
|
|
||
| // ValidMetricName check if the metric name is valid | ||
| func ValidMetricName(name string) bool { |
pkg/util/prometheushelper/helper.go
Outdated
| } | ||
|
|
||
| // ValidLabelName check if the label name is valid | ||
| func ValidLabelName(label string) bool { |
| // NewGauge create the gauge metric | ||
| func NewGauge(metric string, help string, labels []string) *prometheus. | ||
| GaugeVec { | ||
|
|
| return nil | ||
| } | ||
|
|
||
| if m, find := counterMap[metricName]; find { |
There was a problem hiding this comment.
will NewCounter be called concurrently? if yes, here could be a race condition.
localvar
approved these changes
Dec 19, 2022
|
@suchen-sci please review the PR ASAP |
suchen-sci
approved these changes
Dec 21, 2022
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Background
Add Prometheus support for Easegress Objects & Filters, for details, see
metrics.md.Changing
DefaultRegistererautomatically.pool.collectMetricsfunction.serveHTTPfunctionimplementation
Why not take the easemonitor approach to implementation