Continue Refactoring TracerProvider. (#1035)

cijothomas · reyang · web-flow · commit 8af9b326c6b1 · 2020-08-10T15:18:19.000-07:00
* optimize the flow

* fix instrumenataion

* move mroe things to provider ctor

* move logic arounds

Co-authored-by: Reiley Yang &lt;reyang@microsoft.com&gt;
diff --git a/docs/trace/building-your-own-exporter/Program.cs b/docs/trace/building-your-own-exporter/Program.cs
@@ -25,11 +25,7 @@ public class Program
     public static void Main()
     {
         using var tracerProvider = Sdk.CreateTracerProviderBuilder()
-            .AddActivitySources(
-            new string[]
-            {
-                "MyCompany.MyProduct.MyLibrary",
-            })
+            .AddActivitySource("MyCompany.MyProduct.MyLibrary")
             .AddMyExporter()
             .Build();
 
diff --git a/docs/trace/building-your-own-processor/Program.cs b/docs/trace/building-your-own-processor/Program.cs
@@ -26,10 +26,7 @@ public class Program
     public static void Main()
     {
         using var tracerProvider = Sdk.CreateTracerProviderBuilder()
-            .AddActivitySources(new string[]
-            {
-                "MyCompany.MyProduct.MyLibrary",
-            })
+            .AddActivitySource("MyCompany.MyProduct.MyLibrary")
             .AddProcessor(new MyActivityProcessor("A"))
             .AddProcessor(new MyActivityProcessor("B"))
             .Build();
diff --git a/docs/trace/building-your-own-sampler/Program.cs b/docs/trace/building-your-own-sampler/Program.cs
@@ -27,11 +27,7 @@ public class Program
     public static void Main()
     {
         using var tracerProvider = Sdk.CreateTracerProviderBuilder()
-            .AddActivitySources(
-            new string[]
-            {
-                "MyCompany.MyProduct.MyLibrary",
-            })
+            .AddActivitySource("MyCompany.MyProduct.MyLibrary")
             .SetSampler(new MySampler())
             .AddProcessor(new SimpleActivityProcessor(new ConsoleExporter(new ConsoleExporterOptions())))
             .Build();
diff --git a/docs/trace/getting-started/Program.cs b/docs/trace/getting-started/Program.cs
@@ -27,11 +27,7 @@ public class Program
     public static void Main()
     {
         using var tracerProvider = Sdk.CreateTracerProviderBuilder()
-            .AddActivitySources(
-            new string[]
-            {
-                "MyCompany.MyProduct.MyLibrary",
-            })
+            .AddActivitySource("MyCompany.MyProduct.MyLibrary")
             .AddProcessor(new SimpleActivityProcessor(new ConsoleExporter(new ConsoleExporterOptions())))
             .Build();
 
diff --git a/src/OpenTelemetry/Trace/TracerProviderBuilder.cs b/src/OpenTelemetry/Trace/TracerProviderBuilder.cs
@@ -15,7 +15,6 @@
 // </copyright>
 using System;
 using System.Collections.Generic;
-using System.Diagnostics;
 using OpenTelemetry.Resources;
 using OpenTelemetry.Trace.Samplers;
 
@@ -26,17 +25,17 @@ namespace OpenTelemetry.Trace
     /// </summary>
     public class TracerProviderBuilder
     {
-        internal TracerProviderBuilder()
-        {
-        }
+        private readonly List<ActivityProcessor> processors = new List<ActivityProcessor>();
 
-        internal Sampler Sampler { get; private set; }
+        private readonly List<string> sources = new List<string>();
 
-        internal Resource Resource { get; private set; } = Resource.Empty;
+        private Sampler sampler = new ParentOrElseSampler(new AlwaysOnSampler());
 
-        internal ActivityProcessor ActivityProcessor { get; private set; }
+        private Resource resource = Resource.Empty;
 
-        internal Dictionary<string, bool> ActivitySourceNames { get; private set; }
+        internal TracerProviderBuilder()
+        {
+        }
 
         internal List<InstrumentationFactory> InstrumentationFactories { get; private set; }
 
@@ -47,7 +46,7 @@ internal TracerProviderBuilder()
         /// <returns>Returns <see cref="TracerProviderBuilder"/> for chaining.</returns>
         public TracerProviderBuilder SetSampler(Sampler sampler)
         {
-            this.Sampler = sampler ?? throw new ArgumentNullException(nameof(sampler));
+            this.sampler = sampler;
             return this;
         }
 
@@ -58,45 +57,44 @@ public TracerProviderBuilder SetSampler(Sampler sampler)
         /// <returns>Returns <see cref="TracerProviderBuilder"/> for chaining.</returns>
         public TracerProviderBuilder SetResource(Resource resource)
         {
-            this.Resource = resource ?? Resource.Empty;
+            this.resource = resource ?? Resource.Empty;
             return this;
         }
 
         /// <summary>
         /// Adds given activitysource name to the list of subscribed sources.
         /// </summary>
-        /// <param name="activitySourceName">Activity source name.</param>
+        /// <param name="name">Activity source name.</param>
         /// <returns>Returns <see cref="TracerProviderBuilder"/> for chaining.</returns>
-        public TracerProviderBuilder AddActivitySource(string activitySourceName)
-        {
+        public TracerProviderBuilder AddActivitySource(string name)
+        {
+            if (string.IsNullOrWhiteSpace(name))
+            {
+                throw new ArgumentException($"{nameof(name)} is null or whitespace.");
+            }
+
             // TODO: We need to fix the listening model.
             // Today it ignores version.
-            if (this.ActivitySourceNames == null)
-            {
-                this.ActivitySourceNames = new Dictionary<string, bool>(StringComparer.OrdinalIgnoreCase);
-            }
+            this.sources.Add(name);
 
-            this.ActivitySourceNames[activitySourceName] = true;
             return this;
         }
 
         /// <summary>
         /// Adds given activitysource names to the list of subscribed sources.
         /// </summary>
-        /// <param name="activitySourceNames">Activity source names.</param>
+        /// <param name="names">Activity source names.</param>
         /// <returns>Returns <see cref="TracerProviderBuilder"/> for chaining.</returns>
-        public TracerProviderBuilder AddActivitySources(IEnumerable<string> activitySourceNames)
-        {
-            // TODO: We need to fix the listening model.
-            // Today it ignores version.
-            if (this.ActivitySourceNames == null)
-            {
-                this.ActivitySourceNames = new Dictionary<string, bool>(StringComparer.OrdinalIgnoreCase);
+        public TracerProviderBuilder AddActivitySource(IEnumerable<string> names)
+        {
+            if (names == null)
+            {
+                throw new ArgumentNullException(nameof(names));
             }
 
-            foreach (var activitySourceName in activitySourceNames)
+            foreach (var name in names)
             {
-                this.ActivitySourceNames[activitySourceName] = true;
+                this.AddActivitySource(name);
             }
 
             return this;
@@ -108,24 +106,14 @@ public TracerProviderBuilder AddActivitySources(IEnumerable<string> activitySour
         /// <param name="activityProcessor">Activity processor to add.</param>
         /// <returns>Returns <see cref="TracerProviderBuilder"/> for chaining.</returns>
         public TracerProviderBuilder AddProcessor(ActivityProcessor activityProcessor)
-        {
-            if (this.ActivityProcessor == null)
-            {
-                this.ActivityProcessor = activityProcessor;
-            }
-            else if (this.ActivityProcessor is CompositeActivityProcessor compositeProcessor)
-            {
-                compositeProcessor.AddProcessor(activityProcessor);
-            }
-            else
-            {
-                this.ActivityProcessor = new CompositeActivityProcessor(new[]
-                {
-                    this.ActivityProcessor,
-                    activityProcessor,
-                });
+        {
+            if (activityProcessor == null)
+            {
+                throw new ArgumentNullException(nameof(activityProcessor));
             }
 
+            this.processors.Add(activityProcessor);
+
             return this;
         }
 
@@ -160,104 +148,7 @@ public TracerProviderBuilder AddInstrumentation<TInstrumentation>(
 
         public TracerProvider Build()
         {
-            this.Sampler = this.Sampler ?? new ParentOrElseSampler(new AlwaysOnSampler());
-
-            var provider = new TracerProviderSdk
-            {
-                Resource = this.Resource,
-                Sampler = this.Sampler,
-                ActivityProcessor = this.ActivityProcessor,
-            };
-
-            var activitySource = new ActivitySourceAdapter(provider.Sampler, provider.ActivityProcessor, provider.Resource);
-
-            if (this.InstrumentationFactories != null)
-            {
-                foreach (var instrumentation in this.InstrumentationFactories)
-                {
-                    provider.Instrumentations.Add(instrumentation.Factory(activitySource));
-                }
-            }
-
-            provider.ActivityListener = new ActivityListener
-            {
-                // Callback when Activity is started.
-                ActivityStarted = (activity) =>
-                {
-                    if (activity.IsAllDataRequested)
-                    {
-                        activity.SetResource(this.Resource);
-                    }
-
-                    provider.ActivityProcessor?.OnStart(activity);
-                },
-
-                // Callback when Activity is stopped.
-                ActivityStopped = (activity) =>
-                {
-                    provider.ActivityProcessor?.OnEnd(activity);
-                },
-
-                // Function which takes ActivitySource and returns true/false to indicate if it should be subscribed to
-                // or not.
-                ShouldListenTo = (activitySource) =>
-                {
-                    if (this.ActivitySourceNames == null)
-                    {
-                        return false;
-                    }
-
-                    return this.ActivitySourceNames.ContainsKey(activitySource.Name);
-                },
-
-                // Setting this to true means TraceId will be always
-                // available in sampling callbacks and will be the actual
-                // traceid used, if activity ends up getting created.
-                AutoGenerateRootContextTraceId = true,
-
-                // This delegate informs ActivitySource about sampling decision when the parent context is an ActivityContext.
-                GetRequestedDataUsingContext = (ref ActivityCreationOptions<ActivityContext> options) => ComputeActivityDataRequest(options, this.Sampler),
-            };
-
-            ActivitySource.AddActivityListener(provider.ActivityListener);
-
-            return provider;
-        }
-
-        internal static ActivityDataRequest ComputeActivityDataRequest(
-            in ActivityCreationOptions<ActivityContext> options,
-            Sampler sampler)
-        {
-            var isRootSpan = /*TODO: Put back once AutoGenerateRootContextTraceId is removed.
-                              options.Parent.TraceId == default ||*/ options.Parent.SpanId == default;
-
-            if (sampler != null)
-            {
-                // As we set ActivityListener.AutoGenerateRootContextTraceId = true,
-                // Parent.TraceId will always be the TraceId of the to-be-created Activity,
-                // if it get created.
-                ActivityTraceId traceId = options.Parent.TraceId;
-
-                var samplingParameters = new SamplingParameters(
-                    options.Parent,
-                    traceId,
-                    options.Name,
-                    options.Kind,
-                    options.Tags,
-                    options.Links);
-
-                var shouldSample = sampler.ShouldSample(samplingParameters);
-                if (shouldSample.IsSampled)
-                {
-                    return ActivityDataRequest.AllDataAndRecorded;
-                }
-            }
-
-            // If it is the root span select PropagationData so the trace ID is preserved
-            // even if no activity of the trace is recorded (sampled per OpenTelemetry parlance).
-            return isRootSpan
-                ? ActivityDataRequest.PropagationData
-                : ActivityDataRequest.None;
+            return new TracerProviderSdk(this.sources, this.processors, this.InstrumentationFactories, this.sampler, this.resource);
         }
 
         internal readonly struct InstrumentationFactory
diff --git a/src/OpenTelemetry/Trace/TracerProviderSdk.cs b/src/OpenTelemetry/Trace/TracerProviderSdk.cs
