Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement TracerProvider Shutdown #1489

Merged
Merged
Show file tree
Hide file tree
Changes from 15 commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -231,6 +231,7 @@ static OpenTelemetry.SuppressInstrumentationScope.Enter() -> int
static OpenTelemetry.Trace.SamplingResult.operator !=(OpenTelemetry.Trace.SamplingResult decision1, OpenTelemetry.Trace.SamplingResult decision2) -> bool
static OpenTelemetry.Trace.SamplingResult.operator ==(OpenTelemetry.Trace.SamplingResult decision1, OpenTelemetry.Trace.SamplingResult decision2) -> bool
static OpenTelemetry.Trace.TracerProviderExtensions.AddProcessor(this OpenTelemetry.Trace.TracerProvider provider, OpenTelemetry.BaseProcessor<System.Diagnostics.Activity> processor) -> OpenTelemetry.Trace.TracerProvider
static OpenTelemetry.Trace.TracerProviderExtensions.Shutdown(this OpenTelemetry.Trace.TracerProvider provider, int timeoutMilliseconds = -1) -> bool
virtual OpenTelemetry.BaseExporter<T>.Dispose(bool disposing) -> void
virtual OpenTelemetry.BaseExporter<T>.OnShutdown(int timeoutMilliseconds) -> bool
virtual OpenTelemetry.BaseProcessor<T>.Dispose(bool disposing) -> void
Expand Down
1 change: 1 addition & 0 deletions src/OpenTelemetry/.publicApi/net46/PublicAPI.Unshipped.txt
Original file line number Diff line number Diff line change
Expand Up @@ -231,6 +231,7 @@ static OpenTelemetry.SuppressInstrumentationScope.Enter() -> int
static OpenTelemetry.Trace.SamplingResult.operator !=(OpenTelemetry.Trace.SamplingResult decision1, OpenTelemetry.Trace.SamplingResult decision2) -> bool
static OpenTelemetry.Trace.SamplingResult.operator ==(OpenTelemetry.Trace.SamplingResult decision1, OpenTelemetry.Trace.SamplingResult decision2) -> bool
static OpenTelemetry.Trace.TracerProviderExtensions.AddProcessor(this OpenTelemetry.Trace.TracerProvider provider, OpenTelemetry.BaseProcessor<System.Diagnostics.Activity> processor) -> OpenTelemetry.Trace.TracerProvider
static OpenTelemetry.Trace.TracerProviderExtensions.Shutdown(this OpenTelemetry.Trace.TracerProvider provider, int timeoutMilliseconds = -1) -> bool
virtual OpenTelemetry.BaseExporter<T>.Dispose(bool disposing) -> void
virtual OpenTelemetry.BaseExporter<T>.OnShutdown(int timeoutMilliseconds) -> bool
virtual OpenTelemetry.BaseProcessor<T>.Dispose(bool disposing) -> void
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -251,6 +251,7 @@ static OpenTelemetry.SuppressInstrumentationScope.Enter() -> int
static OpenTelemetry.Trace.SamplingResult.operator !=(OpenTelemetry.Trace.SamplingResult decision1, OpenTelemetry.Trace.SamplingResult decision2) -> bool
static OpenTelemetry.Trace.SamplingResult.operator ==(OpenTelemetry.Trace.SamplingResult decision1, OpenTelemetry.Trace.SamplingResult decision2) -> bool
static OpenTelemetry.Trace.TracerProviderExtensions.AddProcessor(this OpenTelemetry.Trace.TracerProvider provider, OpenTelemetry.BaseProcessor<System.Diagnostics.Activity> processor) -> OpenTelemetry.Trace.TracerProvider
static OpenTelemetry.Trace.TracerProviderExtensions.Shutdown(this OpenTelemetry.Trace.TracerProvider provider, int timeoutMilliseconds = -1) -> bool
virtual OpenTelemetry.BaseExporter<T>.Dispose(bool disposing) -> void
virtual OpenTelemetry.BaseExporter<T>.OnShutdown(int timeoutMilliseconds) -> bool
virtual OpenTelemetry.BaseProcessor<T>.Dispose(bool disposing) -> void
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -251,6 +251,7 @@ static OpenTelemetry.SuppressInstrumentationScope.Enter() -> int
static OpenTelemetry.Trace.SamplingResult.operator !=(OpenTelemetry.Trace.SamplingResult decision1, OpenTelemetry.Trace.SamplingResult decision2) -> bool
static OpenTelemetry.Trace.SamplingResult.operator ==(OpenTelemetry.Trace.SamplingResult decision1, OpenTelemetry.Trace.SamplingResult decision2) -> bool
static OpenTelemetry.Trace.TracerProviderExtensions.AddProcessor(this OpenTelemetry.Trace.TracerProvider provider, OpenTelemetry.BaseProcessor<System.Diagnostics.Activity> processor) -> OpenTelemetry.Trace.TracerProvider
static OpenTelemetry.Trace.TracerProviderExtensions.Shutdown(this OpenTelemetry.Trace.TracerProvider provider, int timeoutMilliseconds = -1) -> bool
virtual OpenTelemetry.BaseExporter<T>.Dispose(bool disposing) -> void
virtual OpenTelemetry.BaseExporter<T>.OnShutdown(int timeoutMilliseconds) -> bool
virtual OpenTelemetry.BaseProcessor<T>.Dispose(bool disposing) -> void
Expand Down
8 changes: 4 additions & 4 deletions src/OpenTelemetry/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,15 +11,15 @@
`CustomProperty`.
([#1463](https://github.com/open-telemetry/opentelemetry-dotnet/pull/1463))
* Remove RentrantExportProcessor as it is not required by spec.
* Implemented Shutdown for TracerProvider
([#1489](https://github.com/open-telemetry/opentelemetry-dotnet/pull/1489))

## 0.8.0-beta.1

Released 2020-Nov-5

* TracerProviderBuilder API changes
Renamed AddInstrumentation to AddDiagnosticSourceInstrumentation
and made internal.
Added AddInstrumentation
* TracerProviderBuilder API changes Renamed AddInstrumentation to
AddDiagnosticSourceInstrumentation and made internal. Added AddInstrumentation
([#1454](https://github.com/open-telemetry/opentelemetry-dotnet/pull/1454))

* DiagnosticSource subscription helper classes (DiagnosticSourceSubscriber,
Expand Down
15 changes: 15 additions & 0 deletions src/OpenTelemetry/Internal/OpenTelemetrySdkEventSource.cs
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,15 @@ public void SelfDiagnosticsFileCreateException(string logDirectory, Exception ex
}
}

[NonEvent]
public void TracerProviderException(string evnt, Exception ex)
{
if (this.IsEnabled(EventLevel.Warning, (EventKeywords)(-1)))
{
this.TracerProviderException(evnt, ex.ToInvariantString());
}
}

[Event(1, Message = "Span processor queue size reached maximum. Throttling spans.", Level = EventLevel.Warning)]
public void SpanProcessorQueueIsExhausted()
{
Expand Down Expand Up @@ -266,6 +275,12 @@ public void SelfDiagnosticsFileCreateException(string logDirectory, string excep
this.WriteEvent(26, logDirectory, exception);
}

[Event(27, Message = "Unknown error in TracerProvider '{0}': '{1}'.", Level = EventLevel.Warning)]
public void TracerProviderException(string evnt, string ex)
{
this.WriteEvent(27, evnt, ex);
}

#if DEBUG
public class OpenTelemetryEventListener : EventListener
{
Expand Down
54 changes: 54 additions & 0 deletions src/OpenTelemetry/Trace/TracerProviderExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@

using System;
using System.Diagnostics;
using System.Threading;
using OpenTelemetry.Internal;

namespace OpenTelemetry.Trace
{
Expand All @@ -40,5 +42,57 @@ public static TracerProvider AddProcessor(this TracerProvider provider, BaseProc

return provider;
}

/// <summary>
/// Attempts to shutdown the TracerProviderSdk, blocks the current thread until
/// shutdown completed or timed out.
/// </summary>
/// <param name="provider">TracerProviderSdk instance on which Shutdown will be called.</param>
/// <param name="timeoutMilliseconds">
/// The number of milliseconds to wait, or <c>Timeout.Infinite</c> to
/// wait indefinitely.
/// </param>
/// <returns>
/// Returns <c>true</c> when shutdown succeeded; otherwise, <c>false</c>.
/// </returns>
/// <exception cref="System.ArgumentOutOfRangeException">
/// Thrown when the <c>timeoutMilliseconds</c> is smaller than -1.
/// </exception>
/// <remarks>
/// This function guarantees thread-safety. Only the first call will
/// win, subsequent calls will be no-op.
/// </remarks>
public static bool Shutdown(this TracerProvider provider, int timeoutMilliseconds = Timeout.Infinite)
{
if (provider == null)
{
throw new ArgumentNullException(nameof(provider));
}

if (provider is TracerProviderSdk tracerProviderSdk)
{
if (timeoutMilliseconds < 0 && timeoutMilliseconds != Timeout.Infinite)
{
throw new ArgumentOutOfRangeException(nameof(timeoutMilliseconds), timeoutMilliseconds, "timeoutMilliseconds should be non-negative.");
}

if (Interlocked.Increment(ref tracerProviderSdk.ShutdownCount) > 1)
{
return false; // shutdown already called
}

try
{
return tracerProviderSdk.OnShutdown(timeoutMilliseconds);
}
catch (Exception ex)
{
OpenTelemetrySdkEventSource.Log.TracerProviderException(nameof(tracerProviderSdk.OnShutdown), ex);
return false;
}
}

return true;
}
}
}
38 changes: 38 additions & 0 deletions src/OpenTelemetry/Trace/TracerProviderSdk.cs
Original file line number Diff line number Diff line change
Expand Up @@ -20,13 +20,16 @@
using System.Linq;
using System.Runtime.CompilerServices;
using System.Text.RegularExpressions;
using System.Threading;
using OpenTelemetry.Internal;
using OpenTelemetry.Resources;

namespace OpenTelemetry.Trace
{
internal class TracerProviderSdk : TracerProvider
{
internal int ShutdownCount;

private readonly List<object> instrumentations = new List<object>();
private readonly ActivityListener listener;
private readonly Sampler sampler;
Expand Down Expand Up @@ -196,6 +199,41 @@ internal TracerProviderSdk AddProcessor(BaseProcessor<Activity> processor)
return this;
}

/// <summary>
/// Called by <c>Shutdown</c>. This function should block the current
/// thread until shutdown completed or timed out.
/// </summary>
/// <param name="timeoutMilliseconds">
/// The number of milliseconds to wait, or <c>Timeout.Infinite</c> to
/// wait indefinitely.
/// </param>
/// <returns>
/// Returns <c>true</c> when shutdown succeeded; otherwise, <c>false</c>.
/// </returns>
/// <remarks>
/// This function is called synchronously on the thread which made the
/// first call to <c>Shutdown</c>. This function should not throw
/// exceptions.
/// </remarks>
internal bool OnShutdown(int timeoutMilliseconds)
{
// TO DO Put OnShutdown logic in a task to run within the user provider timeOutMilliseconds
bool? result;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why not dispose the instrumentations?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Spec just says at least processors must be shutdown.
i think shutdown should do same as dispose.

if (this.instrumentations != null)
{
foreach (var item in this.instrumentations)
{
(item as IDisposable)?.Dispose();
}

this.instrumentations.Clear();
}

result = this.processor?.Shutdown(timeoutMilliseconds);
this.listener?.Dispose();
return result ?? true;
}

protected override void Dispose(bool disposing)
{
if (this.instrumentations != null)
Expand Down