Skip to content

Commit

Permalink
feat: improve configurability of prometheus metrics (#450)
Browse files Browse the repository at this point in the history
Closes #446

This patch introduces a new boolean configuration parameter `collapse_request_paths` in the prometheus configuration stanza of the `oathkeeper.yml` file:

```
serve:
  prometheus:
    port: 9000
    host: localhost
    metrics_path: /metrics
    collapse_request_paths: true
```

The parameter is optional and if not set it defaults to `true`.

When set to `true` it modifies the behaviour of the metrics Middleware so that for all the request metrics that contain the `request` label (which is equal to the request context path) its value is collapsed to only the first element of the segment. Eg.

```
curl http://localhost:4456/decisions/service1/users
curl http://localhost:4456/decisions/service1/topics
```
will result in the following request metrics (trimmed for reading convenience):
```
# TYPE ory_oathkeeper_request_duration_seconds histogram
ory_oathkeeper_request_duration_seconds_count{method="GET",request="/decisions",service="oathkeeper-api",status_code="404"} 2
# HELP ory_oathkeeper_requests_total Total number of requests
# TYPE ory_oathkeeper_requests_total counter
ory_oathkeeper_requests_total{method="GET",request="/decisions",service="oathkeeper-api",status_code="404"} 2
```

When `collapse_request_paths` is set to `false` the metrics will keep the path previous fine granularity:
```
# HELP ory_oathkeeper_request_duration_seconds Time spent serving requests.
# TYPE ory_oathkeeper_request_duration_seconds histogram
ory_oathkeeper_request_duration_seconds_count{method="GET",request="/decisions/service1/topics",service="oathkeeper-api",status_code="404"} 1
ory_oathkeeper_request_duration_seconds_count{method="GET",request="/decisions/service1/users",service="oathkeeper-api",status_code="404"} 1
# HELP ory_oathkeeper_requests_total Total number of requests
# TYPE ory_oathkeeper_requests_total counter
ory_oathkeeper_requests_total{method="GET",request="/decisions/service1/topics",service="oathkeeper-api",status_code="404"} 1
ory_oathkeeper_requests_total{method="GET",request="/decisions/service1/users",service="oathkeeper-api",status_code="404"} 1
```
  • Loading branch information
claudio-benfatto authored Jun 26, 2020
1 parent 0185070 commit ddcb226
Show file tree
Hide file tree
Showing 8 changed files with 196 additions and 27 deletions.
6 changes: 6 additions & 0 deletions .schema/config.schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -1098,6 +1098,12 @@
"default": "/metrics",
"title": "Path",
"description": "The path to provide metrics on"
},
"collapse_request_paths": {
"type": "boolean",
"default": true,
"title": "CollapsePaths",
"description": "When set to true the request label will include just the first segment of the request path"
}
}
}
Expand Down
8 changes: 6 additions & 2 deletions cmd/server/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,9 @@ func runProxy(d driver.Driver, n *negroni.Negroni, logger *logrusx.Logger, prom
Transport: proxy,
}

n.Use(metrics.NewMiddleware(prom, "oathkeeper-proxy").ExcludePaths(healthx.ReadyCheckPath, healthx.AliveCheckPath))
promCollapsePaths := d.Configuration().PrometheusCollapseRequestPaths()

n.Use(metrics.NewMiddleware(prom, "oathkeeper-proxy").ExcludePaths(healthx.ReadyCheckPath, healthx.AliveCheckPath).CollapsePaths(promCollapsePaths))
n.Use(reqlog.NewMiddlewareFromLogger(logger, "oathkeeper-proxy").ExcludePaths(healthx.ReadyCheckPath, healthx.AliveCheckPath))
n.UseHandler(handler)

Expand Down Expand Up @@ -81,7 +83,9 @@ func runAPI(d driver.Driver, n *negroni.Negroni, logger *logrusx.Logger, prom *m
d.Registry().HealthHandler().SetRoutes(router.Router, true)
d.Registry().CredentialHandler().SetRoutes(router)

n.Use(metrics.NewMiddleware(prom, "oathkeeper-api").ExcludePaths(healthx.ReadyCheckPath, healthx.AliveCheckPath))
promCollapsePaths := d.Configuration().PrometheusCollapseRequestPaths()

n.Use(metrics.NewMiddleware(prom, "oathkeeper-api").ExcludePaths(healthx.ReadyCheckPath, healthx.AliveCheckPath).CollapsePaths(promCollapsePaths))
n.Use(reqlog.NewMiddlewareFromLogger(logger, "oathkeeper-api").ExcludePaths(healthx.ReadyCheckPath, healthx.AliveCheckPath))
n.Use(d.Registry().DecisionHandler()) // This needs to be the last entry, otherwise the judge API won't work

Expand Down
3 changes: 2 additions & 1 deletion driver/configuration/provider.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,9 +51,10 @@ type Provider interface {

ProxyServeAddress() string
APIServeAddress() string
PrometheusServeAddress() string

PrometheusServeAddress() string
PrometheusMetricsPath() string
PrometheusCollapseRequestPaths() bool

ToScopeStrategy(value string, key string) fosite.ScopeStrategy
ParseURLs(sources []string) ([]url.URL, error)
Expand Down
35 changes: 20 additions & 15 deletions driver/configuration/provider_viper.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,21 +38,22 @@ func init() {
}

const (
ViperKeyProxyReadTimeout = "serve.proxy.timeout.read"
ViperKeyProxyWriteTimeout = "serve.proxy.timeout.write"
ViperKeyProxyIdleTimeout = "serve.proxy.timeout.idle"
ViperKeyProxyServeAddressHost = "serve.proxy.host"
ViperKeyProxyServeAddressPort = "serve.proxy.port"
ViperKeyAPIServeAddressHost = "serve.api.host"
ViperKeyAPIServeAddressPort = "serve.api.port"
ViperKeyAPIReadTimeout = "serve.api.timeout.read"
ViperKeyAPIWriteTimeout = "serve.api.timeout.write"
ViperKeyAPIIdleTimeout = "serve.api.timeout.idle"
ViperKeyPrometheusServeAddressHost = "serve.prometheus.host"
ViperKeyPrometheusServeAddressPort = "serve.prometheus.port"
ViperKeyPrometheusServeMetricsPath = "serve.prometheus.metrics_path"
ViperKeyAccessRuleRepositories = "access_rules.repositories"
ViperKeyAccessRuleMatchingStrategy = "access_rules.matching_strategy"
ViperKeyProxyReadTimeout = "serve.proxy.timeout.read"
ViperKeyProxyWriteTimeout = "serve.proxy.timeout.write"
ViperKeyProxyIdleTimeout = "serve.proxy.timeout.idle"
ViperKeyProxyServeAddressHost = "serve.proxy.host"
ViperKeyProxyServeAddressPort = "serve.proxy.port"
ViperKeyAPIServeAddressHost = "serve.api.host"
ViperKeyAPIServeAddressPort = "serve.api.port"
ViperKeyAPIReadTimeout = "serve.api.timeout.read"
ViperKeyAPIWriteTimeout = "serve.api.timeout.write"
ViperKeyAPIIdleTimeout = "serve.api.timeout.idle"
ViperKeyPrometheusServeAddressHost = "serve.prometheus.host"
ViperKeyPrometheusServeAddressPort = "serve.prometheus.port"
ViperKeyPrometheusServeMetricsPath = "serve.prometheus.metrics_path"
ViperKeyPrometheusServeCollapseRequestPaths = "serve.prometheus.collapse_request_paths"
ViperKeyAccessRuleRepositories = "access_rules.repositories"
ViperKeyAccessRuleMatchingStrategy = "access_rules.matching_strategy"
)

// Authorizers
Expand Down Expand Up @@ -208,6 +209,10 @@ func (v *ViperProvider) PrometheusMetricsPath() string {
return viperx.GetString(v.l, ViperKeyPrometheusServeMetricsPath, "/metrics")
}

func (v *ViperProvider) PrometheusCollapseRequestPaths() bool {
return viperx.GetBool(v.l, ViperKeyPrometheusServeCollapseRequestPaths, true)
}

func (v *ViperProvider) ParseURLs(sources []string) ([]url.URL, error) {
r := make([]url.URL, len(sources))
for k, u := range sources {
Expand Down
1 change: 1 addition & 0 deletions driver/configuration/provider_viper_public_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -190,6 +190,7 @@ func TestViperProvider(t *testing.T) {
t.Run("group=prometheus", func(t *testing.T) {
assert.Equal(t, "localhost:9000", p.PrometheusServeAddress())
assert.Equal(t, "/metrics", p.PrometheusMetricsPath())
assert.Equal(t, true, p.PrometheusCollapseRequestPaths())
})

t.Run("group=cors", func(t *testing.T) {
Expand Down
1 change: 1 addition & 0 deletions internal/config/.oathkeeper.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,7 @@ serve:
port: 9000
host: localhost
metrics_path: /metrics
collapse_request_paths: true

# Configures Access Rules
access_rules:
Expand Down
47 changes: 38 additions & 9 deletions metrics/middleware.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package metrics

import (
"net/http"
"strings"
"sync"
"time"

Expand All @@ -27,24 +28,26 @@ func (rc *realClock) Since(t time.Time) time.Duration {
type Middleware struct {
// Name is the name of the application as recorded in latency metrics
Name string
// Promtheus repository
// Prometheus repository
Prometheus *PrometheusRepository

clock timer

// Silence metrics for specific URL paths
// it is protected by the mutex
mutex sync.RWMutex
silencePaths map[string]bool
mutex sync.RWMutex
silencePaths map[string]bool
collapsePaths 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{},
Name: name,
Prometheus: prom,
clock: &realClock{},
silencePaths: map[string]bool{},
collapsePaths: true,
}
}

Expand All @@ -58,14 +61,40 @@ func (m *Middleware) ExcludePaths(paths ...string) *Middleware {
return m
}

// CollapsePaths if set to true, forces the value of the "request" label
// of the prometheus request metrics to be collapsed to the first context path segment only.
// eg. (when set to true):
// - /decisions/service/my-service -> /decisions
// - /decisions -> /decisions
func (m *Middleware) CollapsePaths(flag bool) *Middleware {
m.mutex.Lock()
m.collapsePaths = 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 {
return "/" + uriSegments[1]
}
return "/"

}

func (m *Middleware) ServeHTTP(rw http.ResponseWriter, r *http.Request, next http.HandlerFunc) {
start := m.clock.Now()
next(rw, r)
latency := m.clock.Since(start)
res := rw.(negroni.ResponseWriter)

if _, silent := m.silencePaths[r.URL.Path]; !silent {
m.Prometheus.RequestDurationObserve(m.Name, r.RequestURI, r.Method, res.Status())(float64(latency.Seconds()))
m.Prometheus.UpdateRequest(m.Name, r.RequestURI, r.Method, res.Status())
requestURI := r.RequestURI
if m.collapsePaths {
requestURI = m.getFirstPathSegment(requestURI)
}
m.Prometheus.RequestDurationObserve(m.Name, requestURI, r.Method, res.Status())(float64(latency.Seconds()))
m.Prometheus.UpdateRequest(m.Name, requestURI, r.Method, res.Status())
}
}
122 changes: 122 additions & 0 deletions metrics/middleware_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,122 @@
package metrics

import (
"fmt"
"net/http"
"net/http/httptest"
"strings"
"testing"

"github.com/julienschmidt/httprouter"
"github.com/prometheus/client_golang/prometheus"
"github.com/prometheus/client_golang/prometheus/testutil"
"github.com/urfave/negroni"
)

var (
metricMetadata string = `
# HELP ory_oathkeeper_requests_total Total number of requests
# TYPE ory_oathkeeper_requests_total counter
`
rootMetric string = `
ory_oathkeeper_requests_total{method="GET",request="/",service="test",status_code="200"} 1
`
metricsNotCollapsed string = metricMetadata + rootMetric + `
ory_oathkeeper_requests_total{method="GET",request="/hello/world",service="test",status_code="200"} 1
`
metricsCollapsed string = metricMetadata + rootMetric + `
ory_oathkeeper_requests_total{method="GET",request="/hello",service="test",status_code="200"} 1
`
serverContextPaths []string = []string{"/", "/hello/world"}
)

func NewTestPrometheusRepository(collector prometheus.Collector) *PrometheusRepository {
r := prometheus.NewRegistry()

pr := &PrometheusRepository{
Registry: r,
metrics: []prometheus.Collector{collector},
}

return pr
}

func PrometheusTestApp(middleware *Middleware) http.Handler {
n := negroni.Classic()
n.Use(middleware)

r := httprouter.New()

for _, path := range serverContextPaths {
r.GET(path, func(res http.ResponseWriter, req *http.Request, p httprouter.Params) {
fmt.Fprint(res, "OK")
})
}
n.UseHandler(r)
return n
}

var prometheusParams = []struct {
name string
collapsePaths bool
expectedMetrics string
}{
{"Not collapsed paths", false, metricsNotCollapsed},
{"Collapsed paths", true, metricsCollapsed},
}

func TestPrometheusRequestTotalMetrics(t *testing.T) {
for _, tt := range prometheusParams {
t.Run(tt.name, func(t *testing.T) {
// re-initialize to prevent double counts
RequestTotal.Reset()

promRepo := NewTestPrometheusRepository(RequestTotal)
promMiddleware := NewMiddleware(promRepo, "test")
promMiddleware.CollapsePaths(tt.collapsePaths)

ts := httptest.NewServer(PrometheusTestApp(promMiddleware))
defer ts.Close()

for _, path := range serverContextPaths {
req, err := http.NewRequest("GET", ts.URL+path, nil)
if err != nil {
t.Fatal(err)
}
client := &http.Client{}
_, err = client.Do(req)
if err != nil {
t.Fatal(err)
}
}
if err := testutil.CollectAndCompare(RequestTotal, strings.NewReader(tt.expectedMetrics), "ory_oathkeeper_requests_total"); err != nil {
t.Fatal(err)
}
})
}
}

var requestURIParams = []struct {
name string
originalPath string
firstSegment string
}{
{"root path", "/", "/"},
{"single segment", "/test", "/test"},
{"two segments", "/test/path", "/test"},
{"multiple segments", "/test/path/segments", "/test"},
}

func TestMiddlewareGetFirstPathSegment(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)
if collapsed != tt.firstSegment {
t.Fatalf("Expected first segment: %s to be equal to: %s", collapsed, tt.firstSegment)
}
})
}
}

0 comments on commit ddcb226

Please sign in to comment.