fix: correctly handle HTTP route patterns in metrics

GitOrigin-RevId: 534f4347eb820a3c51357b9d8defcefecd9845dd

Author: hperl

cmd/courier/watch.go

@@ -63,7 +63,6 @@ func ServeMetrics(ctx context.Context, r driver.Registry, port int) error {
 
 	router.Handle(prometheusx.MetricsPrometheusPath, promhttp.Handler())
 	n.Use(reqlog.NewMiddlewareFromLogger(l, "admin#"+cfg.BaseURL.String()))
-	n.Use(r.PrometheusManager())
 
 	n.UseHandler(router)
 

cmd/daemon/serve.go

@@ -68,9 +68,7 @@ func servePublic(ctx context.Context, r *driver.RegistryDefault, cmd *cobra.Comm
 	n.Use(x.HTTPLoaderContextMiddleware(r))
 	n.Use(sqa(ctx, cmd, r))
 
-	n.Use(r.PrometheusManager())
-
-	router := x.NewRouterPublic()
+	router := x.NewRouterPublic(r)
 	csrf := nosurfx.NewCSRFHandler(router, r)
 
 	// we need to always load the CORS middleware even if it is disabled, to allow hot-enabling CORS
@@ -158,9 +156,8 @@ func serveAdmin(ctx context.Context, r *driver.RegistryDefault, cmd *cobra.Comma
 	n.UseFunc(x.RedirectAdminMiddleware)
 	n.Use(x.HTTPLoaderContextMiddleware(r))
 	n.Use(sqa(ctx, cmd, r))
-	n.Use(r.PrometheusManager())
 
-	router := x.NewRouterAdmin()
+	router := x.NewRouterAdmin(r)
 	r.RegisterAdminRoutes(ctx, router)
 
 	n.UseHandler(http.MaxBytesHandler(router, 5*1024*1024 /* 5 MB */))

driver/config/handler_test.go

@@ -28,7 +28,7 @@ func (c *configProvider) Config() *config.Config {
 func TestNewConfigHashHandler(t *testing.T) {
 	ctx := context.Background()
 	cfg := internal.NewConfigurationWithDefaults(t)
-	router := x.NewRouterPublic()
+	router := x.NewTestRouterPublic(t)
 	config.NewConfigHashHandler(&configProvider{cfg: cfg}, router)
 	ts := contextx.NewConfigurableTestServer(router)
 	t.Cleanup(ts.Close)

driver/registry_default.go

@@ -64,7 +64,7 @@ import (
 	"github.com/ory/x/otelx"
 	otelsql "github.com/ory/x/otelx/sql"
 	"github.com/ory/x/popx"
-	prometheus "github.com/ory/x/prometheusx"
+	"github.com/ory/x/prometheusx"
 	"github.com/ory/x/servicelocatorx"
 	"github.com/ory/x/sqlcon"
 )
@@ -83,10 +83,10 @@ type RegistryDefault struct {
 
 	nosurf         nosurf.Handler
 	trc            *otelx.Tracer
-	pmm            *prometheus.MetricsManager
+	pmm            *prometheusx.MetricsManager
 	writer         herodot.Writer
 	healthxHandler *healthx.Handler
-	metricsHandler *prometheus.Handler
+	metricsHandler *prometheusx.Handler
 
 	persister       persistence.Persister
 	migrationStatus popx.MigrationStatuses
@@ -287,9 +287,9 @@ func (m *RegistryDefault) HealthHandler(_ context.Context) *healthx.Handler {
 	return m.healthxHandler
 }
 
-func (m *RegistryDefault) MetricsHandler() *prometheus.Handler {
+func (m *RegistryDefault) MetricsHandler() *prometheusx.Handler {
 	if m.metricsHandler == nil {
-		m.metricsHandler = prometheus.NewHandler(m.Writer(), config.Version)
+		m.metricsHandler = prometheusx.NewHandler(m.Writer(), config.Version)
 	}
 
 	return m.metricsHandler
@@ -835,11 +835,11 @@ func (m *RegistryDefault) IdentityManager() *identity.Manager {
 	return m.identityManager
 }
 
-func (m *RegistryDefault) PrometheusManager() *prometheus.MetricsManager {
+func (m *RegistryDefault) PrometheusManager() *prometheusx.MetricsManager {
 	m.rwl.Lock()
 	defer m.rwl.Unlock()
 	if m.pmm == nil {
-		m.pmm = prometheus.NewMetricsManagerWithPrefix("kratos", prometheus.HTTPMetrics, m.buildVersion, m.buildHash, m.buildDate)
+		m.pmm = prometheusx.NewMetricsManagerWithPrefix("kratos", prometheusx.HTTPMetrics, m.buildVersion, m.buildHash, m.buildDate)
 	}
 	return m.pmm
 }

driver/registry_default_test.go

@@ -6,11 +6,14 @@ package driver_test
 import (
 	"context"
 	"fmt"
+	"io"
+	"net/http"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 
+	"github.com/ory/kratos/internal/testhelpers"
 	"github.com/ory/x/configx"
 	"github.com/ory/x/contextx"
 	"github.com/ory/x/logrusx"
@@ -960,3 +963,26 @@ func TestGetActiveVerificationStrategy(t *testing.T) {
 		}
 	})
 }
+
+func TestMetricsRouterPaths(t *testing.T) {
+	t.Parallel()
+	_, reg := internal.NewVeryFastRegistryWithoutDB(t)
+	publicTS, adminTS := testhelpers.NewKratosServerWithCSRF(t, reg)
+
+	// Make some requests that should be recorded in the metrics
+	req, _ := http.NewRequest(http.MethodDelete, publicTS.URL+"/sessions/session-id", nil)
+	_, err := publicTS.Client().Do(req)
+	require.NoError(t, err)
+	_, err = adminTS.Client().Get(adminTS.URL + "/admin/identities/some-id/sessions")
+	require.NoError(t, err)
+
+	res, err := adminTS.Client().Get(adminTS.URL + "/admin/metrics/prometheus")
+	require.NoError(t, err)
+	require.EqualValues(t, http.StatusOK, res.StatusCode)
+	respBody, err := io.ReadAll(res.Body)
+	body := string(respBody)
+
+	require.NoError(t, err)
+	assert.Contains(t, body, `endpoint="DELETE /sessions/{param}"`, body)
+	assert.Contains(t, body, `endpoint="GET /admin/identities/{param}/sessions"`, body)
+}

internal/registrationhelpers/helpers.go

@@ -38,8 +38,8 @@ import (
 
 func setupServer(t *testing.T, reg *driver.RegistryDefault) *httptest.Server {
 	conf := reg.Config()
-	router := x.NewRouterPublic()
-	admin := x.NewRouterAdmin()
+	router := x.NewRouterPublic(reg)
+	admin := x.NewRouterAdmin(reg)
 
 	publicTS, _ := testhelpers.NewKratosServerWithRouters(t, reg, router, admin)
 	redirTS := testhelpers.NewRedirSessionEchoTS(t, reg)

internal/testhelpers/selfservice_settings.go

@@ -176,7 +176,7 @@ func NewSettingsLoginAcceptAPIServer(t *testing.T, publicClient *kratos.APIClien
 
 func NewSettingsAPIServer(t *testing.T, reg *driver.RegistryDefault, ids map[string]*identity.Identity) (*httptest.Server, *httptest.Server, map[string]*http.Client) {
 	ctx := context.Background()
-	public, admin := x.NewRouterPublic(), x.NewRouterAdmin()
+	public, admin := x.NewRouterPublic(reg), x.NewRouterAdmin(reg)
 	reg.SettingsHandler().RegisterAdminRoutes(admin)
 
 	n := negroni.Classic()

internal/testhelpers/server.go

@@ -20,7 +20,7 @@ import (
 )
 
 func NewKratosServer(t *testing.T, reg driver.Registry) (public, admin *httptest.Server) {
-	return NewKratosServerWithRouters(t, reg, x.NewRouterPublic(), x.NewRouterAdmin())
+	return NewKratosServerWithRouters(t, reg, x.NewRouterPublic(reg), x.NewRouterAdmin(reg))
 }
 
 func NewKratosServerWithCSRF(t *testing.T, reg driver.Registry) (public, admin *httptest.Server) {
@@ -29,15 +29,18 @@ func NewKratosServerWithCSRF(t *testing.T, reg driver.Registry) (public, admin *
 }
 
 func NewKratosServerWithCSRFAndRouters(t *testing.T, reg driver.Registry) (public, admin *httptest.Server, rp *x.RouterPublic, ra *x.RouterAdmin) {
-	rp, ra = x.NewRouterPublic(), x.NewRouterAdmin()
+	rp, ra = x.NewRouterPublic(reg), x.NewRouterAdmin(reg)
 	csrfHandler := nosurfx.NewTestCSRFHandler(rp, reg)
 	reg.WithCSRFHandler(csrfHandler)
+
 	ran := negroni.New()
 	ran.UseFunc(x.RedirectAdminMiddleware)
 	ran.UseHandler(ra)
+
 	rpn := negroni.New()
 	rpn.UseFunc(x.HTTPLoaderContextMiddleware(reg))
 	rpn.UseHandler(rp)
+
 	public = httptest.NewServer(nosurfx.NewTestCSRFHandler(rpn, reg))
 	admin = httptest.NewServer(ran)
 	ctx := context.Background()
@@ -82,9 +85,9 @@ func InitKratosServers(t *testing.T, reg driver.Registry, public, admin *httptes
 	reg.RegisterRoutes(context.Background(), public.Config.Handler.(*x.RouterPublic), admin.Config.Handler.(*x.RouterAdmin))
 }
 
-func NewKratosServers(t *testing.T) (public, admin *httptest.Server) {
-	public = httptest.NewServer(x.NewRouterPublic())
-	admin = httptest.NewServer(x.NewRouterAdmin())
+func NewKratosServers(t *testing.T, reg driver.Registry) (public, admin *httptest.Server) {
+	public = httptest.NewServer(x.NewRouterPublic(reg))
+	admin = httptest.NewServer(x.NewRouterAdmin(reg))
 
 	public.URL = strings.Replace(public.URL, "127.0.0.1", "localhost", -1)
 	t.Cleanup(public.Close)

schema/handler_test.go

@@ -30,7 +30,7 @@ import (
 func TestHandler(t *testing.T) {
 	ctx := context.Background()
 	conf, reg := internal.NewFastRegistryWithMocks(t)
-	router := x.NewRouterPublic()
+	router := x.NewTestRouterPublic(t)
 	reg.SchemaHandler().RegisterPublicRoutes(router)
 	ts := httptest.NewServer(router)
 	defer ts.Close()

selfservice/errorx/handler_test.go

@@ -34,7 +34,7 @@ func TestHandler(t *testing.T) {
 	h := errorx.NewHandler(reg)
 
 	t.Run("case=public authorization", func(t *testing.T) {
-		router := x.NewRouterPublic()
+		router := x.NewTestRouterPublic(t)
 		ns := nosurfx.NewTestCSRFHandler(router, reg)
 
 		h.RegisterPublicRoutes(router)
@@ -74,7 +74,7 @@ func TestHandler(t *testing.T) {
 	})
 
 	t.Run("case=stubs", func(t *testing.T) {
-		router := x.NewRouterPublic()
+		router := x.NewTestRouterPublic(t)
 		h.RegisterPublicRoutes(router)
 		ts := httptest.NewServer(router)
 		defer ts.Close()
@@ -90,7 +90,7 @@ func TestHandler(t *testing.T) {
 	})
 
 	t.Run("case=errors types", func(t *testing.T) {
-		router := x.NewRouterPublic()
+		router := x.NewTestRouterPublic(t)
 		h.RegisterPublicRoutes(router)
 		ts := httptest.NewServer(router)
 		defer ts.Close()