Implement TracerProvider Shutdown #1489
Conversation
| { | ||
| bool? result; | ||
| var sw = Stopwatch.StartNew(); | ||
| this.listener?.Dispose(); |
There was a problem hiding this comment.
It is a bit strange to call dispose in shutdown.
Consider clean up the dispose and shutdown methods (e.g. Dispose will dispose the underlying listener).
There was a problem hiding this comment.
The spec says that after the TracerProvider Shutdown, any subsequent calls to get a Tracer should result in a no-op tracer. To achieve the equivalent of that requirement, we have to dispose the listener. If we don't dispose the listener we would continue to export the data as the exporters don't implement OnShutdown.
Codecov Report
@@ Coverage Diff @@
## master #1489 +/- ##
==========================================
- Coverage 81.30% 81.03% -0.28%
==========================================
Files 233 233
Lines 6370 6395 +25
==========================================
+ Hits 5179 5182 +3
- Misses 1191 1213 +22
|
…er Shutdown andOnShutdown methods
…roviderSdk-Shutdown
…roviderSdk-Shutdown
…down' into utpilla/Add-TracerProviderSdk-Shutdown
| /// </remarks> | ||
| internal bool OnShutdown(int timeoutMilliseconds) | ||
| { | ||
| bool? result; |
There was a problem hiding this comment.
why not dispose the instrumentations?
There was a problem hiding this comment.
The spec just says that any calls to TracerPorvider after Shutdown should result in a no-op. It also explicitly says to call shutdown of each of the processors. We can achieve this by only disposing the listener and calling the CompositeProcessor Shutdown. I didn't dispose instrumentation to keep the Shutdown functionality as minimum as possible and in accordance with the spec.
We could Dispose the instrumentations too and then TracerProviderSdk Shutdown will be functionally pretty close to TracerProviderSdk Dispose with only Sampler and the base TracerProvider not getting disposed.
There was a problem hiding this comment.
Spec just says at least processors must be shutdown.
i think shutdown should do same as dispose.
…ub.com/utpilla/opentelemetry-dotnet into utpilla/Add-TracerProviderSdk-Shutdown
Fixes #1451
Changes
Implemented Shutdown method for TracerProvider for the issue #1451