make `Agent.destroy()` async

[trentm]

Currently in index.d.ts:

  export interface Agent {
    // ...
    destroy (): void;
  }

In the coming OTel Metrics work, Agent.destroy() will possibly call an async method to shutdown an OTel MeterProvider. This means that agent shutdown code can possibly be running after the sync Agent.destroy() method returns. That's unfortunate.

The fix is to make Agent.destroy() async -- i.e. returns a Promise -- then the user can await it. However, is changing that return type in the TypeScript types a breaking change? I'm not sure.

[david-luna]

Hi trentm, here is my 2 cents.

Being very strict you can consider any type change in the API params or return values a breaking change. But does this change actually breaks code instrumented with previous versions of the agent?

Let's pretend we have a service instrumented with the nodejs-agent which makes use of the destroy() method. The line will look like this

// Block 1: some logic which asserts the agent should be destroyed 
agent.destroy() // no assignment expression because it returns undefined
// Block 2: other logic assuming the agent is destroyed

Updating the API and returning a Promise instead of undefined will not trigger an Error per se. By looking at OTel metrics Work it doesn't look like since agent is creating & removing OTel provider references when starting/destroying. So code in Block 2 will even be able to restart the agent.

But in the other hand we may introduce in the future some destroy logic that actually must be awaited. Or even the developer may use a provider which its shutdown process need to be awaited.

As a conclusion IMHO its safer to mark this as a breaking change instead of doing it later without any change on the public API.

[trentm]

As a conclusion IMHO its safer to mark this as a breaking change

Sounds good to me, thanks. We'll save this change for a major version bump. There isn't a really strong need to have this change soon.

[trentm]

I can take this, but I want #3536 to get into the dev/4.x branch first for the added docs/upgrade-to-4.x.asciidoc file, so I can document this change there.

[trentm]

This was done in #3547 for the "dev/4.x" branch, which will be the active 4.x release branch soonish.