open-telemetry
Metrics SDK - can we cleanup after a shutdown?
Though shutdown prevents metrics from ever getting collected/exported, they are still kept in the HashMaps/Aggregations. There is no check in hot path to see if the provider is already shutdown (it may be too expensive to do that check). This maybe okay, but opening an issue to discuss more.
Also, given the Counter/Meter from the API use a Arc<dyn> object pointing to something from the SDK, it is not easily possible to do much cleanup in the SDK, unless the user explicitly drops the Counter/Meters themselves.
As you have mentioned, I would prefer to avoid having this check in hot path as well.
However, we should probably clear Pipeline (more precisely inner: Mutex<PipelineInner>) during shutdown, so that memory can be cleaned up once user explicitly drops meters themselves.
Also, given the Counter/Meter from the API use a
Arc<dyn>object pointing to something from the SDK, it is not easily possible to do much cleanup in the SDK
We can use Weak instead of Arc in the instruments to address this issue. I created #2497 as a proof of concept to show that instruments can be updated to respect the MeterProvider shutdown. We can do the same for Meters as well. In the PR, I have only updated Counter to use Weak.
This comes with a performance cost though as the Weak to Arc upgrade happens on the hot path. On my machine, the benchmarks saw a 5% decline and the stress test throughput went from 9M to 7M iterations per second.
unless the user explicitly drops the Counter/Meters themselves.
This would not always be possible. Especially, if the users are using some instrumentation library and have no visibility and access to the instruments and Meters.
This comes with a performance cost though as the Weak to Arc upgrade happens on the hot path. On my machine, the benchmarks saw a 5% decline and the stress test throughput went from 9M to 7M iterations per second.
This is not related, but just to add. The Arc upgrade indeed is perf heavy operation, which was the reason it was removed from Logger/LoggerProvider in #1455.
Based on the prototype @utpilla did I think the cost is not worth the benifit. Most of the MeterProvider should have the same lifecycle as the application
TODO: Also check this for other signals like Logs. Add tests to confirm the current behavior and if they are behaving as expected or not.
Removing from stable milestone. This is nice-to-have, but not a blocker for SDK stable release.