Core changes for k6/x/execution JS module

[imiric]

These are core changes needed for the k6/x/execution JS module.

Initially the module was part of core as well, but we decided to extract it into an extension to be able to quickly iterate on the API. See the JS usage and example in the extension repo.

This also inadvertently fixes the segmentation of iterations for the ramping-arrival-rate executor. See #1863 (comment).

Part of #1320

[mstoykov]

Haven't taken a deeper look or tried the code, but some comments:

1. IMO we should allow getting the info in all cases where any info is available
2. The things that call functions should probably call the functions on the spot (without any caching) or be functions in the first place
3. Given that a lot of the *State objects seem "static" can we have 1 of those and just give it everywhere
4. given 3. (and even without it) can you check and make certain that a user script can't change any of the values?

[imiric]

@mstoykov:

IMO we should allow getting the info in all cases where any info is available

Are you referring to being able to call this from init? Most of this information doesn't make sense outside the context of a running VU/scenario. Only vus_max seems relevant in init and we could get it from the config instead of *ExecutionState, but besides complicating things slightly it would be weird to have to document which information is available and which isn't, instead of just disallowing it entirely. If we go with your 2nd suggestion below, it might make sense, so I'll think about it some more. :)

The things that call functions should probably call the functions on the spot (without any caching) or be functions in the first place

Hhmm I'm not sure I prefer the API to be something like getTestStats().duration() or complicating it with DynamicObject like we discussed in #1320. Returning an already evaluated object is easier to understand (users don't expect proxies) and these are small objects so the performance benefit would likely be negligible.

I could give it a shot if you feel strongly about it, though would prefer the DynamicObject approach in that case.

Given that a lot of the *State objects seem "static" can we have 1 of those and just give it everywhere

Hhmm you mean about *ScenarioState specifically and how it's set in the context each time a VU is activated? True, it would be better if it was set on the context without being passed around in VUActivationParams. I'll look into it, thanks.

But *State and *ExecutionState are only set once.

Ah, no, you're referring about the map[string]interface{} being created every time? Indeed, good point, will avoid that too.

given 3. (and even without it) can you check and make certain that a user script can't change any of the values?

Sure. With dumb / already evaluated objects this shouldn't matter, but I'll confirm after your suggested changes.

[na--]

I have only skimmed the code changes, so I haven't fully grokked the PR yet, but I have a more generic comment here. I think the MVP version of #1320 shouldn't be exposing information users can already mostly get. It should be exposing things that they cannot currently get in any way. Primarily because it would require some refactoring to the executors and may change how the rest of the feature is implemented.

From what I can see, exec.getVUStats() would return exactly __VU and __ITER, right? While it is nice to have a better UX than the current super-global "constants" for that information, it's not necessary to do so initially, users manage just fine without it. exec.getScenarioStats() is new information, so better, but even now you can get most of that information by just having different exec function (or via environment variables in each scenario). The rest of the information from exec.getScenarioStats() and the whole exec.getTestStats() is novel and previously inaccessible information, true, but probably wouldn't actually be useful to users from inside of the script very often...

I think the MVP of this feature should have these two/three pieces of information, with everything else being a nice-to-have bonus:

• in the current scenario, what is the ID number of the VU currently running the iteration
• in the current scenario, what is the sequential number of the iteration for this VU that is currently being executed
• for shared-iterations, constant-arrival-rate and variable-arrival-rate scenarios, a way to get the sequential number of the current iteration among all of the iterations for the current scenario, across all VUs and even instances.

The first 2 are the equivalent of __VU and __ITER, but only in terms of a single scenario. The last one is a very useful feature for data sharding and it should be easy to calculate with the execution segments. But all of these are information that is completely unavailable to user scripts right now. So, yeah, I may be wrong, but I think we should start with the "hard" (i.e. unknown) parts of this issue first, and implement the "easy" (i.e. known) parts last, as a polish to everything.

[imiric]

@na-- OK, so to clarify, that would involve adding a few additional executor-specific counters, right? Except for the last global across instances values which could be calculated given the execution segment?

[na--]

OK, so to clarify, that would involve adding a few additional executor-specific counters, right?

Yeah, basically a local VU number for the scenario (basically immutable after the VU activation in the scenario) and a local iteration number for the specific VU in the scenario (mutable, but the only lock contention here would be between whatever increments it and this new exec method used to retrieve it, so pretty light). Still, as I said, those things require some refactoring of the executors... 😞

[codecov-io]

Codecov Report

Merging #1863 (a7456c3) into master (51790fc) will increase coverage by 0.20%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master    #1863      +/-   ##
==========================================
+ Coverage   71.50%   71.70%   +0.20%     
==========================================
  Files         180      181       +1     
  Lines       13969    14008      +39     
==========================================
+ Hits         9988    10045      +57     
+ Misses       3344     3327      -17     
+ Partials      637      636       -1     

Flag	Coverage Δ
ubuntu	71.64% <100.00%> (+0.17%)	⬆️
windows	71.29% <100.00%> (+0.19%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
lib/executors.go	93.10% <ø> (ø)
core/local/local.go	75.82% <100.00%> (+0.11%)	⬆️
js/modules/k6/execution/execution.go	100.00% <100.00%> (ø)
js/runner.go	80.97% <100.00%> (+0.11%)	⬆️
lib/context.go	100.00% <100.00%> (ø)
lib/executor/constant_arrival_rate.go	96.42% <100.00%> (ø)
lib/executor/constant_vus.go	96.20% <100.00%> (ø)
lib/executor/externally_controlled.go	76.22% <100.00%> (+0.45%)	⬆️
lib/executor/helpers.go	96.38% <100.00%> (ø)
lib/executor/per_vu_iterations.go	97.05% <100.00%> (ø)
... and 8 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 51790fc...4be00f3. Read the comment docs.

[imiric]

So it's unfortunately safe to say that this won't make the v0.31.0 release. 😞

I think the functionality described here is implemented now, but it will need thorough testing and actually making the tests pass consistently.

Please give the code an overview, the commits should be fairly isolated, and test it on your machine to ensure it's at least functionally OK.

What's pending:

• Fixing the flaky tests. I think I managed to find a timing sweet spot for the *GlobalIters tests (they passed for more than 30m with this script), but it's clear from the Windows failures that the others are still flaky. I'm not sure how to refactor them to avoid this, so any ideas are welcome.

• Addressing @mstoykov's comments above, which I started doing but ran into issues with sharing the object across goja runtimes.

• Addressing any additional feedback.

[imiric]

We're currently only tracking completed/full iterations. Would tracking interrupted iterations too be interesting to users? It would essentially double the amount of counters for everything... :-/

[codecov-commenter]

Codecov Report

Merging #1863 (739bb55) into master (9f3dd60) will increase coverage by 0.17%.
The diff coverage is 92.83%.

❗ Current head 739bb55 differs from pull request most recent head 0279c67. Consider uploading reports for the commit 0279c67 to get more accurate results

@@            Coverage Diff             @@
##           master    #1863      +/-   ##
==========================================
+ Coverage   71.99%   72.17%   +0.17%     
==========================================
  Files         181      181              
  Lines       14149    14283     +134     
==========================================
+ Hits        10187    10309     +122     
- Misses       3338     3348      +10     
- Partials      624      626       +2     

Flag	Coverage Δ
ubuntu	72.08% <92.83%> (+0.14%)	⬆️
windows	71.80% <92.83%> (+0.17%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
js/bundle.go	93.54% <ø> (ø)
lib/executors.go	93.10% <ø> (ø)
lib/state.go	100.00% <ø> (ø)
lib/context.go	46.66% <20.00%> (-53.34%)	⬇️
js/runner.go	82.40% <86.53%> (-0.42%)	⬇️
lib/testutils/minirunner/minirunner.go	81.15% <87.09%> (+0.72%)	⬆️
lib/executor/base_executor.go	84.00% <87.50%> (+6.22%)	⬆️
core/local/local.go	78.19% <100.00%> (+0.10%)	⬆️
lib/execution.go	92.59% <100.00%> (+0.35%)	⬆️
lib/execution_segment.go	92.73% <100.00%> (+1.57%)	⬆️
... and 13 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9f3dd60...0279c67. Read the comment docs.

[imiric]

@mstoykov @na-- Could you take another look and let me know if there's anything pending here?

From what we discussed, allowing the use of this module in the init context is the major thing still missing. After taking another look, I still think it would require substantial refactoring to achieve, and most of the information just doesn't make sense in the init context.

The big (only?) use case that does make sense is retrieving the max test duration before the test starts, as mentioned on the forum and here. The problem with this is that the init context is evaluated before the ExecutionScheduler/ExecutionState is created.
This is a chicken/egg situation since we need to have access to the consolidated options, including the ones exported in JS, to be able to return the correct duration, but these aren't consolidated until after the init context is evaluated for the first time (see cmd/run.go).
I considered adding lib.Options to common.InitEnvironment to precalculate the max duration/VUs and avoid depending on ExecutionScheduler, but we run into the same chicken/egg thing, since options aren't consolidated when InitEnvironment is first initialized.

A possible approach would be to offer a pure JS API to calculate and return just the max duration, but that would duplicate a lot of the same logic we have in Go, and it doesn't seem worth it considering it can be done with the current implementation from the default function, and once we have #1001 to abort the test, it would fulfill the requirement of the person on the forum.

[na--]

[mstoykov]

LGTM 👏