grafana / k6

A modern load testing tool, using Go and JavaScript - https://k6.io
GNU Affero General Public License v3.0
26.03k stars 1.27k forks source link

Provide a file storing API #3017

Open ka3de opened 1 year ago

ka3de commented 1 year ago

Feature Description

Many times extensions might want to store files associated with the test run. For example, in the case of k6 browser, the user has the capability of taking screenshots of the test execution and store them as a file in FS. Currently this functionality is delegated to the extensions implementation, but it would probably make sense to centralize the file storing functionality in k6 and expose its API to extensions.

There are a couple of benefits that I can think for this:

In future iterations, this API could possibly be also exposed to the JS API.

Suggested Solution (optional)

From the options perspective, there could be a new option specifically to toggle the files storing functionality, for example between local (default) and cloud. E.g.:

k6 run --files cloud script.js

Maybe some restrictions should apply to this option in sync with --out option. E.g.: File storing option can only be set to cloud if metrics output option is also set to the cloud.

From the Go API perspective, a simple API can be exposed in order to handle file storing functionality associated with the test run. E.g.:

// This is very raw, just to put an example of how it could look like
// Buf could be an io.Reader abstraction
Store(path string, contentType string, buf []byte) error

Path will contain the path and name for the file. In this case, if k6 is running with --files cloud option, the path might be ignored or be used as a "key" for the remote storing platform.

Already existing or connected issues / PRs (optional)

Superseeds https://github.com/grafana/xk6-browser/issues/839

na-- commented 1 year ago

:+1: in general, though this is definitely deep evaluation needed territory. We need to consider some things more carefully, for example:

  1. We probably don't want to start with an API that has contentType string in the signature. The content type is usually determined by the contents themselves. The k6 API shouldn't care too much about the contents, it should just be concerned with saving some blobs.
  2. We don't want buf []byte, since that will be a problem for big files (e.g. video recordings). We already have that problem when reading files with open(), we shouldn't duplicate it when writing files... :sweat_smile: See https://github.com/grafana/k6/issues/2978 and https://github.com/grafana/k6/issues/2974 and their connected issues for more details. Writing files should happen in some streaming fashion, e.g. with an io.Writer or something like that.
  3. Given that we are in the midst of refactoring how k6 handles reading from the file system (see issues :arrow_up: + https://github.com/grafana/k6/issues/2977, https://github.com/grafana/k6/issues/1079), it makes sense to consider whether this API shouldn't just be a part of that effort. That might have a lot of benefits, including better permission management (e.g. from allowing k6 to write to the FS to disallowing it to even read stuff from the local filesystem, which it can currently do by default).
  4. Similarly to outputs and JS modules, this file API is a very good candidate for extending with xk6 extensions. Maybe not initially, but this is definitely something that should be considered in the design. For example, if we allow users to read and write files directly from S3, it might solve the use case for loading large files with test fixtures for a lot of users.

We don't need to figure out all of these things and have a perfect holistic design at the start. However, we should at least consider them carefully, and we should ensure that any public options or APIs we expose to users don't tie our hands and prevent us from implementing a better and more complete solution in the future. For example, --files flag as it was suggested is probably wildly insufficient and needs some careful design work and future-proofing :sweat_smile:

ka3de commented 1 year ago

Thanks for your quick response @na-- !

In regards of the points you mentioned:

  1. We probably don't want to start with an API that has contentType string in the signature.

Yes, I thought maybe contentType could be handy in order to add metadata to the file upload that could be useful int future for clients retrieving these files (e.g.: cloud FE), but we can also work with "abstract" blobs and detect that later on if necessary.

  1. We don't want buf []byte, since that will be a problem for big files (e.g. video recordings).

Absolutely, that was a kind of biased comment based on screenshots functionality, but I also made a comment that we probably want to use some some reader abstraction that allows us to read and upload the file in chunks so we can work with large files in the future :+1:

  1. ... it makes sense to consider whether this API shouldn't just be a part of that effort ...

I think that makes sense.

  1. Similarly to outputs and JS modules, this file API is a very good candidate for extending with xk6 extensions.

That's a very good point. Initially I thought on keeping this simple, but I can definitely see use cases to extend this, for example to use different authentication mechanisms with the remote storage than the ones implemented by default in k6, or some other constraints that users might require to upload test related files to their own storage directly :+1:

--files flag as it was suggested is probably wildly insufficient and needs some careful design work and future-proofing sweat_smile

Agree, I'm not much sold on --files flag either, it was just an example. Initially I was thinking on some --media flag, but this probably extends to more things so.. Don't really have a candidate, naming is difficult :smile: