open-telemetry
Add ability to add timeout duration to shutdown
All Providers have shutdown() method now, and it internally uses a hardcoded 5sec timeout to perform shutdown. (usually passed on to reader/processor/exporter). Opening an issue to let user provide their own timeout.
Option1
fn shutdown(&self, timeout: Option<Duration>)
Option2
fn shutdown(&self)
{
self.shutdown_with_timeout(5secs)
}
fn shutdown_with_timeout(&self, timeout: Duration);
I am inclined to do 2, as it keeps backward compatibility, and we can add this anytime, not necessarily before 1.0. But given https://github.com/open-telemetry/opentelemetry-rust/pull/2573/files and changes in this area, it'd be better to club them in together.
Removing from 0.28 as option2 is additive change, and can be done post 0.28 as well.
@mohammadVatandoost Thanks! Assigned. To confirm - you'll be pursuing the option2 from the issue, right?
@mohammadVatandoost Thanks! Assigned. To confirm - you'll be pursuing the option2 from the issue, right?
Yes, backward compatibility is important.
I have a question: we should implement option2 for reader and option1 for exporter and processor. Since we only use reader shutdown from outside, and reader shutdown function runs the exporter/processor's shutdown function. This will not effect the backward compatibility,right? I didn't find any usecase that we need to run exporter/processor's shutdown function directly. For logger will be like this:
impl SdkLoggerProvider {
fn shutdown(&self) -> OTelSdkResult{
self.shutdown_with_timeout(Duration::from_secs(5))
}
fn shutdown_with_timeout(&self, timeout: Duration) -> OTelSdkResult {
.....
}
}
// ====================
trait LogProcessor {
fn shutdown(&self, timeout: Duration) -> OTelSdkResult;
}
// =========
trait LogExporter {
fn shutdown(&mut self, timeout: Duration) -> OTelSdkResult;
}
@cijothomas What is your idea?
Only providers need option2. reader, processor, exporter are not expected to be used outside of providers.
Removing from logging-stable milestone. This is nice-to-have for that milestone, but given this is back-compatible, no need to make it mandatory for the stabilization milestone.
Only providers need option2. reader, processor, exporter are not expected to be used outside of providers.
Apologies, I have a correction to make. Though we don't expect users to directly call processor/readers, they still need to do the option2, and have a new method that takes timeout, just like providers. These are part of public api, and we can follow the same API share as provider.
@cijothomas The shutdown_with_timeout method is added for the metric, log and trace signals. If i miss anthing, please let me know. I think we can close the issue.