Use IsEnabled DS callback to filter out requests (#259)

* Use IsEnabled DS callback to filter out requests

* Update src/OpenTelemetry.Collector.Dependencies/DependenciesCollectorOptions.cs

Co-Authored-By: Bruno Garcia <bruno@brunogarcia.com>

* Update src/OpenTelemetry.Collector.AspNetCore/RequestsCollectorOptions.cs

samples/Exporters/TestHttpClient.cs

@@ -40,7 +40,7 @@ internal static object Run()
 
             var tracerFactory = new TracerFactory(new BatchingSpanProcessor(exporter));
             var tracer = tracerFactory.GetTracer(string.Empty);
-            using (new DependenciesCollector(new DependenciesCollectorOptions(), tracerFactory, Samplers.AlwaysSample))
+            using (new DependenciesCollector(new DependenciesCollectorOptions(), tracerFactory))
             {
                 using (tracer.WithSpan(tracer.SpanBuilder("incoming request").SetSampler(Samplers.AlwaysSample).StartSpan()))
                 {

src/OpenTelemetry.Collector.AspNetCore/Implementation/HttpInListener.cs

@@ -24,7 +24,7 @@ namespace OpenTelemetry.Collector.AspNetCore.Implementation
     using Microsoft.AspNetCore.Http.Features;
     using OpenTelemetry.Trace;
 
-    internal class HttpInListener : ListenerHandler<HttpRequest>
+    internal class HttpInListener : ListenerHandler
     {
         private static readonly string UnknownHostName = "UNKNOWN-HOST";
         private readonly PropertyFetcher startContextFetcher = new PropertyFetcher("HttpContext");
@@ -34,8 +34,8 @@ internal class HttpInListener : ListenerHandler<HttpRequest>
         private readonly PropertyFetcher beforeActionTemplateFetcher = new PropertyFetcher("Template");
         private readonly bool hostingSupportsW3C = false;
 
-        public HttpInListener(string name, ITracer tracer, Func<HttpRequest, ISampler> samplerFactory)
-            : base(name, tracer, samplerFactory)
+        public HttpInListener(string name, ITracer tracer)
+            : base(name, tracer)
         {
             this.hostingSupportsW3C = typeof(HttpRequest).Assembly.GetName().Version.Major >= 3;
         }
@@ -65,8 +65,7 @@ public override void OnStartActivity(Activity activity, object payload)
             var path = (request.PathBase.HasValue || request.Path.HasValue) ? (request.PathBase + request.Path).ToString() : "/";
 
             var spanBuilder = this.Tracer.SpanBuilder(path)
-                .SetSpanKind(SpanKind.Server)
-                .SetSampler(this.SamplerFactory(request));
+                .SetSpanKind(SpanKind.Server);
 
             if (this.hostingSupportsW3C)
             {

src/OpenTelemetry.Collector.AspNetCore/RequestsCollector.cs

@@ -29,45 +29,30 @@ namespace OpenTelemetry.Collector.AspNetCore
     /// </summary>
     public class RequestsCollector : IDisposable
     {
-        private readonly DiagnosticSourceSubscriber<HttpRequest> diagnosticSourceSubscriber;
+        private readonly DiagnosticSourceSubscriber diagnosticSourceSubscriber;
 
         /// <summary>
         /// Initializes a new instance of the <see cref="RequestsCollector"/> class.
         /// </summary>
         /// <param name="options">Configuration options for dependencies collector.</param>
         /// <param name="tracerFactory">TracerFactory which creates the Tracer to record traced with.</param>
-        /// <param name="sampler">Sampler to use to sample dependency calls.</param>
-        public RequestsCollector(RequestsCollectorOptions options, ITracerFactory tracerFactory, ISampler sampler)
+        public RequestsCollector(RequestsCollectorOptions options, ITracerFactory tracerFactory)
         {
             const string name = "Microsoft.AspNetCore";
-            this.diagnosticSourceSubscriber = new DiagnosticSourceSubscriber<HttpRequest>(
-                new Dictionary<string, Func<ITracerFactory, Func<HttpRequest, ISampler>, ListenerHandler<HttpRequest>>>()
+            this.diagnosticSourceSubscriber = new DiagnosticSourceSubscriber(
+                new Dictionary<string, Func<ITracerFactory, ListenerHandler>>()
                 {
                     {
-                        name, (t, s) =>
+                        name, (t) =>
                         {
                             var version = typeof(RequestDelegate).Assembly.GetName().Version;
                             var tracer = tracerFactory.GetTracer(typeof(RequestsCollector).Namespace, "semver:" + version.ToString());
-                            return new HttpInListener(name, tracer, s);
+                            return new HttpInListener(name, tracer);
                         }
                     },
                 },
                 tracerFactory,
-                x =>
-                {
-                    ISampler s = null;
-                    try
-                    {
-                        s = options.CustomSampler(x);
-                    }
-                    catch (Exception e)
-                    {
-                        s = null;
-                        CollectorEventSource.Log.ExceptionInCustomSampler(e);
-                    }
-
-                    return s ?? sampler;
-                });
+                options.EventFilter);
             this.diagnosticSourceSubscriber.Subscribe();
         }
 

src/OpenTelemetry.Collector.AspNetCore/RequestsCollectorOptions.cs

@@ -17,29 +17,39 @@
 namespace OpenTelemetry.Collector.AspNetCore
 {
     using System;
-    using Microsoft.AspNetCore.Http;
-    using OpenTelemetry.Trace;
 
     /// <summary>
     /// Options for requests collector.
     /// </summary>
     public class RequestsCollectorOptions
     {
-        private static readonly Func<HttpRequest, ISampler> DefaultSampler = (req) => { return null; };
+        /// <summary>
+        /// Initializes a new instance of the <see cref="RequestsCollectorOptions"/> class.
+        /// </summary>
+        public RequestsCollectorOptions()
+        {
+            this.EventFilter = DefaultFilter;
+        }
 
         /// <summary>
         /// Initializes a new instance of the <see cref="RequestsCollectorOptions"/> class.
         /// </summary>
-        /// <param name="sampler">Custom sampling function, if any.</param>
-        public RequestsCollectorOptions(Func<HttpRequest, ISampler> sampler = null)
+        /// <param name="eventFilter">Custom filtering predicate for DiagnosticSource events, if any.</param>
+        internal RequestsCollectorOptions(Func<string, object, object, bool> eventFilter = null)
         {
-            this.CustomSampler = sampler ?? DefaultSampler;
+            // TODO This API is unusable and likely to change, let's not expose it for now.
+
+            this.EventFilter = eventFilter;
         }
 
         /// <summary>
-        /// Gets a hook to exclude calls based on domain
-        /// or other per-request criterion.
+        /// Gets a hook to exclude calls based on domain or other per-request criterion.
         /// </summary>
-        public Func<HttpRequest, ISampler> CustomSampler { get; private set; }
+        internal Func<string, object, object, bool> EventFilter { get; }
+
+        private static bool DefaultFilter(string activityName, object arg1, object unused)
+        {
+            return true;
+        }
     }
 }

src/OpenTelemetry.Collector.Dependencies/AssemblyInfo.cs

@@ -13,3 +13,11 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 // </copyright>
+
+using System.Runtime.CompilerServices;
+
+#if SIGNED
+[assembly: InternalsVisibleTo("OpenTelemetry.Collector.Dependencies.Tests, PublicKey=002400000480000094000000060200000024000052534131000400000100010051c1562a090fb0c9f391012a32198b5e5d9a60e9b80fa2d7b434c9e5ccb7259bd606e66f9660676afc6692b8cdc6793d190904551d2103b7b22fa636dcbb8208839785ba402ea08fc00c8f1500ccef28bbf599aa64ffb1e1d5dc1bf3420a3777badfe697856e9d52070a50c3ea5821c80bef17ca3acffa28f89dd413f096f898")]
+#else
+[assembly: InternalsVisibleTo("OpenTelemetry.Collector.Dependencies.Tests")]
+#endif

src/OpenTelemetry.Collector.Dependencies/AzureSdkDiagnosticListener.cs

@@ -23,15 +23,13 @@ namespace OpenTelemetry.Collector.Dependencies
     using OpenTelemetry.Collector.Dependencies.Implementation;
     using OpenTelemetry.Trace;
 
-    internal class AzureSdkDiagnosticListener : ListenerHandler<HttpRequestMessage>
+    internal class AzureSdkDiagnosticListener : ListenerHandler
     {
         private static readonly PropertyFetcher LinksPropertyFetcher = new PropertyFetcher("Links");
-        private readonly ISampler sampler;
 
-        public AzureSdkDiagnosticListener(string sourceName, ITracer tracer, ISampler sampler)
-            : base(sourceName, tracer, null)
+        public AzureSdkDiagnosticListener(string sourceName, ITracer tracer)
+            : base(sourceName, tracer)
         {
-            this.sampler = sampler;
         }
 
         public void OnCompleted()
@@ -65,8 +63,7 @@ public override void OnStartActivity(Activity current, object valueValue)
             }
 
             var spanBuilder = this.Tracer.SpanBuilder(operationName)
-                .SetCreateChild(false)
-                .SetSampler(this.sampler);
+                .SetCreateChild(false);
 
             var links = LinksPropertyFetcher.Fetch(valueValue) as IEnumerable<Activity> ?? Array.Empty<Activity>();
 

src/OpenTelemetry.Collector.Dependencies/DependenciesCollector.cs

@@ -18,68 +18,50 @@ namespace OpenTelemetry.Collector.Dependencies
 {
     using System;
     using System.Collections.Generic;
-    using System.Net.Http;
-    using System.Reflection;
     using OpenTelemetry.Collector.Dependencies.Implementation;
     using OpenTelemetry.Trace;
-    using OpenTelemetry.Utils;
 
     /// <summary>
     /// Dependencies collector.
     /// </summary>
     public class DependenciesCollector : IDisposable
     {
-        private readonly DiagnosticSourceSubscriber<HttpRequestMessage> diagnosticSourceSubscriber;
+        private readonly DiagnosticSourceSubscriber diagnosticSourceSubscriber;
 
         /// <summary>
         /// Initializes a new instance of the <see cref="DependenciesCollector"/> class.
         /// </summary>
         /// <param name="options">Configuration options for dependencies collector.</param>
         /// <param name="tracerFactory">TracerFactory to create a Tracer to record traced with.</param>
-        /// <param name="sampler">Sampler to use to sample dependency calls.</param>
-        public DependenciesCollector(DependenciesCollectorOptions options, ITracerFactory tracerFactory, ISampler sampler)
+        public DependenciesCollector(DependenciesCollectorOptions options, ITracerFactory tracerFactory)
         {
-            this.diagnosticSourceSubscriber = new DiagnosticSourceSubscriber<HttpRequestMessage>(
-                new Dictionary<string, Func<ITracerFactory, Func<HttpRequestMessage, ISampler>, ListenerHandler<HttpRequestMessage>>>()
+            this.diagnosticSourceSubscriber = new DiagnosticSourceSubscriber(
+                new Dictionary<string, Func<ITracerFactory, ListenerHandler>>()
                 {
                     {
-                        "HttpHandlerDiagnosticListener", (tf, s) =>
+                        "HttpHandlerDiagnosticListener", (tf) =>
                         {
-                            var tracer = tracerFactory.GetTracer("OpenTelemetry.Collector.Dependencies.HttpHandlerDiagnosticListener");
-                            return new HttpHandlerDiagnosticListener(tracer, s);
+                            var tracer = tf.GetTracer("OpenTelemetry.Collector.Dependencies.HttpHandlerDiagnosticListener");
+                            return new HttpHandlerDiagnosticListener(tracer);
                         }
                     },
                     {
-                        "Azure.Clients", (tf, s) =>
+                        "Azure.Clients", (tf) =>
                         {
-                            var tracer = tracerFactory.GetTracer("OpenTelemetry.Collector.Dependencies.Azure.Clients");
-                            return new AzureSdkDiagnosticListener("Azure.Clients", tracer, sampler);
+                            var tracer = tf.GetTracer("OpenTelemetry.Collector.Dependencies.Azure.Clients");
+                            return new AzureSdkDiagnosticListener("Azure.Clients", tracer);
                         }
                     },
                     {
-                        "Azure.Pipeline", (tf, s) =>
+                        "Azure.Pipeline", (tf) =>
                         {
-                            var tracer = tracerFactory.GetTracer("OpenTelemetry.Collector.Dependencies.Azure.Pipeline");
-                            return new AzureSdkDiagnosticListener("Azure.Pipeline", tracer, sampler);
+                            var tracer = tf.GetTracer("OpenTelemetry.Collector.Dependencies.Azure.Pipeline");
+                            return new AzureSdkDiagnosticListener("Azure.Pipeline", tracer);
                         }
                     },
                 },
                 tracerFactory,
-                x =>
-                {
-                    ISampler s = null;
-                    try
-                    {
-                        s = options.CustomSampler(x);
-                    }
-                    catch (Exception e)
-                    {
-                        s = null;
-                        CollectorEventSource.Log.ExceptionInCustomSampler(e);
-                    }
-
-                    return s ?? sampler;
-                    });
+                options.EventFilter);
             this.diagnosticSourceSubscriber.Subscribe();
         }
 

src/OpenTelemetry.Collector.Dependencies/DependenciesCollectorOptions.cs

@@ -18,29 +18,66 @@ namespace OpenTelemetry.Collector.Dependencies
 {
     using System;
     using System.Net.Http;
-    using OpenTelemetry.Trace;
-    using OpenTelemetry.Trace.Sampler;
 
     /// <summary>
     /// Options for dependencies collector.
     /// </summary>
     public class DependenciesCollectorOptions
     {
-        private static readonly Func<HttpRequestMessage, ISampler> DefaultSampler = (req) => { return ((req.RequestUri != null) && req.RequestUri.ToString().Contains("zipkin.azurewebsites.net")) ? Samplers.NeverSample : null; };
+        /// <summary>
+        /// Initializes a new instance of the <see cref="DependenciesCollectorOptions"/> class.
+        /// </summary>
+        public DependenciesCollectorOptions()
+        {
+            this.EventFilter = DefaultFilter;
+        }
 
         /// <summary>
         /// Initializes a new instance of the <see cref="DependenciesCollectorOptions"/> class.
         /// </summary>
-        /// <param name="sampler">Custom sampling function, if any.</param>
-        public DependenciesCollectorOptions(Func<HttpRequestMessage, ISampler> sampler = null)
+        /// <param name="eventFilter">Custom filtering predicate for DiagnosticSource events, if any.</param>
+        internal DependenciesCollectorOptions(Func<string, object, object, bool> eventFilter)
         {
-            this.CustomSampler = sampler ?? DefaultSampler;
+            // TODO This API is unusable and likely to change, let's not expose it for now.
+
+            this.EventFilter = eventFilter;
         }
 
         /// <summary>
-        /// Gets a hook to exclude calls based on domain
-        /// or other per-request criterion.
+        /// Gets a hook to exclude calls based on domain or other per-request criterion.
         /// </summary>
-        public Func<HttpRequestMessage, ISampler> CustomSampler { get; private set; }
+        internal Func<string, object, object, bool> EventFilter { get; }
+
+        private static bool DefaultFilter(string activityName, object arg1, object unused)
+        {
+            // TODO: there is some preliminary consensus that we should introduce 'terminal' spans or context.
+            // exporters should ensure they set it
+
+            if (activityName == "System.Net.Http.HttpRequestOut" &&
+                arg1 is HttpRequestMessage request &&
+                request.RequestUri != null &&
+                request.Method == HttpMethod.Post)
+            {
+                var originalString = request.RequestUri.OriginalString;
+
+                // zipkin
+                if (originalString.Contains(":9411/api/v2/spans"))
+                {
+                    return false;
+                }
+
+                // applicationinsights
+                if (originalString.StartsWith("https://dc.services.visualstudio") ||
+                    originalString.StartsWith("https://rt.services.visualstudio") ||
+                    originalString.StartsWith("https://dc.applicationinsights") ||
+                    originalString.StartsWith("https://live.applicationinsights") ||
+                    originalString.StartsWith("https://quickpulse.applicationinsights"))
+                {
+                    return false;
+                }
+            }
+
+            return true;
+        }
     }
 }

src/OpenTelemetry.Collector.Dependencies/Implementation/HttpHandlerDiagnosticListener.cs

@@ -26,16 +26,16 @@ namespace OpenTelemetry.Collector.Dependencies.Implementation
     using System.Threading.Tasks;
     using OpenTelemetry.Trace;
 
-    internal class HttpHandlerDiagnosticListener : ListenerHandler<HttpRequestMessage>
+    internal class HttpHandlerDiagnosticListener : ListenerHandler
     {
         private readonly PropertyFetcher startRequestFetcher = new PropertyFetcher("Request");
         private readonly PropertyFetcher stopResponseFetcher = new PropertyFetcher("Response");
         private readonly PropertyFetcher stopExceptionFetcher = new PropertyFetcher("Exception");
         private readonly PropertyFetcher stopRequestStatusFetcher = new PropertyFetcher("RequestTaskStatus");
         private readonly bool httpClientSupportsW3C = false;
 
-        public HttpHandlerDiagnosticListener(ITracer tracer, Func<HttpRequestMessage, ISampler> samplerFactory)
-            : base("HttpHandlerDiagnosticListener", tracer, samplerFactory)
+        public HttpHandlerDiagnosticListener(ITracer tracer)
+            : base("HttpHandlerDiagnosticListener", tracer)
         {
             var framework = Assembly
                 .GetEntryAssembly()?
@@ -68,7 +68,6 @@ public override void OnStartActivity(Activity activity, object payload)
 
             var span = this.Tracer.SpanBuilder(request.RequestUri.AbsolutePath)
                 .SetSpanKind(SpanKind.Client)
-                .SetSampler(this.SamplerFactory(request))
                 .SetCreateChild(false)
                 .StartSpan();
 

src/OpenTelemetry/Collector/DiagnosticSourceListener.cs

@@ -20,11 +20,11 @@ namespace OpenTelemetry.Collector
     using System.Collections.Generic;
     using System.Diagnostics;
 
-    internal class DiagnosticSourceListener<TInput> : IObserver<KeyValuePair<string, object>>, IDisposable
+    internal class DiagnosticSourceListener : IObserver<KeyValuePair<string, object>>, IDisposable
     {
-        private readonly ListenerHandler<TInput> handler;
+        private readonly ListenerHandler handler;
 
-        public DiagnosticSourceListener(ListenerHandler<TInput> handler)
+        public DiagnosticSourceListener(ListenerHandler handler)
         {
             this.handler = handler ?? throw new ArgumentNullException(nameof(handler));
         }