I don't quite follow the reason for this change here -- it looks like this code has been removed from the sendError callback function, which (may?) chance the timing here subtly. Can you talk a bit about why this change was necessary?

Good question.

First, I removed the use of the sendError callback, because both the sent apmError and the flush token added to the http client Client by agent.flush() use the same queue: the Client stream (it inherites from stream.Writable). So there is no need to sequence apm.flush() asynchronously after sendError.

Second, waiting for the callback from sendError can actually delay the start of the intake request:

Client.prototype.sendError = function (error, cb) {
  if (this._isUnsafeToWrite() || this._shouldDropEvent()) {
    return
  }
  this._maybeCork()
  return this.write({ error }, Client.encoding.ERROR, cb)
}

That _maybeCork will delay the start of an intake request, if there isn't one already by bufferWindowTime, which defaults to 20ms. I restored the "use the sendError callback" and added some logging and re-ran this test in test/agent.test.js:

test.only('#captureError()', function (t) {
  ...
  // Passing a callback to `captureError` means agent.flush() will be called.
  t.test('with callback', function (t) {

The logging shows a ~20ms delay from the "Sending error to Elastic APM" message to "intake request start":

[2021-10-20T22:11:14.313Z]  INFO (elastic-apm-node): Sending error to Elastic APM: {"id":"52b6dfb48f99b95c923b3c186ca1df3e"}
XXX corking
XXX sync after sendError
XXX uncorking1 (after bufferWindowTime)
[2021-10-20T22:11:14.338Z] TRACE (elastic-apm-node): intake request start
    event.module: "apmclient"
    reqId: "69557af9d5c862a96a45eb4119397f5d"
[2021-10-20T22:11:14.340Z] TRACE (elastic-apm-node): _write: encode object
    event.module: "apmclient"
    fullTimeMs: 5.267449
    numEvents: 1
    numBytes: 6831
[2021-10-20T22:11:14.341Z] TRACE (elastic-apm-node): intakeReq "socket": unref it
    event.module: "apmclient"
    reqId: "69557af9d5c862a96a45eb4119397f5d"
XXX in callback from sendError
[2021-10-20T22:11:14.341Z] TRACE (elastic-apm-node): _writeFlush
    event.module: "apmclient"
    active: true

The apm.flush() call comes after that in once sendError calls back.

With the currently proposed code the intake request is starts right away (a few ms of processing time):

[2021-10-20T22:04:42.229Z]  INFO (elastic-apm-node): Sending error to Elastic APM: {"id":"96ef84fc56d978ab34e3201b392c1de0"}
[2021-10-20T22:04:42.234Z] TRACE (elastic-apm-node): intake request start
    event.module: "apmclient"
    reqId: "33fd40c31c5ad99af7dd9de8ab8b0e40"
[2021-10-20T22:04:42.235Z] TRACE (elastic-apm-node): _writeBatch
    event.module: "apmclient"
    encodeTimeMs: 0.690205
    fullTimeMs: 5.48049
    numEvents: 1
    numBytes: 6871

because the apm.flush() called right after the sendError:

[2021-10-20T22:23:39.171Z]  INFO (elastic-apm-node): Sending error to Elastic APM: {"id":"affa9a3c0dc1c8958515255657c3ea95"}
XXX corking
XXX sync after sendError
XXX uncorking2
[2021-10-20T22:23:39.177Z] TRACE (elastic-apm-node): intake request start
    event.module: "apmclient"
    reqId: "6e2b4c7c250e96a3f5f9ea6e056a4e9b"
[2021-10-20T22:23:39.178Z] TRACE (elastic-apm-node): _writeBatch

Because the Client.flush uncorks immediately:

Client.prototype.flush = function (cb) {
  this._maybeUncork()

Last, I'll admit to not being 100% sure there isn't some interaction difference if there an ongoing intake request and enough data on the Client buffer such that the just sent apmError will need to wait for one or more separate intake requests to get fully flushed. I haven't followed what happens with the this._write(error, cb) callback when it is passed to the stream chopper.

It looks like we lost this comment in the new version. Is this something that's still relevant?

This was intentional. That TODO was added a long while back and there is no details that I found that explains why AsyncResource would be better and how it would be used. I think _instrumentation.bindFunction(cb) is the right thing to do to ensure that cb executes in the correct run context. If at some point our Run Context Manager uses AsyncResource to handle tracking, then great, but code like this should just use the Instrumentation class API.