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 @@ namespace OpenTelemetry
         /// <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 @@ namespace OpenTelemetry.Trace
             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>

src/OpenTelemetry/Trace/TracerProviderOptions.cs

@@ -1,27 +0,0 @@
-// <copyright file="TracerProviderOptions.cs" company="OpenTelemetry Authors">
-// Copyright The OpenTelemetry Authors
-//
-// Licensed under the Apache License, Version 2.0 (the "License");
-// you may not use this file except in compliance with the License.
-// You may obtain a copy of the License at
-//
-//     http://www.apache.org/licenses/LICENSE-2.0
-//
-// Unless required by applicable law or agreed to in writing, software
-// distributed under the License is distributed on an "AS IS" BASIS,
-// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-// See the License for the specific language governing permissions and
-// limitations under the License.
-// </copyright>
-
-namespace OpenTelemetry.Trace
-{
-    public class TracerProviderOptions
-    {
-        /// <summary>
-        /// Gets or sets a value indicating 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>
-        public bool SetErrorStatusOnException { get; set; } = false;
-    }
-}

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 @@ namespace OpenTelemetry.Trace.Tests
             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);
+        }
     }
 }

test/OpenTelemetry.Tests/Trace/TracerProviderOptionsTest.cs

@@ -1,81 +0,0 @@
-// <copyright file="TracerProviderOptionsTest.cs" company="OpenTelemetry Authors">
-// Copyright The OpenTelemetry Authors
-//
-// Licensed under the Apache License, Version 2.0 (the "License");
-// you may not use this file except in compliance with the License.
-// You may obtain a copy of the License at
-//
-//     http://www.apache.org/licenses/LICENSE-2.0
-//
-// Unless required by applicable law or agreed to in writing, software
-// distributed under the License is distributed on an "AS IS" BASIS,
-// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-// See the License for the specific language governing permissions and
-// limitations under the License.
-// </copyright>
-
-using System;
-using System.Diagnostics;
-using OpenTelemetry.Tests;
-using Xunit;
-
-namespace OpenTelemetry.Trace.Tests
-{
-    public class TracerProviderOptionsTest
-    {
-        private const string ActivitySourceName = "TracerProviderOptionsTest";
-
-        [Fact]
-        public void SetErrorStatusOnExceptionEnabled()
-        {
-            using var activitySource = new ActivitySource(ActivitySourceName);
-            using var tracerProvider = Sdk.CreateTracerProviderBuilder(options =>
-                {
-                    options.SetErrorStatusOnException = true;
-                })
-                .AddSource(ActivitySourceName)
-                .SetSampler(new AlwaysOnSampler())
-                .Build();
-
-            var activity = activitySource.StartActivity("Activity");
-
-            try
-            {
-                using (activity)
-                {
-                    throw new Exception("Oops!");
-                }
-            }
-            catch (Exception)
-            {
-            }
-
-            Assert.Equal(StatusCode.Error, activity.GetStatus().StatusCode);
-        }
-
-        [Fact]
-        public void SetErrorStatusOnExceptionDefault()
-        {
-            using var activitySource = new ActivitySource(ActivitySourceName);
-            using var tracerProvider = Sdk.CreateTracerProviderBuilder(options => { })
-                .AddSource(ActivitySourceName)
-                .SetSampler(new AlwaysOnSampler())
-                .Build();
-
-            var activity = activitySource.StartActivity("Activity");
-
-            try
-            {
-                using (activity)
-                {
-                    throw new Exception("Oops!");
-                }
-            }
-            catch (Exception)
-            {
-            }
-
-            Assert.Equal(StatusCode.Unset, activity.GetStatus().StatusCode);
-        }
-    }
-}