From df8d5ee92b07dced6d0c13355fc21ff9b15aabbb Mon Sep 17 00:00:00 2001 From: Ralf Haferkamp Date: Wed, 15 Jun 2022 12:34:30 +0200 Subject: [PATCH] Attach TracerProvider to context To generate more useful tracing (especially when running multiple services in a single process) we'd like to generate Tracers from the per service TracerProvider() instead of the global default provider. Therefore we now pass the TracerProvider via context (similar to what is already done for the logger. --- internal/grpc/interceptors/appctx/appctx.go | 1 + .../services/gateway/usershareprovider.go | 4 +--- .../publicstorageprovider.go | 9 ++++---- internal/http/interceptors/appctx/appctx.go | 1 + internal/http/services/owncloud/ocdav/meta.go | 5 ++-- .../owncloud/ocdav/propfind/propfind.go | 7 +++--- pkg/appctx/appctx.go | 14 +++++++++++ .../utils/decomposedfs/decomposedfs.go | 5 ++-- pkg/trace/trace.go | 23 +++++++++++++++++++ 9 files changed, 51 insertions(+), 18 deletions(-) diff --git a/internal/grpc/interceptors/appctx/appctx.go b/internal/grpc/interceptors/appctx/appctx.go index 8718c3dc3a..705edd7ce5 100644 --- a/internal/grpc/interceptors/appctx/appctx.go +++ b/internal/grpc/interceptors/appctx/appctx.go @@ -47,6 +47,7 @@ func NewUnary(log zerolog.Logger, tp trace.TracerProvider) grpc.UnaryServerInter sub := log.With().Str("traceid", span.SpanContext().TraceID().String()).Logger() ctx = appctx.WithLogger(ctx, &sub) + ctx = appctx.WithTracerProvider(ctx, tp) res, err := handler(ctx, req) return res, err } diff --git a/internal/grpc/services/gateway/usershareprovider.go b/internal/grpc/services/gateway/usershareprovider.go index b375dfd2be..fff53cceb7 100644 --- a/internal/grpc/services/gateway/usershareprovider.go +++ b/internal/grpc/services/gateway/usershareprovider.go @@ -33,7 +33,6 @@ import ( "github.com/cs3org/reva/v2/pkg/rgrpc/todo/pool" "github.com/cs3org/reva/v2/pkg/storage/utils/grants" "github.com/cs3org/reva/v2/pkg/storagespace" - rtrace "github.com/cs3org/reva/v2/pkg/trace" "github.com/cs3org/reva/v2/pkg/utils" "github.com/pkg/errors" ) @@ -190,8 +189,7 @@ func (s *svc) GetReceivedShare(ctx context.Context, req *collaboration.GetReceiv // 1) if received share is mounted: we also do a rename in the storage // 2) if received share is not mounted: we only rename in user share provider. func (s *svc) UpdateReceivedShare(ctx context.Context, req *collaboration.UpdateReceivedShareRequest) (*collaboration.UpdateReceivedShareResponse, error) { - t := rtrace.DefaultProvider().Tracer("reva") - ctx, span := t.Start(ctx, "Gateway.UpdateReceivedShare") + ctx, span := appctx.GetTracerProvider(ctx).Tracer("gateway").Start(ctx, "Gateway.UpdateReceivedShare") defer span.End() // sanity checks diff --git a/internal/grpc/services/publicstorageprovider/publicstorageprovider.go b/internal/grpc/services/publicstorageprovider/publicstorageprovider.go index 41098649be..6c36bb64ac 100644 --- a/internal/grpc/services/publicstorageprovider/publicstorageprovider.go +++ b/internal/grpc/services/publicstorageprovider/publicstorageprovider.go @@ -38,7 +38,6 @@ import ( "github.com/cs3org/reva/v2/pkg/rgrpc/status" "github.com/cs3org/reva/v2/pkg/rgrpc/todo/pool" "github.com/cs3org/reva/v2/pkg/storagespace" - rtrace "github.com/cs3org/reva/v2/pkg/trace" "github.com/cs3org/reva/v2/pkg/utils" "github.com/mitchellh/mapstructure" "github.com/pkg/errors" @@ -555,7 +554,7 @@ func (s *service) CreateContainer(ctx context.Context, req *provider.CreateConta _, req.Ref.ResourceId.StorageId = storagespace.SplitStorageID(req.Ref.ResourceId.StorageId) } - ctx, span := rtrace.DefaultProvider().Tracer(tracerName).Start(ctx, "CreateContainer") + ctx, span := appctx.GetTracerProvider(ctx).Tracer(tracerName).Start(ctx, "CreateContainer") defer span.End() span.SetAttributes(attribute.KeyValue{ @@ -616,7 +615,7 @@ func (s *service) Delete(ctx context.Context, req *provider.DeleteRequest) (*pro _, req.Ref.ResourceId.StorageId = storagespace.SplitStorageID(req.Ref.ResourceId.StorageId) } - ctx, span := rtrace.DefaultProvider().Tracer(tracerName).Start(ctx, "Delete") + ctx, span := appctx.GetTracerProvider(ctx).Tracer(tracerName).Start(ctx, "Delete") defer span.End() span.SetAttributes(attribute.KeyValue{ @@ -663,7 +662,7 @@ func (s *service) Move(ctx context.Context, req *provider.MoveRequest) (*provide _, req.Destination.ResourceId.StorageId = storagespace.SplitStorageID(req.Destination.ResourceId.StorageId) } - ctx, span := rtrace.DefaultProvider().Tracer(tracerName).Start(ctx, "Move") + ctx, span := appctx.GetTracerProvider(ctx).Tracer(tracerName).Start(ctx, "Move") defer span.End() span.SetAttributes( @@ -730,7 +729,7 @@ func (s *service) Stat(ctx context.Context, req *provider.StatRequest) (*provide _, req.Ref.ResourceId.StorageId = storagespace.SplitStorageID(req.Ref.ResourceId.StorageId) } - ctx, span := rtrace.DefaultProvider().Tracer(tracerName).Start(ctx, "Stat") + ctx, span := appctx.GetTracerProvider(ctx).Tracer(tracerName).Start(ctx, "Stat") defer span.End() span.SetAttributes( diff --git a/internal/http/interceptors/appctx/appctx.go b/internal/http/interceptors/appctx/appctx.go index 35c92bbd9d..b86caa8c1d 100644 --- a/internal/http/interceptors/appctx/appctx.go +++ b/internal/http/interceptors/appctx/appctx.go @@ -53,6 +53,7 @@ func handler(log zerolog.Logger, tp trace.TracerProvider, h http.Handler) http.H sub := log.With().Str("traceid", span.SpanContext().TraceID().String()).Logger() ctx = appctx.WithLogger(ctx, &sub) + ctx = appctx.WithTracerProvider(ctx, tp) r = r.WithContext(ctx) h.ServeHTTP(w, r) }) diff --git a/internal/http/services/owncloud/ocdav/meta.go b/internal/http/services/owncloud/ocdav/meta.go index da99a15de8..f67f1f5049 100644 --- a/internal/http/services/owncloud/ocdav/meta.go +++ b/internal/http/services/owncloud/ocdav/meta.go @@ -33,7 +33,6 @@ import ( "github.com/cs3org/reva/v2/pkg/appctx" "github.com/cs3org/reva/v2/pkg/rhttp/router" "github.com/cs3org/reva/v2/pkg/storagespace" - rtrace "github.com/cs3org/reva/v2/pkg/trace" ) // MetaHandler handles meta requests @@ -91,7 +90,7 @@ func (h *MetaHandler) Handler(s *svc) http.Handler { } func (h *MetaHandler) handlePathForUser(w http.ResponseWriter, r *http.Request, s *svc, rid *provider.ResourceId) { - ctx, span := rtrace.DefaultProvider().Tracer(tracerName).Start(r.Context(), "meta_propfind") + ctx, span := appctx.GetTracerProvider(r.Context()).Tracer(tracerName).Start(r.Context(), "meta_propfind") defer span.End() id := storagespace.FormatResourceID(*rid) @@ -178,7 +177,7 @@ func (h *MetaHandler) handlePathForUser(w http.ResponseWriter, r *http.Request, } func (h *MetaHandler) handleEmptyID(w http.ResponseWriter, r *http.Request) { - ctx, span := rtrace.DefaultProvider().Tracer(tracerName).Start(r.Context(), "meta_propfind") + ctx, span := appctx.GetTracerProvider(r.Context()).Tracer(tracerName).Start(r.Context(), "meta_propfind") defer span.End() sublog := appctx.GetLogger(ctx).With().Str("path", r.URL.Path).Logger() diff --git a/internal/http/services/owncloud/ocdav/propfind/propfind.go b/internal/http/services/owncloud/ocdav/propfind/propfind.go index 0ecf244b31..78dc3f8c32 100644 --- a/internal/http/services/owncloud/ocdav/propfind/propfind.go +++ b/internal/http/services/owncloud/ocdav/propfind/propfind.go @@ -47,7 +47,6 @@ import ( "github.com/cs3org/reva/v2/pkg/publicshare" "github.com/cs3org/reva/v2/pkg/rhttp/router" "github.com/cs3org/reva/v2/pkg/storagespace" - rtrace "github.com/cs3org/reva/v2/pkg/trace" "github.com/cs3org/reva/v2/pkg/utils" "github.com/rs/zerolog" "go.opentelemetry.io/otel/codes" @@ -179,7 +178,7 @@ func NewHandler(publicURL string, getClientFunc GetGatewayServiceClientFunc) *Ha // HandlePathPropfind handles a path based propfind request // ns is the namespace that is prefixed to the path in the cs3 namespace func (p *Handler) HandlePathPropfind(w http.ResponseWriter, r *http.Request, ns string) { - ctx, span := rtrace.DefaultProvider().Tracer(tracerName).Start(r.Context(), fmt.Sprintf("%s %v", r.Method, r.URL.Path)) + ctx, span := appctx.GetTracerProvider(r.Context()).Tracer(tracerName).Start(r.Context(), fmt.Sprintf("%s %v", r.Method, r.URL.Path)) defer span.End() fn := path.Join(ns, r.URL.Path) // TODO do we still need to jail if we query the registry about the spaces? @@ -238,7 +237,7 @@ func (p *Handler) HandlePathPropfind(w http.ResponseWriter, r *http.Request, ns // HandleSpacesPropfind handles a spaces based propfind request func (p *Handler) HandleSpacesPropfind(w http.ResponseWriter, r *http.Request, spaceID string) { - ctx, span := rtrace.DefaultProvider().Tracer(tracerName).Start(r.Context(), "spaces_propfind") + ctx, span := appctx.GetTracerProvider(r.Context()).Tracer(tracerName).Start(r.Context(), "spaces_propfind") defer span.End() sublog := appctx.GetLogger(ctx).With().Str("path", r.URL.Path).Str("spaceid", spaceID).Logger() @@ -285,7 +284,7 @@ func (p *Handler) HandleSpacesPropfind(w http.ResponseWriter, r *http.Request, s } func (p *Handler) propfindResponse(ctx context.Context, w http.ResponseWriter, r *http.Request, namespace, spaceType string, pf XML, sendTusHeaders bool, resourceInfos []*provider.ResourceInfo, log zerolog.Logger) { - ctx, span := rtrace.DefaultProvider().Tracer(tracerName).Start(ctx, "propfind_response") + ctx, span := appctx.GetTracerProvider(r.Context()).Tracer(tracerName).Start(ctx, "propfind_response") defer span.End() filters := make([]*link.ListPublicSharesRequest_Filter, 0, len(resourceInfos)) diff --git a/pkg/appctx/appctx.go b/pkg/appctx/appctx.go index 8c63c3ddae..c8a56537a2 100644 --- a/pkg/appctx/appctx.go +++ b/pkg/appctx/appctx.go @@ -21,7 +21,9 @@ package appctx import ( "context" + rtrace "github.com/cs3org/reva/v2/pkg/trace" "github.com/rs/zerolog" + "go.opentelemetry.io/otel/trace" ) // DeletingSharedResource flags to a storage a shared resource is being deleted not by the owner. @@ -37,3 +39,15 @@ func WithLogger(ctx context.Context, l *zerolog.Logger) context.Context { func GetLogger(ctx context.Context) *zerolog.Logger { return zerolog.Ctx(ctx) } + +// WithTracerProvider returns a context with an associated TracerProvider +func WithTracerProvider(ctx context.Context, p trace.TracerProvider) context.Context { + return rtrace.ContextSetTracerProvider(ctx, p) +} + +// GetTracerProvider returns the TracerProvider associated with +// the given context. (Or the global default TracerProvider if there +// is no TracerProvider in the context) +func GetTracerProvider(ctx context.Context) trace.TracerProvider { + return rtrace.ContextGetTracerProvider(ctx) +} diff --git a/pkg/storage/utils/decomposedfs/decomposedfs.go b/pkg/storage/utils/decomposedfs/decomposedfs.go index a20a702c0c..bb5d8d5856 100644 --- a/pkg/storage/utils/decomposedfs/decomposedfs.go +++ b/pkg/storage/utils/decomposedfs/decomposedfs.go @@ -50,7 +50,6 @@ import ( "github.com/cs3org/reva/v2/pkg/storage/utils/decomposedfs/tree" "github.com/cs3org/reva/v2/pkg/storage/utils/decomposedfs/xattrs" "github.com/cs3org/reva/v2/pkg/storage/utils/templates" - rtrace "github.com/cs3org/reva/v2/pkg/trace" "github.com/cs3org/reva/v2/pkg/utils" "github.com/pkg/errors" "go.opentelemetry.io/otel/codes" @@ -347,7 +346,7 @@ func (fs *Decomposedfs) TouchFile(ctx context.Context, ref *provider.Reference) // To mimic the eos and owncloud driver we only allow references as children of the "/Shares" folder // FIXME: This comment should explain briefly what a reference is in this context. func (fs *Decomposedfs) CreateReference(ctx context.Context, p string, targetURI *url.URL) (err error) { - ctx, span := rtrace.DefaultProvider().Tracer("reva").Start(ctx, "CreateReference") + ctx, span := appctx.GetTracerProvider(ctx).Tracer("reva").Start(ctx, "CreateReference") defer span.End() p = strings.Trim(p, "/") @@ -496,7 +495,7 @@ func (fs *Decomposedfs) ListFolder(ctx context.Context, ref *provider.Reference, return } - ctx, span := rtrace.DefaultProvider().Tracer(tracerName).Start(ctx, "ListFolder") + ctx, span := appctx.GetTracerProvider(ctx).Tracer(tracerName).Start(ctx, "ListFolder") defer span.End() if !n.Exists { diff --git a/pkg/trace/trace.go b/pkg/trace/trace.go index e04e6767ae..9aab17679d 100644 --- a/pkg/trace/trace.go +++ b/pkg/trace/trace.go @@ -19,6 +19,7 @@ package trace import ( + "context" "fmt" "net/url" "os" @@ -47,6 +48,28 @@ type revaDefaultTracerProvider struct { provider trace.TracerProvider } +type ctxKey struct{} + +// ContextSetTracerProvider returns a copy of ctx with p associated. +func ContextSetTracerProvider(ctx context.Context, p trace.TracerProvider) context.Context { + if tp, ok := ctx.Value(ctxKey{}).(trace.TracerProvider); ok { + if tp == p { + return ctx + } + } + return context.WithValue(ctx, ctxKey{}, p) +} + +// ContextGetTracerProvider returns the TracerProvider associated with the ctx. +// If no TracerProvider is associated is associated, the global default TracerProvider +// is returned +func ContextGetTracerProvider(ctx context.Context) trace.TracerProvider { + if p, ok := ctx.Value(ctxKey{}).(trace.TracerProvider); ok { + return p + } + return DefaultProvider() +} + // InitDefaultTracerProvider initializes a global default TracerProvider at a package level. func InitDefaultTracerProvider(collectorEndpoint string, agentEndpoint string) { defaultProvider.mutex.Lock()