From 8a80b990a81b513f51ed8e8142346d264b70a3db Mon Sep 17 00:00:00 2001 From: "A.Unger" Date: Tue, 10 Aug 2021 00:09:39 +0200 Subject: [PATCH] safer code --- accounts/pkg/service/v0/accounts.go | 60 ++++++++++++--------------- accounts/pkg/tracing/tracing.go | 4 +- glauth/pkg/tracing/tracing.go | 4 +- graph-explorer/pkg/tracing/tracing.go | 4 +- graph/pkg/tracing/tracing.go | 4 +- idp/pkg/tracing/tracing.go | 4 +- ocs/pkg/service/v0/groups.go | 36 +++++++--------- ocs/pkg/service/v0/users.go | 12 +++--- ocs/pkg/tracing/tracing.go | 4 +- onlyoffice/pkg/tracing/tracing.go | 4 +- proxy/pkg/proxy/proxy.go | 7 ++-- proxy/pkg/tracing/tracing.go | 4 +- settings/pkg/tracing/tracing.go | 4 +- store/pkg/tracing/tracing.go | 4 +- thumbnails/pkg/tracing/tracing.go | 4 +- web/pkg/tracing/tracing.go | 4 +- webdav/pkg/tracing/tracing.go | 4 +- 17 files changed, 77 insertions(+), 90 deletions(-) diff --git a/accounts/pkg/service/v0/accounts.go b/accounts/pkg/service/v0/accounts.go index d0aa54175..53b3f1278 100644 --- a/accounts/pkg/service/v0/accounts.go +++ b/accounts/pkg/service/v0/accounts.go @@ -128,15 +128,13 @@ func (s Service) getInMemoryServiceUser() proto.Account { // the query contains account properties func (s Service) ListAccounts(ctx context.Context, in *proto.ListAccountsRequest, out *proto.ListAccountsResponse) (err error) { var span trace.Span - if s.Config.Tracing.Enabled { - ctx, span = accTracing.TraceProvider.Tracer("accounts").Start(ctx, "Accounts.ListAccounts") - defer span.End() + ctx, span = accTracing.TraceProvider.Tracer("accounts").Start(ctx, "Accounts.ListAccounts") + defer span.End() - span.SetAttributes( - attribute.KeyValue{Key: "page_size", Value: attribute.Int64Value(int64(in.PageSize))}, - attribute.KeyValue{Key: "page_token", Value: attribute.StringValue(in.PageToken)}, - ) - } + span.SetAttributes( + attribute.KeyValue{Key: "page_size", Value: attribute.Int64Value(int64(in.PageSize))}, + attribute.KeyValue{Key: "page_token", Value: attribute.StringValue(in.PageToken)}, + ) hasSelf := s.hasSelfManagementPermissions(ctx) hasManagement := s.hasAccountManagementPermissions(ctx) @@ -284,14 +282,13 @@ func (s Service) findAccountsByQuery(ctx context.Context, query string) ([]strin // GetAccount implements the AccountsServiceHandler interface func (s Service) GetAccount(ctx context.Context, in *proto.GetAccountRequest, out *proto.Account) (err error) { var span trace.Span - if s.Config.Tracing.Enabled { - ctx, span = accTracing.TraceProvider.Tracer("accounts").Start(ctx, "Accounts.GetAccount") - defer span.End() - span.SetAttributes( - attribute.KeyValue{Key: "account_id", Value: attribute.StringValue(in.Id)}, - ) - } + ctx, span = accTracing.TraceProvider.Tracer("accounts").Start(ctx, "Accounts.GetAccount") + defer span.End() + + span.SetAttributes( + attribute.KeyValue{Key: "account_id", Value: attribute.StringValue(in.Id)}, + ) hasSelf := s.hasSelfManagementPermissions(ctx) hasManagement := s.hasAccountManagementPermissions(ctx) @@ -342,14 +339,13 @@ func (s Service) GetAccount(ctx context.Context, in *proto.GetAccountRequest, ou // CreateAccount implements the AccountsServiceHandler interface func (s Service) CreateAccount(ctx context.Context, in *proto.CreateAccountRequest, out *proto.Account) (err error) { var span trace.Span - if s.Config.Tracing.Enabled { - ctx, span = accTracing.TraceProvider.Tracer("accounts").Start(ctx, "Accounts.CreateAccount") - defer span.End() - span.SetAttributes( - attribute.KeyValue{Key: "account", Value: attribute.StringValue(in.Account.String())}, - ) - } + ctx, span = accTracing.TraceProvider.Tracer("accounts").Start(ctx, "Accounts.CreateAccount") + defer span.End() + + span.SetAttributes( + attribute.KeyValue{Key: "account", Value: attribute.StringValue(in.Account.String())}, + ) if !s.hasAccountManagementPermissions(ctx) { return merrors.Forbidden(s.id, "no permission for CreateAccount") @@ -484,14 +480,13 @@ func (s Service) rollbackCreateAccount(ctx context.Context, acc *proto.Account) // TODO how can we unset specific values? using the update mask func (s Service) UpdateAccount(ctx context.Context, in *proto.UpdateAccountRequest, out *proto.Account) (err error) { var span trace.Span - if s.Config.Tracing.Enabled { - ctx, span = accTracing.TraceProvider.Tracer("accounts").Start(ctx, "Accounts.UpdateAccount") - defer span.End() - span.SetAttributes( - attribute.KeyValue{Key: "account", Value: attribute.StringValue(in.Account.String())}, - ) - } + ctx, span = accTracing.TraceProvider.Tracer("accounts").Start(ctx, "Accounts.UpdateAccount") + defer span.End() + + span.SetAttributes( + attribute.KeyValue{Key: "account", Value: attribute.StringValue(in.Account.String())}, + ) hasSelf := s.hasSelfManagementPermissions(ctx) hasManagement := s.hasAccountManagementPermissions(ctx) @@ -660,10 +655,9 @@ var updatableAccountPaths = map[string]struct{}{ // DeleteAccount implements the AccountsServiceHandler interface func (s Service) DeleteAccount(ctx context.Context, in *proto.DeleteAccountRequest, out *empty.Empty) (err error) { var span trace.Span - if s.Config.Tracing.Enabled { - ctx, span = accTracing.TraceProvider.Tracer("accounts").Start(ctx, "Accounts.DeleteAccount") - defer span.End() - } + + ctx, span = accTracing.TraceProvider.Tracer("accounts").Start(ctx, "Accounts.DeleteAccount") + defer span.End() if !s.hasAccountManagementPermissions(ctx) { return merrors.Forbidden(s.id, "no permission for DeleteAccount") diff --git a/accounts/pkg/tracing/tracing.go b/accounts/pkg/tracing/tracing.go index 25408837b..2a8263a85 100644 --- a/accounts/pkg/tracing/tracing.go +++ b/accounts/pkg/tracing/tracing.go @@ -3,12 +3,12 @@ package tracing import ( "github.com/owncloud/ocis/accounts/pkg/config" pkgtrace "github.com/owncloud/ocis/ocis-pkg/tracing" - sdktrace "go.opentelemetry.io/otel/sdk/trace" + "go.opentelemetry.io/otel/trace" ) var ( // TraceProvider is the global trace provider for the proxy service. - TraceProvider *sdktrace.TracerProvider + TraceProvider = trace.NewNoopTracerProvider() ) func Configure(cfg *config.Config) error { diff --git a/glauth/pkg/tracing/tracing.go b/glauth/pkg/tracing/tracing.go index b721b7574..1ea50a2d6 100644 --- a/glauth/pkg/tracing/tracing.go +++ b/glauth/pkg/tracing/tracing.go @@ -3,12 +3,12 @@ package tracing import ( "github.com/owncloud/ocis/glauth/pkg/config" pkgtrace "github.com/owncloud/ocis/ocis-pkg/tracing" - sdktrace "go.opentelemetry.io/otel/sdk/trace" + "go.opentelemetry.io/otel/trace" ) var ( // TraceProvider is the global trace provider for the glauth service. - TraceProvider *sdktrace.TracerProvider + TraceProvider = trace.NewNoopTracerProvider() ) func Configure(cfg *config.Config) error { diff --git a/graph-explorer/pkg/tracing/tracing.go b/graph-explorer/pkg/tracing/tracing.go index f1e5e3fac..d76a0c3e8 100644 --- a/graph-explorer/pkg/tracing/tracing.go +++ b/graph-explorer/pkg/tracing/tracing.go @@ -3,12 +3,12 @@ package tracing import ( "github.com/owncloud/ocis/graph-explorer/pkg/config" pkgtrace "github.com/owncloud/ocis/ocis-pkg/tracing" - sdktrace "go.opentelemetry.io/otel/sdk/trace" + "go.opentelemetry.io/otel/trace" ) var ( // TraceProvider is the global trace provider for the proxy service. - TraceProvider *sdktrace.TracerProvider + TraceProvider = trace.NewNoopTracerProvider() ) func Configure(cfg *config.Config) error { diff --git a/graph/pkg/tracing/tracing.go b/graph/pkg/tracing/tracing.go index 2180fb315..e0ee16cef 100644 --- a/graph/pkg/tracing/tracing.go +++ b/graph/pkg/tracing/tracing.go @@ -3,12 +3,12 @@ package tracing import ( "github.com/owncloud/ocis/graph/pkg/config" pkgtrace "github.com/owncloud/ocis/ocis-pkg/tracing" - sdktrace "go.opentelemetry.io/otel/sdk/trace" + "go.opentelemetry.io/otel/trace" ) var ( // TraceProvider is the global trace provider for the graph service. - TraceProvider *sdktrace.TracerProvider + TraceProvider = trace.NewNoopTracerProvider() ) func Configure(cfg *config.Config) error { diff --git a/idp/pkg/tracing/tracing.go b/idp/pkg/tracing/tracing.go index 1bbd6f3b0..4426d6762 100644 --- a/idp/pkg/tracing/tracing.go +++ b/idp/pkg/tracing/tracing.go @@ -3,12 +3,12 @@ package tracing import ( "github.com/owncloud/ocis/idp/pkg/config" pkgtrace "github.com/owncloud/ocis/ocis-pkg/tracing" - sdktrace "go.opentelemetry.io/otel/sdk/trace" + "go.opentelemetry.io/otel/trace" ) var ( // TraceProvider is the global trace provider for the idp service. - TraceProvider *sdktrace.TracerProvider + TraceProvider = trace.NewNoopTracerProvider() ) func Configure(cfg *config.Config) error { diff --git a/ocs/pkg/service/v0/groups.go b/ocs/pkg/service/v0/groups.go index a8c461d00..f592ad742 100644 --- a/ocs/pkg/service/v0/groups.go +++ b/ocs/pkg/service/v0/groups.go @@ -33,14 +33,12 @@ func (o Ocs) ListUserGroups(w http.ResponseWriter, r *http.Request) { // the OCS API is a REST API and it uses the username to look for groups. If the id from the user in the context // differs from that of the url we can assume we are an admin because we are past the selfOrAdmin middleware. - if o.config.Tracing.Enabled { - _, span := ocstracing.TraceProvider. - Tracer("ocs"). - Start(r.Context(), "ListUserGroups") - defer span.End() + _, span := ocstracing.TraceProvider. + Tracer("ocs"). + Start(r.Context(), "ListUserGroups") + defer span.End() - span.SetAttributes(attribute.Any("groups", u.Groups)) - } + span.SetAttributes(attribute.Any("groups", u.Groups)) if len(u.Groups) > 0 { mustNotFail(render.Render(w, r, response.DataRender(&data.Groups{Groups: u.Groups}))) @@ -93,14 +91,12 @@ func (o Ocs) ListUserGroups(w http.ResponseWriter, r *http.Request) { o.logger.Error().Err(err).Int("count", len(groups)).Str("userid", account.Id).Msg("listing groups for user") - if o.config.Tracing.Enabled { - _, span := ocstracing.TraceProvider. - Tracer("ocs"). - Start(r.Context(), "ListUserGroups") - defer span.End() + _, span := ocstracing.TraceProvider. + Tracer("ocs"). + Start(r.Context(), "ListUserGroups") + defer span.End() - span.SetAttributes(attribute.Any("groups", groups)) - } + span.SetAttributes(attribute.Any("groups", groups)) mustNotFail(render.Render(w, r, response.DataRender(&data.Groups{Groups: groups}))) } @@ -265,14 +261,12 @@ func (o Ocs) ListGroups(w http.ResponseWriter, r *http.Request) { groups = append(groups, res.Groups[i].OnPremisesSamAccountName) } - if o.config.Tracing.Enabled { - _, span := ocstracing.TraceProvider. - Tracer("ocs"). - Start(r.Context(), "ListGroups") - defer span.End() + _, span := ocstracing.TraceProvider. + Tracer("ocs"). + Start(r.Context(), "ListGroups") + defer span.End() - span.SetAttributes(attribute.Any("groups", groups)) - } + span.SetAttributes(attribute.Any("groups", groups)) mustNotFail(render.Render(w, r, response.DataRender(&data.Groups{Groups: groups}))) } diff --git a/ocs/pkg/service/v0/users.go b/ocs/pkg/service/v0/users.go index 75a052365..cca80e0b1 100644 --- a/ocs/pkg/service/v0/users.go +++ b/ocs/pkg/service/v0/users.go @@ -148,14 +148,12 @@ func (o Ocs) GetUser(w http.ResponseWriter, r *http.Request) { }, } - if o.config.Tracing.Enabled { - _, span := ocstracing.TraceProvider. - Tracer("ocs"). - Start(r.Context(), "GetUser") - defer span.End() + _, span := ocstracing.TraceProvider. + Tracer("ocs"). + Start(r.Context(), "GetUser") + defer span.End() - span.SetAttributes(attribute.Any("user", d)) - } + span.SetAttributes(attribute.Any("user", d)) mustNotFail(render.Render(w, r, response.DataRender(d))) } diff --git a/ocs/pkg/tracing/tracing.go b/ocs/pkg/tracing/tracing.go index 228587b92..e8c6f101e 100644 --- a/ocs/pkg/tracing/tracing.go +++ b/ocs/pkg/tracing/tracing.go @@ -3,12 +3,12 @@ package tracing import ( pkgtrace "github.com/owncloud/ocis/ocis-pkg/tracing" "github.com/owncloud/ocis/ocs/pkg/config" - sdktrace "go.opentelemetry.io/otel/sdk/trace" + "go.opentelemetry.io/otel/trace" ) var ( // TraceProvider is the global trace provider for the ocs service. - TraceProvider *sdktrace.TracerProvider + TraceProvider = trace.NewNoopTracerProvider() ) func Configure(cfg *config.Config) error { diff --git a/onlyoffice/pkg/tracing/tracing.go b/onlyoffice/pkg/tracing/tracing.go index d775ffa4b..1713bfb6d 100644 --- a/onlyoffice/pkg/tracing/tracing.go +++ b/onlyoffice/pkg/tracing/tracing.go @@ -3,12 +3,12 @@ package tracing import ( pkgtrace "github.com/owncloud/ocis/ocis-pkg/tracing" "github.com/owncloud/ocis/onlyoffice/pkg/config" - sdktrace "go.opentelemetry.io/otel/sdk/trace" + "go.opentelemetry.io/otel/trace" ) var ( // TraceProvider is the global trace provider for the onlyoffice service. - TraceProvider *sdktrace.TracerProvider + TraceProvider = trace.NewNoopTracerProvider() ) func Configure(cfg *config.Config) error { diff --git a/proxy/pkg/proxy/proxy.go b/proxy/pkg/proxy/proxy.go index 5f4536e32..7aa16aa7f 100644 --- a/proxy/pkg/proxy/proxy.go +++ b/proxy/pkg/proxy/proxy.go @@ -222,10 +222,11 @@ func (p *MultiHostReverseProxy) ServeHTTP(w http.ResponseWriter, r *http.Request span trace.Span ) + tracer := proxytracing.TraceProvider.Tracer("proxy") + ctx, span = tracer.Start(ctx, fmt.Sprintf("%s %v", r.Method, r.URL.Path)) + defer span.End() + if p.config.Tracing.Enabled { - tracer := proxytracing.TraceProvider.Tracer("proxy") - ctx, span = tracer.Start(ctx, fmt.Sprintf("%s %v", r.Method, r.URL.Path)) - defer span.End() pkgtrace.Propagator.Inject(ctx, propagation.HeaderCarrier(r.Header)) } diff --git a/proxy/pkg/tracing/tracing.go b/proxy/pkg/tracing/tracing.go index ee9f3dfcf..ffe29a220 100644 --- a/proxy/pkg/tracing/tracing.go +++ b/proxy/pkg/tracing/tracing.go @@ -3,12 +3,12 @@ package tracing import ( pkgtrace "github.com/owncloud/ocis/ocis-pkg/tracing" "github.com/owncloud/ocis/proxy/pkg/config" - sdktrace "go.opentelemetry.io/otel/sdk/trace" + "go.opentelemetry.io/otel/trace" ) var ( // TraceProvider is the global trace provider for the proxy service. - TraceProvider *sdktrace.TracerProvider + TraceProvider = trace.NewNoopTracerProvider() ) func Configure(cfg *config.Config) error { diff --git a/settings/pkg/tracing/tracing.go b/settings/pkg/tracing/tracing.go index bc86e3af1..678ee7f55 100644 --- a/settings/pkg/tracing/tracing.go +++ b/settings/pkg/tracing/tracing.go @@ -3,12 +3,12 @@ package tracing import ( pkgtrace "github.com/owncloud/ocis/ocis-pkg/tracing" "github.com/owncloud/ocis/settings/pkg/config" - sdktrace "go.opentelemetry.io/otel/sdk/trace" + "go.opentelemetry.io/otel/trace" ) var ( // TraceProvider is the global trace provider for the settings service. - TraceProvider *sdktrace.TracerProvider + TraceProvider = trace.NewNoopTracerProvider() ) func Configure(cfg *config.Config) error { diff --git a/store/pkg/tracing/tracing.go b/store/pkg/tracing/tracing.go index 726566224..6f91bb967 100644 --- a/store/pkg/tracing/tracing.go +++ b/store/pkg/tracing/tracing.go @@ -3,12 +3,12 @@ package tracing import ( pkgtrace "github.com/owncloud/ocis/ocis-pkg/tracing" "github.com/owncloud/ocis/store/pkg/config" - sdktrace "go.opentelemetry.io/otel/sdk/trace" + "go.opentelemetry.io/otel/trace" ) var ( // TraceProvider is the global trace provider for the store service. - TraceProvider *sdktrace.TracerProvider + TraceProvider = trace.NewNoopTracerProvider() ) func Configure(cfg *config.Config) error { diff --git a/thumbnails/pkg/tracing/tracing.go b/thumbnails/pkg/tracing/tracing.go index c9c45e5ef..e5ca712bc 100644 --- a/thumbnails/pkg/tracing/tracing.go +++ b/thumbnails/pkg/tracing/tracing.go @@ -3,12 +3,12 @@ package tracing import ( pkgtrace "github.com/owncloud/ocis/ocis-pkg/tracing" "github.com/owncloud/ocis/thumbnails/pkg/config" - sdktrace "go.opentelemetry.io/otel/sdk/trace" + "go.opentelemetry.io/otel/trace" ) var ( // TraceProvider is the global trace provider for the thumbnails service. - TraceProvider *sdktrace.TracerProvider + TraceProvider = trace.NewNoopTracerProvider() ) func Configure(cfg *config.Config) error { diff --git a/web/pkg/tracing/tracing.go b/web/pkg/tracing/tracing.go index 1f788eca2..d3e2a9365 100644 --- a/web/pkg/tracing/tracing.go +++ b/web/pkg/tracing/tracing.go @@ -3,12 +3,12 @@ package tracing import ( pkgtrace "github.com/owncloud/ocis/ocis-pkg/tracing" "github.com/owncloud/ocis/web/pkg/config" - sdktrace "go.opentelemetry.io/otel/sdk/trace" + "go.opentelemetry.io/otel/trace" ) var ( // TraceProvider is the global trace provider for the web service. - TraceProvider *sdktrace.TracerProvider + TraceProvider = trace.NewNoopTracerProvider() ) func Configure(cfg *config.Config) error { diff --git a/webdav/pkg/tracing/tracing.go b/webdav/pkg/tracing/tracing.go index 6f1fbc9ac..ebc16f2f8 100644 --- a/webdav/pkg/tracing/tracing.go +++ b/webdav/pkg/tracing/tracing.go @@ -3,12 +3,12 @@ package tracing import ( pkgtrace "github.com/owncloud/ocis/ocis-pkg/tracing" "github.com/owncloud/ocis/webdav/pkg/config" - sdktrace "go.opentelemetry.io/otel/sdk/trace" + "go.opentelemetry.io/otel/trace" ) var ( // TraceProvider is the global trace provider for the proxy service. - TraceProvider *sdktrace.TracerProvider + TraceProvider = trace.NewNoopTracerProvider() ) func Configure(cfg *config.Config) error {