Update Processor.Shutdown to meet with the latest spec (#1282)

* processor.Shutdown returns bool according to the spec * update changelog
open-telemetry · Sep 17, 2020 · 8a4621c · 8a4621c
1 parent 7863cee
commit 8a4621c
Show file tree

Hide file tree

Showing 8 changed files with 39 additions and 20 deletions.
diff --git a/docs/trace/extending-the-sdk/MyProcessor.cs b/docs/trace/extending-the-sdk/MyProcessor.cs
@@ -44,9 +44,10 @@ protected override bool OnForceFlush(int timeoutMilliseconds)
         return true;
     }
 
-    protected override void OnShutdown(int timeoutMilliseconds)
+    protected override bool OnShutdown(int timeoutMilliseconds)
     {
         Console.WriteLine($"{this.name}.OnShutdown({timeoutMilliseconds})");
+        return true;
     }
 
     protected override void Dispose(bool disposing)

diff --git a/src/OpenTelemetry/CHANGELOG.md b/src/OpenTelemetry/CHANGELOG.md
@@ -2,6 +2,10 @@
 
 ## Unreleased
 
+* Changed `ActivityProcessor.OnShutdown` and `ActivityProcessor.Shutdown` to
+  return boolean value
+  ([#1282](https://github.com/open-telemetry/opentelemetry-dotnet/pull/1282))
+
 ## 0.6.0-beta.1
 
 Released 2020-Sep-15

diff --git a/src/OpenTelemetry/Trace/ActivityExporter.cs b/src/OpenTelemetry/Trace/ActivityExporter.cs
@@ -59,14 +59,17 @@ public abstract class ActivityExporter : IDisposable
         /// 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 void Shutdown(int timeoutMilliseconds = Timeout.Infinite)
+        public bool Shutdown(int timeoutMilliseconds = Timeout.Infinite)
         {
             if (timeoutMilliseconds < 0 && timeoutMilliseconds != Timeout.Infinite)
             {
@@ -75,16 +78,18 @@ public void Shutdown(int timeoutMilliseconds = Timeout.Infinite)
 
             if (Interlocked.Increment(ref this.shutdownCount) > 1)
             {
-                return; // shutdown already called
+                return false; // shutdown already called
             }
 
             try
             {
                 this.OnShutdown(timeoutMilliseconds);
+                return true; // TODO: update exporter.OnShutdown to return boolean
             }
             catch (Exception ex)
             {
                 OpenTelemetrySdkEventSource.Log.SpanProcessorException(nameof(this.Shutdown), ex);
+                return false;
             }
         }
 

diff --git a/src/OpenTelemetry/Trace/ActivityProcessor.cs b/src/OpenTelemetry/Trace/ActivityProcessor.cs
@@ -101,14 +101,17 @@ public bool ForceFlush(int timeoutMilliseconds = Timeout.Infinite)
         /// 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 void Shutdown(int timeoutMilliseconds = Timeout.Infinite)
+        public bool Shutdown(int timeoutMilliseconds = Timeout.Infinite)
         {
             if (timeoutMilliseconds < 0 && timeoutMilliseconds != Timeout.Infinite)
             {
@@ -117,16 +120,17 @@ public void Shutdown(int timeoutMilliseconds = Timeout.Infinite)
 
             if (Interlocked.Increment(ref this.shutdownCount) > 1)
             {
-                return; // shutdown already called
+                return false; // shutdown already called
             }
 
             try
             {
-                this.OnShutdown(timeoutMilliseconds);
+                return this.OnShutdown(timeoutMilliseconds);
             }
             catch (Exception ex)
             {
                 OpenTelemetrySdkEventSource.Log.SpanProcessorException(nameof(this.Shutdown), ex);
+                return false;
             }
         }
 
@@ -166,13 +170,17 @@ protected virtual bool OnForceFlush(int 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>
-        protected virtual void OnShutdown(int timeoutMilliseconds)
+        protected virtual bool OnShutdown(int timeoutMilliseconds)
         {
+            return true;
         }
 
         /// <summary>

diff --git a/src/OpenTelemetry/Trace/BaseExportActivityProcessor.cs b/src/OpenTelemetry/Trace/BaseExportActivityProcessor.cs
@@ -46,9 +46,9 @@ public sealed override void OnStart(Activity activity)
         public abstract override void OnEnd(Activity activity);
 
         /// <inheritdoc />
-        protected override void OnShutdown(int timeoutMilliseconds)
+        protected override bool OnShutdown(int timeoutMilliseconds)
         {
-            this.exporter.Shutdown(timeoutMilliseconds);
+            return this.exporter.Shutdown(timeoutMilliseconds);
         }
 
         /// <inheritdoc/>

diff --git a/src/OpenTelemetry/Trace/BatchExportActivityProcessor.cs b/src/OpenTelemetry/Trace/BatchExportActivityProcessor.cs
@@ -174,28 +174,26 @@ protected override bool OnForceFlush(int timeoutMilliseconds)
         }
 
         /// <inheritdoc/>
-        protected override void OnShutdown(int timeoutMilliseconds)
+        protected override bool OnShutdown(int timeoutMilliseconds)
         {
             this.shutdownDrainTarget = this.circularBuffer.AddedCount;
             this.shutdownTrigger.Set();
 
             if (timeoutMilliseconds == Timeout.Infinite)
             {
                 this.exporterThread.Join();
-                this.exporter.Shutdown();
-                return;
+                return this.exporter.Shutdown();
             }
 
             if (timeoutMilliseconds == 0)
             {
-                this.exporter.Shutdown(0);
-                return;
+                return this.exporter.Shutdown(0);
             }
 
             var sw = Stopwatch.StartNew();
             this.exporterThread.Join(timeoutMilliseconds);
             var timeout = (long)timeoutMilliseconds - sw.ElapsedMilliseconds;
-            this.exporter.Shutdown((int)Math.Max(timeout, 0));
+            return this.exporter.Shutdown((int)Math.Max(timeout, 0));
         }
 
         private void ExporterProc()

diff --git a/src/OpenTelemetry/Trace/CompositeActivityProcessor.cs b/src/OpenTelemetry/Trace/CompositeActivityProcessor.cs
@@ -129,28 +129,30 @@ protected override bool OnForceFlush(int timeoutMilliseconds)
         }
 
         /// <inheritdoc/>
-        protected override void OnShutdown(int timeoutMilliseconds)
+        protected override bool OnShutdown(int timeoutMilliseconds)
         {
             var cur = this.head;
-
+            var result = true;
             var sw = Stopwatch.StartNew();
 
             while (cur != null)
             {
                 if (timeoutMilliseconds == Timeout.Infinite)
                 {
-                    cur.Value.Shutdown(Timeout.Infinite);
+                    result = cur.Value.Shutdown(Timeout.Infinite) && result;
                 }
                 else
                 {
                     var timeout = (long)timeoutMilliseconds - sw.ElapsedMilliseconds;
 
                     // notify all the processors, even if we run overtime
-                    cur.Value.Shutdown((int)Math.Max(timeout, 0));
+                    result = cur.Value.Shutdown((int)Math.Max(timeout, 0)) && result;
                 }
 
                 cur = cur.Next;
             }
+
+            return result;
         }
 
         protected override void Dispose(bool disposing)

diff --git a/test/OpenTelemetry.Tests/Shared/TestActivityProcessor.cs b/test/OpenTelemetry.Tests/Shared/TestActivityProcessor.cs
@@ -57,9 +57,10 @@ protected override bool OnForceFlush(int timeoutMilliseconds)
             return true;
         }
 
-        protected override void OnShutdown(int timeoutMilliseconds)
+        protected override bool OnShutdown(int timeoutMilliseconds)
         {
             this.ShutdownCalled = true;
+            return true;
         }
 
         protected override void Dispose(bool disposing)