From a5d27b2f989ac5fee03607ca963f24f643896f11 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juan=20Pablo=20Villaf=C3=A1=C3=B1ez?= Date: Wed, 24 Jul 2024 18:18:34 +0200 Subject: [PATCH 1/5] feat: include additional metadata for tracing the collaboration service --- .../pkg/connector/contentconnector.go | 6 +++ .../collaboration/pkg/middleware/tracing.go | 39 +++++++++++++++++++ .../collaboration/pkg/server/http/server.go | 12 ++++-- services/proxy/pkg/middleware/tracing.go | 2 + 4 files changed, 55 insertions(+), 4 deletions(-) create mode 100644 services/collaboration/pkg/middleware/tracing.go 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/tracing.go b/services/collaboration/pkg/middleware/tracing.go new file mode 100644 index 00000000000..e4f4b890f53 --- /dev/null +++ b/services/collaboration/pkg/middleware/tracing.go @@ -0,0 +1,39 @@ +package middleware + +import ( + "net/http" + + "go.opentelemetry.io/otel/attribute" + "go.opentelemetry.io/otel/trace" +) + +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.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/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), From 083a2e14848cd4ce96b4fe72627e71a0b87a8f93 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juan=20Pablo=20Villaf=C3=A1=C3=B1ez?= Date: Thu, 25 Jul 2024 13:01:26 +0200 Subject: [PATCH 2/5] feat: include tracing for grpc openInApp function --- services/collaboration/pkg/command/server.go | 1 + services/collaboration/pkg/server/grpc/server.go | 12 +++++++++++- 2 files changed, 12 insertions(+), 1 deletion(-) 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/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( From eb40aa04608c8e145aae2353455238111ddad184 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juan=20Pablo=20Villaf=C3=A1=C3=B1ez?= Date: Tue, 6 Aug 2024 13:36:25 +0200 Subject: [PATCH 3/5] feat: include X-WOPI-SessionId header in the logs and tracing --- services/collaboration/pkg/middleware/tracing.go | 1 + services/collaboration/pkg/middleware/wopicontext.go | 1 + 2 files changed, 2 insertions(+) diff --git a/services/collaboration/pkg/middleware/tracing.go b/services/collaboration/pkg/middleware/tracing.go index e4f4b890f53..3e90d4df412 100644 --- a/services/collaboration/pkg/middleware/tracing.go +++ b/services/collaboration/pkg/middleware/tracing.go @@ -23,6 +23,7 @@ func CollaborationTracingMiddleware(next http.Handler) http.Handler { 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()), 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")). From 29700046d71a42da760c1220430a669b136af24a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juan=20Pablo=20Villaf=C3=A1=C3=B1ez?= Date: Mon, 12 Aug 2024 12:00:42 +0200 Subject: [PATCH 4/5] docs: add missing code comments in the middlewares --- services/collaboration/pkg/middleware/proofkeys.go | 10 ++++++++++ services/collaboration/pkg/middleware/tracing.go | 7 +++++++ 2 files changed, 17 insertions(+) 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 index 3e90d4df412..816f81e129b 100644 --- a/services/collaboration/pkg/middleware/tracing.go +++ b/services/collaboration/pkg/middleware/tracing.go @@ -7,6 +7,13 @@ import ( "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()) From 90e2322bfa6bf2d8e75da55557728e39c8952ee4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juan=20Pablo=20Villaf=C3=A1=C3=B1ez?= Date: Tue, 13 Aug 2024 10:21:26 +0200 Subject: [PATCH 5/5] chore: add changelog entry --- changelog/unreleased/collaboration-improved-tracing.md | 6 ++++++ 1 file changed, 6 insertions(+) create mode 100644 changelog/unreleased/collaboration-improved-tracing.md 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