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.

Sign up for GitHub

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

Jump to bottom

fix: query string included for the first collapsed path segment #1159

Merged
merged 1 commit into from
Apr 15, 2024
Merged

fix: query string included for the first collapsed path segment #1159

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion metrics/middleware.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,9 @@ 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]
// Remove any query string from the segment
// For example /my?query=string should return /my
return "/" + strings.SplitN(uriSegments[1], "?", 2)[0]
}
return "/"

Expand Down
20 changes: 13 additions & 7 deletions metrics/middleware_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,14 +32,18 @@ var (
`
metricsNotCollapsed string = metricMetadata + rootMetric + `
ory_oathkeeper_requests_total{method="GET",request="/hello/world",service="test",status_code="200"} 1
ory_oathkeeper_requests_total{method="GET",request="/hello/world?foo=bar",service="test",status_code="200"} 1
ory_oathkeeper_requests_total{method="GET",request="/hello?foo=bar",service="test",status_code="200"} 1
`
metricsCollapsed string = metricMetadata + rootMetric + `
ory_oathkeeper_requests_total{method="GET",request="/hello",service="test",status_code="200"} 1
ory_oathkeeper_requests_total{method="GET",request="/hello",service="test",status_code="200"} 3
`
metricsHidden string = metricMetadata + `
ory_oathkeeper_requests_total{method="GET",request="",service="test",status_code="200"} 2
ory_oathkeeper_requests_total{method="GET",request="",service="test",status_code="200"} 4
`
serverContextPaths []string = []string{"/", "/hello/world"}

serverConfigPaths []string = []string{"/", "/hello", "/hello/world"}
serverRequestPaths []string = []string{"/", "/hello?foo=bar", "/hello/world", "/hello/world?foo=bar"}

configurableMetricMetadata string = `
# HELP http_requests_total Total number of requests
Expand All @@ -49,10 +53,12 @@ var (
http_requests_total{method="GET",request="/",service="test",status_code="200"} 1
`
configurableMetricsNotCollapsed string = configurableMetricMetadata + configurableRootMetric + `
http_requests_total{method="GET",request="/hello?foo=bar",service="test",status_code="200"} 1
http_requests_total{method="GET",request="/hello/world",service="test",status_code="200"} 1
http_requests_total{method="GET",request="/hello/world?foo=bar",service="test",status_code="200"} 1
`
configurableMetricsCollapsed string = configurableMetricMetadata + configurableRootMetric + `
http_requests_total{method="GET",request="/hello",service="test",status_code="200"} 1
http_requests_total{method="GET",request="/hello",service="test",status_code="200"} 3
`
)

Expand All @@ -73,7 +79,7 @@ func PrometheusTestApp(middleware *Middleware) http.Handler {

r := httprouter.New()

for _, path := range serverContextPaths {
for _, path := range serverConfigPaths {
r.GET(path, func(res http.ResponseWriter, req *http.Request, p httprouter.Params) {
fmt.Fprint(res, "OK")
})
Expand Down Expand Up @@ -108,7 +114,7 @@ func TestPrometheusRequestTotalMetrics(t *testing.T) {
ts := httptest.NewServer(PrometheusTestApp(promMiddleware))
defer ts.Close()

for _, path := range serverContextPaths {
for _, path := range serverRequestPaths {
req, err := http.NewRequest("GET", ts.URL+path, nil)
if err != nil {
t.Fatal(err)
Expand Down Expand Up @@ -156,7 +162,7 @@ serve:
ts := httptest.NewServer(PrometheusTestApp(promMiddleware))
defer ts.Close()

for _, path := range serverContextPaths {
for _, path := range serverRequestPaths {
req, err := http.NewRequest("GET", ts.URL+path, nil)
if err != nil {
t.Fatal(err)
Expand Down
Loading