diff --git a/changelog/unreleased/collaboration-improved-tracing.md b/changelog/unreleased/collaboration-improved-tracing.md new file mode 100644 index 00000000000..fe921b620d1 --- /dev/null +++ b/changelog/unreleased/collaboration-improved-tracing.md @@ -0,0 +1,6 @@ +Enhancement: Tracing improvements in the collaboration service + +Uploads and downloads through the collaboration service will be traced. The openInApp request will also be linked properly with other requests in the tracing. +In addition, the collaboration service will include some additional information in the traces. Filtering based on that information might be an option. + +https://github.com/owncloud/ocis/pull/9684 diff --git a/services/collaboration/pkg/command/server.go b/services/collaboration/pkg/command/server.go index 799448d4cf3..dff0bb4b21c 100644 --- a/services/collaboration/pkg/command/server.go +++ b/services/collaboration/pkg/command/server.go @@ -63,6 +63,7 @@ func Server(cfg *config.Config) *cli.Command { grpc.AppURLs(appUrls), grpc.Config(cfg), grpc.Logger(logger), + grpc.TraceProvider(traceProvider), ) defer teardown() if err != nil { diff --git a/services/collaboration/pkg/connector/contentconnector.go b/services/collaboration/pkg/connector/contentconnector.go index ee7cad2d0c9..d357af05ee3 100644 --- a/services/collaboration/pkg/connector/contentconnector.go +++ b/services/collaboration/pkg/connector/contentconnector.go @@ -15,9 +15,11 @@ import ( providerv1beta1 "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1" types "github.com/cs3org/go-cs3apis/cs3/types/v1beta1" revactx "github.com/cs3org/reva/v2/pkg/ctx" + "github.com/owncloud/ocis/v2/ocis-pkg/tracing" "github.com/owncloud/ocis/v2/services/collaboration/pkg/config" "github.com/owncloud/ocis/v2/services/collaboration/pkg/middleware" "github.com/rs/zerolog" + "go.opentelemetry.io/otel/propagation" ) // ContentConnectorService is the interface to implement the "File contents" @@ -143,6 +145,8 @@ func (c *ContentConnector) GetFile(ctx context.Context, writer io.Writer) error } else { httpReq.Header.Add("X-Access-Token", wopiContext.AccessToken) } + tracingProp := tracing.GetPropagator() + tracingProp.Inject(ctx, propagation.HeaderCarrier(httpReq.Header)) httpResp, err := httpClient.Do(httpReq) if err != nil { @@ -341,6 +345,8 @@ func (c *ContentConnector) PutFile(ctx context.Context, stream io.Reader, stream //if lockID, ok := ctxpkg.ContextGetLockID(ctx); ok { // httpReq.Header.Add("X-Lock-Id", lockID) //} + tracingProp := tracing.GetPropagator() + tracingProp.Inject(ctx, propagation.HeaderCarrier(httpReq.Header)) httpResp, err := httpClient.Do(httpReq) if err != nil { diff --git a/services/collaboration/pkg/middleware/proofkeys.go b/services/collaboration/pkg/middleware/proofkeys.go index 064e541318d..c3b96835e01 100644 --- a/services/collaboration/pkg/middleware/proofkeys.go +++ b/services/collaboration/pkg/middleware/proofkeys.go @@ -10,6 +10,16 @@ import ( "github.com/rs/zerolog" ) +// ProofKeysMiddleware will verify the proof keys of the requests. +// This is a middleware that could be disabled / not set. +// +// Requests will fail with a 500 HTTP status if the verification fails. +// As said, this can be disabled (via configuration) if you want to skip +// the verification. +// The middleware requires hitting the "/hosting/discovery" endpoint of the +// WOPI app in order to get the keys. The keys will be cached in memory for +// 12 hours (or the configured value) before hitting the endpoint again to +// request new / updated keys. func ProofKeysMiddleware(cfg *config.Config, next http.Handler) http.Handler { wopiDiscovery := cfg.App.Addr + "/hosting/discovery" insecure := cfg.App.Insecure diff --git a/services/collaboration/pkg/middleware/tracing.go b/services/collaboration/pkg/middleware/tracing.go new file mode 100644 index 00000000000..816f81e129b --- /dev/null +++ b/services/collaboration/pkg/middleware/tracing.go @@ -0,0 +1,47 @@ +package middleware + +import ( + "net/http" + + "go.opentelemetry.io/otel/attribute" + "go.opentelemetry.io/otel/trace" +) + +// CollaborationTracingMiddleware adds a new middleware in order to include +// more attributes in the traced span. +// +// In order not to mess with the expected responses, this middleware won't do +// anything if there is no available WOPI context set in the request (there is +// nothing to report). This means that the WopiContextAuthMiddleware should be +// set before this middleware. +func CollaborationTracingMiddleware(next http.Handler) http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + wopiContext, err := WopiContextFromCtx(r.Context()) + if err != nil { + // if we can't get the context, skip this middleware + next.ServeHTTP(w, r) + } + + span := trace.SpanFromContext(r.Context()) + + wopiMethod := r.Header.Get("X-WOPI-Override") + + wopiFile := wopiContext.FileReference + wopiUser := wopiContext.User.GetId() + + attrs := []attribute.KeyValue{ + attribute.String("ocis.wopi.sessionid", r.Header.Get("X-WOPI-SessionId")), + attribute.String("ocis.wopi.method", wopiMethod), + attribute.String("ocis.wopi.resource.id.storage", wopiFile.GetResourceId().GetStorageId()), + attribute.String("ocis.wopi.resource.id.opaque", wopiFile.GetResourceId().GetOpaqueId()), + attribute.String("ocis.wopi.resource.id.space", wopiFile.GetResourceId().GetSpaceId()), + attribute.String("ocis.wopi.resource.path", wopiFile.GetPath()), + attribute.String("ocis.wopi.user.idp", wopiUser.GetIdp()), + attribute.String("ocis.wopi.user.opaque", wopiUser.GetOpaqueId()), + attribute.String("ocis.wopi.user.type", wopiUser.GetType().String()), + } + span.SetAttributes(attrs...) + + next.ServeHTTP(w, r) + }) +} diff --git a/services/collaboration/pkg/middleware/wopicontext.go b/services/collaboration/pkg/middleware/wopicontext.go index 7a02d8e6c3c..a6dbd13ef1e 100644 --- a/services/collaboration/pkg/middleware/wopicontext.go +++ b/services/collaboration/pkg/middleware/wopicontext.go @@ -90,6 +90,7 @@ func WopiContextAuthMiddleware(cfg *config.Config, next http.Handler) http.Handl // although some headers might not be sent depending on the client. logger := zerolog.Ctx(ctx) ctx = logger.With(). + Str("WopiSessionId", r.Header.Get("X-WOPI-SessionId")). Str("WopiOverride", r.Header.Get("X-WOPI-Override")). Str("WopiProof", r.Header.Get("X-WOPI-Proof")). Str("WopiProofOld", r.Header.Get("X-WOPI-ProofOld")). diff --git a/services/collaboration/pkg/server/grpc/server.go b/services/collaboration/pkg/server/grpc/server.go index a0be59dccab..43964afe229 100644 --- a/services/collaboration/pkg/server/grpc/server.go +++ b/services/collaboration/pkg/server/grpc/server.go @@ -2,15 +2,25 @@ package grpc import ( appproviderv1beta1 "github.com/cs3org/go-cs3apis/cs3/app/provider/v1beta1" + "github.com/owncloud/ocis/v2/ocis-pkg/tracing" svc "github.com/owncloud/ocis/v2/services/collaboration/pkg/service/grpc/v0" + "go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc" "google.golang.org/grpc" ) // Server initializes a new grpc service ready to run // THIS SERVICE IS REGISTERED AGAINST REVA, NOT GO-MICRO func Server(opts ...Option) (*grpc.Server, func(), error) { - grpcOpts := []grpc.ServerOption{} options := newOptions(opts...) + + grpcOpts := []grpc.ServerOption{ + grpc.StatsHandler( + otelgrpc.NewServerHandler( + otelgrpc.WithTracerProvider(options.TraceProvider), + otelgrpc.WithPropagators(tracing.GetPropagator()), + ), + ), + } grpcServer := grpc.NewServer(grpcOpts...) handle, teardown, err := svc.NewHandler( diff --git a/services/collaboration/pkg/server/http/server.go b/services/collaboration/pkg/server/http/server.go index 0b6cde51b86..e00cdb403c6 100644 --- a/services/collaboration/pkg/server/http/server.go +++ b/services/collaboration/pkg/server/http/server.go @@ -73,6 +73,7 @@ func Server(opts ...Option) (http.Service, error) { otelchi.WithChiRoutes(mux), otelchi.WithTracerProvider(options.TracerProvider), otelchi.WithPropagators(tracing.GetPropagator()), + otelchi.WithRequestMethodInSpanName(true), ), ) @@ -115,10 +116,13 @@ func prepareRoutes(r *chi.Mux, options Options) { r.Route("/files/{fileid}", func(r chi.Router) { - r.Use(func(h stdhttp.Handler) stdhttp.Handler { - // authentication and wopi context - return colabmiddleware.WopiContextAuthMiddleware(options.Config, h) - }) + r.Use( + func(h stdhttp.Handler) stdhttp.Handler { + // authentication and wopi context + return colabmiddleware.WopiContextAuthMiddleware(options.Config, h) + }, + colabmiddleware.CollaborationTracingMiddleware, + ) // check whether we should check for proof keys if !options.Config.App.ProofKeys.Disable { diff --git a/services/proxy/pkg/middleware/tracing.go b/services/proxy/pkg/middleware/tracing.go index 9b897720dce..aed4b675693 100644 --- a/services/proxy/pkg/middleware/tracing.go +++ b/services/proxy/pkg/middleware/tracing.go @@ -32,6 +32,8 @@ func (m tracer) ServeHTTP(w http.ResponseWriter, r *http.Request) { span trace.Span ) + ctx = pkgtrace.Propagator.Extract(ctx, propagation.HeaderCarrier(r.Header)) + tracer := m.traceProvider.Tracer("proxy") spanOpts := []trace.SpanStartOption{ trace.WithSpanKind(trace.SpanKindServer),