Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Provide a file storing API #3017

Open
ka3de opened this issue Apr 11, 2023 · 2 comments
Open

Provide a file storing API #3017

ka3de opened this issue Apr 11, 2023 · 2 comments
Labels
evaluation needed proposal needs to be validated or tested before fully implementing it in k6 feature

Comments

@ka3de
Copy link
Contributor

ka3de commented Apr 11, 2023

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:

  • k6 can decide when these test run associated files have to be stored locally or uploaded to the cloud or another remote storage based on environment and/or cmdline flags.
  • k6 is in a better position (compared to extensions) in order to guarantee file upload based on the global test execution. For example waiting for possible remaining file upload processes at the end of the test run (up to a specified TO).

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 grafana/xk6-browser#839

@ka3de ka3de added the feature label Apr 11, 2023
@na-- na-- added the evaluation needed proposal needs to be validated or tested before fully implementing it in k6 label Apr 11, 2023
@na--
Copy link
Member

na-- commented Apr 11, 2023

👍 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... 😅 See Add Streams API support to k6 #2978 and Improving handling of large files in k6 #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 ⬆️ + Design a File API for k6 #2977, Replace afero with the new Go io/fs package #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 😅

@ka3de
Copy link
Contributor Author

ka3de commented Apr 12, 2023

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. ... 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 👍

--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 😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
evaluation needed proposal needs to be validated or tested before fully implementing it in k6 feature
Projects
None yet
Development

No branches or pull requests

2 participants