Fix double shutdown (#1017)

* fix double shutdown * add consistency * nit
open-telemetry · Aug 7, 2020 · e43356a · e43356a
1 parent 1b01f3c
commit e43356a
Show file tree

Hide file tree

Showing 18 changed files with 31 additions and 43 deletions.
diff --git a/docs/Directory.Build.props b/docs/Directory.Build.props
@@ -11,7 +11,7 @@
       Please sort alphabetically.
       Refer to https://docs.microsoft.com/en-us/nuget/concepts/package-versioning for semver syntax.
     -->
-    <OpenTelemetryPkgVer>[0.4.0-beta.2,1.0)</OpenTelemetryPkgVer>
     <OpenTelemetryExporterConsolePkgVer>[0.4.0-beta.2,1.0)</OpenTelemetryExporterConsolePkgVer>
+    <OpenTelemetryPkgVer>[0.4.0-beta.2,1.0)</OpenTelemetryPkgVer>
   </PropertyGroup>
 </Project>
diff --git a/docs/trace/building-your-own-exporter/Program.cs b/docs/trace/building-your-own-exporter/Program.cs
@@ -16,7 +16,6 @@
 
 using System.Diagnostics;
 using OpenTelemetry;
-using OpenTelemetry.Trace;
 
 public class Program
 {

diff --git a/docs/trace/building-your-own-exporter/TracerProviderExtensions.cs b/docs/trace/building-your-own-exporter/TracerProviderExtensions.cs
@@ -14,7 +14,6 @@
 // limitations under the License.
 // </copyright>
 
-using System;
 using OpenTelemetry.Trace;
 
 internal static class TracerProviderExtensions

diff --git a/docs/trace/building-your-own-sampler/Program.cs b/docs/trace/building-your-own-sampler/Program.cs
@@ -38,6 +38,7 @@ public static void Main()
         {
             activity?.SetTag("foo", 1);
             activity?.SetTag("bar", "Hello, World!");
+            activity?.SetTag("baz", new int[] { 1, 2, 3 });
         }
     }
 }
diff --git a/docs/trace/getting-started/Program.cs b/docs/trace/getting-started/Program.cs
@@ -30,9 +30,8 @@ public static void Main()
             new string[]
             {
                 "MyCompany.MyProduct.MyLibrary",
-            });
-
-        tracerProvider.AddProcessor(new SimpleActivityProcessor(new ConsoleExporter(new ConsoleExporterOptions())));
+            })
+            .AddProcessor(new SimpleActivityProcessor(new ConsoleExporter(new ConsoleExporterOptions())));
 
         using (var activity = MyActivitySource.StartActivity("SayHello"))
         {

diff --git a/src/OpenTelemetry.Exporter.Console/ConsoleExporter.cs b/src/OpenTelemetry.Exporter.Console/ConsoleExporter.cs
@@ -15,7 +15,6 @@
 // </copyright>
 
 using System;
-using System.Collections;
 using System.Collections.Generic;
 using System.Diagnostics;
 using System.Linq;

diff --git a/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/Implementation/ActivityExtensions.cs b/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/Implementation/ActivityExtensions.cs
@@ -20,7 +20,6 @@
 using System.Linq;
 
 using Google.Protobuf;
-using Opentelemetry.Proto.Common.V1;
 using OpenTelemetry.Resources;
 using OpenTelemetry.Trace;
 using OtlpCommon = Opentelemetry.Proto.Common.V1;

diff --git a/src/OpenTelemetry.Exporter.ZPages/ZPagesProcessor.cs b/src/OpenTelemetry.Exporter.ZPages/ZPagesProcessor.cs
@@ -17,7 +17,6 @@
 using System;
 using System.Diagnostics;
 using System.Threading;
-using System.Threading.Tasks;
 using OpenTelemetry.Exporter.ZPages.Implementation;
 #if NET452
 using OpenTelemetry.Internal;

diff --git a/src/OpenTelemetry.Instrumentation.Http/Implementation/HttpTagHelper.cs b/src/OpenTelemetry.Instrumentation.Http/Implementation/HttpTagHelper.cs
@@ -15,7 +15,6 @@
 // </copyright>
 using System;
 using System.Collections.Concurrent;
-using System.Net;
 using System.Net.Http;
 
 namespace OpenTelemetry.Instrumentation.Http.Implementation

diff --git a/src/OpenTelemetry/Metrics/MeterProviderSdk.cs b/src/OpenTelemetry/Metrics/MeterProviderSdk.cs
@@ -14,7 +14,6 @@
 // limitations under the License.
 // </copyright>
 
-using System;
 using System.Collections.Generic;
 using System.Threading;
 using OpenTelemetry.Metrics.Export;

diff --git a/src/OpenTelemetry/Sdk.cs b/src/OpenTelemetry/Sdk.cs
@@ -18,9 +18,7 @@
 using System.Collections.Generic;
 using System.Diagnostics;
 using System.Linq;
-using System.Runtime.CompilerServices;
 using System.Threading;
-using OpenTelemetry.Context;
 using OpenTelemetry.Metrics;
 using OpenTelemetry.Metrics.Export;
 using OpenTelemetry.Resources;

diff --git a/src/OpenTelemetry/Trace/ActivityProcessor.cs b/src/OpenTelemetry/Trace/ActivityProcessor.cs
@@ -27,6 +27,8 @@ namespace OpenTelemetry.Trace
     /// </summary>
     public abstract class ActivityProcessor : IDisposable
     {
+        private bool disposed;
+
         /// <summary>
         /// Activity start hook.
         /// </summary>
@@ -80,14 +82,24 @@ public void Dispose()
 
         protected virtual void Dispose(bool disposing)
         {
-            try
+            if (this.disposed)
             {
-                this.ShutdownAsync(CancellationToken.None).GetAwaiter().GetResult();
+                return;
             }
-            catch (Exception ex)
+
+            if (disposing)
             {
-                OpenTelemetrySdkEventSource.Log.SpanProcessorException(nameof(this.Dispose), ex);
+                try
+                {
+                    this.ShutdownAsync(CancellationToken.None).GetAwaiter().GetResult();
+                }
+                catch (Exception ex)
+                {
+                    OpenTelemetrySdkEventSource.Log.SpanProcessorException(nameof(this.Dispose), ex);
+                }
             }
+
+            this.disposed = true;
         }
     }
 }
diff --git a/src/OpenTelemetry/Trace/CompositeActivityProcessor.cs b/src/OpenTelemetry/Trace/CompositeActivityProcessor.cs
@@ -17,7 +17,6 @@
 using System;
 using System.Collections.Generic;
 using System.Diagnostics;
-using System.Linq;
 using System.Threading;
 using System.Threading.Tasks;
 using OpenTelemetry.Internal;
@@ -55,6 +54,11 @@ public CompositeActivityProcessor(IEnumerable<ActivityProcessor> processors)
 
         public CompositeActivityProcessor AddProcessor(ActivityProcessor processor)
         {
+            if (processor == null)
+            {
+                throw new ArgumentNullException(nameof(processor));
+            }
+
             var node = new DoublyLinkedListNode<ActivityProcessor>(processor)
             {
                 Previous = this.tail,
@@ -132,22 +136,22 @@ protected override void Dispose(bool disposing)
                     {
                         cur.Value?.Dispose();
                     }
-                    catch (Exception e)
+                    catch (Exception ex)
                     {
-                        OpenTelemetrySdkEventSource.Log.SpanProcessorException("Dispose", e);
+                        OpenTelemetrySdkEventSource.Log.SpanProcessorException(nameof(this.Dispose), ex);
                     }
 
                     cur = cur.Next;
                 }
             }
 
-            base.Dispose(disposing);
-
             this.disposed = true;
         }
 
         private class DoublyLinkedListNode<T>
         {
+            public readonly T Value;
+
             public DoublyLinkedListNode(T value)
             {
                 this.Value = value;
@@ -156,8 +160,6 @@ public DoublyLinkedListNode(T value)
             public DoublyLinkedListNode<T> Previous { get; set; }
 
             public DoublyLinkedListNode<T> Next { get; set; }
-
-            public T Value { get; set; }
         }
     }
 }
diff --git a/src/OpenTelemetry/Trace/Internal/NoopActivityProcessor.cs b/src/OpenTelemetry/Trace/Internal/NoopActivityProcessor.cs
@@ -14,10 +14,6 @@
 // limitations under the License.
 // </copyright>
 
-using System.Diagnostics;
-using System.Threading;
-using System.Threading.Tasks;
-
 namespace OpenTelemetry.Trace.Internal
 {
     internal sealed class NoopActivityProcessor : ActivityProcessor

diff --git a/src/OpenTelemetry/Trace/TracerProviderExtensions.cs b/src/OpenTelemetry/Trace/TracerProviderExtensions.cs
@@ -15,9 +15,6 @@
 // </copyright>
 
 using System;
-using System.Collections.Generic;
-using System.Diagnostics;
-using OpenTelemetry.Resources;
 
 namespace OpenTelemetry.Trace
 {

diff --git a/src/OpenTelemetry/Trace/TracerProviderSdk.cs b/src/OpenTelemetry/Trace/TracerProviderSdk.cs
@@ -43,18 +43,12 @@ protected override void Dispose(bool disposing)
         {
             foreach (var item in this.Instrumentations)
             {
-                if (item is IDisposable disposable)
-                {
-                    disposable.Dispose();
-                }
+                (item as IDisposable)?.Dispose();
             }
 
             this.Instrumentations.Clear();
 
-            if (this.ActivityProcessor is IDisposable disposableProcessor)
-            {
-                disposableProcessor.Dispose();
-            }
+            this.ActivityProcessor?.Dispose();
 
             // Shutdown the listener last so that anything created while instrumentation cleans up will still be processed.
             // Redis instrumentation, for example, flushes during dispose which creates Activity objects for any profiling

diff --git a/test/OpenTelemetry.Tests/Implementation/Trace/Config/ActivityProcessorPipelineTests.cs b/test/OpenTelemetry.Tests/Implementation/Trace/Config/ActivityProcessorPipelineTests.cs
@@ -15,10 +15,7 @@
 // </copyright>
 
 using System;
-using System.Diagnostics;
 using System.Linq;
-using System.Threading;
-using System.Threading.Tasks;
 using OpenTelemetry.Testing.Export;
 using OpenTelemetry.Trace;
 using OpenTelemetry.Trace.Internal;

diff --git a/test/OpenTelemetry.Tests/Implementation/Trace/TraceSdkTest.cs b/test/OpenTelemetry.Tests/Implementation/Trace/TraceSdkTest.cs
@@ -15,7 +15,6 @@
 // </copyright>
 
 using System.Diagnostics;
-using OpenTelemetry;
 using OpenTelemetry.Trace;
 using Xunit;