Add ability to add timeout duration to shutdown

[cijothomas]

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.

[cijothomas]

Removing from 0.28 as option2 is additive change, and can be done post 0.28 as well.

[mohammadVatandoost]

I would be happy to work on this. Please assign it to me

[cijothomas]

@mohammadVatandoost Thanks! Assigned. To confirm - you'll be pursuing the option2 from the issue, right?

[mohammadVatandoost]

@mohammadVatandoost Thanks! Assigned. To confirm - you'll be pursuing the option2 from the issue, right?

Yes, backward compatibility is important.

[mohammadVatandoost]

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?

[cijothomas]

Only providers need option2.
reader, processor, exporter are not expected to be used outside of providers.

[cijothomas]

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.

[cijothomas]

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.