From a2714ab1f1d71542f897e08e89ca46bc6bacc886 Mon Sep 17 00:00:00 2001 From: Alexandre NICOLAIE Date: Thu, 27 Jul 2023 18:51:08 +0200 Subject: [PATCH] outposts: make metrics compliant with Prometheus best-practices (#6398) web/outpost: make metrics compliant with Prometheus best-practices Today, all NewHistogramVec store values in nanoseconds without changing the default histogram bucket, which are made for seconds, making them a bit useless. In addition, some metrics names are not self-explanatoryand and do not comply with Prometheus best practices. This commit tries to fix all of this "issues". NOTE: I kept old metrics in order to avoid breaking changes with existing dashboards and metrics. Signed-off-by: Alexandre NICOLAIE --- internal/outpost/flow/executor.go | 21 ++++++++++++- internal/outpost/ldap/bind.go | 12 ++++++++ internal/outpost/ldap/bind/direct/bind.go | 30 +++++++++++++++++++ internal/outpost/ldap/metrics/metrics.go | 12 +++++++- internal/outpost/ldap/search.go | 6 ++++ internal/outpost/ldap/search/direct/direct.go | 24 +++++++++++++++ internal/outpost/ldap/search/memory/memory.go | 18 +++++++++++ internal/outpost/ldap/unbind.go | 14 ++++++++- .../proxyv2/application/application.go | 10 +++++-- .../outpost/proxyv2/application/mode_proxy.go | 11 +++++-- internal/outpost/proxyv2/handlers.go | 20 ++++++++++--- internal/outpost/proxyv2/metrics/metrics.go | 12 +++++++- .../outpost/radius/handle_access_request.go | 20 +++++++++++++ internal/outpost/radius/handler.go | 5 ++++ internal/outpost/radius/metrics/metrics.go | 12 +++++++- internal/web/metrics.go | 6 ++++ internal/web/proxy.go | 18 +++++++++-- 17 files changed, 235 insertions(+), 16 deletions(-) diff --git a/internal/outpost/flow/executor.go b/internal/outpost/flow/executor.go index 585159841..998249e5c 100644 --- a/internal/outpost/flow/executor.go +++ b/internal/outpost/flow/executor.go @@ -8,6 +8,7 @@ import ( "net/http/cookiejar" "net/url" "strings" + "time" "github.com/getsentry/sentry-go" "github.com/prometheus/client_golang/prometheus" @@ -21,10 +22,20 @@ import ( var ( FlowTimingGet = promauto.NewHistogramVec(prometheus.HistogramOpts{ + Name: "authentik_outpost_flow_timing_get_seconds", + Help: "Duration it took to get a challenge in seconds", + }, []string{"stage", "flow"}) + FlowTimingPost = promauto.NewHistogramVec(prometheus.HistogramOpts{ + Name: "authentik_outpost_flow_timing_post_seconds", + Help: "Duration it took to send a challenge in seconds", + }, []string{"stage", "flow"}) + + // NOTE: the following metrics are kept for compatibility purpose + FlowTimingGetLegacy = promauto.NewHistogramVec(prometheus.HistogramOpts{ Name: "authentik_outpost_flow_timing_get", Help: "Duration it took to get a challenge", }, []string{"stage", "flow"}) - FlowTimingPost = promauto.NewHistogramVec(prometheus.HistogramOpts{ + FlowTimingPostLegacy = promauto.NewHistogramVec(prometheus.HistogramOpts{ Name: "authentik_outpost_flow_timing_post", Help: "Duration it took to send a challenge", }, []string{"stage", "flow"}) @@ -186,6 +197,10 @@ func (fe *FlowExecutor) getInitialChallenge() (*api.ChallengeTypes, error) { FlowTimingGet.With(prometheus.Labels{ "stage": ch.GetComponent(), "flow": fe.flowSlug, + }).Observe(float64(gcsp.EndTime.Sub(gcsp.StartTime)) / float64(time.Second)) + FlowTimingGetLegacy.With(prometheus.Labels{ + "stage": ch.GetComponent(), + "flow": fe.flowSlug, }).Observe(float64(gcsp.EndTime.Sub(gcsp.StartTime))) return challenge, nil } @@ -243,6 +258,10 @@ func (fe *FlowExecutor) solveFlowChallenge(challenge *api.ChallengeTypes, depth FlowTimingPost.With(prometheus.Labels{ "stage": ch.GetComponent(), "flow": fe.flowSlug, + }).Observe(float64(scsp.EndTime.Sub(scsp.StartTime)) / float64(time.Second)) + FlowTimingPostLegacy.With(prometheus.Labels{ + "stage": ch.GetComponent(), + "flow": fe.flowSlug, }).Observe(float64(scsp.EndTime.Sub(scsp.StartTime))) if depth >= 10 { diff --git a/internal/outpost/ldap/bind.go b/internal/outpost/ldap/bind.go index cbf668662..5ab1e42c1 100644 --- a/internal/outpost/ldap/bind.go +++ b/internal/outpost/ldap/bind.go @@ -2,6 +2,7 @@ package ldap import ( "net" + "time" "beryju.io/ldap" "github.com/getsentry/sentry-go" @@ -20,6 +21,11 @@ func (ls *LDAPServer) Bind(bindDN string, bindPW string, conn net.Conn) (ldap.LD "outpost_name": ls.ac.Outpost.Name, "type": "bind", "app": selectedApp, + }).Observe(float64(span.EndTime.Sub(span.StartTime)) / float64(time.Second)) + metrics.RequestsLegacy.With(prometheus.Labels{ + "outpost_name": ls.ac.Outpost.Name, + "type": "bind", + "app": selectedApp, }).Observe(float64(span.EndTime.Sub(span.StartTime))) req.Log().WithField("took-ms", span.EndTime.Sub(span.StartTime).Milliseconds()).Info("Bind request") }() @@ -49,6 +55,12 @@ func (ls *LDAPServer) Bind(bindDN string, bindPW string, conn net.Conn) (ldap.LD "reason": "no_provider", "app": "", }).Inc() + metrics.RequestsRejectedLegacy.With(prometheus.Labels{ + "outpost_name": ls.ac.Outpost.Name, + "type": "bind", + "reason": "no_provider", + "app": "", + }).Inc() return ldap.LDAPResultInsufficientAccessRights, nil } diff --git a/internal/outpost/ldap/bind/direct/bind.go b/internal/outpost/ldap/bind/direct/bind.go index 67151033e..ce74d18f8 100644 --- a/internal/outpost/ldap/bind/direct/bind.go +++ b/internal/outpost/ldap/bind/direct/bind.go @@ -52,6 +52,12 @@ func (db *DirectBinder) Bind(username string, req *bind.Request) (ldap.LDAPResul "reason": "flow_error", "app": db.si.GetAppSlug(), }).Inc() + metrics.RequestsRejectedLegacy.With(prometheus.Labels{ + "outpost_name": db.si.GetOutpostName(), + "type": "bind", + "reason": "flow_error", + "app": db.si.GetAppSlug(), + }).Inc() req.Log().WithError(err).Warning("failed to execute flow") return ldap.LDAPResultInvalidCredentials, nil } @@ -62,6 +68,12 @@ func (db *DirectBinder) Bind(username string, req *bind.Request) (ldap.LDAPResul "reason": "invalid_credentials", "app": db.si.GetAppSlug(), }).Inc() + metrics.RequestsRejectedLegacy.With(prometheus.Labels{ + "outpost_name": db.si.GetOutpostName(), + "type": "bind", + "reason": "invalid_credentials", + "app": db.si.GetAppSlug(), + }).Inc() req.Log().Info("Invalid credentials") return ldap.LDAPResultInvalidCredentials, nil } @@ -75,6 +87,12 @@ func (db *DirectBinder) Bind(username string, req *bind.Request) (ldap.LDAPResul "reason": "access_denied", "app": db.si.GetAppSlug(), }).Inc() + metrics.RequestsRejectedLegacy.With(prometheus.Labels{ + "outpost_name": db.si.GetOutpostName(), + "type": "bind", + "reason": "access_denied", + "app": db.si.GetAppSlug(), + }).Inc() return ldap.LDAPResultInsufficientAccessRights, nil } if err != nil { @@ -84,6 +102,12 @@ func (db *DirectBinder) Bind(username string, req *bind.Request) (ldap.LDAPResul "reason": "access_check_fail", "app": db.si.GetAppSlug(), }).Inc() + metrics.RequestsRejectedLegacy.With(prometheus.Labels{ + "outpost_name": db.si.GetOutpostName(), + "type": "bind", + "reason": "access_check_fail", + "app": db.si.GetAppSlug(), + }).Inc() req.Log().WithError(err).Warning("failed to check access") return ldap.LDAPResultOperationsError, nil } @@ -98,6 +122,12 @@ func (db *DirectBinder) Bind(username string, req *bind.Request) (ldap.LDAPResul "reason": "user_info_fail", "app": db.si.GetAppSlug(), }).Inc() + metrics.RequestsRejectedLegacy.With(prometheus.Labels{ + "outpost_name": db.si.GetOutpostName(), + "type": "bind", + "reason": "user_info_fail", + "app": db.si.GetAppSlug(), + }).Inc() req.Log().WithError(err).Warning("failed to get user info") return ldap.LDAPResultOperationsError, nil } diff --git a/internal/outpost/ldap/metrics/metrics.go b/internal/outpost/ldap/metrics/metrics.go index d644964d4..150f89e7f 100644 --- a/internal/outpost/ldap/metrics/metrics.go +++ b/internal/outpost/ldap/metrics/metrics.go @@ -15,10 +15,20 @@ import ( var ( Requests = promauto.NewHistogramVec(prometheus.HistogramOpts{ + Name: "authentik_outpost_ldap_request_duration_seconds", + Help: "LDAP request latencies in seconds", + }, []string{"outpost_name", "type", "app"}) + RequestsRejected = promauto.NewCounterVec(prometheus.CounterOpts{ + Name: "authentik_outpost_ldap_requests_rejected_total", + Help: "Total number of rejected requests", + }, []string{"outpost_name", "type", "reason", "app"}) + + // NOTE: the following metrics are kept for compatibility purpose + RequestsLegacy = promauto.NewHistogramVec(prometheus.HistogramOpts{ Name: "authentik_outpost_ldap_requests", Help: "The total number of configured providers", }, []string{"outpost_name", "type", "app"}) - RequestsRejected = promauto.NewCounterVec(prometheus.CounterOpts{ + RequestsRejectedLegacy = promauto.NewCounterVec(prometheus.CounterOpts{ Name: "authentik_outpost_ldap_requests_rejected", Help: "Total number of rejected requests", }, []string{"outpost_name", "type", "reason", "app"}) diff --git a/internal/outpost/ldap/search.go b/internal/outpost/ldap/search.go index a082cd8e9..a330e3fb8 100644 --- a/internal/outpost/ldap/search.go +++ b/internal/outpost/ldap/search.go @@ -2,6 +2,7 @@ package ldap import ( "net" + "time" "beryju.io/ldap" "github.com/getsentry/sentry-go" @@ -21,6 +22,11 @@ func (ls *LDAPServer) Search(bindDN string, searchReq ldap.SearchRequest, conn n "outpost_name": ls.ac.Outpost.Name, "type": "search", "app": selectedApp, + }).Observe(float64(span.EndTime.Sub(span.StartTime)) / float64(time.Second)) + metrics.RequestsLegacy.With(prometheus.Labels{ + "outpost_name": ls.ac.Outpost.Name, + "type": "search", + "app": selectedApp, }).Observe(float64(span.EndTime.Sub(span.StartTime))) req.Log().WithField("attributes", searchReq.Attributes).WithField("took-ms", span.EndTime.Sub(span.StartTime).Milliseconds()).Info("Search request") }() diff --git a/internal/outpost/ldap/search/direct/direct.go b/internal/outpost/ldap/search/direct/direct.go index 7ac59f834..9f7ccd124 100644 --- a/internal/outpost/ldap/search/direct/direct.go +++ b/internal/outpost/ldap/search/direct/direct.go @@ -45,6 +45,12 @@ func (ds *DirectSearcher) Search(req *search.Request) (ldap.ServerSearchResult, "reason": "empty_bind_dn", "app": ds.si.GetAppSlug(), }).Inc() + metrics.RequestsRejectedLegacy.With(prometheus.Labels{ + "outpost_name": ds.si.GetOutpostName(), + "type": "search", + "reason": "empty_bind_dn", + "app": ds.si.GetAppSlug(), + }).Inc() return ldap.ServerSearchResult{ResultCode: ldap.LDAPResultInsufficientAccessRights}, fmt.Errorf("Search Error: Anonymous BindDN not allowed %s", req.BindDN) } if !utils.HasSuffixNoCase(req.BindDN, ","+baseDN) { @@ -54,6 +60,12 @@ func (ds *DirectSearcher) Search(req *search.Request) (ldap.ServerSearchResult, "reason": "invalid_bind_dn", "app": ds.si.GetAppSlug(), }).Inc() + metrics.RequestsRejectedLegacy.With(prometheus.Labels{ + "outpost_name": ds.si.GetOutpostName(), + "type": "search", + "reason": "invalid_bind_dn", + "app": ds.si.GetAppSlug(), + }).Inc() return ldap.ServerSearchResult{ResultCode: ldap.LDAPResultInsufficientAccessRights}, fmt.Errorf("Search Error: BindDN %s not in our BaseDN %s", req.BindDN, ds.si.GetBaseDN()) } @@ -66,6 +78,12 @@ func (ds *DirectSearcher) Search(req *search.Request) (ldap.ServerSearchResult, "reason": "user_info_not_cached", "app": ds.si.GetAppSlug(), }).Inc() + metrics.RequestsRejectedLegacy.With(prometheus.Labels{ + "outpost_name": ds.si.GetOutpostName(), + "type": "search", + "reason": "user_info_not_cached", + "app": ds.si.GetAppSlug(), + }).Inc() return ldap.ServerSearchResult{ResultCode: ldap.LDAPResultInsufficientAccessRights}, errors.New("access denied") } accsp.Finish() @@ -78,6 +96,12 @@ func (ds *DirectSearcher) Search(req *search.Request) (ldap.ServerSearchResult, "reason": "filter_parse_fail", "app": ds.si.GetAppSlug(), }).Inc() + metrics.RequestsRejectedLegacy.With(prometheus.Labels{ + "outpost_name": ds.si.GetOutpostName(), + "type": "search", + "reason": "filter_parse_fail", + "app": ds.si.GetAppSlug(), + }).Inc() return ldap.ServerSearchResult{ResultCode: ldap.LDAPResultOperationsError}, fmt.Errorf("Search Error: error parsing filter: %s", req.Filter) } diff --git a/internal/outpost/ldap/search/memory/memory.go b/internal/outpost/ldap/search/memory/memory.go index 04a39996a..d877b76e5 100644 --- a/internal/outpost/ldap/search/memory/memory.go +++ b/internal/outpost/ldap/search/memory/memory.go @@ -62,6 +62,12 @@ func (ms *MemorySearcher) Search(req *search.Request) (ldap.ServerSearchResult, "reason": "empty_bind_dn", "app": ms.si.GetAppSlug(), }).Inc() + metrics.RequestsRejectedLegacy.With(prometheus.Labels{ + "outpost_name": ms.si.GetOutpostName(), + "type": "search", + "reason": "empty_bind_dn", + "app": ms.si.GetAppSlug(), + }).Inc() return ldap.ServerSearchResult{ResultCode: ldap.LDAPResultInsufficientAccessRights}, fmt.Errorf("Search Error: Anonymous BindDN not allowed %s", req.BindDN) } if !utils.HasSuffixNoCase(req.BindDN, ","+baseDN) { @@ -71,6 +77,12 @@ func (ms *MemorySearcher) Search(req *search.Request) (ldap.ServerSearchResult, "reason": "invalid_bind_dn", "app": ms.si.GetAppSlug(), }).Inc() + metrics.RequestsRejectedLegacy.With(prometheus.Labels{ + "outpost_name": ms.si.GetOutpostName(), + "type": "search", + "reason": "invalid_bind_dn", + "app": ms.si.GetAppSlug(), + }).Inc() return ldap.ServerSearchResult{ResultCode: ldap.LDAPResultInsufficientAccessRights}, fmt.Errorf("Search Error: BindDN %s not in our BaseDN %s", req.BindDN, ms.si.GetBaseDN()) } @@ -83,6 +95,12 @@ func (ms *MemorySearcher) Search(req *search.Request) (ldap.ServerSearchResult, "reason": "user_info_not_cached", "app": ms.si.GetAppSlug(), }).Inc() + metrics.RequestsRejectedLegacy.With(prometheus.Labels{ + "outpost_name": ms.si.GetOutpostName(), + "type": "search", + "reason": "user_info_not_cached", + "app": ms.si.GetAppSlug(), + }).Inc() return ldap.ServerSearchResult{ResultCode: ldap.LDAPResultInsufficientAccessRights}, errors.New("access denied") } accsp.Finish() diff --git a/internal/outpost/ldap/unbind.go b/internal/outpost/ldap/unbind.go index 3b0db3e5f..35c608c32 100644 --- a/internal/outpost/ldap/unbind.go +++ b/internal/outpost/ldap/unbind.go @@ -2,9 +2,10 @@ package ldap import ( "net" + "time" - "github.com/getsentry/sentry-go" "beryju.io/ldap" + "github.com/getsentry/sentry-go" "github.com/prometheus/client_golang/prometheus" log "github.com/sirupsen/logrus" "goauthentik.io/internal/outpost/ldap/bind" @@ -20,6 +21,11 @@ func (ls *LDAPServer) Unbind(boundDN string, conn net.Conn) (ldap.LDAPResultCode "outpost_name": ls.ac.Outpost.Name, "type": "unbind", "app": selectedApp, + }).Observe(float64(span.EndTime.Sub(span.StartTime)) / float64(time.Second)) + metrics.RequestsLegacy.With(prometheus.Labels{ + "outpost_name": ls.ac.Outpost.Name, + "type": "unbind", + "app": selectedApp, }).Observe(float64(span.EndTime.Sub(span.StartTime))) req.Log().WithField("took-ms", span.EndTime.Sub(span.StartTime).Milliseconds()).Info("Unbind request") }() @@ -49,5 +55,11 @@ func (ls *LDAPServer) Unbind(boundDN string, conn net.Conn) (ldap.LDAPResultCode "reason": "no_provider", "app": "", }).Inc() + metrics.RequestsRejectedLegacy.With(prometheus.Labels{ + "outpost_name": ls.ac.Outpost.Name, + "type": "unbind", + "reason": "no_provider", + "app": "", + }).Inc() return ldap.LDAPResultOperationsError, nil } diff --git a/internal/outpost/proxyv2/application/application.go b/internal/outpost/proxyv2/application/application.go index 000f8704c..324909562 100644 --- a/internal/outpost/proxyv2/application/application.go +++ b/internal/outpost/proxyv2/application/application.go @@ -163,13 +163,19 @@ func NewApplication(p api.ProxyOutpostConfig, c *http.Client, server Server) (*A } before := time.Now() inner.ServeHTTP(rw, r) - after := time.Since(before) + elapsed := time.Since(before) metrics.Requests.With(prometheus.Labels{ "outpost_name": a.outpostName, "type": "app", "method": r.Method, "host": web.GetHost(r), - }).Observe(float64(after)) + }).Observe(float64(elapsed) / float64(time.Second)) + metrics.RequestsLegacy.With(prometheus.Labels{ + "outpost_name": a.outpostName, + "type": "app", + "method": r.Method, + "host": web.GetHost(r), + }).Observe(float64(elapsed)) }) }) if server.API().GlobalConfig.ErrorReporting.Enabled { diff --git a/internal/outpost/proxyv2/application/mode_proxy.go b/internal/outpost/proxyv2/application/mode_proxy.go index 0d687b657..07a53f2d6 100644 --- a/internal/outpost/proxyv2/application/mode_proxy.go +++ b/internal/outpost/proxyv2/application/mode_proxy.go @@ -55,7 +55,7 @@ func (a *Application) configureProxy() error { } before := time.Now() rp.ServeHTTP(rw, r) - after := time.Since(before) + elapsed := time.Since(before) metrics.UpstreamTiming.With(prometheus.Labels{ "outpost_name": a.outpostName, @@ -63,7 +63,14 @@ func (a *Application) configureProxy() error { "method": r.Method, "scheme": r.URL.Scheme, "host": web.GetHost(r), - }).Observe(float64(after)) + }).Observe(float64(elapsed) / float64(time.Second)) + metrics.UpstreamTimingLegacy.With(prometheus.Labels{ + "outpost_name": a.outpostName, + "upstream_host": r.URL.Host, + "method": r.Method, + "scheme": r.URL.Scheme, + "host": web.GetHost(r), + }).Observe(float64(elapsed)) }) return nil } diff --git a/internal/outpost/proxyv2/handlers.go b/internal/outpost/proxyv2/handlers.go index bbcd567b2..97d86a9fb 100644 --- a/internal/outpost/proxyv2/handlers.go +++ b/internal/outpost/proxyv2/handlers.go @@ -19,25 +19,37 @@ import ( func (ps *ProxyServer) HandlePing(rw http.ResponseWriter, r *http.Request) { before := time.Now() rw.WriteHeader(204) - after := time.Since(before) + elapsed := time.Since(before) metrics.Requests.With(prometheus.Labels{ "outpost_name": ps.akAPI.Outpost.Name, "method": r.Method, "host": web.GetHost(r), "type": "ping", - }).Observe(float64(after)) + }).Observe(float64(elapsed) / float64(time.Second)) + metrics.RequestsLegacy.With(prometheus.Labels{ + "outpost_name": ps.akAPI.Outpost.Name, + "method": r.Method, + "host": web.GetHost(r), + "type": "ping", + }).Observe(float64(elapsed)) } func (ps *ProxyServer) HandleStatic(rw http.ResponseWriter, r *http.Request) { before := time.Now() web.DisableIndex(http.StripPrefix("/outpost.goauthentik.io/static/dist", staticWeb.StaticHandler)).ServeHTTP(rw, r) - after := time.Since(before) + elapsed := time.Since(before) metrics.Requests.With(prometheus.Labels{ "outpost_name": ps.akAPI.Outpost.Name, "method": r.Method, "host": web.GetHost(r), "type": "static", - }).Observe(float64(after)) + }).Observe(float64(elapsed) / float64(time.Second)) + metrics.RequestsLegacy.With(prometheus.Labels{ + "outpost_name": ps.akAPI.Outpost.Name, + "method": r.Method, + "host": web.GetHost(r), + "type": "static", + }).Observe(float64(elapsed)) } func (ps *ProxyServer) lookupApp(r *http.Request) (*application.Application, string) { diff --git a/internal/outpost/proxyv2/metrics/metrics.go b/internal/outpost/proxyv2/metrics/metrics.go index 048ddaf79..564a010d0 100644 --- a/internal/outpost/proxyv2/metrics/metrics.go +++ b/internal/outpost/proxyv2/metrics/metrics.go @@ -15,10 +15,20 @@ import ( var ( Requests = promauto.NewHistogramVec(prometheus.HistogramOpts{ + Name: "authentik_outpost_proxy_request_duration_seconds", + Help: "Proxy request latencies in seconds", + }, []string{"outpost_name", "method", "host", "type"}) + UpstreamTiming = promauto.NewHistogramVec(prometheus.HistogramOpts{ + Name: "authentik_outpost_proxy_upstream_response_duration_seconds", + Help: "Proxy upstream response latencies in seconds", + }, []string{"outpost_name", "method", "scheme", "host", "upstream_host"}) + + // NOTE: the following metric is kept for compatibility purpose + RequestsLegacy = promauto.NewHistogramVec(prometheus.HistogramOpts{ Name: "authentik_outpost_proxy_requests", Help: "The total number of configured providers", }, []string{"outpost_name", "method", "host", "type"}) - UpstreamTiming = promauto.NewHistogramVec(prometheus.HistogramOpts{ + UpstreamTimingLegacy = promauto.NewHistogramVec(prometheus.HistogramOpts{ Name: "authentik_outpost_proxy_upstream_time", Help: "A summary of the duration we wait for the upstream reply", }, []string{"outpost_name", "method", "scheme", "host", "upstream_host"}) diff --git a/internal/outpost/radius/handle_access_request.go b/internal/outpost/radius/handle_access_request.go index 73644d980..465f1d267 100644 --- a/internal/outpost/radius/handle_access_request.go +++ b/internal/outpost/radius/handle_access_request.go @@ -32,6 +32,11 @@ func (rs *RadiusServer) Handle_AccessRequest(w radius.ResponseWriter, r *RadiusR "reason": "flow_error", "app": r.pi.appSlug, }).Inc() + metrics.RequestsRejectedLegacy.With(prometheus.Labels{ + "outpost_name": rs.ac.Outpost.Name, + "reason": "flow_error", + "app": r.pi.appSlug, + }).Inc() _ = w.Write(r.Response(radius.CodeAccessReject)) return } @@ -41,6 +46,11 @@ func (rs *RadiusServer) Handle_AccessRequest(w radius.ResponseWriter, r *RadiusR "reason": "invalid_credentials", "app": r.pi.appSlug, }).Inc() + metrics.RequestsRejectedLegacy.With(prometheus.Labels{ + "outpost_name": rs.ac.Outpost.Name, + "reason": "invalid_credentials", + "app": r.pi.appSlug, + }).Inc() _ = w.Write(r.Response(radius.CodeAccessReject)) return } @@ -53,6 +63,11 @@ func (rs *RadiusServer) Handle_AccessRequest(w radius.ResponseWriter, r *RadiusR "reason": "access_check_fail", "app": r.pi.appSlug, }).Inc() + metrics.RequestsRejectedLegacy.With(prometheus.Labels{ + "outpost_name": rs.ac.Outpost.Name, + "reason": "access_check_fail", + "app": r.pi.appSlug, + }).Inc() return } if !access { @@ -63,6 +78,11 @@ func (rs *RadiusServer) Handle_AccessRequest(w radius.ResponseWriter, r *RadiusR "reason": "access_denied", "app": r.pi.appSlug, }).Inc() + metrics.RequestsRejectedLegacy.With(prometheus.Labels{ + "outpost_name": rs.ac.Outpost.Name, + "reason": "access_denied", + "app": r.pi.appSlug, + }).Inc() return } _ = w.Write(r.Response(radius.CodeAccessAccept)) diff --git a/internal/outpost/radius/handler.go b/internal/outpost/radius/handler.go index e98d77606..e1d3cb531 100644 --- a/internal/outpost/radius/handler.go +++ b/internal/outpost/radius/handler.go @@ -2,6 +2,7 @@ package radius import ( "crypto/sha512" + "time" "github.com/getsentry/sentry-go" "github.com/google/uuid" @@ -45,6 +46,10 @@ func (rs *RadiusServer) ServeRADIUS(w radius.ResponseWriter, r *radius.Request) metrics.Requests.With(prometheus.Labels{ "outpost_name": rs.ac.Outpost.Name, "app": selectedApp, + }).Observe(float64(span.EndTime.Sub(span.StartTime)) / float64(time.Second)) + metrics.RequestsLegacy.With(prometheus.Labels{ + "outpost_name": rs.ac.Outpost.Name, + "app": selectedApp, }).Observe(float64(span.EndTime.Sub(span.StartTime))) }() diff --git a/internal/outpost/radius/metrics/metrics.go b/internal/outpost/radius/metrics/metrics.go index ec96e0dc5..1bcecfdc2 100644 --- a/internal/outpost/radius/metrics/metrics.go +++ b/internal/outpost/radius/metrics/metrics.go @@ -15,10 +15,20 @@ import ( var ( Requests = promauto.NewHistogramVec(prometheus.HistogramOpts{ + Name: "authentik_outpost_radius_request_duration_seconds", + Help: "RADIUS request latencies in seconds", + }, []string{"outpost_name", "app"}) + RequestsRejected = promauto.NewCounterVec(prometheus.CounterOpts{ + Name: "authentik_outpost_radius_requests_rejected_total", + Help: "Total number of rejected requests", + }, []string{"outpost_name", "reason", "app"}) + + // NOTE: the following metric is kept for compatibility purpose + RequestsLegacy = promauto.NewHistogramVec(prometheus.HistogramOpts{ Name: "authentik_outpost_radius_requests", Help: "The total number of successful requests", }, []string{"outpost_name", "app"}) - RequestsRejected = promauto.NewCounterVec(prometheus.CounterOpts{ + RequestsRejectedLegacy = promauto.NewCounterVec(prometheus.CounterOpts{ Name: "authentik_outpost_radius_requests_rejected", Help: "Total number of rejected requests", }, []string{"outpost_name", "reason", "app"}) diff --git a/internal/web/metrics.go b/internal/web/metrics.go index 942c9e6d6..0f22f59ab 100644 --- a/internal/web/metrics.go +++ b/internal/web/metrics.go @@ -15,6 +15,12 @@ import ( var ( Requests = promauto.NewHistogramVec(prometheus.HistogramOpts{ + Name: "authentik_main_request_duration_seconds", + Help: "API request latencies in seconds", + }, []string{"dest"}) + + // NOTE: the following metric is kept for compatibility purpose + RequestsLegacy = promauto.NewHistogramVec(prometheus.HistogramOpts{ Name: "authentik_main_requests", Help: "The total number of configured providers", }, []string{"dest"}) diff --git a/internal/web/proxy.go b/internal/web/proxy.go index e0fce7b22..13c2c76fe 100644 --- a/internal/web/proxy.go +++ b/internal/web/proxy.go @@ -34,9 +34,13 @@ func (ws *WebServer) configureProxy() { if ws.ProxyServer != nil { before := time.Now() ws.ProxyServer.Handle(rw, r) + elapsed := time.Since(before) Requests.With(prometheus.Labels{ "dest": "embedded_outpost", - }).Observe(float64(time.Since(before))) + }).Observe(float64(elapsed) / float64(time.Second)) + RequestsLegacy.With(prometheus.Labels{ + "dest": "embedded_outpost", + }).Observe(float64(elapsed)) return } ws.proxyErrorHandler(rw, r, fmt.Errorf("proxy not running")) @@ -52,15 +56,23 @@ func (ws *WebServer) configureProxy() { before := time.Now() if ws.ProxyServer != nil { if ws.ProxyServer.HandleHost(rw, r) { + elapsed := time.Since(before) Requests.With(prometheus.Labels{ "dest": "embedded_outpost", - }).Observe(float64(time.Since(before))) + }).Observe(float64(elapsed) / float64(time.Second)) + RequestsLegacy.With(prometheus.Labels{ + "dest": "embedded_outpost", + }).Observe(float64(elapsed)) return } } + elapsed := time.Since(before) Requests.With(prometheus.Labels{ "dest": "core", - }).Observe(float64(time.Since(before))) + }).Observe(float64(elapsed) / float64(time.Second)) + RequestsLegacy.With(prometheus.Labels{ + "dest": "core", + }).Observe(float64(elapsed)) r.Body = http.MaxBytesReader(rw, r.Body, 32*1024*1024) rp.ServeHTTP(rw, r) }))