Shutdown hook in auto-instrumentations-node can block process termination if downstream collector is unavailable

[basti1302]

What version of OpenTelemetry are you using?

Current main branch of open-telemetry/opentelemetry-js-contrib with @opentelemetry/sdk-node@0.210.0

What version of Node are you using?

v24.13.0

What did you do?

The Node.js SDK auto-instrumentation-node installs a shutdown hook. This can block process termination by ~ 5 seconds in certain error scenarios, for example if the configured exporter cannot connect to its downstream collector (e.g. OTEL_EXPORTER_OTLP_ENDPOINT points to something that is currently not network-reachable).

As far as I can tell, this has always been the case for certain error conditions, but the likelihood of this happening seems to have increased with this change in otlp-exporter-base.

I agree that flushing telemetry via sdk.shutdown at process termination is the right call, and blocking process termination for a few tens or maybe hundreds of milliseconds is acceptable for that. Blocking process termination for up to 5 seconds can be problematic though, in particular in Kubernetes etc., where pod restart times are important.

What did you expect to see?

Process termination not being blocked by auto-instrumentations-node.

What did you see instead?

Process termination is blocked for ~5 seconds.

Additional context

There is a reproducer integration test here: #3348

To be honest, I'm not entirely sure if this can be solved in opentelemetry-js-contrib or if this needs to be solved in opentelemetry-js. My guess is that the shutdown call maybe should accept an optional timeout parameter? Not sure.

_{Tip: React with 👍 to help prioritize this issue. Please use comments to provide useful context, avoiding +1 or me too, to help us triage it. Learn more here.}

[trentm]

Marc's theory is this is due to retries having been added in open-telemetry/opentelemetry-js#6147

[basti1302]

Marc's theory is this is due to retries having been added in open-telemetry/opentelemetry-js#6147

That's what I said in my initial description as well :-) But thanks for confirming you agree with this.

[pichlermarc]

yep - I think we need to change forceFlush() on the exporter to cancel retries for pending exports (or try to do them ASAP), then adjust the PeriodicExportingMetricReader to only include the "effects" of PerodicExportingMetricReader#forceFlush() (collection and scheduling the export, but not awaiting it before force-flusing the exporter).

Looks like that's the intention of the spec, too:

This is a hint to ensure that the export of any Metrics the exporter has received prior to the call to ForceFlush SHOULD be completed as soon as possible, preferably before returning from this method.

I'm aiming to get a PR up today to fix that.

[pichlermarc]

Turns out this is a lot more work to fix than I thought. It was fairly easy to fix for metrics and in the exporters, but the the BatchLogRecordProcessor and BatchSpanProcessor need a lot more work to avoid scheduling more exports. I'll continue chipping away on it, but I probably won't be able to finish this today.