elastic / elastic/elastic-otel-node
SDK shutdown handling
- Dominant language
- JavaScript
- Stars
- 12
- Forks
- 22
- Avg merge
- 16h 54m
- Merged PRs (30d)
- 5
Description
Shutdown TODO notes extracted from "start.js":
```js
// TODO sdk shutdown: also SIGINT?
// TODO sdk shutdown handlers: better log on err
// TODO sdk shutdown: make these handlers configurable?
// - If so, could move these into sdk.js and it could use its logger for logging an error.
// - Note, only want luggite.warn over console.warn if errSerializer gets
// all attributes (console.warn shows more data for ECONNREFUSED)
// TODO sdk shutdown beforeExit: skip this for Lambda (also Azure Fns?)
// TODO sdh shutdown: call process.exit?
// TODO: Whether we have a signal handler for sdk.shutdown() is debatable. It
// definitely *can* change program behaviour. Let's reconsider.
// Note: See https://github.com/open-telemetry/opentelemetry-js/issues/1521
// for some thoughts on automatic handling to shutdown the SDK.
```
These should all be worked through.
My current thoughts are:
- We *should* have shutdown handling by default for our basic/default use case.
- We *should* provide a configuration option to turn off the signal handler(s) and mention this in troubleshooting and/or in the advanced/details section of the user guide. This is because users can have legitimate other uses for SIGTERM, etc. and ours will conflict. The docs for this option should show how a user using the golden path or a custom "instrumentation.js" can setup shutdown signal handlers.
- We should add an equivalent handler for SIGINT *and/or* document why not.
- We should document limitations on Windows and have separate later work to support controlled shutdown on Windows. Note (https://nodejs.org/api/process.html#signal-events) that SIGTERM and SIGINT are not supported on Windows. There is SIGBREAK, but I'm not sure of the default handler behaviour there.
- Multiple shutdowns: When a MeterProvider is involved (and possibly in other cases), shutdown adds async tasks, so we get `beforeExit` again later. That results in calling sdk.shutdown() twice. Currently that isn't problematic (there may be a diag warn about this?), but we should be able to do better with a module-level global to only attempt to shutdown once.
- The logging-related issues for shutdown handling and perhaps the Windows-related issues should be moved to a separate issue for later work.
Contributor guide
Assessment
This issue has not been assessed yet.