runtime performance metrics

[ronag]

What do we need for introspection and metrics?

I propose the following events:

• 'queued' - on request queued
• 'running' - on request running
• 'complete'- on request complete
• 'failed' - on request failed

Refs: https://github.com/nodejs/undici/discussions/673

[szmarczak]

1. pending -> queued (in order to avoid confusing w/ running)
2. Does the failed event have an error argument?

[ronag]

1. pending -> queued (in order to avoid confusing w/ running)

Fixed:

2. Does the failed event have an error argument?

Yes. Alternatively we could combine complete/failed and have an err argument.

[mcollina]

I think we might want to provide a "stats provider" of sort that could be attached to a Agent or a Dispatcher so that we could track all those metrics automatically - and potentially be easily collectable by prometheus.

[artur-ma]

Since undici does not use node's built-in agent, I guess it means tools like Newrelic will not track network requests correctly Will this be enough for such tools as NR?

[mcollina]

Since undici does not use node's built-in agent, I guess it means tools like Newrelic will not track network requests correctly

Exactly. We should look into providing some hooks for APMs in the future.

Will this be enough for such tools as NR?

I do not think so. This issue will provide metrics about how the process operates - data that is relatively difficult to extract from Node.js own client.

NR (and all others) will need a way to inject custom headers for each request.

[ronag]

NR (and all others) will need a way to inject custom headers for each request.

That should be reasonably simple already. Just extend Agent and pass another handler wrapping the user's handler. Similar to how we do redirects. I don't think we need to add anything to undici.

i.e.

class NRAgent extends Agent {
  dispatch(opts, handler) {
    return super.dispatch(opts, new NRHandler(opts, handler))
  }
}
setGlobalDispatcher(new NRAgent())

[artur-ma]

@ronag a question regarding your example

in case im NR APM, and I want to add new Agent, I have to assume that my current globalDispatcher is Agent, not some other class that already extends Agent .

For example, my app can execute this code, before NR APM is required

class MyAppCustomAgent extends Agent {
  constructor(opts) {
    super()
    this.opts = opts
  }
}

setGlobalDispatcher(new MyAppCustomAgent())

theoretically I can do

class NrAgent extends getGlobalDispatcher().constructor {
  constructor(appId) {
    super()
    this.appId = appId
  }
}

also then I dont know what original dispatcher gets in the constructor (that is returned from getGlobalDispatcher)

so the other way to do it, is to wrap the original dispatcher I guess

class NrAgent extends  Agent {
  constructor(appId, originalDispatcher) {
    super()
    this.originalDispatcher = originalDispatcher
    this.appId = appId
  }

  onHeaders() { return this.originalDispatcher.onHeaders() }
  ... 
}

setGlobalDispatcher(new NrAgent("someApp", getGlobalDispatcher()))

Is this expected solution ?