diff --git a/.schema/config.schema.json b/.schema/config.schema.json index 28fb647df2..21fac4259c 100644 --- a/.schema/config.schema.json +++ b/.schema/config.schema.json @@ -1256,6 +1256,12 @@ "default": true, "title": "CollapsePaths", "description": "When set to true the request label will include just the first segment of the request path" + }, + "collapse_request_paths_depth": { + "type": "integer", + "default": 1, + "title": "CollapsePathsDepth", + "description": "When set to a value greater than 0 the request label will include the first N segments of the request path" } } } diff --git a/cmd/server/server.go b/cmd/server/server.go index 8b35a41b62..6e26033932 100644 --- a/cmd/server/server.go +++ b/cmd/server/server.go @@ -69,7 +69,9 @@ func runProxy(d driver.Driver, n *negroni.Negroni, logger *logrusx.Logger, prom promHidePaths := d.Configuration().PrometheusHideRequestPaths() promCollapsePaths := d.Configuration().PrometheusCollapseRequestPaths() - n.Use(metrics.NewMiddleware(prom, "oathkeeper-proxy").ExcludePaths(healthx.ReadyCheckPath, healthx.AliveCheckPath).HidePaths(promHidePaths).CollapsePaths(promCollapsePaths)) + promCollapsePathsDepth := d.Configuration().PrometheusCollapseRequestPathsDepth() + + n.Use(metrics.NewMiddleware(prom, "oathkeeper-proxy").ExcludePaths(healthx.ReadyCheckPath, healthx.AliveCheckPath).HidePaths(promHidePaths).CollapsePaths(promCollapsePaths, promCollapsePathsDepth)) n.Use(reqlog.NewMiddlewareFromLogger(logger, "oathkeeper-proxy").ExcludePaths(healthx.ReadyCheckPath, healthx.AliveCheckPath)) n.Use(corsx.ContextualizedMiddleware(func(ctx context.Context) (opts cors.Options, enabled bool) { //nolint:staticcheck // legacy middleware still supported return d.Configuration().CORS("proxy") @@ -115,8 +117,9 @@ func runAPI(d driver.Driver, n *negroni.Negroni, logger *logrusx.Logger, prom *m promHidePaths := d.Configuration().PrometheusHideRequestPaths() promCollapsePaths := d.Configuration().PrometheusCollapseRequestPaths() + promCollapsePathsDepth := d.Configuration().PrometheusCollapseRequestPathsDepth() - n.Use(metrics.NewMiddleware(prom, "oathkeeper-api").ExcludePaths(healthx.ReadyCheckPath, healthx.AliveCheckPath).HidePaths(promHidePaths).CollapsePaths(promCollapsePaths)) + n.Use(metrics.NewMiddleware(prom, "oathkeeper-api").ExcludePaths(healthx.ReadyCheckPath, healthx.AliveCheckPath).HidePaths(promHidePaths).CollapsePaths(promCollapsePaths, promCollapsePathsDepth)) n.Use(reqlog.NewMiddlewareFromLogger(logger, "oathkeeper-api").ExcludePaths(healthx.ReadyCheckPath, healthx.AliveCheckPath)) n.Use(corsx.ContextualizedMiddleware(func(ctx context.Context) (opts cors.Options, enabled bool) { //nolint:staticcheck // legacy middleware still supported return d.Configuration().CORS("api") diff --git a/driver/configuration/config_keys.go b/driver/configuration/config_keys.go index 0a834be635..938e7900d5 100644 --- a/driver/configuration/config_keys.go +++ b/driver/configuration/config_keys.go @@ -6,25 +6,26 @@ package configuration type Key = string const ( - ProxyReadTimeout Key = "serve.proxy.timeout.read" - ProxyWriteTimeout Key = "serve.proxy.timeout.write" - ProxyIdleTimeout Key = "serve.proxy.timeout.idle" - ProxyServeAddressHost Key = "serve.proxy.host" - ProxyServeAddressPort Key = "serve.proxy.port" - ProxyTrustForwardedHeaders Key = "serve.proxy.trust_forwarded_headers" - APIServeAddressHost Key = "serve.api.host" - APIServeAddressPort Key = "serve.api.port" - APIReadTimeout Key = "serve.api.timeout.read" - APIWriteTimeout Key = "serve.api.timeout.write" - APIIdleTimeout Key = "serve.api.timeout.idle" - PrometheusServeAddressHost Key = "serve.prometheus.host" - PrometheusServeAddressPort Key = "serve.prometheus.port" - PrometheusServeMetricsPath Key = "serve.prometheus.metrics_path" - PrometheusServeMetricsNamePrefix Key = "serve.prometheus.metric_name_prefix" - PrometheusServeHideRequestPaths Key = "serve.prometheus.hide_request_paths" - PrometheusServeCollapseRequestPaths Key = "serve.prometheus.collapse_request_paths" - AccessRuleRepositories Key = "access_rules.repositories" - AccessRuleMatchingStrategy Key = "access_rules.matching_strategy" + ProxyReadTimeout Key = "serve.proxy.timeout.read" + ProxyWriteTimeout Key = "serve.proxy.timeout.write" + ProxyIdleTimeout Key = "serve.proxy.timeout.idle" + ProxyServeAddressHost Key = "serve.proxy.host" + ProxyServeAddressPort Key = "serve.proxy.port" + ProxyTrustForwardedHeaders Key = "serve.proxy.trust_forwarded_headers" + APIServeAddressHost Key = "serve.api.host" + APIServeAddressPort Key = "serve.api.port" + APIReadTimeout Key = "serve.api.timeout.read" + APIWriteTimeout Key = "serve.api.timeout.write" + APIIdleTimeout Key = "serve.api.timeout.idle" + PrometheusServeAddressHost Key = "serve.prometheus.host" + PrometheusServeAddressPort Key = "serve.prometheus.port" + PrometheusServeMetricsPath Key = "serve.prometheus.metrics_path" + PrometheusServeMetricsNamePrefix Key = "serve.prometheus.metric_name_prefix" + PrometheusServeHideRequestPaths Key = "serve.prometheus.hide_request_paths" + PrometheusServeCollapseRequestPaths Key = "serve.prometheus.collapse_request_paths" + PrometheusServeCollapseRequestPathsDepth Key = "serve.prometheus.collapse_request_paths_depth" + AccessRuleRepositories Key = "access_rules.repositories" + AccessRuleMatchingStrategy Key = "access_rules.matching_strategy" ) // Authorizers diff --git a/driver/configuration/provider.go b/driver/configuration/provider.go index 4293d97092..1f86821406 100644 --- a/driver/configuration/provider.go +++ b/driver/configuration/provider.go @@ -69,6 +69,7 @@ type Provider interface { PrometheusMetricsNamePrefix() string PrometheusHideRequestPaths() bool PrometheusCollapseRequestPaths() bool + PrometheusCollapseRequestPathsDepth() int ToScopeStrategy(value string, key string) fosite.ScopeStrategy ParseURLs(sources []string) ([]url.URL, error) diff --git a/driver/configuration/provider_koanf.go b/driver/configuration/provider_koanf.go index 209b851a1f..5a14d857eb 100644 --- a/driver/configuration/provider_koanf.go +++ b/driver/configuration/provider_koanf.go @@ -247,6 +247,10 @@ func (v *KoanfProvider) PrometheusCollapseRequestPaths() bool { return v.source.BoolF(PrometheusServeCollapseRequestPaths, true) } +func (v *KoanfProvider) PrometheusCollapseRequestPathsDepth() int { + return v.source.IntF(PrometheusServeCollapseRequestPathsDepth, 1) +} + func (v *KoanfProvider) ParseURLs(sources []string) ([]url.URL, error) { r := make([]url.URL, len(sources)) for k, u := range sources { diff --git a/driver/configuration/provider_koanf_public_test.go b/driver/configuration/provider_koanf_public_test.go index 35ba2829c8..7cba2b61ee 100644 --- a/driver/configuration/provider_koanf_public_test.go +++ b/driver/configuration/provider_koanf_public_test.go @@ -168,6 +168,7 @@ func TestKoanfProvider(t *testing.T) { assert.Equal(t, "localhost:9000", p.PrometheusServeAddress()) assert.Equal(t, "/metrics", p.PrometheusMetricsPath()) assert.Equal(t, true, p.PrometheusCollapseRequestPaths()) + assert.Equal(t, 1, p.PrometheusCollapseRequestPathsDepth()) }) t.Run("group=cors", func(t *testing.T) { diff --git a/internal/config/.oathkeeper.yaml b/internal/config/.oathkeeper.yaml index db604289f4..8d885a3051 100644 --- a/internal/config/.oathkeeper.yaml +++ b/internal/config/.oathkeeper.yaml @@ -85,6 +85,7 @@ serve: metrics_path: /metrics hide_request_paths: false collapse_request_paths: true + collapse_request_paths_depth: 1 metric_name_prefix: ory_oathkeeper_ # Configures Access Rules diff --git a/metrics/middleware.go b/metrics/middleware.go index b0819fb93a..eb57f9d4a8 100644 --- a/metrics/middleware.go +++ b/metrics/middleware.go @@ -38,21 +38,23 @@ type Middleware struct { // Silence metrics for specific URL paths // it is protected by the mutex - mutex sync.RWMutex - silencePaths map[string]bool - collapsePaths bool - hidePaths bool + mutex sync.RWMutex + silencePaths map[string]bool + collapsePaths bool + collapsePathsDepth int + hidePaths bool } // NewMiddleware returns a new *Middleware, yay! func NewMiddleware(prom *PrometheusRepository, name string) *Middleware { return &Middleware{ - Name: name, - Prometheus: prom, - clock: &realClock{}, - silencePaths: map[string]bool{}, - collapsePaths: true, - hidePaths: false, + Name: name, + Prometheus: prom, + clock: &realClock{}, + silencePaths: map[string]bool{}, + collapsePaths: true, + collapsePathsDepth: 1, + hidePaths: false, } } @@ -71,10 +73,14 @@ func (m *Middleware) ExcludePaths(paths ...string) *Middleware { // eg. (when set to true): // - /decisions/service/my-service -> /decisions // - /decisions -> /decisions -func (m *Middleware) CollapsePaths(flag bool) *Middleware { +func (m *Middleware) CollapsePaths(flag bool, depth int) *Middleware { m.mutex.Lock() + defer m.mutex.Unlock() m.collapsePaths = flag - m.mutex.Unlock() + if depth < 1 { + depth = 1 + } + m.collapsePathsDepth = depth return m } @@ -83,21 +89,19 @@ func (m *Middleware) CollapsePaths(flag bool) *Middleware { func (m *Middleware) HidePaths(flag bool) *Middleware { m.mutex.Lock() + defer m.mutex.Unlock() m.hidePaths = flag - m.mutex.Unlock() return m } -func (m *Middleware) getFirstPathSegment(requestURI string) string { - // Will split /my/example/uri in []string{"", "my", "example/uri"} - uriSegments := strings.SplitN(requestURI, "/", 3) - if len(uriSegments) > 1 { - // Remove any query string from the segment - // For example /my?query=string should return /my - return "/" + strings.SplitN(uriSegments[1], "?", 2)[0] +func (m *Middleware) getFirstNPathSegments(requestURI string, n int) string { + uriSegments := strings.SplitN(requestURI, "/", n+2) + end := n + 1 + if end > len(uriSegments) { + end = len(uriSegments) } - return "/" - + joinedPath := "/" + strings.Join(uriSegments[1:end], "/") + return strings.Split(joinedPath, "?")[0] } func (m *Middleware) ServeHTTP(rw http.ResponseWriter, r *http.Request, next http.HandlerFunc) { @@ -106,13 +110,20 @@ func (m *Middleware) ServeHTTP(rw http.ResponseWriter, r *http.Request, next htt latency := m.clock.Since(start) res := rw.(negroni.ResponseWriter) - if _, silent := m.silencePaths[r.URL.Path]; !silent { + m.mutex.RLock() + _, silent := m.silencePaths[r.URL.Path] + hidePaths := m.hidePaths + collapsePaths := m.collapsePaths + collapsePathsDepth := m.collapsePathsDepth + m.mutex.RUnlock() + + if !silent { requestURI := r.RequestURI - if m.hidePaths { + if hidePaths { requestURI = "" } else { - if m.collapsePaths { - requestURI = m.getFirstPathSegment(requestURI) + if collapsePaths { + requestURI = m.getFirstNPathSegments(requestURI, collapsePathsDepth) } } diff --git a/metrics/middleware_test.go b/metrics/middleware_test.go index acf07981a2..abff77179c 100644 --- a/metrics/middleware_test.go +++ b/metrics/middleware_test.go @@ -88,15 +88,16 @@ func PrometheusTestApp(middleware *Middleware) http.Handler { } var prometheusParams = []struct { - name string - collapsePaths bool - hidePaths bool - expectedMetrics string + name string + collapsePaths bool + collapsePathsDepth int + hidePaths bool + expectedMetrics string }{ - {"Not collapsed paths", false, false, metricsNotCollapsed}, - {"Collapsed paths", true, false, metricsCollapsed}, - {"Hidden not collapsed paths", false, true, metricsHidden}, - {"Hidden collapsed paths", true, true, metricsHidden}, + {"Not collapsed paths", false, 1, false, metricsNotCollapsed}, + {"Collapsed paths", true, 1, false, metricsCollapsed}, + {"Hidden not collapsed paths", false, 1, true, metricsHidden}, + {"Hidden collapsed paths", true, 1, true, metricsHidden}, } func TestPrometheusRequestTotalMetrics(t *testing.T) { @@ -107,7 +108,7 @@ func TestPrometheusRequestTotalMetrics(t *testing.T) { promRepo := NewTestPrometheusRepository(RequestTotal) promMiddleware := NewMiddleware(promRepo, "test") - promMiddleware.CollapsePaths(tt.collapsePaths) + promMiddleware.CollapsePaths(tt.collapsePaths, tt.collapsePathsDepth) promMiddleware.HidePaths(tt.hidePaths) ts := httptest.NewServer(PrometheusTestApp(promMiddleware)) @@ -132,12 +133,13 @@ func TestPrometheusRequestTotalMetrics(t *testing.T) { } var configurablePrometheusParams = []struct { - name string - collapsePaths bool - expectedMetrics string + name string + collapsePaths bool + collapsePathsDepth int + expectedMetrics string }{ - {"Not collapsed paths", false, configurableMetricsNotCollapsed}, - {"Collapsed paths", true, configurableMetricsCollapsed}, + {"Not collapsed paths", false, 1, configurableMetricsNotCollapsed}, + {"Collapsed paths", true, 1, configurableMetricsCollapsed}, } func TestConfigurablePrometheusRequestTotalMetrics(t *testing.T) { @@ -156,7 +158,7 @@ serve: ) promRepo := NewConfigurablePrometheusRepository(d, logger) promMiddleware := NewMiddleware(promRepo, "test") - promMiddleware.CollapsePaths(tt.collapsePaths) + promMiddleware.CollapsePaths(tt.collapsePaths, tt.collapsePathsDepth) ts := httptest.NewServer(PrometheusTestApp(promMiddleware)) defer ts.Close() @@ -180,23 +182,48 @@ serve: } var requestURIParams = []struct { - name string - originalPath string - firstSegment string + name string + originalPath string + firstSegment string + collapseDepth int }{ - {"root path", "/", "/"}, - {"single segment", "/test", "/test"}, - {"two segments", "/test/path", "/test"}, - {"multiple segments", "/test/path/segments", "/test"}, + // depth = 1 (default behaviour) + {"root path depth 1", "/", "/", 1}, + {"single segment depth 1", "/test", "/test", 1}, + {"single segment depth 1 with query", "/test?foo=bar", "/test", 1}, + {"two segments depth 1", "/test/path", "/test", 1}, + {"multiple segments depth 1", "/test/path/segments", "/test", 1}, + {"with query depth 1", "/test/path?foo=bar", "/test", 1}, + {"three segments depth 1", "/test/path/segments", "/test", 1}, + {"four segments depth 1", "/test/path/segments/foo", "/test", 1}, + + // depth = 2 + {"root path depth 2", "/", "/", 2}, + {"single segment depth 2", "/test", "/test", 2}, + {"single segment depth 2 with query", "/test?foo=bar", "/test", 2}, + {"two segments depth 2", "/test/path", "/test/path", 2}, + {"two segments depth 2 with query", "/test/path?foo=bar", "/test/path", 2}, + {"multiple segments depth 2", "/test/path/segments", "/test/path", 2}, + {"with query depth 2", "/test/path/segments?foo=bar", "/test/path", 2}, + + // depth = 3 + {"root path depth 3", "/", "/", 3}, + {"single segment depth 3", "/test", "/test", 3}, + {"two segments depth 3", "/test/path", "/test/path", 3}, + {"two segments depth 3 with query", "/test/path?foo=bar", "/test/path", 3}, + {"multiple segments depth 3", "/test/path/segments", "/test/path/segments", 3}, + {"with query depth 3", "/test/path/segments?foo=bar", "/test/path/segments", 3}, + {"four segments depth 3", "/test/path/segments/foo", "/test/path/segments", 3}, + {"five segments depth 3", "/test/path/segments/foo/bar", "/test/path/segments", 3}, } -func TestMiddlewareGetFirstPathSegment(t *testing.T) { +func TestMiddlewareGetFirstNPathSegments(t *testing.T) { promMiddleware := NewMiddleware(nil, "test") for _, tt := range requestURIParams { t.Run(tt.name, func(t *testing.T) { - promMiddleware.CollapsePaths(true) - collapsed := promMiddleware.getFirstPathSegment(tt.originalPath) + promMiddleware.CollapsePaths(true, tt.collapseDepth) + collapsed := promMiddleware.getFirstNPathSegments(tt.originalPath, tt.collapseDepth) if collapsed != tt.firstSegment { t.Fatalf("Expected first segment: %s to be equal to: %s", collapsed, tt.firstSegment) } diff --git a/spec/config.schema.json b/spec/config.schema.json index bb2bef34fe..fc7fbb88e2 100644 --- a/spec/config.schema.json +++ b/spec/config.schema.json @@ -1256,6 +1256,12 @@ "default": true, "title": "CollapsePaths", "description": "When set to true the request label will include just the first segment of the request path" + }, + "collapse_request_paths_depth": { + "type": "integer", + "default": 1, + "title": "CollapsePathsDepth", + "description": "When set to a value greater than 0 the request label will include the first N segments of the request path" } } }