Skip to content

Commit 8aee364

Browse files
zepatrikory-bot
authored andcommitted
fix(hydra): instrument metrics also on public endpoints
GitOrigin-RevId: 84ae1df26bd3d9a025655e50792ea7312f250cca
1 parent 5745f7d commit 8aee364

19 files changed

+128
-178
lines changed

client/handler_test.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -141,7 +141,7 @@ func TestHandler(t *testing.T) {
141141
router := x.NewRouterAdmin(metrics)
142142
h.SetAdminRoutes(router)
143143
router.Handler("GET", prometheusx.MetricsPrometheusPath, promhttp.Handler())
144-
n.UseHandler(router.Mux)
144+
n.UseHandler(router)
145145

146146
adminTs = httptest.NewServer(n)
147147
t.Cleanup(adminTs.Close)
@@ -152,9 +152,9 @@ func TestHandler(t *testing.T) {
152152
n.UseFunc(httprouterx.TrimTrailingSlashNegroni)
153153
n.UseFunc(httprouterx.NoCacheNegroni)
154154

155-
router := x.NewRouterPublic()
155+
router := x.NewRouterPublic(metrics)
156156
h.SetPublicRoutes(router)
157-
n.UseHandler(router.Mux)
157+
n.UseHandler(router)
158158

159159
publicTs = httptest.NewServer(n)
160160
t.Cleanup(publicTs.Close)

client/sdk_test.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -70,17 +70,17 @@ func TestClientSDK(t *testing.T) {
7070

7171
metrics := prometheusx.NewMetricsManagerWithPrefix("hydra", prometheusx.HTTPMetrics, config.Version, config.Commit, config.Date)
7272
routerAdmin := x.NewRouterAdmin(metrics)
73-
routerPublic := x.NewRouterPublic()
73+
routerPublic := x.NewRouterPublic(metrics)
7474
clHandler := client.NewHandler(r)
7575
clHandler.SetPublicRoutes(routerPublic)
7676
clHandler.SetAdminRoutes(routerAdmin)
7777
o2Handler := oauth2.NewHandler(r)
7878
o2Handler.SetPublicRoutes(routerPublic, func(h http.Handler) http.Handler { return h })
7979
o2Handler.SetAdminRoutes(routerAdmin)
8080

81-
server := httptest.NewServer(routerAdmin.Mux)
81+
server := httptest.NewServer(routerAdmin)
8282
t.Cleanup(server.Close)
83-
publicServer := httptest.NewServer(routerPublic.Mux)
83+
publicServer := httptest.NewServer(routerPublic)
8484
t.Cleanup(publicServer.Close)
8585
r.Config().MustSet(ctx, config.KeyAdminURL, server.URL)
8686
r.Config().MustSet(ctx, config.KeyOAuth2TokenURL, publicServer.URL+"/oauth2/token")

cmd/cmd_perform_authorization_code.go

Lines changed: 13 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,6 @@ import (
3030
"github.com/ory/hydra/v2/cmd/cliclient"
3131
"github.com/ory/x/cmdx"
3232
"github.com/ory/x/flagx"
33-
"github.com/ory/x/httprouterx"
3433
"github.com/ory/x/pointerx"
3534
"github.com/ory/x/randx"
3635
"github.com/ory/x/tlsx"
@@ -237,7 +236,7 @@ and success, unless if the --no-shutdown flag is provided.`,
237236
}
238237
authCodeURL, state := generateAuthCodeURL()
239238

240-
r := httprouterx.NewRouterPublic()
239+
r := http.NewServeMux()
241240
var tlsc *tls.Config
242241
if isSSL {
243242
key, err := rsa.GenerateKey(rand.Reader, 2048)
@@ -254,23 +253,23 @@ and success, unless if the --no-shutdown flag is provided.`,
254253

255254
server := graceful.WithDefaults(&http.Server{
256255
Addr: fmt.Sprintf(":%d", port),
257-
Handler: r.Mux, TLSConfig: tlsc,
256+
Handler: r, TLSConfig: tlsc,
258257
ReadHeaderTimeout: time.Second * 5,
259258
})
260-
var shutdown = func() {
259+
shutdown := func() {
261260
time.Sleep(time.Second * 1)
262261
ctx, cancel := context.WithTimeout(context.Background(), time.Second*5)
263262
defer cancel()
264263
_ = server.Shutdown(ctx)
265264
}
266265

267-
r.GET("/", func(w http.ResponseWriter, r *http.Request) {
266+
r.Handle("GET /", http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
268267
_ = tokenUserWelcome.Execute(w, &struct{ URL string }{URL: authCodeURL})
269-
})
268+
}))
270269

271-
r.GET("/perform-flow", func(w http.ResponseWriter, r *http.Request) {
270+
r.Handle("GET /perform-flow", http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
272271
http.Redirect(w, r, authCodeURL, http.StatusFound)
273-
})
272+
}))
274273

275274
rt := router{
276275
cl: client,
@@ -290,12 +289,12 @@ and success, unless if the --no-shutdown flag is provided.`,
290289
noShutdown: noShutdown,
291290
}
292291

293-
r.GET("/login", rt.loginGET)
294-
r.POST("/login", rt.loginPOST)
295-
r.GET("/consent", rt.consentGET)
296-
r.POST("/consent", rt.consentPOST)
297-
r.GET("/callback", rt.callback)
298-
r.POST("/callback", rt.callbackPOSTForm)
292+
r.Handle("GET /login", http.HandlerFunc(rt.loginGET))
293+
r.Handle("POST /login", http.HandlerFunc(rt.loginPOST))
294+
r.Handle("GET /consent", http.HandlerFunc(rt.consentGET))
295+
r.Handle("POST /consent", http.HandlerFunc(rt.consentPOST))
296+
r.Handle("GET /callback", http.HandlerFunc(rt.callback))
297+
r.Handle("POST /callback", http.HandlerFunc(rt.callbackPOSTForm))
299298

300299
if !flagx.MustGetBool(cmd, "no-open") {
301300
_ = webbrowser.Open(serverLocation) // ignore errors

cmd/server/handler.go

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -135,7 +135,6 @@ func adminServer(ctx context.Context, d *driver.RegistrySQL, metricsService *met
135135
n.UseFunc(httprouterx.AddAdminPrefixIfNotPresentNegroni)
136136
n.UseFunc(semconv.Middleware)
137137
n.Use(logger)
138-
n.Use(prometheusManager)
139138

140139
if cfg.TLS.Enabled && !networkx.AddressIsUnixSocket(cfg.Host) {
141140
mw, err := tlsx.EnforceTLSRequests(d, cfg.TLS.AllowTerminationFrom)
@@ -158,10 +157,10 @@ func adminServer(ctx context.Context, d *driver.RegistrySQL, metricsService *met
158157
})
159158
n.Use(metricsService)
160159

161-
router := httprouterx.NewRouterAdmin(prometheusManager)
160+
router := httprouterx.NewRouterAdminWithPrefix(prometheusManager)
162161
d.RegisterAdminRoutes(router)
163162

164-
n.UseHandler(router.Mux)
163+
n.UseHandler(router)
165164

166165
return func() error {
167166
return serve(ctx, d, cfg, n, "admin")
@@ -185,7 +184,6 @@ func publicServer(ctx context.Context, d *driver.RegistrySQL, metricsService *me
185184
n.UseFunc(httprouterx.NoCacheNegroni)
186185
n.UseFunc(semconv.Middleware)
187186
n.Use(logger)
188-
n.Use(prometheusManager)
189187
if cfg.TLS.Enabled && !networkx.AddressIsUnixSocket(cfg.Host) {
190188
mw, err := tlsx.EnforceTLSRequests(d, cfg.TLS.AllowTerminationFrom)
191189
if err != nil {
@@ -207,10 +205,10 @@ func publicServer(ctx context.Context, d *driver.RegistrySQL, metricsService *me
207205
})
208206
n.Use(metricsService)
209207

210-
router := x.NewRouterPublic()
208+
router := x.NewRouterPublic(metricsService)
211209
d.RegisterPublicRoutes(ctx, router)
212210

213-
n.UseHandler(router.Mux)
211+
n.UseHandler(router)
214212
return func() error {
215213
return serve(ctx, d, cfg, n, "public")
216214
}, nil

consent/handler_test.go

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ func TestGetLogoutRequest(t *testing.T) {
3737
h := NewHandler(reg)
3838
r := x.NewRouterAdmin(prometheusx.NewMetricsManagerWithPrefix("hydra", prometheusx.HTTPMetrics, config.Version, config.Commit, config.Date))
3939
h.SetRoutes(r)
40-
ts := httptest.NewServer(r.Mux)
40+
ts := httptest.NewServer(r)
4141
defer ts.Close()
4242

4343
cl := &client.Client{
@@ -103,7 +103,7 @@ func TestGetLoginRequest(t *testing.T) {
103103
h := NewHandler(reg)
104104
r := x.NewRouterAdmin(prometheusx.NewMetricsManagerWithPrefix("hydra", prometheusx.HTTPMetrics, config.Version, config.Commit, config.Date))
105105
h.SetRoutes(r)
106-
ts := httptest.NewServer(r.Mux)
106+
ts := httptest.NewServer(r)
107107
defer ts.Close()
108108

109109
cl := &client.Client{
@@ -169,7 +169,7 @@ func TestGetConsentRequest(t *testing.T) {
169169
h := NewHandler(reg)
170170
r := x.NewRouterAdmin(prometheusx.NewMetricsManagerWithPrefix("hydra", prometheusx.HTTPMetrics, config.Version, config.Commit, config.Date))
171171
h.SetRoutes(r)
172-
ts := httptest.NewServer(r.Mux)
172+
ts := httptest.NewServer(r)
173173
defer ts.Close()
174174

175175
cl := &client.Client{
@@ -249,7 +249,7 @@ func TestAcceptLoginRequestDouble(t *testing.T) {
249249
h := NewHandler(reg)
250250
r := x.NewRouterAdmin(prometheusx.NewMetricsManagerWithPrefix("hydra", prometheusx.HTTPMetrics, config.Version, config.Commit, config.Date))
251251
h.SetRoutes(r)
252-
ts := httptest.NewServer(r.Mux)
252+
ts := httptest.NewServer(r)
253253
defer ts.Close()
254254

255255
// marshal User to json
@@ -289,7 +289,7 @@ func TestAcceptCodeDeviceRequest(t *testing.T) {
289289
h := NewHandler(reg)
290290
r := x.NewRouterAdmin(prometheusx.NewMetricsManagerWithPrefix("hydra", prometheusx.HTTPMetrics, config.Version, config.Commit, config.Date))
291291
h.SetRoutes(r)
292-
ts := httptest.NewServer(r.Mux)
292+
ts := httptest.NewServer(r)
293293
t.Cleanup(ts.Close)
294294

295295
submitCode := func(t *testing.T, reqBody any, challenge string) *http.Response {

health/handler_test.go

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,8 @@ import (
99
"net/http/httptest"
1010
"testing"
1111

12+
"github.com/urfave/negroni"
13+
1214
"github.com/stretchr/testify/assert"
1315
"github.com/stretchr/testify/require"
1416

@@ -71,10 +73,10 @@ func TestPublicHealthHandler(t *testing.T) {
7173
t.Run(tc.name, func(t *testing.T) {
7274
reg := testhelpers.NewRegistryMemory(t, driver.WithConfigOptions(configx.WithValues(tc.config)))
7375

74-
public := x.NewRouterPublic()
76+
public := x.NewRouterPublic(negroni.HandlerFunc(func(w http.ResponseWriter, r *http.Request, next http.HandlerFunc) { next(w, r) }))
7577
reg.RegisterPublicRoutes(ctx, public)
7678

77-
ts := httptest.NewServer(public.Mux)
79+
ts := httptest.NewServer(public)
7880

7981
tc.verifyResponse(t, doCORSRequest(t, ts.URL+healthx.AliveCheckPath))
8082
tc.verifyResponse(t, doCORSRequest(t, ts.URL+healthx.ReadyCheckPath))

internal/testhelpers/oauth2.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,7 @@ func NewConfigurableOAuth2Server(ctx context.Context, t testing.TB, reg *driver.
8484

8585
router := x.NewRouterAdmin(metrics)
8686
reg.RegisterAdminRoutes(router)
87-
n.UseHandler(router.Mux)
87+
n.UseHandler(router)
8888

8989
adminTS = httptest.NewServer(n)
9090
t.Cleanup(adminTS.Close)
@@ -95,9 +95,9 @@ func NewConfigurableOAuth2Server(ctx context.Context, t testing.TB, reg *driver.
9595
n.UseFunc(httprouterx.TrimTrailingSlashNegroni)
9696
n.UseFunc(httprouterx.NoCacheNegroni)
9797

98-
router := x.NewRouterPublic()
98+
router := x.NewRouterPublic(metrics)
9999
reg.RegisterPublicRoutes(ctx, router)
100-
n.UseHandler(router.Mux)
100+
n.UseHandler(router)
101101

102102
publicTS = httptest.NewServer(n)
103103
t.Cleanup(publicTS.Close)

jwk/handler.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -55,8 +55,8 @@ func NewHandler(r InternalRegistry) *Handler {
5555
}
5656

5757
func (h *Handler) SetPublicRoutes(r *httprouterx.RouterPublic, corsMiddleware func(http.Handler) http.Handler) {
58-
r.Handle("OPTIONS", WellKnownKeysPath, corsMiddleware(http.HandlerFunc(h.handleOptions)))
59-
r.Handle("GET", WellKnownKeysPath, corsMiddleware(http.HandlerFunc(h.discoverJsonWebKeys)))
58+
r.Handler("OPTIONS", WellKnownKeysPath, corsMiddleware(http.HandlerFunc(h.handleOptions)))
59+
r.Handler("GET", WellKnownKeysPath, corsMiddleware(http.HandlerFunc(h.discoverJsonWebKeys)))
6060
}
6161

6262
func (h *Handler) SetAdminRoutes(r *httprouterx.RouterAdmin) {

jwk/handler_test.go

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,8 @@ import (
1010
"net/http/httptest"
1111
"testing"
1212

13+
"github.com/urfave/negroni"
14+
1315
"github.com/ory/hydra/v2/driver"
1416

1517
"github.com/go-jose/go-jose/v3"
@@ -28,12 +30,12 @@ func TestHandlerWellKnown(t *testing.T) {
2830
t.Parallel()
2931

3032
reg := testhelpers.NewRegistryMemory(t, driver.WithConfigOptions(configx.WithValue(config.KeyWellKnownKeys, []string{x.OpenIDConnectKeyName, x.OpenIDConnectKeyName})))
31-
router := x.NewRouterPublic()
33+
router := x.NewRouterPublic(negroni.HandlerFunc(func(w http.ResponseWriter, r *http.Request, next http.HandlerFunc) { next(w, r) }))
3234
h := jwk.NewHandler(reg)
3335
h.SetPublicRoutes(router, func(h http.Handler) http.Handler {
3436
return h
3537
})
36-
testServer := httptest.NewServer(router.Mux)
38+
testServer := httptest.NewServer(router)
3739
JWKPath := "/.well-known/jwks.json"
3840

3941
t.Run("Test_Handler_WellKnown/Run_public_key_With_public_prefix", func(t *testing.T) {

jwk/sdk_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ func TestJWKSDK(t *testing.T) {
2929
router := x.NewRouterAdmin(metrics)
3030
h := NewHandler(reg)
3131
h.SetAdminRoutes(router)
32-
server := httptest.NewServer(router.Mux)
32+
server := httptest.NewServer(router)
3333
reg.Config().MustSet(ctx, config.KeyAdminURL, server.URL)
3434

3535
sdk := hydra.NewAPIClient(hydra.NewConfiguration())

0 commit comments

Comments
 (0)