Hooks for extensions - e.g. graceful shutdown, per VU/iteration, etc.

[imiric]

Feature Description

Currently k6 extensions are abruptly terminated in some scenarios, e.g. when pressing ^C, as we've found out in grafana/xk6-browser#268 (comment). This doesn't leave any time for extensions to cleanup and do a clean shutdown, which in xk6-browser's case would involve closing the browser connection, though in general k6 should ensure that any pending event loop operations are processed as well. For example, it would be good DX to allow the Promise rejection handlers to run, but this is up for debate whether ^C should interrupt those as well.

Suggested Solution

One or more of these might be a good idea:

• Passing a cancelable context to extensions via the moduleVUImpl here. This context could then be cancelled first whenever k6 starts shutting down (whether that's abruptly with ^C or otherwise).

• Allow extensions to specify a callback for "deregistration"/cleanup, either via the modules.Register() method, or another moduleVUImpl method. k6 would then run this first before shutdown. A drawback with this might be that callbacks might run an arbitrary amount of time, but we can always timeout on it.

• Maybe alternatively to the point above, pass a "shutdown" channel to extensions that extensions can use to signal that they're done with cleanup. This might be more flexible and simpler to timeout on.

• If we do decide to have a timeout, it should probably use a sane default value but be user configurable.

Already existing or connected issues / PRs

• Implement Browser.on(event) xk6-browser#268

[mstoykov]

I feel like maybe some kind of full blown event system that possibly is also available to js scripts will be better solution. Some links:

• https://doc.deno.land/deno/stable/~/Event
• https://doc.deno.land/deno/stable/~/ErrorEvent
• https://doc.deno.land/deno/stable/~/addEventListener
• https://doc.deno.land/deno/stable/~/dispatchEvent
• https://doc.deno.land/deno/stable/~/MessageEvent
• https://nodejs.org/dist/latest-v12.x/docs/api/events.html#events_passing_arguments_and_this_to_listeners
• https://developer.mozilla.org/en-US/docs/Web/Events

[ankur22]

I'm going to describe the problem that we're having in the context of xk6-browser:

1. We can't guarantee that we can delete any temporary directories that we create, which is important as the browser may have cached sensitive information when running the test (mostly important when we get this running in the cloud for our customers)
2. We can't guarantee a graceful shutdown of the browser itself which could lead to misleading logs or unexpected behaviour when the browser is killed (go kills sub processes when the context is closed).

The suggestions outlined in the description of this issue are good. Just some points that I think should be covered or considered in this work:

• Since multiple extensions can be combined, it might be a good idea to make the process of signalling and waiting for the extensions to shutdown concurrent.
• It would also be a good idea to set some sort of timeout using a new context that is passed to the extension's shutdown method. This which would allow the extension to log a message if the shutdown is taking too long and so allow the developer to find ways to speed up the shutdown OR increase the shutdown timeout.
• It's probably ok to have a global timeout for all extensions since we will need to set a timeout and wait for the extension that takes the longest.
• It might be useful for the extension to know why the shutdown is occurring, so maybe pass it the signal from the system.
• It might be best to implement this feature so that it doesn't break backwards compatibility with other extensions, so an optional registration method might be a good approach.

[mstoykov]

For your particular case I would expect that you can workaround, as long as you don't need anything to be executed on the event loop - js code usually:

1. When creating the browser/the temp directory (per browser) - call RegisterCallback and start a goroutine
2. When the context get canceled k6 will wait until you call the callback - do all the shutdowning here
3. In the other places where you would want to shutdown as well have a different signal in the goroutine to shortcircuit it.

Something a kin to

func (b *Browser) connect() {
  // other code 
  callback := b.vu.RegisterCallack()
  go func() {
    defer.callback()
    <-b.vu.Context(), <-b.stop:
    // all cleanup here
  }()
  // rest of connect code  
}

Otherwise - yes a lot of those are likely going to be required which is why the issue is a lot more involved than it might seem at first glance.

[ankur22]

For your particular case I would expect that you can workaround, as long as you don't need anything to be executed on the event loop - js code usually:

1. When creating the browser/the temp directory (per browser) - call RegisterCallback and start a goroutine
2. When the context get canceled k6 will wait until you call the callback - do all the shutdowning here
3. In the other places where you would want to shutdown as well have a different signal in the goroutine to shortcircuit it.

Something a kin to

func (b *Browser) connect() {
  // other code 
  callback := b.vu.RegisterCallack()
  go func() {
    defer.callback()
    <-b.vu.Context(), <-b.stop:
    // all cleanup here
  }()
  // rest of connect code  
}

Otherwise - yes a lot of those are likely going to be required which is why the issue is a lot more involved than it might seem at first glance.

Thanks for this, we will give this work around a go if we have time in the next cycle.

[ankur22]

The browser module is making changes to abstract away the chromium browser lifecycle away from the user. This will allow the browser module to better utilise the test generator resources so that a suitable number of chromium instances can be launched before the test is started, and those same instances are used throughout the test run instead of launching and closing many chrome instances per iteration. This abstraction will affect the browser module APIs, which are being explored in https://github.com/grafana/k6-cloud/issues/1077.

While working through the API changes, we realised that there is one issue that we keep hitting upon, which is that the browser module is unable to close the chromium subprocess as it's missing a signal from k6. Here are two solutions worth mentioning and the problems associated to each of them:

1. Start a chromium process at the start of the test run. The blocker here is that there is no "end of test run" hook/signal in k6 which could allow us to close the chromium browser when the whole test run ends and k6 is shutting down.
2. Start a chromium process when the vu is initialised. The blocker here is that there is no "end of vu" hook/signal in k6 which allows us to close the chromium browser, and this is likely to be very challenging if not impossible to determine by k6.

When the vu ends unsuccessfully, the browser module can rely on the workaround that is described in this comment. When the vu ends successfully the browser module cannot rely on this workaround as it seems to hang indefinitely waiting on the context to close.

When an iteration ends, the browser module can rely on the context in the vu to close, which is a useful signal that it will likely use to cleanup the test iteration that has ended.

What the browser module now needs is a signal from k6 when it is shutting down regardless of whether the whole test run was successful or not.

[ankur22]

Something else that would help the browser module is a "start of test run" (similar to the setup JS method). The differences being that it would be an API at the module level (in go), and we would need the initialised vu.State.

This would help us to create all the chrome/firefox/etc instances, browserTypes, browsers, browserProcesses, and connectionss for the whole test up front. Essentially creating a pool of everything the whole test run will need based on wha the user has defined in scenario.options.browser. During the test it would then be a case of just taking the pre-warmed types and connections needed for the current vu/iteration.

Currently, if we wanted to do the same, we'd need to do it during the test runtime i.e. when a browser module method is called. Fo example in the following test we're having to do all this up front creation of the pool when newPage is called for the very first time. Sunbsequent calls to newPage will skip over the creation of the pool and use what was created earlier:

import { browser } from 'k6/x/browser'
import { sleep } from 'k6'

export default async function () {
  const page = browser.newPage() // At the moment we're doing all this up front creation here

  try {
    await page.goto('https://test.k6.io/')
    sleep(1);
  } finally {
    page.close()
    context.close()
  }
}

It's a stretch goal though, as we think that the challenge might be getting the initialised vu.State in a module level setup.

[imiric]

After discussing this with Ankur over Slack, the browser module essentially needs a setup/teardown hook at the module level. Hooking into the script's setup() and teardown() functions would be unreliable, since these can be disabled and are conditionally run, so the JS module itself needs to have this.

A quick hack that doesn't involve a full event system could be to add an interface that wraps modules.Module and has these additional Setup() and Teardown() methods that k6 can call on startup/shutdown. The tricky part would be passing the configuration in Setup(), since vu.State isn't available at that point, and they don't need anything else but the script options.

Let's discuss this at today's sync meeting and align on the way forward.