From 86db525cec8f2cc88ac313aff0da536d15bbf9bb Mon Sep 17 00:00:00 2001 From: Ralf Haferkamp Date: Mon, 1 Sep 2025 17:14:35 +0200 Subject: [PATCH] feat(tracing): Improve tracing for proxy middlewares Each middleware adds a new span with a useful name now. --- services/proxy/pkg/command/server.go | 4 ++++ services/proxy/pkg/middleware/account_resolver.go | 12 +++++++++--- services/proxy/pkg/middleware/authentication.go | 11 ++++++++--- services/proxy/pkg/middleware/create_home.go | 13 +++++++++++-- services/proxy/pkg/middleware/policies.go | 9 ++++++++- services/proxy/pkg/middleware/selector_cookie.go | 10 ++++++++++ 6 files changed, 50 insertions(+), 9 deletions(-) diff --git a/services/proxy/pkg/command/server.go b/services/proxy/pkg/command/server.go index 6a86288c5..ca0d75998 100644 --- a/services/proxy/pkg/command/server.go +++ b/services/proxy/pkg/command/server.go @@ -370,6 +370,7 @@ func loadMiddlewares(logger log.Logger, cfg *config.Config, ), middleware.AccountResolver( middleware.Logger(logger), + middleware.TraceProvider(traceProvider), middleware.UserProvider(userProvider), middleware.UserRoleAssigner(roleAssigner), middleware.SkipUserInfo(cfg.OIDC.SkipUserInfo), @@ -380,17 +381,20 @@ func loadMiddlewares(logger log.Logger, cfg *config.Config, ), middleware.SelectorCookie( middleware.Logger(logger), + middleware.TraceProvider(traceProvider), middleware.PolicySelectorConfig(*cfg.PolicySelector), ), middleware.Policies( cfg.PoliciesMiddleware.Query, middleware.Logger(logger), + middleware.TraceProvider(traceProvider), middleware.WithRevaGatewaySelector(gatewaySelector), middleware.PoliciesProviderService(policiesProviderClient), ), // finally, trigger home creation when a user logs in middleware.CreateHome( middleware.Logger(logger), + middleware.TraceProvider(traceProvider), middleware.WithRevaGatewaySelector(gatewaySelector), middleware.RoleQuotas(cfg.RoleQuotas), ), diff --git a/services/proxy/pkg/middleware/account_resolver.go b/services/proxy/pkg/middleware/account_resolver.go index 5b3a3be95..90f730ecb 100644 --- a/services/proxy/pkg/middleware/account_resolver.go +++ b/services/proxy/pkg/middleware/account_resolver.go @@ -10,6 +10,7 @@ import ( "github.com/opencloud-eu/opencloud/services/proxy/pkg/router" "github.com/opencloud-eu/opencloud/services/proxy/pkg/user/backend" "github.com/opencloud-eu/opencloud/services/proxy/pkg/userroles" + "go.opentelemetry.io/otel/trace" cs3user "github.com/cs3org/go-cs3apis/cs3/identity/user/v1beta1" "github.com/opencloud-eu/opencloud/pkg/log" @@ -24,6 +25,7 @@ import ( func AccountResolver(optionSetters ...Option) func(next http.Handler) http.Handler { options := newOptions(optionSetters...) logger := options.Logger + tracer := getTraceProvider(options).Tracer("proxy.middleware.account_resolver") lastGroupSyncCache := ttlcache.New( ttlcache.WithTTL[string, struct{}](5*time.Minute), @@ -35,6 +37,7 @@ func AccountResolver(optionSetters ...Option) func(next http.Handler) http.Handl return &accountResolver{ next: next, logger: logger, + tracer: tracer, userProvider: options.UserProvider, userOIDCClaim: options.UserOIDCClaim, userCS3Claim: options.UserCS3Claim, @@ -49,6 +52,7 @@ func AccountResolver(optionSetters ...Option) func(next http.Handler) http.Handl type accountResolver struct { next http.Handler logger log.Logger + tracer trace.Tracer userProvider backend.UserBackend userRoleAssigner userroles.UserRoleAssigner autoProvisionAccounts bool @@ -98,12 +102,14 @@ func readUserIDClaim(path string, claims map[string]interface{}) (string, error) // TODO do not use the context to store values: https://medium.com/@cep21/how-to-correctly-use-context-context-in-go-1-7-8f2c0fafdf39 func (m accountResolver) ServeHTTP(w http.ResponseWriter, req *http.Request) { - ctx := req.Context() + ctx, span := m.tracer.Start(req.Context(), fmt.Sprintf("%s %s", req.Method, req.URL.Path), trace.WithSpanKind(trace.SpanKindServer)) claims := oidc.FromContext(ctx) user, ok := revactx.ContextGetUser(ctx) token, hasToken := revactx.ContextGetToken(ctx) - + req = req.WithContext(ctx) + defer span.End() if claims == nil && !ok { + span.End() m.next.ServeHTTP(w, req) return } @@ -219,6 +225,6 @@ func (m accountResolver) ServeHTTP(w http.ResponseWriter, req *http.Request) { if !ri.SkipXAccessToken() { req.Header.Set(revactx.TokenHeader, token) } - + span.End() m.next.ServeHTTP(w, req) } diff --git a/services/proxy/pkg/middleware/authentication.go b/services/proxy/pkg/middleware/authentication.go index 61c69323f..7703fdf62 100644 --- a/services/proxy/pkg/middleware/authentication.go +++ b/services/proxy/pkg/middleware/authentication.go @@ -9,6 +9,7 @@ import ( "github.com/opencloud-eu/opencloud/services/proxy/pkg/router" "github.com/opencloud-eu/opencloud/services/proxy/pkg/webdav" + "go.opentelemetry.io/otel/attribute" "go.opentelemetry.io/otel/trace" "golang.org/x/text/cases" "golang.org/x/text/language" @@ -52,7 +53,7 @@ type Authenticator interface { func Authentication(auths []Authenticator, opts ...Option) func(next http.Handler) http.Handler { options := newOptions(opts...) configureSupportedChallenges(options) - tracer := getTraceProvider(options).Tracer("proxy") + tracer := getTraceProvider(options).Tracer("proxy.middleware.authentication") spanOpts := []trace.SpanStartOption{ trace.WithSpanKind(trace.SpanKindServer), @@ -60,24 +61,28 @@ func Authentication(auths []Authenticator, opts ...Option) func(next http.Handle return func(next http.Handler) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - ctx, span := tracer.Start(r.Context(), fmt.Sprintf("%s %v", r.Method, r.URL.Path), spanOpts...) - defer span.End() + ctx, span := tracer.Start(r.Context(), fmt.Sprintf("%s %s", r.Method, r.URL.Path), spanOpts...) r = r.WithContext(ctx) + defer span.End() ri := router.ContextRoutingInfo(ctx) if isOIDCTokenAuth(r) || ri.IsRouteUnprotected() || r.Method == "OPTIONS" { // Either this is a request that does not need any authentication or // the authentication for this request is handled by the IdP. + span.SetAttributes(attribute.Bool("routeunprotected", true)) + span.End() next.ServeHTTP(w, r) return } for _, a := range auths { if req, ok := a.Authenticate(r); ok { + span.End() next.ServeHTTP(w, req) return } } + if !isPublicPath(r.URL.Path) { // Failed basic authentication attempts receive the Www-Authenticate header in the response var touch bool diff --git a/services/proxy/pkg/middleware/create_home.go b/services/proxy/pkg/middleware/create_home.go index b24904aed..bb329ab75 100644 --- a/services/proxy/pkg/middleware/create_home.go +++ b/services/proxy/pkg/middleware/create_home.go @@ -1,6 +1,7 @@ package middleware import ( + "fmt" "net/http" "strconv" @@ -14,6 +15,7 @@ import ( "github.com/opencloud-eu/reva/v2/pkg/rgrpc/status" "github.com/opencloud-eu/reva/v2/pkg/rgrpc/todo/pool" "github.com/opencloud-eu/reva/v2/pkg/utils" + "go.opentelemetry.io/otel/trace" "google.golang.org/grpc/metadata" ) @@ -21,11 +23,13 @@ import ( func CreateHome(optionSetters ...Option) func(next http.Handler) http.Handler { options := newOptions(optionSetters...) logger := options.Logger + tracer := getTraceProvider(options).Tracer("proxy.middleware.create_home") return func(next http.Handler) http.Handler { return &createHome{ next: next, logger: logger, + tracer: tracer, revaGatewaySelector: options.RevaGatewaySelector, roleQuotas: options.RoleQuotas, } @@ -35,12 +39,17 @@ func CreateHome(optionSetters ...Option) func(next http.Handler) http.Handler { type createHome struct { next http.Handler logger log.Logger + tracer trace.Tracer revaGatewaySelector pool.Selectable[gateway.GatewayAPIClient] roleQuotas map[string]uint64 } func (m createHome) ServeHTTP(w http.ResponseWriter, req *http.Request) { + ctx, span := m.tracer.Start(req.Context(), fmt.Sprintf("%s %s", req.Method, req.URL.Path), trace.WithSpanKind(trace.SpanKindServer)) + req = req.WithContext(ctx) + defer span.End() if !m.shouldServe(req) { + span.End() m.next.ServeHTTP(w, req) return } @@ -49,7 +58,7 @@ func (m createHome) ServeHTTP(w http.ResponseWriter, req *http.Request) { // we need to pass the token to authenticate the CreateHome request. //ctx := tokenpkg.ContextSetToken(r.Context(), token) - ctx := metadata.AppendToOutgoingContext(req.Context(), revactx.TokenHeader, token) + ctx = metadata.AppendToOutgoingContext(req.Context(), revactx.TokenHeader, token) createHomeReq := &provider.CreateHomeRequest{} u, ok := revactx.ContextGetUser(ctx) @@ -79,7 +88,7 @@ func (m createHome) ServeHTTP(w http.ResponseWriter, req *http.Request) { } } } - + span.End() m.next.ServeHTTP(w, req) } diff --git a/services/proxy/pkg/middleware/policies.go b/services/proxy/pkg/middleware/policies.go index 36d8bc062..4a3c39f95 100644 --- a/services/proxy/pkg/middleware/policies.go +++ b/services/proxy/pkg/middleware/policies.go @@ -1,6 +1,7 @@ package middleware import ( + "fmt" "net/http" "path" "path/filepath" @@ -11,6 +12,7 @@ import ( "github.com/go-chi/chi/v5/middleware" "github.com/go-chi/render" tusd "github.com/tus/tusd/v2/pkg/handler" + "go.opentelemetry.io/otel/trace" "google.golang.org/grpc/metadata" revactx "github.com/opencloud-eu/reva/v2/pkg/ctx" @@ -43,12 +45,17 @@ const DeniedMessage = "Operation denied due to security policies" func Policies(qs string, opts ...Option) func(next http.Handler) http.Handler { options := newOptions(opts...) logger := options.Logger + tracer := getTraceProvider(options).Tracer("proxy.middleware.policies") gatewaySelector := options.RevaGatewaySelector policiesProviderClient := options.PoliciesProviderService return func(next http.Handler) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + ctx, span := tracer.Start(r.Context(), fmt.Sprintf("%s %s", r.Method, r.URL.Path), trace.WithSpanKind(trace.SpanKindServer)) + r = r.WithContext(ctx) + defer span.End() if qs == "" { + span.End() next.ServeHTTP(w, r) return } @@ -134,7 +141,7 @@ func Policies(qs string, opts ...Option) func(next http.Handler) http.Handler { RenderError(w, r, req, http.StatusForbidden, DeniedMessage) return } - + span.End() next.ServeHTTP(w, r) }) } diff --git a/services/proxy/pkg/middleware/selector_cookie.go b/services/proxy/pkg/middleware/selector_cookie.go index d5117e64f..a62751fbb 100644 --- a/services/proxy/pkg/middleware/selector_cookie.go +++ b/services/proxy/pkg/middleware/selector_cookie.go @@ -1,12 +1,14 @@ package middleware import ( + "fmt" "net/http" "github.com/opencloud-eu/opencloud/pkg/log" "github.com/opencloud-eu/opencloud/pkg/oidc" "github.com/opencloud-eu/opencloud/services/proxy/pkg/config" "github.com/opencloud-eu/opencloud/services/proxy/pkg/proxy/policy" + "go.opentelemetry.io/otel/trace" ) // SelectorCookie provides a middleware which @@ -14,11 +16,13 @@ func SelectorCookie(optionSetters ...Option) func(next http.Handler) http.Handle options := newOptions(optionSetters...) logger := options.Logger policySelector := options.PolicySelector + tracer := getTraceProvider(options).Tracer("proxy.middleware.selector_cookie") return func(next http.Handler) http.Handler { return &selectorCookie{ next: next, logger: logger, + tracer: tracer, policySelector: policySelector, } } @@ -27,12 +31,17 @@ func SelectorCookie(optionSetters ...Option) func(next http.Handler) http.Handle type selectorCookie struct { next http.Handler logger log.Logger + tracer trace.Tracer policySelector config.PolicySelector } func (m selectorCookie) ServeHTTP(w http.ResponseWriter, req *http.Request) { + ctx, span := m.tracer.Start(req.Context(), fmt.Sprintf("%s %s", req.Method, req.URL.Path), trace.WithSpanKind(trace.SpanKindServer)) + req = req.WithContext(ctx) + defer span.End() if m.policySelector.Regex == nil && m.policySelector.Claims == nil { // only set selector cookie for regex and claim selectors + span.End() m.next.ServeHTTP(w, req) return } @@ -65,5 +74,6 @@ func (m selectorCookie) ServeHTTP(w http.ResponseWriter, req *http.Request) { http.SetCookie(w, &cookie) } + defer span.End() m.next.ServeHTTP(w, req) }