Remove options, stick with builder (#1875)

* remove options, stick with builder

* update changelog

* sanity

* update changelog

Co-authored-by: Cijo Thomas <cithomas@microsoft.com>

docs/trace/exception-handling/Program.cs

@@ -26,12 +26,10 @@ public class Program
 
     public static void Main()
     {
-        using var tracerProvider = Sdk.CreateTracerProviderBuilder(options =>
-            {
-                options.SetErrorStatusOnException = true;
-            })
+        using var tracerProvider = Sdk.CreateTracerProviderBuilder()
             .AddSource("MyCompany.MyProduct.MyLibrary")
             .SetSampler(new AlwaysOnSampler())
+            .SetErrorStatusOnException()
             .AddConsoleExporter()
             .Build();
 

docs/trace/exception-handling/README.md

@@ -34,9 +34,9 @@ The following configuration will automatically detect exception and set the
 activity status to `Error`:
 
 ```csharp
-Sdk.CreateTracerProviderBuilder(options => {
-    options.SetErrorStatusOnException = true;
-});
+Sdk.CreateTracerProviderBuilder()
+    .SetErrorStatusOnException()
+    // ...
 ```
 
 A complete example can be found [here](./Program.cs).
@@ -79,12 +79,10 @@ public class Program
     {
         AppDomain.CurrentDomain.UnhandledException += UnhandledExceptionHandler;
 
-        using var tracerProvider = Sdk.CreateTracerProviderBuilder(options =>
-            {
-                options.SetErrorStatusOnException = true;
-            })
+        using var tracerProvider = Sdk.CreateTracerProviderBuilder()
             .AddSource("MyCompany.MyProduct.MyLibrary")
             .SetSampler(new AlwaysOnSampler())
+            .SetErrorStatusOnException()
             .AddConsoleExporter()
             .Build();
 

src/OpenTelemetry/.publicApi/net452/PublicAPI.Unshipped.txt

@@ -1,8 +1,4 @@
 OpenTelemetry.Trace.ParentBasedSampler.ParentBasedSampler(OpenTelemetry.Trace.Sampler rootSampler, OpenTelemetry.Trace.Sampler remoteParentSampled = null, OpenTelemetry.Trace.Sampler remoteParentNotSampled = null, OpenTelemetry.Trace.Sampler localParentSampled = null, OpenTelemetry.Trace.Sampler localParentNotSampled = null) -> void
-OpenTelemetry.Trace.TracerProviderOptions
-OpenTelemetry.Trace.TracerProviderOptions.SetErrorStatusOnException.get -> bool
-OpenTelemetry.Trace.TracerProviderOptions.SetErrorStatusOnException.set -> void
-OpenTelemetry.Trace.TracerProviderOptions.TracerProviderOptions() -> void
-static OpenTelemetry.Sdk.CreateTracerProviderBuilder(System.Action<OpenTelemetry.Trace.TracerProviderOptions> configure = null) -> OpenTelemetry.Trace.TracerProviderBuilder
 static OpenTelemetry.Trace.TracerProviderBuilderExtensions.AddLegacyActivity(this OpenTelemetry.Trace.TracerProviderBuilder tracerProviderBuilder, string operationName) -> OpenTelemetry.Trace.TracerProviderBuilder
+static OpenTelemetry.Trace.TracerProviderBuilderExtensions.SetErrorStatusOnException(this OpenTelemetry.Trace.TracerProviderBuilder tracerProviderBuilder, bool enabled = true) -> OpenTelemetry.Trace.TracerProviderBuilder
 static OpenTelemetry.Trace.TracerProviderExtensions.ForceFlush(this OpenTelemetry.Trace.TracerProvider provider, int timeoutMilliseconds = -1) -> bool

src/OpenTelemetry/.publicApi/net46/PublicAPI.Unshipped.txt

@@ -1,8 +1,4 @@
 OpenTelemetry.Trace.ParentBasedSampler.ParentBasedSampler(OpenTelemetry.Trace.Sampler rootSampler, OpenTelemetry.Trace.Sampler remoteParentSampled = null, OpenTelemetry.Trace.Sampler remoteParentNotSampled = null, OpenTelemetry.Trace.Sampler localParentSampled = null, OpenTelemetry.Trace.Sampler localParentNotSampled = null) -> void
-OpenTelemetry.Trace.TracerProviderOptions
-OpenTelemetry.Trace.TracerProviderOptions.SetErrorStatusOnException.get -> bool
-OpenTelemetry.Trace.TracerProviderOptions.SetErrorStatusOnException.set -> void
-OpenTelemetry.Trace.TracerProviderOptions.TracerProviderOptions() -> void
-static OpenTelemetry.Sdk.CreateTracerProviderBuilder(System.Action<OpenTelemetry.Trace.TracerProviderOptions> configure = null) -> OpenTelemetry.Trace.TracerProviderBuilder
 static OpenTelemetry.Trace.TracerProviderBuilderExtensions.AddLegacyActivity(this OpenTelemetry.Trace.TracerProviderBuilder tracerProviderBuilder, string operationName) -> OpenTelemetry.Trace.TracerProviderBuilder
+static OpenTelemetry.Trace.TracerProviderBuilderExtensions.SetErrorStatusOnException(this OpenTelemetry.Trace.TracerProviderBuilder tracerProviderBuilder, bool enabled = true) -> OpenTelemetry.Trace.TracerProviderBuilder
 static OpenTelemetry.Trace.TracerProviderExtensions.ForceFlush(this OpenTelemetry.Trace.TracerProvider provider, int timeoutMilliseconds = -1) -> bool

src/OpenTelemetry/.publicApi/net461/PublicAPI.Unshipped.txt

@@ -8,10 +8,6 @@ OpenTelemetry.Logs.OpenTelemetryLoggerOptions.IncludeScopes.set -> void
 OpenTelemetry.Logs.OpenTelemetryLoggerOptions.ParseStateValues.get -> bool
 OpenTelemetry.Logs.OpenTelemetryLoggerOptions.ParseStateValues.set -> void
 OpenTelemetry.Trace.ParentBasedSampler.ParentBasedSampler(OpenTelemetry.Trace.Sampler rootSampler, OpenTelemetry.Trace.Sampler remoteParentSampled = null, OpenTelemetry.Trace.Sampler remoteParentNotSampled = null, OpenTelemetry.Trace.Sampler localParentSampled = null, OpenTelemetry.Trace.Sampler localParentNotSampled = null) -> void
-OpenTelemetry.Trace.TracerProviderOptions
-OpenTelemetry.Trace.TracerProviderOptions.SetErrorStatusOnException.get -> bool
-OpenTelemetry.Trace.TracerProviderOptions.SetErrorStatusOnException.set -> void
-OpenTelemetry.Trace.TracerProviderOptions.TracerProviderOptions() -> void
-static OpenTelemetry.Sdk.CreateTracerProviderBuilder(System.Action<OpenTelemetry.Trace.TracerProviderOptions> configure = null) -> OpenTelemetry.Trace.TracerProviderBuilder
 static OpenTelemetry.Trace.TracerProviderBuilderExtensions.AddLegacyActivity(this OpenTelemetry.Trace.TracerProviderBuilder tracerProviderBuilder, string operationName) -> OpenTelemetry.Trace.TracerProviderBuilder
+static OpenTelemetry.Trace.TracerProviderBuilderExtensions.SetErrorStatusOnException(this OpenTelemetry.Trace.TracerProviderBuilder tracerProviderBuilder, bool enabled = true) -> OpenTelemetry.Trace.TracerProviderBuilder
 static OpenTelemetry.Trace.TracerProviderExtensions.ForceFlush(this OpenTelemetry.Trace.TracerProvider provider, int timeoutMilliseconds = -1) -> bool

src/OpenTelemetry/.publicApi/netstandard2.0/PublicAPI.Unshipped.txt

@@ -8,10 +8,6 @@ OpenTelemetry.Logs.OpenTelemetryLoggerOptions.IncludeScopes.set -> void
 OpenTelemetry.Logs.OpenTelemetryLoggerOptions.ParseStateValues.get -> bool
 OpenTelemetry.Logs.OpenTelemetryLoggerOptions.ParseStateValues.set -> void
 OpenTelemetry.Trace.ParentBasedSampler.ParentBasedSampler(OpenTelemetry.Trace.Sampler rootSampler, OpenTelemetry.Trace.Sampler remoteParentSampled = null, OpenTelemetry.Trace.Sampler remoteParentNotSampled = null, OpenTelemetry.Trace.Sampler localParentSampled = null, OpenTelemetry.Trace.Sampler localParentNotSampled = null) -> void
-OpenTelemetry.Trace.TracerProviderOptions
-OpenTelemetry.Trace.TracerProviderOptions.SetErrorStatusOnException.get -> bool
-OpenTelemetry.Trace.TracerProviderOptions.SetErrorStatusOnException.set -> void
-OpenTelemetry.Trace.TracerProviderOptions.TracerProviderOptions() -> void
-static OpenTelemetry.Sdk.CreateTracerProviderBuilder(System.Action<OpenTelemetry.Trace.TracerProviderOptions> configure = null) -> OpenTelemetry.Trace.TracerProviderBuilder
 static OpenTelemetry.Trace.TracerProviderBuilderExtensions.AddLegacyActivity(this OpenTelemetry.Trace.TracerProviderBuilder tracerProviderBuilder, string operationName) -> OpenTelemetry.Trace.TracerProviderBuilder
+static OpenTelemetry.Trace.TracerProviderBuilderExtensions.SetErrorStatusOnException(this OpenTelemetry.Trace.TracerProviderBuilder tracerProviderBuilder, bool enabled = true) -> OpenTelemetry.Trace.TracerProviderBuilder
 static OpenTelemetry.Trace.TracerProviderExtensions.ForceFlush(this OpenTelemetry.Trace.TracerProvider provider, int timeoutMilliseconds = -1) -> bool

src/OpenTelemetry/CHANGELOG.md

@@ -9,8 +9,10 @@ please check the latest changes
 
 ## Unreleased
 
-* Added `TracerProviderOptions` and `SetErrorStatusOnException`.
-  ([#1858](https://github.com/open-telemetry/opentelemetry-dotnet/pull/1858))
+* Added `TracerProviderBuilder.SetErrorStatusOnException` which automatically
+  sets the activity status to `Error` when exception happened.
+  ([#1858](https://github.com/open-telemetry/opentelemetry-dotnet/pull/1858)
+  [#1875](https://github.com/open-telemetry/opentelemetry-dotnet/pull/1875))
 * Added `ForceFlush` to `TracerProvider`.
   ([#1837](https://github.com/open-telemetry/opentelemetry-dotnet/pull/1837))
 * Added a TracerProvierBuilder extension method called
@@ -46,9 +48,9 @@ Released 2021-Feb-09
 Released 2021-Feb-04
 
 * Default `Resource` will now contain service.name instead of Telemetry SDK.
-  [#1744](https://github.com/open-telemetry/opentelemetry-dotnet/pull/1744)
+  ([#1744](https://github.com/open-telemetry/opentelemetry-dotnet/pull/1744))
 * Added GetDefaultResource() method to `Provider`.
-  [#1768](https://github.com/open-telemetry/opentelemetry-dotnet/pull/1768)
+  ([#1768](https://github.com/open-telemetry/opentelemetry-dotnet/pull/1768))
 
 ## 1.0.0-rc2
 
@@ -149,14 +151,14 @@ Released 2020-Oct-16
 * Changed `ActivityExporter.OnShutdown`, `ActivityExporter.Shutdown`,
   `ActivityProcessor.OnShutdown` and `ActivityProcessor.Shutdown` to return
   boolean value
-  ([#1282](https://github.com/open-telemetry/opentelemetry-dotnet/pull/1282))
-  ([#1285](https://github.com/open-telemetry/opentelemetry-dotnet/pull/1285))
+  ([#1282](https://github.com/open-telemetry/opentelemetry-dotnet/pull/1282)
+  [#1285](https://github.com/open-telemetry/opentelemetry-dotnet/pull/1285))
 * Renamed `SamplingDecision` options (`NotRecord` to `Drop`, `Record` to
   `RecordOnly`, and `RecordAndSampled` to `RecordAndSample`)
   ([#1297](https://github.com/open-telemetry/opentelemetry-dotnet/pull/1297))
 * Added `ILogger`/`Microsoft.Extensions.Logging` integration
-  ([#1308](https://github.com/open-telemetry/opentelemetry-dotnet/pull/1308))
-  ([#1315](https://github.com/open-telemetry/opentelemetry-dotnet/pull/1315))
+  ([#1308](https://github.com/open-telemetry/opentelemetry-dotnet/pull/1308)
+  [#1315](https://github.com/open-telemetry/opentelemetry-dotnet/pull/1315))
 * Changed exporter and processor to generic types
   ([#1328](https://github.com/open-telemetry/opentelemetry-dotnet/pull/1328)):
   * `ActivityExporter` changed to `BaseExporter<Activity>`

src/OpenTelemetry/Sdk.cs

@@ -60,19 +60,7 @@ public static void SetDefaultTextMapPropagator(TextMapPropagator textMapPropagat
         /// <returns>TracerProviderBuilder instance, which should be used to build TracerProvider.</returns>
         public static TracerProviderBuilder CreateTracerProviderBuilder()
         {
-            return CreateTracerProviderBuilder(null);
-        }
-
-        /// <summary>
-        /// Creates TracerProviderBuilder which should be used to build TracerProvider.
-        /// </summary>
-        /// <param name="configure">TracerProvider configuration options.</param>
-        /// <returns>TracerProviderBuilder instance, which should be used to build TracerProvider.</returns>
-        public static TracerProviderBuilder CreateTracerProviderBuilder(Action<TracerProviderOptions> configure = null)
-        {
-            var options = new TracerProviderOptions();
-            configure?.Invoke(options);
-            return new TracerProviderBuilderSdk(options);
+            return new TracerProviderBuilderSdk();
         }
     }
 }

src/OpenTelemetry/Trace/TracerProviderBuilderExtensions.cs

@@ -22,6 +22,23 @@ namespace OpenTelemetry.Trace
 {
     public static class TracerProviderBuilderExtensions
     {
+        /// <summary>
+        /// Sets whether the status of <see cref="System.Diagnostics.Activity"/>
+        /// should be set to <c>Status.Error</c> when it ended abnormally due to an unhandled exception.
+        /// </summary>
+        /// <param name="tracerProviderBuilder">TracerProviderBuilder instance.</param>
+        /// <param name="enabled">Enabled or not. Default value is <c>true</c>.</param>
+        /// <returns>Returns <see cref="TracerProviderBuilder"/> for chaining.</returns>
+        public static TracerProviderBuilder SetErrorStatusOnException(this TracerProviderBuilder tracerProviderBuilder, bool enabled = true)
+        {
+            if (tracerProviderBuilder is TracerProviderBuilderSdk tracerProviderBuilderSdk)
+            {
+                tracerProviderBuilderSdk.SetErrorStatusOnException(enabled);
+            }
+
+            return tracerProviderBuilder;
+        }
+
         /// <summary>
         /// Sets sampler.
         /// </summary>

src/OpenTelemetry/Trace/TracerProviderBuilderSdk.cs

@@ -27,29 +27,14 @@ namespace OpenTelemetry.Trace
     internal class TracerProviderBuilderSdk : TracerProviderBuilder
     {
         private readonly List<InstrumentationFactory> instrumentationFactories = new List<InstrumentationFactory>();
-
-        private readonly TracerProviderOptions options;
         private readonly List<BaseProcessor<Activity>> processors = new List<BaseProcessor<Activity>>();
         private readonly List<string> sources = new List<string>();
         private readonly Dictionary<string, bool> legacyActivityOperationNames = new Dictionary<string, bool>(StringComparer.OrdinalIgnoreCase);
         private ResourceBuilder resourceBuilder = ResourceBuilder.CreateDefault();
         private Sampler sampler = new ParentBasedSampler(new AlwaysOnSampler());
 
-        internal TracerProviderBuilderSdk(TracerProviderOptions options)
+        internal TracerProviderBuilderSdk()
         {
-            this.options = options ?? new TracerProviderOptions();
-
-            if (options.SetErrorStatusOnException)
-            {
-                try
-                {
-                    this.AddProcessor(new ExceptionProcessor());
-                }
-                catch (Exception ex)
-                {
-                    throw new NotSupportedException($"{nameof(options.SetErrorStatusOnException)} is not supported on this platform.", ex);
-                }
-            }
         }
 
         /// <summary>
@@ -103,6 +88,47 @@ public override TracerProviderBuilder AddSource(params string[] names)
             return this;
         }
 
+        /// <summary>
+        /// Sets whether the status of <see cref="System.Diagnostics.Activity"/>
+        /// should be set to <c>Status.Error</c> when it ended abnormally due to an unhandled exception.
+        /// </summary>
+        /// <param name="enabled">Enabled or not.</param>
+        /// <returns>Returns <see cref="TracerProviderBuilder"/> for chaining.</returns>
+        internal TracerProviderBuilder SetErrorStatusOnException(bool enabled)
+        {
+            ExceptionProcessor existingExceptionProcessor = null;
+
+            if (this.processors.Count > 0)
+            {
+                existingExceptionProcessor = this.processors[0] as ExceptionProcessor;
+            }
+
+            if (enabled)
+            {
+                if (existingExceptionProcessor == null)
+                {
+                    try
+                    {
+                        this.processors.Insert(0, new ExceptionProcessor());
+                    }
+                    catch (Exception ex)
+                    {
+                        throw new NotSupportedException("SetErrorStatusOnException is not supported on this platform.", ex);
+                    }
+                }
+            }
+            else
+            {
+                if (existingExceptionProcessor != null)
+                {
+                    this.processors.RemoveAt(0);
+                    existingExceptionProcessor.Dispose();
+                }
+            }
+
+            return this;
+        }
+
         /// <summary>
         /// Sets sampler.
         /// </summary>

test/OpenTelemetry.Tests/Trace/TracerProviderBuilderExtensionsTest.cs

@@ -23,6 +23,8 @@ namespace OpenTelemetry.Trace.Tests
 {
     public class TracerProviderBuilderExtensionsTest
     {
+        private const string ActivitySourceName = "TracerProviderBuilderExtensionsTest";
+
         [Fact]
         public void AddLegacyOperationName_NullBuilder_Noop()
         {
@@ -57,5 +59,88 @@ public void AddLegacyOperationNameAddsActivityListenerForEmptyActivitySource()
             using var provider = builder.Build();
             Assert.True(emptyActivitySource.HasListeners());
         }
+
+        [Fact]
+        public void SetErrorStatusOnExceptionEnabled()
+        {
+            using var activitySource = new ActivitySource(ActivitySourceName);
+            using var tracerProvider = Sdk.CreateTracerProviderBuilder()
+                .AddSource(ActivitySourceName)
+                .SetSampler(new AlwaysOnSampler())
+                .SetErrorStatusOnException(false)
+                .SetErrorStatusOnException(false)
+                .SetErrorStatusOnException(true)
+                .SetErrorStatusOnException(true)
+                .SetErrorStatusOnException(false)
+                .SetErrorStatusOnException()
+                .Build();
+
+            Activity activity = null;
+
+            try
+            {
+                using (activity = activitySource.StartActivity("Activity"))
+                {
+                    throw new Exception("Oops!");
+                }
+            }
+            catch (Exception)
+            {
+            }
+
+            Assert.Equal(StatusCode.Error, activity.GetStatus().StatusCode);
+        }
+
+        [Fact]
+        public void SetErrorStatusOnExceptionDisabled()
+        {
+            using var activitySource = new ActivitySource(ActivitySourceName);
+            using var tracerProvider = Sdk.CreateTracerProviderBuilder()
+                .AddSource(ActivitySourceName)
+                .SetSampler(new AlwaysOnSampler())
+                .SetErrorStatusOnException()
+                .SetErrorStatusOnException(false)
+                .Build();
+
+            Activity activity = null;
+
+            try
+            {
+                using (activity = activitySource.StartActivity("Activity"))
+                {
+                    throw new Exception("Oops!");
+                }
+            }
+            catch (Exception)
+            {
+            }
+
+            Assert.Equal(StatusCode.Unset, activity.GetStatus().StatusCode);
+        }
+
+        [Fact]
+        public void SetErrorStatusOnExceptionDefault()
+        {
+            using var activitySource = new ActivitySource(ActivitySourceName);
+            using var tracerProvider = Sdk.CreateTracerProviderBuilder()
+                .AddSource(ActivitySourceName)
+                .SetSampler(new AlwaysOnSampler())
+                .Build();
+
+            Activity activity = null;
+
+            try
+            {
+                using (activity = activitySource.StartActivity("Activity"))
+                {
+                    throw new Exception("Oops!");
+                }
+            }
+            catch (Exception)
+            {
+            }
+
+            Assert.Equal(StatusCode.Unset, activity.GetStatus().StatusCode);
+        }
     }
 }