grafana / k6x

Run k6 with extensions
GNU Affero General Public License v3.0
13 stars 3 forks source link

Add --with option to specify extensions to build #5

Open pablochacin opened 11 months ago

pablochacin commented 11 months ago

In some cases, it would be desirable to allow the user to specify the extensions that should be built into the binary produced by k6x:

  1. With the build subcommand, for building a custom binary to be used with multiple tests (similar to using xk6)
k6x build --with=github.com/grafana/xk6-disruptor
  1. With the run subcommand, to allow specifying the version of the extensions without having to modify the code
k6x --with=github.com/grafana/xk6-disruptor@v0.3.0  run test.js
  1. With the run subcommand, when we want to ensure only a set of pre-approved extensions can be used and prevent the user from using other extensions in their tests (accidentally or maliciously)
k6x  --with=github.com/grafana/xk6-disruptor run test.js

Cases 1 and 3 require disabling the automatic dependency discovery. In the case of the build subcommand, the test script does not need to be provided.

In case 2 it could still be valid to combine information from the --with argument with the discovery of dependencies, but this creates some complexity in the CLI interface, so we suggest disabling auto-discovery when the with option is specified.

szkiba commented 11 months ago

Hi @pablochacin , Thanks for the suggestion. Frankly, in its current form, the --with option does not fit the concept of k6x, nor does skipping the extension detection step. Automatic extension detection and discovery is one of the main features of k6x. In addition, this option can be confusing from a UX point of view.

I think I understand the use cases, I will try to describe what I understood and look for a solution that does not contradict the concept of k6x.

  1. Loading the k6 binary into the cache (aka build) independently of the test script, based on a given extension list

  2. Defining a version constraint for extensions in the command line

  3. Limit the extensions that can be used in the test script (aka filter the extension registry for specific extensions)

Please confirm whether I understood the requirements correctly and then I will work out how this can be implemented in accordance with the concept of k6x.

pablochacin commented 11 months ago

Hi @szkiba I understand that this feature does not fit the main concept from which k6x was developed, but I think k6x does more than just autodiscovery and the idea of this option is to reuse these other functionalities in other contexts

The ability to build and run k6 with the required extensions in one single step is still valuable, regardless of the auto-discovery. More so when considering the cache, that would improve the process in case multiple tests require different combinations of extensions.

One alternative I see is to retrofit some of these functionalities in xk6, but this seems a more complex task than exposing them from k6x.

szkiba commented 11 months ago

Hi @pablochacin, As I wrote, I am trying to understand what the requirements are and to provide a solution that does not contradict the concept of the k6x.

Yes, k6x is not only about automatic detection of extensions. Building and running in one step is also its main function. I understand that it could be used for something else and I would like to solve it. However, it is important to ensure that the user experience is not confused by parameters with different syntax used for the same purpose.

The syntax of the proposed --with option is different from the syntax of the "use k6..." pragma. The --with switch specifies the extension as go mdule with the "use k6..." pragma as the name used by the user. The --with switch specifies the version in the form of @tag and the "use k6 ..." pragma specifies the version in the form of a version constraints expression. Because of such contradictions, I wrote that in its current form it worsens the user experience.

The expectations you expressed inspired the following features:

Using these, your use cases can be solved as follows:

  1. building a custom binary to be used with multiple tests (similar to using xk6)

    k6x build --use "with k6/x/disruptor" /dev/null
  2. specifying the version of the extensions without having to modify the code

    k6x --use "with k6/x/disruptor v0.3.0"  run test.js
  3. only a set of pre-approved extensions can be used and prevent the user from using other extensions in their tests

    k6x  --use "import allow k6/x/disruptor" --use "import deny k6/x/**" run test.js
pablochacin commented 11 months ago

Hi @szkiba for me the name of the flag (--with or --use) is much less relevant than the functionality.

It also makes sense to use the same syntax as the pragmas that are used in the js code.

From the usability perspective, I only would say that forcing to pass /dev/null to the build subcommand is not ergonomic. I think in this subcommand the input script should be optional.

szkiba commented 11 months ago

Hi @pablochacin , The k6x API consists of the command line and pragmas, i.e. the command line flags are part of the API design and therefore the user experience. It's relevant to me :)

In the case of /dev/null, the concept was that its use explicitly indicates that there is no input. That is, for example, if the argument is substituted from an environment variable, it does not happen that the build is accidentally run with an empty variable value and the error occurs later. Since this is not a very common use and --use flag will be introduced, I have reconsidered and the argument will be optional for the build (and deps) command.

Since you didn't say anything else, I assume you will be able to solve your use cases with the help of the above two planned features.

pablochacin commented 11 months ago

In the case of /dev/null, the concept was that its use explicitly indicates that there is no input.

I hadn't considered the case of variable substitution. Good point.

Since you didn't say anything else, I assume you will be able to solve your use cases with the help of the above two planned features

Yes, I think your proposal covers the use cases pretty well. Thanks.

szkiba commented 11 months ago

Hi @pablochacin , During implementation, I decided to implement #10 instead of #8, because it gives a better user experience. In addition, your use cases can be solved without #7. #7 is not included in the scope of the k6x, so I did not implement it.

According to these, your use cases can be solved as follows:

  1. building a custom binary to be used with multiple tests (similar to using xk6)

    k6x build --with k6/x/disruptor
  2. specifying the version of the extensions without having to modify the code

k6x run --with "k6/x/disruptor v0.3.0"  run test.js
  1. only a set of pre-approved extensions can be used and prevent the user from using other extensions in their tests
k6x build --with k6/x/disruptor && ./k6 run test.js

or

k6x build --with k6/x/disruptor --bin-dir ./custom-k6 && ./custom-k6/k6 run test.js

The implementation is included in the v0.3.0 release. I would appreciate your feedback if you successfully tried the new release and managed to solve your use cases with it.

szkiba commented 11 months ago

The v0.4.0 includes filtering capability, so 3. can solved by using something like this:

k6x --filter "[?name == 'xk6-disruptor']" run test.js