Skip to content

Commit 724e425

Browse files
alnrory-bot
authored andcommitted
feat: improved tracing
GitOrigin-RevId: 98ae16a6f1de4414538c7692758af850aa0caaac
1 parent e8586df commit 724e425

File tree

9 files changed

+68
-54
lines changed

9 files changed

+68
-54
lines changed

cmd/server/handler.go

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -162,6 +162,7 @@ func adminServer(ctx context.Context, d *driver.RegistrySQL, sqaMetrics *metrics
162162

163163
n.UseHandler(router)
164164

165+
n.UseFunc(otelx.SpanNameRecorderNegroniFunc)
165166
return func() error {
166167
return serve(ctx, d, cfg, n, "admin")
167168
}, nil
@@ -209,6 +210,7 @@ func publicServer(ctx context.Context, d *driver.RegistrySQL, sqaMetrics *metric
209210
d.RegisterPublicRoutes(ctx, router)
210211

211212
n.UseHandler(router)
213+
n.UseFunc(otelx.SpanNameRecorderNegroniFunc)
212214
return func() error {
213215
return serve(ctx, d, cfg, n, "public")
214216
}, nil
@@ -312,12 +314,8 @@ func serve(
312314
ifaceName string,
313315
) error {
314316
if tracer := d.Tracer(ctx); tracer.IsLoaded() {
315-
handler = otelx.TraceHandler(
316-
handler,
317+
handler = otelx.NewMiddleware(handler, ifaceName,
317318
otelhttp.WithTracerProvider(tracer.Provider()),
318-
otelhttp.WithFilter(func(r *http.Request) bool {
319-
return !strings.HasPrefix(r.URL.Path, "/admin/metrics/")
320-
}),
321319
)
322320
}
323321

go.mod

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,6 @@ require (
2626
github.com/gorilla/sessions v1.4.0
2727
github.com/hashicorp/go-retryablehttp v0.7.8
2828
github.com/jackc/pgx/v5 v5.7.5
29-
github.com/julienschmidt/httprouter v1.3.0
3029
github.com/magiconair/properties v1.8.9
3130
github.com/mattn/goveralls v0.0.12
3231
github.com/miekg/pkcs11 v1.1.1
@@ -171,6 +170,7 @@ require (
171170
github.com/jmoiron/sqlx v1.4.0 // indirect
172171
github.com/joho/godotenv v1.5.1 // indirect
173172
github.com/josharian/intern v1.0.0 // indirect
173+
github.com/julienschmidt/httprouter v1.3.1-0.20240130105656-484018016424 // indirect
174174
github.com/kballard/go-shellquote v0.0.0-20180428030007-95032a82bc51 // indirect
175175
github.com/knadh/koanf/maps v0.1.2 // indirect
176176
github.com/knadh/koanf/parsers/json v0.1.0 // indirect

go.sum

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -303,8 +303,8 @@ github.com/josharian/intern v1.0.0 h1:vlS4z54oSdjm0bgjRigI+G1HpF+tI+9rE5LLzOg8Hm
303303
github.com/josharian/intern v1.0.0/go.mod h1:5DoeVV0s6jJacbCEi61lwdGj/aVlrQvzHFFd8Hwg//Y=
304304
github.com/jtolds/gls v4.20.0+incompatible h1:xdiiI2gbIgH/gLH7ADydsJ1uDOEzR8yvV7C0MuV77Wo=
305305
github.com/jtolds/gls v4.20.0+incompatible/go.mod h1:QJZ7F/aHp+rZTRtaJ1ow/lLfFfVYBRgL+9YlvaHOwJU=
306-
github.com/julienschmidt/httprouter v1.3.0 h1:U0609e9tgbseu3rBINet9P48AI/D3oJs4dN7jwJOQ1U=
307-
github.com/julienschmidt/httprouter v1.3.0/go.mod h1:JR6WtHb+2LUe8TCKY3cZOxFyyO8IZAc4RVcycCCAKdM=
306+
github.com/julienschmidt/httprouter v1.3.1-0.20240130105656-484018016424 h1:KsUAkP+Y6n+542zpxWiQDUvOqfh3n429HYleEvq/V7M=
307+
github.com/julienschmidt/httprouter v1.3.1-0.20240130105656-484018016424/go.mod h1:JR6WtHb+2LUe8TCKY3cZOxFyyO8IZAc4RVcycCCAKdM=
308308
github.com/kballard/go-shellquote v0.0.0-20180428030007-95032a82bc51 h1:Z9n2FFNUXsshfwJMBgNA0RU6/i7WVaAegv3PtuIHPMs=
309309
github.com/kballard/go-shellquote v0.0.0-20180428030007-95032a82bc51/go.mod h1:CzGEWj7cYgsdH8dAjBGEr58BoE7ScuLd+fwFZ44+/x8=
310310
github.com/kisielk/errcheck v1.5.0/go.mod h1:pFxgyoBC7bSaBwPgfKdkLd5X25qrDl4LWUI2bnpBCr8=

internal/testhelpers/oauth2.go

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@ import (
1616
"testing"
1717
"time"
1818

19-
"github.com/julienschmidt/httprouter"
2019
"github.com/stretchr/testify/assert"
2120
"github.com/stretchr/testify/require"
2221
"github.com/tidwall/gjson"
@@ -217,8 +216,8 @@ func NewCallbackURL(t testing.TB, prefix string, h http.HandlerFunc) string {
217216
h = HTTPServerNotImplementedHandler
218217
}
219218

220-
r := httprouter.New()
221-
r.GET("/"+prefix, func(w http.ResponseWriter, r *http.Request, _ httprouter.Params) {
219+
r := http.NewServeMux()
220+
r.HandleFunc("/"+prefix, func(w http.ResponseWriter, r *http.Request) {
222221
h(w, r)
223222
})
224223
ts := httptest.NewServer(r)

oryx/go.mod

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ require (
3333
github.com/jackc/pgx/v4 v4.18.3
3434
github.com/jackc/puddle/v2 v2.2.2
3535
github.com/jmoiron/sqlx v1.4.0
36-
github.com/julienschmidt/httprouter v1.3.0
36+
github.com/julienschmidt/httprouter v1.3.1-0.20240130105656-484018016424
3737
github.com/knadh/koanf/maps v0.1.2
3838
github.com/knadh/koanf/parsers/json v0.1.0
3939
github.com/knadh/koanf/parsers/toml v0.1.0

oryx/go.sum

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -304,8 +304,8 @@ github.com/joho/godotenv v1.5.1 h1:7eLL/+HRGLY0ldzfGMeQkb7vMd0as4CfYvUVzLqw0N0=
304304
github.com/joho/godotenv v1.5.1/go.mod h1:f4LDr5Voq0i2e/R5DDNOoa2zzDfwtkZa6DnEwAbqwq4=
305305
github.com/josharian/intern v1.0.0 h1:vlS4z54oSdjm0bgjRigI+G1HpF+tI+9rE5LLzOg8HmY=
306306
github.com/josharian/intern v1.0.0/go.mod h1:5DoeVV0s6jJacbCEi61lwdGj/aVlrQvzHFFd8Hwg//Y=
307-
github.com/julienschmidt/httprouter v1.3.0 h1:U0609e9tgbseu3rBINet9P48AI/D3oJs4dN7jwJOQ1U=
308-
github.com/julienschmidt/httprouter v1.3.0/go.mod h1:JR6WtHb+2LUe8TCKY3cZOxFyyO8IZAc4RVcycCCAKdM=
307+
github.com/julienschmidt/httprouter v1.3.1-0.20240130105656-484018016424 h1:KsUAkP+Y6n+542zpxWiQDUvOqfh3n429HYleEvq/V7M=
308+
github.com/julienschmidt/httprouter v1.3.1-0.20240130105656-484018016424/go.mod h1:JR6WtHb+2LUe8TCKY3cZOxFyyO8IZAc4RVcycCCAKdM=
309309
github.com/kballard/go-shellquote v0.0.0-20180428030007-95032a82bc51 h1:Z9n2FFNUXsshfwJMBgNA0RU6/i7WVaAegv3PtuIHPMs=
310310
github.com/kballard/go-shellquote v0.0.0-20180428030007-95032a82bc51/go.mod h1:CzGEWj7cYgsdH8dAjBGEr58BoE7ScuLd+fwFZ44+/x8=
311311
github.com/kisielk/errcheck v1.5.0/go.mod h1:pFxgyoBC7bSaBwPgfKdkLd5X25qrDl4LWUI2bnpBCr8=

oryx/hasherx/hasher_bcrypt.go

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -3,11 +3,11 @@ package hasherx
33
import (
44
"context"
55

6+
"github.com/ory/x/otelx"
67
"github.com/pkg/errors"
78

89
"go.opentelemetry.io/otel"
910
"go.opentelemetry.io/otel/attribute"
10-
"go.opentelemetry.io/otel/codes"
1111

1212
"golang.org/x/crypto/bcrypt"
1313
)
@@ -35,22 +35,18 @@ func NewHasherBcrypt(c BCryptConfigurator) *Bcrypt {
3535
}
3636

3737
// Generate generates a hash for the given password.
38-
func (h *Bcrypt) Generate(ctx context.Context, password []byte) ([]byte, error) {
38+
func (h *Bcrypt) Generate(ctx context.Context, password []byte) (hash []byte, err error) {
3939
ctx, span := otel.GetTracerProvider().Tracer(tracingComponent).Start(ctx, "hash.Bcrypt.Generate")
40-
defer span.End()
40+
defer otelx.End(span, &err)
4141

4242
if err := validateBcryptPasswordLength(password); err != nil {
43-
span.RecordError(err)
44-
span.SetStatus(codes.Error, err.Error())
4543
return nil, err
4644
}
4745

4846
cost := int(h.c.HasherBcryptConfig(ctx).Cost)
4947
span.SetAttributes(attribute.Int("bcrypt.cost", cost))
50-
hash, err := bcrypt.GenerateFromPassword(password, cost)
48+
hash, err = bcrypt.GenerateFromPassword(password, cost)
5149
if err != nil {
52-
span.RecordError(err)
53-
span.SetStatus(codes.Error, err.Error())
5450
return nil, err
5551
}
5652

oryx/hasherx/hasher_pbkdf2.go

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import (
1111
"fmt"
1212
"hash"
1313

14+
"github.com/ory/x/otelx"
1415
"github.com/pkg/errors"
1516
"go.opentelemetry.io/otel"
1617
"go.opentelemetry.io/otel/codes"
@@ -48,9 +49,9 @@ func NewHasherPBKDF2(c PBKDF2Configurator) *PBKDF2 {
4849
}
4950

5051
// Generate generates a hash for the given password.
51-
func (h *PBKDF2) Generate(ctx context.Context, password []byte) ([]byte, error) {
52-
_, span := otel.GetTracerProvider().Tracer("").Start(ctx, "hash.PBKDF2.Generate")
53-
defer span.End()
52+
func (h *PBKDF2) Generate(ctx context.Context, password []byte) (hash []byte, err error) {
53+
ctx, span := otel.GetTracerProvider().Tracer("").Start(ctx, "hash.PBKDF2.Generate")
54+
defer otelx.End(span, &err)
5455

5556
conf := h.c.HasherPBKDF2Config(ctx)
5657
salt := make([]byte, conf.SaltLength)

oryx/otelx/middleware.go

Lines changed: 49 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -4,48 +4,68 @@
44
package otelx
55

66
import (
7+
"cmp"
8+
"context"
79
"net/http"
810
"strings"
911

1012
"go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp"
1113
)
1214

13-
func isHealthFilter(r *http.Request) bool {
14-
path := r.URL.Path
15-
return !strings.HasPrefix(path, "/health/")
16-
}
15+
var WithDefaultFilters otelhttp.Option = otelhttp.WithFilter(func(r *http.Request) bool {
16+
return !(strings.HasPrefix(r.URL.Path, "/health") ||
17+
strings.HasPrefix(r.URL.Path, "/admin/health") ||
18+
strings.HasPrefix(r.URL.Path, "/metrics") ||
19+
strings.HasPrefix(r.URL.Path, "/admin/metrics"))
20+
})
1721

18-
func isAdminHealthFilter(r *http.Request) bool {
19-
path := r.URL.Path
20-
return !strings.HasPrefix(path, "/admin/health/")
21-
}
22+
type contextKey int
2223

23-
func filterOpts() []otelhttp.Option {
24-
filters := []otelhttp.Filter{
25-
isHealthFilter,
26-
isAdminHealthFilter,
27-
}
28-
opts := []otelhttp.Option{}
29-
for _, f := range filters {
30-
opts = append(opts, otelhttp.WithFilter(f))
31-
}
32-
return opts
24+
const callbackContextKey contextKey = iota
25+
26+
func SpanNameRecorderMiddleware(next http.Handler) http.Handler {
27+
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
28+
defer func() {
29+
cb, _ := r.Context().Value(callbackContextKey).(func(string))
30+
if cb == nil {
31+
return
32+
}
33+
if r.Pattern != "" {
34+
cb(r.Pattern)
35+
}
36+
}()
37+
next.ServeHTTP(w, r)
38+
})
3339
}
3440

35-
// NewHandler returns a wrapped otelhttp.NewHandler with our request filters.
36-
func NewHandler(handler http.Handler, operation string, opts ...otelhttp.Option) http.Handler {
37-
opts = append(filterOpts(), opts...)
38-
return otelhttp.NewHandler(handler, operation, opts...)
41+
func SpanNameRecorderNegroniFunc(w http.ResponseWriter, r *http.Request, next http.HandlerFunc) {
42+
defer func() {
43+
cb, _ := r.Context().Value(callbackContextKey).(func(string))
44+
if cb == nil {
45+
return
46+
}
47+
if r.Pattern != "" {
48+
cb(r.Pattern)
49+
}
50+
}()
51+
next(w, r)
3952
}
4053

41-
// TraceHandler wraps otelx.NewHandler, passing the URL path as the span name.
42-
func TraceHandler(h http.Handler, opts ...otelhttp.Option) http.Handler {
43-
// Use a span formatter to set the span name to the URL path, rather than passing in the operation to NewHandler.
44-
// This allows us to use the same handler for multiple routes.
45-
middlewareOpts := []otelhttp.Option{
54+
func NewMiddleware(next http.Handler, operation string, opts ...otelhttp.Option) http.Handler {
55+
myOpts := []otelhttp.Option{
56+
WithDefaultFilters,
4657
otelhttp.WithSpanNameFormatter(func(operation string, r *http.Request) string {
47-
return r.URL.Path
58+
return cmp.Or(r.Pattern, operation, r.Method+" "+r.URL.Path)
4859
}),
4960
}
50-
return NewHandler(h, "", append(middlewareOpts, opts...)...)
61+
handler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
62+
callback := func(s string) {
63+
r.Pattern = cmp.Or(r.Pattern, s)
64+
}
65+
ctx := context.WithValue(r.Context(), callbackContextKey, callback)
66+
r2 := r.WithContext(ctx)
67+
next.ServeHTTP(w, r2)
68+
r.Pattern = cmp.Or(r2.Pattern, r.Pattern) // best-effort in case callback never is called
69+
})
70+
return otelhttp.NewHandler(handler, operation, append(myOpts, opts...)...)
5171
}

0 commit comments

Comments
 (0)