Add Testing for Shutdown Scenarios.

[hdost]

          > > And will calling shutdown on the global loggerProvider ensure that the logs through these loggers are not exported ?

It would be good to validate this use-case. It seems shutdown_logger_provider will invoke shutdown on the LogProcessors when the inner instance is dropped. And now if any of the loggers have it's reference in separate thread, the shutdown will not be invoked. One option can be to add a new LoggerProvider::Shutdown() method, and explicitly invoke it from within shutdown_logger_provider . This new method will invoke shutdown on all processors, and so ensure that all the existing logs are flushed. And no new logs can be exported with processors in shutdown state.

Thinking about this scenario again, it seems that the potential issue of leaking providers might not be as significant if we can document it clearly to explain the risk. Specifically, providers could be leaked if any of the tasks indefinitely retain their reference, which can result in a shutdown not triggering.

Originally posted by @lalitb in #1455 (comment)

[bIgBV]

Looking into this, I'd love some clarification on how the scenario would work. My current approach has been to set up an integration test based on https://github.com/open-telemetry/opentelemetry-rust/blob/main/examples/logs-basic/src/main.rs and extend it such that we have multiple background tasks with set up with references to the LoggerProvider instance through an Arc.

The shutdown I wanted to test was to create a forced shutdown through an interrupt. I initially tried this with setting up multiple exporters with a single LoggerProvider and got a panic from this line

opentelemetry-rust/opentelemetry-sdk/src/logs/log_emitter.rs

Line 86 in 36c2a8e

.expect("cannot shutdown LoggerProvider when child Loggers are still active")

which I think is expected behavior.

Would taking the same approach with multiple LoggerProvider references in separate tasks be a similar scenario?

[hdost]

The shutdown I wanted to test was to create a forced shutdown through an interrupt. I initially tried this with setting up multiple exporters with a single LoggerProvider and got a panic from this line

opentelemetry-rust/opentelemetry-sdk/src/logs/log_emitter.rs

Line 86 in 36c2a8e

.expect("cannot shutdown LoggerProvider when child Loggers are still active")

which I think is expected behavior.

Would taking the same approach with multiple LoggerProvider references in separate tasks be a similar scenario?

Looking at our API and the spec I actually think we might need a change. https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/logs/sdk.md#shutdown

Right now i think this panic isn't actually doing what I would want/expect. There's already a result being returned. In this case it's a vec of results but I feel like we should be passing back a default of Vec error result.

Shutdown SHOULD provide a way to let the caller know whether it succeeded, failed or timed out.

Panics don't really "let the caller know".

[cijothomas]

Tests are added now and we should be good to close this.
#1042 (comment) is still required to be resolved, but that is separate (though shutdown related!)

[cijothomas]

Reopening, as few topics were discussed here, but not really resolved.. (Will check how to expand the tests to cover everything.)