Deprecate old JS Module "API"

[na--]

I think we should deprecate JS extensions (i.e. extensions to the JS API written in Go :sweat_smile:) that don't implement the JS Module interface: https://github.com/grafana/k6/blob/1993ac1b6d47d3504631648d3cf02aebcd5adfbf/js/modules/modules.go#L27-L32

JS Modules that don't implement this API are basically an interface{} that is directly given to goja to deal with. For some historical context:

• all k6 modules used to be like that a long time ago
• however, we have had the new Module API since k6 v0.34.0/v0.35.0 (~1.5 years ago, see https://github.com/grafana/k6/pull/2108 https://github.com/grafana/k6/pull/2234)
• after that, all native JS modules were moved to the new module interface
• we [dropped the common.Bind() interface]() that old JS modules used to get the VU information in k6 v0.38.0 (May 2022, almost a year ago); at this point, most of the "simple" JS extensions needed to be rewritten if they wanted to send metrics or do anything that involved working with k6

At this point, unless we have a specific reason to support these interface{} JS modules I don't know about, I think it's fine to officially and fully deprecate them, right?

My proposal is that:

1. We simplify the xk6 JS extension docs - completely remove this section from the documentation now and only give an example on how the new Module API is used, i.e. move this section to the top.
2. In k6 v0.44.0, we announce the deprecation of the old modules explicitly mark all built-in modules to the new type (i.e. make this map from map[string]interface{} to map[string]modules.Module) and start emitting a warning if such an xk6 extension module is used that doesn't implement it.
3. Maybe add a warning in xk6 when an old k6 version is used during the build process (https://github.com/grafana/xk6/issues/52).
4. In k6 v0.45.0 or v0.46.0, we completely remove that capability from k6 to run the old interface{} modules, the js/modules.Register() method will start requiring a modules.Module as an argument and any old JS extensions will need to be rewritten.

[imiric]

Repeating my point from Slack: while this would simplify some of our maintenance and docs, I don't see why we should force developers of basic extensions to upgrade to the much more complex modules.Module implementation, if they have no need for any of the advanced functionality.

Like @mstoykov mentioned, 44 out of 73 extensions currently don't use modules.Module, and even if we remove the ones that are broken on recent k6 versions, that still leaves many extensions that work perfectly fine with the simple API, and arguably don't need anything more complex than that.

There are many such extensions, like xk6-file, xk6-csv and Mihail's xk6-counter. All of these build and work with k6 v0.43.1 as is, and will likely countinue to build and work without having to upgrade. Sure, you may consider them "toys", but they're probably very useful to some users.

By deprecating this API we're essentially breaking all of these extensions, and since they now need to actually depend on a larger API that may not be stable, we're forcing them to have to update more frequently, which as we've seen, just isn't likely to happen.

I'm not strongly against this, mind you, just see more reasons against it than for, but I'm curious what everyone else thinks.

[na--]

I disagree with the point that the new API is "much more complex" or harder to grok than the old "API". The old one was basically "yeehaw, somehow your Go code is usable in JS, but don't ask me how or how it maps to a VU, YOLO!" :rofl: The new API actually has an... API... :sweat_smile: And the boilerplate, if you don't need the new capabilities, is only something like 10-15 lines on top.

This is a minimal example if you don't care about any of the new APIs or doing anything complicated with your module:

func init() {
    modules.Register("k6/x/compare", &RootModule{})
}

type RootModule struct{}

func (*RootModule) NewModuleInstance(_ modules.VU) modules.Instance {
    return &ModuleInstance{}
}

type ModuleInstance struct {}

func (mi *ModuleInstance) Exports() modules.Exports {
    return modules.Exports{
        Default: yourOldLogic,
    }
}

Considering the init() had to be there even before, I don't consider the rest that much of an overhead for all of the sanity that is received in exchange :sweat_smile:

That said, I am not against a longer grace period though. We can wrap the old ad-hoc extensions in some pseudo-Module, as @mstoykov kind of already does in https://github.com/grafana/k6/pull/2881. Especially if we also reverse the xk6 policy from https://github.com/grafana/xk6/pull/44 to not build with the latest k6 version by default, so that extension authors and users will actually see these warnings...

[mstoykov]

Some data, and the way it was "generated":

I did have repos checked out locally from a year+ ago so the previous results were a bit ... wrong :(, sorry.

I basically have a script that checkouts all the repos as in:

 for i in `curl 'https://raw.githubusercontent.com/grafana/k6-docs/master/src/data/doc-extensions/extensions.json' | jq -r '.extensions | .[] | .url'` ; do  git clone ${i} || echo $i >> broken.txt; done

Which gets the data about extensions from the "registry"

This script checkouts 69 repos at this point and there are no repos that are "broken" at this point in time.

Then I run

for i in xk6* ; do cd $i; echo -n "$(git remote -v) " ; rg ModuleInstance . | wc -l  ;cd - ; done  | grep '(push)'

Which will (badly) print the URL of the repo + how many times ModuleInstance is seen in the repo as a very crude way of seeing if it used the new API.

Then if you grep for \b0\b and count with

for i in xk6* ; do cd $i; echo -n "$(git remote -v)" ; rg ModuleInstance . | wc -l  ;cd - ; done  | grep '(push)' | grep "\b0\b" | wc -l  

you get 46 - so that many extension do not have ModuleInstance in the code but if we do remove ones having output

for i in xk6* ; do cd $i; echo -n "$(git remote -v)" ; rg ModuleInstance . | wc -l  ;cd - ; done  | grep push | grep "\b0\b" | grep -v output  | wc -l

you get 37 and 10 of those are by szkiba whose most extensions just won't compile or even if they do they do use the old context.Context as first argument which will not work with current versions.

So we are left with 27

$ for i in xk6* ; do cd $i; echo -n "$(git remote -v)" ; rg ModuleInstance . | wc -l  ;cd - ; done  | grep push | grep "\b0\b" | grep -v output   | grep -v szkiba
origin  https://github.com/grafana/xk6-amqp (push)0
origin  https://github.com/xvzf/xk6-avro (push)0
origin  https://github.com/tmieulet/xk6-cognito (push)0
origin  https://github.com/thotasrinath/xk6-couchbase (push)0
origin  https://github.com/dgzlopes/xk6-datadog (push)0
origin  https://github.com/grafana/xk6-docker (push)0
origin  https://github.com/BarthV/xk6-es (push)0
origin  https://github.com/avitalique/xk6-file (push)0
origin  https://github.com/skibum55/xk6-git (push)0
origin  https://github.com/AckeeCZ/xk6-google-iap (push)0
origin  https://github.com/heww/xk6-harbor (push)0
origin  https://github.com/gpiechnik2/xk6-httpagg (push)0
origin  https://github.com/dgzlopes/xk6-interpret (push)0
origin  https://github.com/dgzlopes/xk6-kv (push)0
origin  https://github.com/gjergjsheldija/xk6-mllp (push)0
origin  https://github.com/GhMartingit/xk6-mongo (push)0
origin  https://github.com/patrick-janeiro/xk6-neo4j (push)0
origin  https://github.com/wosp-io/xk6-playwright (push)0
origin  https://github.com/Juandavi1/xk6-prompt (push)0
origin  https://github.com/olvod/xk6-pubsub (push)0
origin  https://github.com/gpiechnik2/xk6-smtp (push)0
origin  https://github.com/mridehalgh/xk6-sqs (push)0
origin  https://github.com/grafana/xk6-ssh (push)0
origin  https://github.com/NAlexandrov/xk6-tcp (push)0
origin  https://github.com/maksimall89/xk6-telegram (push)0
origin  https://github.com/dgzlopes/xk6-url (push)0
origin  https://github.com/vvarp/xk6-wamp (push)0

A different script

#!/bin/sh
 for i in `curl 'https://raw.githubusercontent.com/grafana/k6-docs/master/src/data/doc-extensions/extensions.json' | jq -r '.extensions | .[] | .url'` ; do  xk6 build master --with ${i##https:\/\/} || echo $i >> broken.txt; done

Will gives us what extensions do not build with the current master and the list is 17 long:

https://github.com/vvarp/xk6-wamp
https://github.com/dgzlopes/xk6-kv
https://github.com/olvod/xk6-pubsub
https://github.com/gjergjsheldija/xk6-mllp
https://github.com/szkiba/xk6-crypto
https://github.com/szkiba/xk6-cache
https://github.com/szkiba/xk6-mock
https://github.com/szkiba/xk6-faker
https://github.com/szkiba/xk6-dashboard
https://github.com/grafana/xk6-output-kafka
https://github.com/xvzf/xk6-avro
https://github.com/grafana/xk6-loki
https://github.com/tinkoff/xk6-output-error
https://github.com/dynatrace/xk6-output-dynatrace
https://github.com/temporalio/xk6-temporal
https://github.com/BarthV/xk6-es
https://github.com/heww/xk6-harbor

This is only about compilation so:

1. https://github.com/grafana/xk6-loki needs a -replace argument to build so it is kind of a false negative
2. Most extensions might build but will not run if they have context.Context in them which using the above technics gives us 34 repos, but it will be a lot harder to actually see if that means they will not run or they have legitimate use for it somewhere else in the code

edit: I have now published some code around this as a repo that ... I might or might not try to maintain

[olegbespalov]

After an internal discussion, we decided to clean a milestone since the issue was jumping between milestones without completion.

Once we determine which milestone it lands, we set the right one.