mirror of
https://github.com/coder/coder.git
synced 2026-09-21 20:51:01 +08:00
fix: report correct request paths from workspace proxy metrics (#21302)
I noticed while looking at scale test metrics that we don't always report a useful path in the API request metrics.  There are a lot of requests with path `/*`. I chased this problem to the workspace proxy, where we mount a the proxy router as a child of a "root" router to support some high level endpoints like `latency-check`. Because we query the path from the Chi route context in the prometheus middleware _before_ the request is actually handled, we can have a partially resolved pattern match only corresponding to the root router. The fix is to always re-resolve the path, rather than accept a partially resolved path.
This commit is contained in:
@@ -106,11 +106,6 @@ func getRoutePattern(r *http.Request) string {
|
||||
return ""
|
||||
}
|
||||
|
||||
if pattern := rctx.RoutePattern(); pattern != "" {
|
||||
// Pattern is already available
|
||||
return pattern
|
||||
}
|
||||
|
||||
routePath := r.URL.Path
|
||||
if r.URL.RawPath != "" {
|
||||
routePath = r.URL.RawPath
|
||||
|
||||
@@ -2,11 +2,13 @@ package httpmw_test
|
||||
|
||||
import (
|
||||
"context"
|
||||
"fmt"
|
||||
"net/http"
|
||||
"net/http/httptest"
|
||||
"testing"
|
||||
|
||||
"github.com/go-chi/chi/v5"
|
||||
"github.com/google/uuid"
|
||||
"github.com/prometheus/client_golang/prometheus"
|
||||
cm "github.com/prometheus/client_model/go"
|
||||
"github.com/stretchr/testify/assert"
|
||||
@@ -164,6 +166,42 @@ func TestPrometheus(t *testing.T) {
|
||||
require.Equal(t, "UNKNOWN", reqProcessed["path"])
|
||||
require.Equal(t, "GET", reqProcessed["method"])
|
||||
})
|
||||
|
||||
t.Run("Subrouter", func(t *testing.T) {
|
||||
t.Parallel()
|
||||
reg := prometheus.NewRegistry()
|
||||
promMW := httpmw.Prometheus(reg)
|
||||
|
||||
r := chi.NewRouter()
|
||||
r.Use(promMW)
|
||||
r.Get("/api/v2/workspaceagents/{workspaceagent}/pty", func(w http.ResponseWriter, r *http.Request) {})
|
||||
|
||||
// Mount under a root router like wsproxy does.
|
||||
rootRouter := chi.NewRouter()
|
||||
rootRouter.Get("/latency-check", func(w http.ResponseWriter, r *http.Request) {})
|
||||
rootRouter.Mount("/", r)
|
||||
|
||||
agentID := uuid.UUID{1}
|
||||
req := httptest.NewRequest("GET", fmt.Sprintf("/api/v2/workspaceagents/%s/pty", agentID.String()), nil)
|
||||
|
||||
sw := &tracing.StatusWriter{ResponseWriter: httptest.NewRecorder()}
|
||||
rootRouter.ServeHTTP(sw, req)
|
||||
|
||||
metrics, err := reg.Gather()
|
||||
require.NoError(t, err)
|
||||
require.Greater(t, len(metrics), 0)
|
||||
metricLabels := getMetricLabels(metrics)
|
||||
|
||||
reqProcessed, ok := metricLabels["coderd_api_requests_processed_total"]
|
||||
require.True(t, ok, "coderd_api_requests_processed_total metric not found")
|
||||
require.Equal(t, "/api/v2/workspaceagents/{workspaceagent}/pty", reqProcessed["path"])
|
||||
require.Equal(t, "GET", reqProcessed["method"])
|
||||
|
||||
concurrentRequests, ok := metricLabels["coderd_api_concurrent_requests"]
|
||||
require.True(t, ok, "coderd_api_concurrent_requests metric not found")
|
||||
require.Equal(t, "/api/v2/workspaceagents/{workspaceagent}/pty", concurrentRequests["path"])
|
||||
require.Equal(t, "GET", concurrentRequests["method"])
|
||||
})
|
||||
}
|
||||
|
||||
func getMetricLabels(metrics []*cm.MetricFamily) map[string]map[string]string {
|
||||
|
||||
Reference in New Issue
Block a user