Skip to content

Commit

Permalink
reduce logging output (#2353)
Browse files Browse the repository at this point in the history 
* reduce logging output

* add error logging to the storage provider

* update `status.NewInternal` API
  • Loading branch information
David Christofas authored Dec 14, 2021
1 parent f6088d1 commit af0d3d1
Show file tree
Hide file tree
Showing 37 changed files with 459 additions and 310 deletions.
6 changes: 6 additions & 0 deletions changelog/unreleased/reduce-log-output.md
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
Change: Reduce log output

Reduced log output. Some errors or warnings were logged multiple times or even unnecesarily.


https://github.com/cs3org/reva/pull/2353
2 changes: 1 addition & 1 deletion internal/grpc/interceptors/auth/auth.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -242,7 +242,7 @@ func dismantleToken(ctx context.Context, tkn string, req interface{}, mgr token.
func getUserGroups(ctx context.Context, u *userpb.User, client gatewayv1beta1.GatewayAPIClient) ([]string, error) {
if groupsIf, err := userGroupsCache.Get(u.Id.OpaqueId); err == nil {
log := appctx.GetLogger(ctx)
log.Info().Msgf("user groups found in cache %s", u.Id.OpaqueId)
log.Info().Str("userid", u.Id.OpaqueId).Msg("user groups found in cache")
return groupsIf.([]string), nil
}

Expand Down
4 changes: 2 additions & 2 deletions internal/grpc/interceptors/auth/scope.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ func expandAndVerifyScope(ctx context.Context, req interface{}, tokenScope map[s
// trying to impersonate the owner, since the share manager doesn't know the
// share path.
if ref.GetPath() != "" {
log.Info().Msgf("resolving path reference to ID to check token scope %+v", ref.GetPath())
log.Info().Str("path", ref.GetPath()).Msg("resolving path reference to ID to check token scope")
for k := range tokenScope {
switch {
case strings.HasPrefix(k, "publicshare"):
Expand Down Expand Up @@ -138,7 +138,7 @@ func expandAndVerifyScope(ctx context.Context, req interface{}, tokenScope map[s
// It's a share ref
// The request might be coming from a share created for a lightweight account
// after the token was minted.
log.Info().Msgf("resolving share reference against received shares to verify token scope %+v", ref)
log.Info().Interface("reference", ref).Msg("resolving share reference against received shares to verify token scope")
client, err := pool.GetGatewayServiceClient(gatewayAddr)
if err != nil {
return err
Expand Down
8 changes: 4 additions & 4 deletions internal/grpc/services/applicationauth/applicationauth.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ func (s *service) GenerateAppPassword(ctx context.Context, req *appauthpb.Genera
pwd, err := s.am.GenerateAppPassword(ctx, req.TokenScope, req.Label, req.Expiration)
if err != nil {
return &appauthpb.GenerateAppPasswordResponse{
Status: status.NewInternal(ctx, err, "error generating app password"),
Status: status.NewInternal(ctx, "error generating app password"),
}, nil
}

Expand All @@ -120,7 +120,7 @@ func (s *service) ListAppPasswords(ctx context.Context, req *appauthpb.ListAppPa
pwds, err := s.am.ListAppPasswords(ctx)
if err != nil {
return &appauthpb.ListAppPasswordsResponse{
Status: status.NewInternal(ctx, err, "error listing app passwords"),
Status: status.NewInternal(ctx, "error listing app passwords"),
}, nil
}

Expand All @@ -134,7 +134,7 @@ func (s *service) InvalidateAppPassword(ctx context.Context, req *appauthpb.Inva
err := s.am.InvalidateAppPassword(ctx, req.Password)
if err != nil {
return &appauthpb.InvalidateAppPasswordResponse{
Status: status.NewInternal(ctx, err, "error invalidating app password"),
Status: status.NewInternal(ctx, "error invalidating app password"),
}, nil
}

Expand All @@ -147,7 +147,7 @@ func (s *service) GetAppPassword(ctx context.Context, req *appauthpb.GetAppPassw
pwd, err := s.am.GetAppPassword(ctx, req.User, req.Password)
if err != nil {
return &appauthpb.GetAppPasswordResponse{
Status: status.NewInternal(ctx, err, "error getting app password via username/password"),
Status: status.NewInternal(ctx, "error getting app password via username/password"),
}, nil
}

Expand Down
11 changes: 5 additions & 6 deletions internal/grpc/services/appprovider/appprovider.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ package appprovider

import (
"context"
"errors"
"os"
"strconv"
"time"
Expand Down Expand Up @@ -106,7 +105,7 @@ func (s *service) registerProvider() {
log := logger.New().With().Int("pid", os.Getpid()).Logger()
pInfo, err := s.provider.GetAppProviderInfo(ctx)
if err != nil {
log.Error().Err(err).Msgf("error registering app provider: could not get provider info")
log.Error().Err(err).Msg("error registering app provider: could not get provider info")
return
}
pInfo.Address = s.conf.AppProviderURL
Expand All @@ -122,7 +121,7 @@ func (s *service) registerProvider() {

client, err := pool.GetGatewayServiceClient(s.conf.GatewaySvc)
if err != nil {
log.Error().Err(err).Msgf("error registering app provider: could not get gateway client")
log.Error().Err(err).Msg("error registering app provider: could not get gateway client")
return
}
req := &registrypb.AddAppProviderRequest{Provider: pInfo}
Expand All @@ -140,12 +139,12 @@ func (s *service) registerProvider() {

res, err := client.AddAppProvider(ctx, req)
if err != nil {
log.Error().Err(err).Msgf("error registering app provider: error calling add app provider")
log.Error().Err(err).Msg("error registering app provider: error calling add app provider")
return
}
if res.Status.Code != rpc.Code_CODE_OK {
err = status.NewErrorFromCode(res.Status.Code, "appprovider")
log.Error().Err(err).Msgf("error registering app provider: add app provider returned error")
log.Error().Err(err).Msg("error registering app provider: add app provider returned error")
return
}
}
Expand Down Expand Up @@ -173,7 +172,7 @@ func (s *service) OpenInApp(ctx context.Context, req *providerpb.OpenInAppReques
appURL, err := s.provider.GetAppURL(ctx, req.ResourceInfo, req.ViewMode, req.AccessToken)
if err != nil {
res := &providerpb.OpenInAppResponse{
Status: status.NewInternal(ctx, errors.New("appprovider: error calling GetAppURL"), err.Error()),
Status: status.NewInternal(ctx, err.Error()),
}
return res, nil
}
Expand Down
12 changes: 6 additions & 6 deletions internal/grpc/services/appregistry/appregistry.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ func (s *svc) GetAppProviders(ctx context.Context, req *registrypb.GetAppProvide
p, err := s.reg.FindProviders(ctx, req.ResourceInfo.MimeType)
if err != nil {
return &registrypb.GetAppProvidersResponse{
Status: status.NewInternal(ctx, err, "error looking for the app provider"),
Status: status.NewInternal(ctx, "error looking for the app provider"),
}, nil
}

Expand All @@ -118,7 +118,7 @@ func (s *svc) AddAppProvider(ctx context.Context, req *registrypb.AddAppProvider
err := s.reg.AddProvider(ctx, req.Provider)
if err != nil {
return &registrypb.AddAppProviderResponse{
Status: status.NewInternal(ctx, err, "error adding the app provider"),
Status: status.NewInternal(ctx, "error adding the app provider"),
}, nil
}

Expand All @@ -132,7 +132,7 @@ func (s *svc) ListAppProviders(ctx context.Context, req *registrypb.ListAppProvi
providers, err := s.reg.ListProviders(ctx)
if err != nil {
return &registrypb.ListAppProvidersResponse{
Status: status.NewInternal(ctx, err, "error listing the app providers"),
Status: status.NewInternal(ctx, "error listing the app providers"),
}, nil
}

Expand All @@ -147,7 +147,7 @@ func (s *svc) ListSupportedMimeTypes(ctx context.Context, req *registrypb.ListSu
mimeTypes, err := s.reg.ListSupportedMimeTypes(ctx)
if err != nil {
return &registrypb.ListSupportedMimeTypesResponse{
Status: status.NewInternal(ctx, err, "error listing the supported mime types"),
Status: status.NewInternal(ctx, "error listing the supported mime types"),
}, nil
}

Expand All @@ -169,7 +169,7 @@ func (s *svc) GetDefaultAppProviderForMimeType(ctx context.Context, req *registr
provider, err := s.reg.GetDefaultProviderForMimeType(ctx, req.MimeType)
if err != nil {
return &registrypb.GetDefaultAppProviderForMimeTypeResponse{
Status: status.NewInternal(ctx, err, "error getting the default app provider for the mimetype"),
Status: status.NewInternal(ctx, "error getting the default app provider for the mimetype"),
}, nil
}

Expand All @@ -184,7 +184,7 @@ func (s *svc) SetDefaultAppProviderForMimeType(ctx context.Context, req *registr
err := s.reg.SetDefaultProviderForMimeType(ctx, req.MimeType, req.Provider)
if err != nil {
return &registrypb.SetDefaultAppProviderForMimeTypeResponse{
Status: status.NewInternal(ctx, err, "error setting the default app provider for the mimetype"),
Status: status.NewInternal(ctx, "error setting the default app provider for the mimetype"),
}, nil
}

Expand Down
3 changes: 2 additions & 1 deletion internal/grpc/services/authprovider/authprovider.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,7 @@ func (s *service) Authenticate(ctx context.Context, req *provider.AuthenticateRe
u, scope, err := s.authmgr.Authenticate(ctx, username, password)
switch v := err.(type) {
case nil:
log.Info().Msgf("user %s authenticated", u.String())
log.Info().Str("user", u.String()).Msg("user authenticated")
return &provider.AuthenticateResponse{
Status: status.NewOK(ctx),
User: u,
Expand All @@ -149,6 +149,7 @@ func (s *service) Authenticate(ctx context.Context, req *provider.AuthenticateRe
Status: status.NewPermissionDenied(ctx, v, "wrong password"),
}, nil
case errtypes.NotFound:
log.Debug().Str("client_id", username).Msg("unknown client id")
return &provider.AuthenticateResponse{
Status: status.NewNotFound(ctx, "unknown client id"),
}, nil
Expand Down
4 changes: 2 additions & 2 deletions internal/grpc/services/authregistry/authregistry.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ func (s *service) ListAuthProviders(ctx context.Context, req *registrypb.ListAut
pinfos, err := s.reg.ListProviders(ctx)
if err != nil {
return &registrypb.ListAuthProvidersResponse{
Status: status.NewInternal(ctx, err, "error getting list of auth providers"),
Status: status.NewInternal(ctx, "error getting list of auth providers"),
}, nil
}

Expand All @@ -120,7 +120,7 @@ func (s *service) GetAuthProvider(ctx context.Context, req *registrypb.GetAuthPr
pinfo, err := s.reg.GetProvider(ctx, req.Type)
if err != nil {
return &registrypb.GetAuthProviderResponse{
Status: status.NewInternal(ctx, err, "error getting auth provider for type: "+req.Type),
Status: status.NewInternal(ctx, "error getting auth provider for type: "+req.Type),
}, nil
}

Expand Down
12 changes: 4 additions & 8 deletions internal/grpc/services/gateway/applicationauth.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,8 @@ import (
func (s *svc) GenerateAppPassword(ctx context.Context, req *appauthpb.GenerateAppPasswordRequest) (*appauthpb.GenerateAppPasswordResponse, error) {
c, err := pool.GetAppAuthProviderServiceClient(s.c.ApplicationAuthEndpoint)
if err != nil {
err = errors.Wrap(err, "gateway: error calling GetAppAuthProviderServiceClient")
return &appauthpb.GenerateAppPasswordResponse{
Status: status.NewInternal(ctx, err, "error getting app auth provider client"),
Status: status.NewInternal(ctx, "error getting app auth provider client"),
}, nil
}

Expand All @@ -47,9 +46,8 @@ func (s *svc) GenerateAppPassword(ctx context.Context, req *appauthpb.GenerateAp
func (s *svc) ListAppPasswords(ctx context.Context, req *appauthpb.ListAppPasswordsRequest) (*appauthpb.ListAppPasswordsResponse, error) {
c, err := pool.GetAppAuthProviderServiceClient(s.c.ApplicationAuthEndpoint)
if err != nil {
err = errors.Wrap(err, "gateway: error calling GetAppAuthProviderServiceClient")
return &appauthpb.ListAppPasswordsResponse{
Status: status.NewInternal(ctx, err, "error getting app auth provider client"),
Status: status.NewInternal(ctx, "error getting app auth provider client"),
}, nil
}

Expand All @@ -64,9 +62,8 @@ func (s *svc) ListAppPasswords(ctx context.Context, req *appauthpb.ListAppPasswo
func (s *svc) InvalidateAppPassword(ctx context.Context, req *appauthpb.InvalidateAppPasswordRequest) (*appauthpb.InvalidateAppPasswordResponse, error) {
c, err := pool.GetAppAuthProviderServiceClient(s.c.ApplicationAuthEndpoint)
if err != nil {
err = errors.Wrap(err, "gateway: error calling GetAppAuthProviderServiceClient")
return &appauthpb.InvalidateAppPasswordResponse{
Status: status.NewInternal(ctx, err, "error getting app auth provider client"),
Status: status.NewInternal(ctx, "error getting app auth provider client"),
}, nil
}

Expand All @@ -81,9 +78,8 @@ func (s *svc) InvalidateAppPassword(ctx context.Context, req *appauthpb.Invalida
func (s *svc) GetAppPassword(ctx context.Context, req *appauthpb.GetAppPasswordRequest) (*appauthpb.GetAppPasswordResponse, error) {
c, err := pool.GetAppAuthProviderServiceClient(s.c.ApplicationAuthEndpoint)
if err != nil {
err = errors.Wrap(err, "gateway: error calling GetAppAuthProviderServiceClient")
return &appauthpb.GetAppPasswordResponse{
Status: status.NewInternal(ctx, err, "error getting app auth provider client"),
Status: status.NewInternal(ctx, "error getting app auth provider client"),
}, nil
}

Expand Down
9 changes: 4 additions & 5 deletions internal/grpc/services/gateway/appprovider.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ func (s *svc) OpenInApp(ctx context.Context, req *gateway.OpenInAppRequest) (*pr
})
if err != nil {
return &providerpb.OpenInAppResponse{
Status: status.NewInternal(ctx, err, "gateway: error calling Stat on the resource path for the app provider: "+req.Ref.GetPath()),
Status: status.NewInternal(ctx, "gateway: error calling Stat on the resource path for the app provider: "+req.Ref.GetPath()),
}, nil
}
if statRes.Status.Code != rpc.Code_CODE_OK {
Expand All @@ -67,7 +67,7 @@ func (s *svc) OpenInApp(ctx context.Context, req *gateway.OpenInAppRequest) (*pr
uri, err := url.Parse(fileInfo.Target)
if err != nil {
return &providerpb.OpenInAppResponse{
Status: status.NewInternal(ctx, err, "gateway: error parsing target uri: "+fileInfo.Target),
Status: status.NewInternal(ctx, "gateway: error parsing target uri: "+fileInfo.Target),
}, nil
}
if uri.Scheme == "webdav" {
Expand All @@ -80,13 +80,12 @@ func (s *svc) OpenInApp(ctx context.Context, req *gateway.OpenInAppRequest) (*pr
})
if err != nil {
return &providerpb.OpenInAppResponse{
Status: status.NewInternal(ctx, err, "gateway: error calling Stat on the resource path for the app provider: "+req.Ref.GetPath()),
Status: status.NewInternal(ctx, "gateway: error calling Stat on the resource path for the app provider: "+req.Ref.GetPath()),
}, nil
}
if res.Status.Code != rpc.Code_CODE_OK {
err := status.NewErrorFromCode(res.Status.GetCode(), "gateway")
return &providerpb.OpenInAppResponse{
Status: status.NewInternal(ctx, err, "Stat failed on the resource path for the app provider: "+req.Ref.GetPath()),
Status: status.NewInternal(ctx, "Stat failed on the resource path for the app provider: "+req.Ref.GetPath()),
}, nil
}
fileInfo = res.Info
Expand Down
18 changes: 6 additions & 12 deletions internal/grpc/services/gateway/appregistry.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,8 @@ import (
func (s *svc) GetAppProviders(ctx context.Context, req *registry.GetAppProvidersRequest) (*registry.GetAppProvidersResponse, error) {
c, err := pool.GetAppRegistryClient(s.c.AppRegistryEndpoint)
if err != nil {
err = errors.Wrap(err, "gateway: error calling GetAppRegistryClient")
return &registry.GetAppProvidersResponse{
Status: status.NewInternal(ctx, err, "error getting app registry client"),
Status: status.NewInternal(ctx, "error getting app registry client"),
}, nil
}

Expand All @@ -47,9 +46,8 @@ func (s *svc) GetAppProviders(ctx context.Context, req *registry.GetAppProviders
func (s *svc) AddAppProvider(ctx context.Context, req *registry.AddAppProviderRequest) (*registry.AddAppProviderResponse, error) {
c, err := pool.GetAppRegistryClient(s.c.AppRegistryEndpoint)
if err != nil {
err = errors.Wrap(err, "gateway: error calling GetAppRegistryClient")
return &registry.AddAppProviderResponse{
Status: status.NewInternal(ctx, err, "error getting app registry client"),
Status: status.NewInternal(ctx, "error getting app registry client"),
}, nil
}

Expand All @@ -64,9 +62,8 @@ func (s *svc) AddAppProvider(ctx context.Context, req *registry.AddAppProviderRe
func (s *svc) ListAppProviders(ctx context.Context, req *registry.ListAppProvidersRequest) (*registry.ListAppProvidersResponse, error) {
c, err := pool.GetAppRegistryClient(s.c.AppRegistryEndpoint)
if err != nil {
err = errors.Wrap(err, "gateway: error calling GetAppRegistryClient")
return &registry.ListAppProvidersResponse{
Status: status.NewInternal(ctx, err, "error getting app registry client"),
Status: status.NewInternal(ctx, "error getting app registry client"),
}, nil
}

Expand All @@ -81,9 +78,8 @@ func (s *svc) ListAppProviders(ctx context.Context, req *registry.ListAppProvide
func (s *svc) ListSupportedMimeTypes(ctx context.Context, req *registry.ListSupportedMimeTypesRequest) (*registry.ListSupportedMimeTypesResponse, error) {
c, err := pool.GetAppRegistryClient(s.c.AppRegistryEndpoint)
if err != nil {
err = errors.Wrap(err, "gateway: error calling GetAppRegistryClient")
return &registry.ListSupportedMimeTypesResponse{
Status: status.NewInternal(ctx, err, "error getting app registry client"),
Status: status.NewInternal(ctx, "error getting app registry client"),
}, nil
}

Expand All @@ -98,9 +94,8 @@ func (s *svc) ListSupportedMimeTypes(ctx context.Context, req *registry.ListSupp
func (s *svc) GetDefaultAppProviderForMimeType(ctx context.Context, req *registry.GetDefaultAppProviderForMimeTypeRequest) (*registry.GetDefaultAppProviderForMimeTypeResponse, error) {
c, err := pool.GetAppRegistryClient(s.c.AppRegistryEndpoint)
if err != nil {
err = errors.Wrap(err, "gateway: error calling GetAppRegistryClient")
return &registry.GetDefaultAppProviderForMimeTypeResponse{
Status: status.NewInternal(ctx, err, "error getting app registry client"),
Status: status.NewInternal(ctx, "error getting app registry client"),
}, nil
}

Expand All @@ -115,9 +110,8 @@ func (s *svc) GetDefaultAppProviderForMimeType(ctx context.Context, req *registr
func (s *svc) SetDefaultAppProviderForMimeType(ctx context.Context, req *registry.SetDefaultAppProviderForMimeTypeRequest) (*registry.SetDefaultAppProviderForMimeTypeResponse, error) {
c, err := pool.GetAppRegistryClient(s.c.AppRegistryEndpoint)
if err != nil {
err = errors.Wrap(err, "gateway: error calling GetAppRegistryClient")
return &registry.SetDefaultAppProviderForMimeTypeResponse{
Status: status.NewInternal(ctx, err, "error getting app registry client"),
Status: status.NewInternal(ctx, "error getting app registry client"),
}, nil
}

Expand Down
Loading

0 comments on commit af0d3d1

Please sign in to comment.