Provide hooks for APMs

[mcollina]

Application performance monitoring tools are used in a lot of Node.js deployments. We should provide them a way to hook into Undici internals without (or with minimal) monkeypatching.

[mcollina]

Tagging @bengl and @Qard... what would you like to have from undici that would make integrating this easier?

[mcollina]

The approach suggested by @bengl on slack is to use https://nodejs.org/api/diagnostics_channel.html. I'm totally +1 for this.

What event would you need @bengl?

[bengl]

Here is our (datadog apm) instrumentation of the http module. The main things we need for tracing are:

1. When the request is started. That is, when the function to initiate a request is called. All its options should be available to the channel.
2. When the response headers are received. Right now we don't do anything with this in tracing in http except to attach to the response object, so this can possibly be omitted but still may be worth it since other APMs may use this event.
3. When the response body is completed (i.e. res.on('end') in http).
4. Any error/timeout events.

At each event, we'd need a context object passed in (i.e. the request/response objects in http).

While we don't currently need request or response bodies, that will change with future products (cc @vdeturckheim @simon-id), so it's probably best to make sure those are available to the diagnostics channel as well.

Also, since I know there's been talk about subclassing components or otherwise using the public-facing API, it's worth mentioning that we want our instrumentations to be completely transparent to users. That is, we don't want to force them to manually add components, or prevent them from doing their own subclassing or other public-facing API efforts. We also would like to (eventually) use diagnostics_channel everywhere, so that instrumentations can be far less invasive than monkey-patching. Indeed, that was the point of adding it to Node.js core in the first place.

[ronag]

You could achieve this by having a wrapping dispatcher:

class APMHandler {
  constructor (opts, parent, channel) {
    this.#opts = opts
    this.#parent = parent
    this.#channel = channel

    this.#channel.publish({
      opts: this.#opts,
      action: 'onCreate'
    })
  }

  onConnect (abort) {
    this.#channel.publish({
      opts: this.#opts,
      action: 'onConnect'
    })
    return this.#parent?.onConnect(abort)
  }

  onUpgrade (statusCode, headers, socket) {
    this.#channel.publish({
      opts: this.#opts,
      statusCode,
      headers,
      action: 'onUpgrade'
    })
    return this.#parent?.onUpgrade(statusCode, headers, socket)
  }

  onError (error) {
    this.#channel.publish({
      opts: this.#opts,
      error,
      action: 'onError'
    })
    return this.#parent?.onError(error)
  }

  onHeaders (statusCode, headers, resume, statusText) {
    this.#channel.publish({
      opts: this.#opts,
      statusCode,
      statusText,
      headers,
      action: 'onHeaders'
    })
    return this.#parent?.onData(statusCode, headers, resume, statusText)
  }

  onData (chunk) {
    this.#channel.publish({
      opts: this.#opts,
      chunk,
      action: 'onData'
    })
    return this.#parent?.onData(chunk)
  }

  onComplete (trailers) {
    this.#channel.publish({
      opts: this.#opts,
      trailers,
      action: 'onComplete'
    })
    return this.#parent?.onComplete(trailers)
  }

  onBodySent (chunk) {
    this.#channel.publish({
      opts: this.#opts,
      trailers,
      action: 'onBodySent'
    })
    return this.#parent?.onBodySent(chunk)
  }
}

class APMDispatcher {
  constructor(parent, channel) {
    this.#parent = parent
    this.#channel = channel
  }

  dispatch (options, handler) {
    return this.#parent.dispatch(opts, new APMHandler(options, handler, this.#channel))
  }

  get closed () {
    return this.#parent.closed
  }

  get destroyed () {
    return this.#parent.destroyed
  }

  close (...args) {
    return this.#parent.close(...args)
  }

  destroy (...args) {
    return this.#parent.destroy(...args)
  }
}

const channel = diagnostics_channel.channel('undici-channel')
setGlobalDispatcher(new APMDispatcher(getGlobalDispatcher(), channel))

[ronag]

We could also just add this to Client so it's always there by default. But the above should already work today so maybe we can start with something like that and see if it works and the consider if we should do it by default?

[mcollina]

The goal for APMs is to not change the default dispatcher because the user could do so as well.

[trentm]

For Elastic's APM, here is our instrumentation for http.get and http.request.

Mostly what @bengl said.

request or response bodies

Yes, please. The Elastic APM may capture the request body (depending on config).

4\. Any error/timeout events.

Include any 'abort' event in this as well.
Nevermind. This is handled by RequestAbortedError. I don't know the undici lib well yet.

[Qard]

Just a note: I'd recommend using differently named channels for each action type rather than one channel with a top-level "action" field. By making each a separate channel you are letting the consumer subscribe to actions individually and therefore have zero cost on any published events that have no subscribers.

If you have them all share a channel and differentiate with an "action" field then a subscriber would receive all events and have to manually filter each individually, which would be very costly.

I'd just do some sort of namespacing, for example: undici.connect, undici.upgrade, undici.complete, etc.

To link "start" and "end" concepts across different channels you need only share some sort of ID or common reference object. In this case, a request object given to both could be used with a WeakMap to find the corresponding event to link to.

[ThisIsMissEm]

Could this also be used to provide a "debug" mode? I'm currently working on an API Client using Undici, but sometimes it seems the wrong request is being issued, and as this module doesn't support the DEBUG env variable, I've very little insight into the requests that undici is making, which makes development harder.

That is, for a good developer experience, we could potentially re-use the APM infrastructure to provide better debuggability.

edit: Unless I've completely missed something in the docs?

[mcollina]

Could this also be used to provide a "debug" mode? I'm currently working on an API Client using Undici, but sometimes it seems the wrong request is being issued, and as this module doesn't support the DEBUG env variable, I've very little insight into the requests that undici is making, which makes development harder.

Could you open a fresh issue about this?

[mcollina]

#1000 here is a draft PR to implement/iterate on this. I'll add the missing channels.