Skip to content
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

Expose additional HTTP metrics #3698

Closed
2 tasks done
IvoGoman opened this issue Aug 15, 2024 · 2 comments · Fixed by #3748
Closed
2 tasks done

Expose additional HTTP metrics #3698

IvoGoman opened this issue Aug 15, 2024 · 2 comments · Fixed by #3748

Comments

@IvoGoman
Copy link
Contributor

Preflight Checklist

  • I agree to follow the Code of Conduct that this project adheres to.
  • I have searched the issue tracker for an issue that matches the one I want to file, without success.

Problem Description

Currently dex exposes only http_requests_total{handler="", code="",method=""} as a metric. The method used to calculate these metrics also collect the Duration and Response size written. https://github.com/felixge/httpsnoop/blob/master/capture_metrics.go#L46-L82
These metrics are not exposed.

Proposed Solution

Additional histogram metrics:

  • http_request_duration_seconds{handler="", code="", method=""}
  • http_response_size_bytes{handler="", code="", method=""}.

Which can be added to the handler instrumentalization here:

dex/server/server.go

Lines 335 to 356 in 5c66c71

instrumentHandlerCounter := func(_ string, handler http.Handler) http.HandlerFunc {
return handler.ServeHTTP
}
if c.PrometheusRegistry != nil {
requestCounter := prometheus.NewCounterVec(prometheus.CounterOpts{
Name: "http_requests_total",
Help: "Count of all HTTP requests.",
}, []string{"handler", "code", "method"})
err = c.PrometheusRegistry.Register(requestCounter)
if err != nil {
return nil, fmt.Errorf("server: Failed to register Prometheus HTTP metrics: %v", err)
}
instrumentHandlerCounter = func(handlerName string, handler http.Handler) http.HandlerFunc {
return func(w http.ResponseWriter, r *http.Request) {
m := httpsnoop.CaptureMetrics(handler, w, r)
requestCounter.With(prometheus.Labels{"handler": handlerName, "code": strconv.Itoa(m.Code), "method": r.Method}).Inc()
}
}
}

Since native histograms are not yet GA, there is the open question of which Buckets make sense for these metrics.

Alternatives Considered

No response

Additional Information

No response

@nabokihms
Copy link
Member

@IvoGoman the idea seems nice. Would you like to go ahead and make the change?

@IvoGoman
Copy link
Contributor Author

IvoGoman commented Sep 10, 2024

Yes, I will open a PR for this later this week.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants