diff --git a/changelog/unreleased/log-uploads.md b/changelog/unreleased/log-uploads.md new file mode 100644 index 0000000000..d0eb570974 --- /dev/null +++ b/changelog/unreleased/log-uploads.md @@ -0,0 +1,6 @@ +Bugfix: enable datatx log + +We now pass a properly initialized logger to the datatx implementations, allowing the tus handler to log with the same level as the rest of reva. + +https://github.com/cs3org/reva/pull/4935 + diff --git a/internal/http/services/dataprovider/dataprovider.go b/internal/http/services/dataprovider/dataprovider.go index f9c14e7b88..0f2558e797 100644 --- a/internal/http/services/dataprovider/dataprovider.go +++ b/internal/http/services/dataprovider/dataprovider.go @@ -22,6 +22,9 @@ import ( "fmt" "net/http" + "github.com/mitchellh/mapstructure" + "github.com/rs/zerolog" + "github.com/cs3org/reva/v2/pkg/appctx" "github.com/cs3org/reva/v2/pkg/events" "github.com/cs3org/reva/v2/pkg/events/stream" @@ -30,8 +33,6 @@ import ( "github.com/cs3org/reva/v2/pkg/rhttp/router" "github.com/cs3org/reva/v2/pkg/storage" "github.com/cs3org/reva/v2/pkg/storage/fs/registry" - "github.com/mitchellh/mapstructure" - "github.com/rs/zerolog" ) func init() { @@ -103,7 +104,7 @@ func New(m map[string]interface{}, log *zerolog.Logger) (global.Service, error) return nil, err } - dataTXs, err := getDataTXs(conf, fs, evstream) + dataTXs, err := getDataTXs(conf, fs, evstream, log) if err != nil { return nil, err } @@ -125,7 +126,7 @@ func getFS(c *config, stream events.Stream) (storage.FS, error) { return nil, fmt.Errorf("driver not found: %s", c.Driver) } -func getDataTXs(c *config, fs storage.FS, publisher events.Publisher) (map[string]http.Handler, error) { +func getDataTXs(c *config, fs storage.FS, publisher events.Publisher, log *zerolog.Logger) (map[string]http.Handler, error) { if c.DataTXs == nil { c.DataTXs = make(map[string]map[string]interface{}) } @@ -144,7 +145,7 @@ func getDataTXs(c *config, fs storage.FS, publisher events.Publisher) (map[strin txs := make(map[string]http.Handler) for t := range c.DataTXs { if f, ok := datatxregistry.NewFuncs[t]; ok { - if tx, err := f(c.DataTXs[t], publisher); err == nil { + if tx, err := f(c.DataTXs[t], publisher, log); err == nil { if handler, err := tx.Handler(fs); err == nil { txs[t] = handler } diff --git a/pkg/rhttp/datatx/manager/registry/registry.go b/pkg/rhttp/datatx/manager/registry/registry.go index 349126be92..3958171d61 100644 --- a/pkg/rhttp/datatx/manager/registry/registry.go +++ b/pkg/rhttp/datatx/manager/registry/registry.go @@ -19,13 +19,15 @@ package registry import ( + "github.com/rs/zerolog" + "github.com/cs3org/reva/v2/pkg/events" "github.com/cs3org/reva/v2/pkg/rhttp/datatx" ) // NewFunc is the function that data transfer implementations // should register at init time. -type NewFunc func(map[string]interface{}, events.Publisher) (datatx.DataTX, error) +type NewFunc func(map[string]interface{}, events.Publisher, *zerolog.Logger) (datatx.DataTX, error) // NewFuncs is a map containing all the registered data transfers. var NewFuncs = map[string]NewFunc{} diff --git a/pkg/rhttp/datatx/manager/simple/simple.go b/pkg/rhttp/datatx/manager/simple/simple.go index 24e4a8f892..5074b3861d 100644 --- a/pkg/rhttp/datatx/manager/simple/simple.go +++ b/pkg/rhttp/datatx/manager/simple/simple.go @@ -22,11 +22,12 @@ import ( "net/http" "time" + userpb "github.com/cs3org/go-cs3apis/cs3/identity/user/v1beta1" + provider "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1" "github.com/mitchellh/mapstructure" "github.com/pkg/errors" + "github.com/rs/zerolog" - userpb "github.com/cs3org/go-cs3apis/cs3/identity/user/v1beta1" - provider "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1" "github.com/cs3org/reva/v2/internal/http/services/owncloud/ocdav/net" "github.com/cs3org/reva/v2/pkg/appctx" "github.com/cs3org/reva/v2/pkg/errtypes" @@ -48,6 +49,7 @@ func init() { type manager struct { conf *cache.Config publisher events.Publisher + log *zerolog.Logger } func parseConfig(m map[string]interface{}) (*cache.Config, error) { @@ -60,22 +62,26 @@ func parseConfig(m map[string]interface{}) (*cache.Config, error) { } // New returns a datatx manager implementation that relies on HTTP PUT/GET. -func New(m map[string]interface{}, publisher events.Publisher) (datatx.DataTX, error) { +func New(m map[string]interface{}, publisher events.Publisher, log *zerolog.Logger) (datatx.DataTX, error) { c, err := parseConfig(m) if err != nil { return nil, err } + l := log.With().Str("datatx", "simple").Logger() + return &manager{ conf: c, publisher: publisher, + log: &l, }, nil } func (m *manager) Handler(fs storage.FS) (http.Handler, error) { h := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + sublog := m.log.With().Str("path", r.URL.Path).Logger() + r = r.WithContext(appctx.WithLogger(r.Context(), &sublog)) ctx := r.Context() - sublog := appctx.GetLogger(ctx).With().Str("datatx", "simple").Logger() switch r.Method { case "GET", "HEAD": diff --git a/pkg/rhttp/datatx/manager/spaces/spaces.go b/pkg/rhttp/datatx/manager/spaces/spaces.go index 743e05d4ae..3003042947 100644 --- a/pkg/rhttp/datatx/manager/spaces/spaces.go +++ b/pkg/rhttp/datatx/manager/spaces/spaces.go @@ -26,6 +26,10 @@ import ( userpb "github.com/cs3org/go-cs3apis/cs3/identity/user/v1beta1" provider "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1" + "github.com/mitchellh/mapstructure" + "github.com/pkg/errors" + "github.com/rs/zerolog" + "github.com/cs3org/reva/v2/internal/http/services/owncloud/ocdav/net" "github.com/cs3org/reva/v2/pkg/appctx" "github.com/cs3org/reva/v2/pkg/errtypes" @@ -39,8 +43,6 @@ import ( "github.com/cs3org/reva/v2/pkg/storage/cache" "github.com/cs3org/reva/v2/pkg/storagespace" "github.com/cs3org/reva/v2/pkg/utils" - "github.com/mitchellh/mapstructure" - "github.com/pkg/errors" ) func init() { @@ -50,6 +52,7 @@ func init() { type manager struct { conf *cache.Config publisher events.Publisher + log *zerolog.Logger } func parseConfig(m map[string]interface{}) (*cache.Config, error) { @@ -62,25 +65,29 @@ func parseConfig(m map[string]interface{}) (*cache.Config, error) { } // New returns a datatx manager implementation that relies on HTTP PUT/GET. -func New(m map[string]interface{}, publisher events.Publisher) (datatx.DataTX, error) { +func New(m map[string]interface{}, publisher events.Publisher, log *zerolog.Logger) (datatx.DataTX, error) { c, err := parseConfig(m) if err != nil { return nil, err } + l := log.With().Str("datatx", "spaces").Logger() + return &manager{ conf: c, publisher: publisher, + log: &l, }, nil } func (m *manager) Handler(fs storage.FS) (http.Handler, error) { h := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - ctx := r.Context() var spaceID string spaceID, r.URL.Path = router.ShiftPath(r.URL.Path) - sublog := appctx.GetLogger(ctx).With().Str("datatx", "spaces").Str("spaceid", spaceID).Logger() + sublog := m.log.With().Str("spaceid", spaceID).Str("path", r.URL.Path).Logger() + r = r.WithContext(appctx.WithLogger(r.Context(), &sublog)) + ctx := r.Context() switch r.Method { case "GET", "HEAD": diff --git a/pkg/rhttp/datatx/manager/tus/tus.go b/pkg/rhttp/datatx/manager/tus/tus.go index bb724c5229..142c23bad3 100644 --- a/pkg/rhttp/datatx/manager/tus/tus.go +++ b/pkg/rhttp/datatx/manager/tus/tus.go @@ -24,11 +24,13 @@ import ( "path" "regexp" + provider "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1" + "github.com/mitchellh/mapstructure" "github.com/pkg/errors" "github.com/rs/zerolog" tusd "github.com/tus/tusd/v2/pkg/handler" + "golang.org/x/exp/slog" - provider "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1" "github.com/cs3org/reva/v2/internal/http/services/owncloud/ocdav/net" "github.com/cs3org/reva/v2/pkg/appctx" "github.com/cs3org/reva/v2/pkg/errtypes" @@ -38,8 +40,6 @@ import ( "github.com/cs3org/reva/v2/pkg/rhttp/datatx/metrics" "github.com/cs3org/reva/v2/pkg/storage" "github.com/cs3org/reva/v2/pkg/storagespace" - "github.com/mitchellh/mapstructure" - "golang.org/x/exp/slog" ) func init() { @@ -59,6 +59,7 @@ type TusConfig struct { type manager struct { conf *TusConfig publisher events.Publisher + log *zerolog.Logger } func parseConfig(m map[string]interface{}) (*TusConfig, error) { @@ -71,14 +72,18 @@ func parseConfig(m map[string]interface{}) (*TusConfig, error) { } // New returns a datatx manager implementation that relies on HTTP PUT/GET. -func New(m map[string]interface{}, publisher events.Publisher) (datatx.DataTX, error) { +func New(m map[string]interface{}, publisher events.Publisher, log *zerolog.Logger) (datatx.DataTX, error) { c, err := parseConfig(m) if err != nil { return nil, err } + + l := log.With().Str("datatx", "tus").Logger() + return &manager{ conf: c, publisher: publisher, + log: &l, }, nil } @@ -100,7 +105,7 @@ func (m *manager) Handler(fs storage.FS) (http.Handler, error) { config := tusd.Config{ StoreComposer: composer, NotifyCompleteUploads: true, - Logger: slog.New(tusdLogger{log: appctx.GetLogger(context.Background())}), // Note: this is a noop logger + Logger: slog.New(tusdLogger{log: m.log}), } if m.conf.CorsEnabled { @@ -154,6 +159,8 @@ func (m *manager) Handler(fs storage.FS) (http.Handler, error) { } h := handler.Middleware(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + sublog := m.log.With().Str("uploadid", r.URL.Path).Logger() + r = r.WithContext(appctx.WithLogger(r.Context(), &sublog)) method := r.Method // https://github.com/tus/tus-resumable-upload-protocol/blob/master/protocol.md#x-http-method-override if r.Header.Get("X-HTTP-Method-Override") != "" { @@ -251,8 +258,16 @@ func (l tusdLogger) Handle(_ context.Context, r slog.Record) error { // Enabled returns true func (l tusdLogger) Enabled(_ context.Context, _ slog.Level) bool { return true } -// WithAttrs is not implemented -func (l tusdLogger) WithAttrs(_ []slog.Attr) slog.Handler { return l } +// WithAttrs creates a new logger with the given attributes +func (l tusdLogger) WithAttrs(attr []slog.Attr) slog.Handler { + fields := make(map[string]interface{}, len(attr)) + for _, a := range attr { + fields[a.Key] = a.Value + } + c := l.log.With().Fields(fields).Logger() + sLog := tusdLogger{log: &c} + return sLog +} // WithGroup is not implemented -func (l tusdLogger) WithGroup(_ string) slog.Handler { return l } +func (l tusdLogger) WithGroup(name string) slog.Handler { return l }