Option to specify context when creating file and location objects

[NathanBaulch]

Something I find lacking in this library is the ability to trace outbound requests. It's possible to configure instrumented clients for most backends but common tracing tools like OpenTelemetry use context to pass state around.

Rather than breaking every API by adding context.Context arguments (as previously discussed), I propose the gs.WithContext pattern be applied more widely to NewLocation and NewFile. This would allow a web application to have a single shared FileSystem (or Location) and inject the current request context when working with specific files.

Here's a minimal example:

ctx := context.Background()
cfg, _ := config.LoadDefaultConfig(ctx)
cli := _s3.NewFromConfig(cfg, func(opts *_s3.Options) {
	// instrument S3 client
	opts.HTTPClient = &http.Client{Transport: otelhttp.NewTransport(http.DefaultTransport)}
})
// pass default global context to fs
fs := s3.NewFileSystem(s3.WithClient(cli), s3.WithContext(ctx))
backend.Register(fs.Name(), fs)

http.HandleFunc("/version", func(w http.ResponseWriter, r *http.Request) {
	// pass request context to file
	f, _ := fs.NewFile("", "version.txt", newfile.WithContext(r.Context()))
	c, _ := io.ReadAll(f)
	_, _ = fmt.Fprintf(w, "version: "+string(c))
})

_ = http.ListenAndServe(":8080", nil)

The other nice thing about this approach is it doesn't pollute the API for backends that don't support context, like mem, os and sftp.

This is a non-breaking additive change so in theory it could be included in v7, but I understand if it needs to wait for v8.

[funkyshu]

I'm still thinking this through but I think the s3 example suggestions make sense.

[funkyshu]

Should this actually be on the FS? What happens when I do:

parent := context.Background()
ctx, cancel := context.WithCancel(ctx) 
loc1, _ := s3fs.NewLocation(auth, somePath, WithContext(timer))
defer cancel()

todoCtx := context.TODO()
loc2, _ := s3fs.NewLocation(auth, otherPath, WithContext(todoCtx))

Since todoCtx replaces, at least in the fs.context (which is always pass the to the GetClient) there really is no cancellation anymore. Same would happen with WithTimeout or some other value.

I kindof feel like there should be a NewFileSystem context option (no option default so context.Background()), that is used by any Location or File action that needs it, UNLESS we've specifically called NewLocation or NewFile with another context. So Location inherits ctx form FileSysetm and File (depending on where it NewFile was called (FileSystem or Location) inherits from its parent. However, any options passed to the "new" would override.
So filesystem.go is like:

func NewFilesyetm(some, args, ...opts) *FileSystem
    // setup fs struct with default background ctx
    fs := &FileSystem{
        context = context.Background()
    }

    // apply option (would override with any provided context option)
    ...
    return fs
}

func (fs *FileSytem) NewLocation(args, opts) *Location {
    // setup Location with default **FileSystem** context
    l := &Location{
        context = fs.context
    }

    // apply option (would override with any provided context option)
    ...
    return l
}

func (fs *FileSytem) NewFile(args, opts) *File {
    // setup File with default **FileSystem** context
    f := &File{
        context = fs.context
    }

    // apply option (would override with any provided context option)
    ...
    return f
}

then in location.go:

func (l *Location) NewFile(args, opts) *File {
    // setup File with default **Location** context
    f := &File{
        context = l.context
    }

    // apply option (would override with any provided context option)
    ...
    return f
}

In this way, any call to sub- struct initializers are independent of its parent when a new context is passed.

What do you think?

[NathanBaulch]

I must be missing something because I've read your comment above a few times and I can't quite figure out what you're asking/suggesting. 😟

Context is only ever set in New* functions and never changed.
Files inherit the context of their parent FS/location unless overridden via newfile.WithContext.

Since todoCtx replaces, at least in the fs.context (which is always pass the to the GetClient) there really is no cancellation anymore.

It's true that in the s3 backend GetClient will always use the context of the root FS, however that's only used briefly when resolving the default AWS config. All client operations take a fresh context when used.
In your example, loc1 remains cancellable while loc2 is not.

You mention // apply option (would override with any provided context option) in the suggested code snippets, but isn't that what I'm already doing with the following code blocks?

ctx := l.ctx
for _, o := range opts {
	switch o := o.(type) {
	case *newfile.Context:
		ctx = context.Context(o)
	default:
	}
}

Apologies if I've misunderstood.

[funkyshu]

I'll try to get back to this tomorrow.