Expose pipecd ops metrics #2279
Conversation
|
Code coverage for golang is
|
|
Code coverage for golang is
|
pkg/cli/cmd.go
Outdated
| @@ -142,15 +145,41 @@ type MetricsBuilder interface { | |||
| Build() (io.Reader, error) | |||
| } | |||
|
|
|||
| func (t Telemetry) CustomMetricsHandlerFor(mb MetricsBuilder) http.Handler { | |||
| func (t Telemetry) CustomMetricsHandlerFor(reg *prometheus.Registry, mb MetricsBuilder) http.Handler { | |||
| if t.Flags.Metrics { | |||
There was a problem hiding this comment.
nit:
if !t.Flags.Metrics {
var empty http.HandlerFunc = func(w http.ResponseWriter, r *http.Request) {
w.Write([]byte(""))
}
return empty
}There was a problem hiding this comment.
I get your point. 👍 Lets keep it as is to unify with others for now, will address it by another PR (apply this pattern to others metrics handler as well)
There was a problem hiding this comment.
Changed my mind, will address it in this PR and apply only for this handler, lol
There was a problem hiding this comment.
Addressed by 6513a2d 🙏
pkg/cli/cmd.go
Outdated
| @@ -142,15 +145,41 @@ type MetricsBuilder interface { | |||
| Build() (io.Reader, error) | |||
| } | |||
|
|
|||
| func (t Telemetry) CustomMetricsHandlerFor(mb MetricsBuilder) http.Handler { | |||
| func (t Telemetry) CustomMetricsHandlerFor(reg *prometheus.Registry, mb MetricsBuilder) http.Handler { | |||
There was a problem hiding this comment.
Passing Gatherer instead of Registry.
There was a problem hiding this comment.
Nice catch 👍 Address by 6513a2d 🙏
|
Looks good! |
|
Yup, looks perfect ;) |
|
This pull request is APPROVED by Approvers can cancel the approval by writing |
What this PR does / why we need it:
Which issue(s) this PR fixes:
Fixes #
Does this PR introduce a user-facing change?: