From 6097bb313697a5feb775af2c48f7ec53a951f3ec Mon Sep 17 00:00:00 2001 From: Liudmila Molkova Date: Tue, 8 Oct 2019 17:24:46 -0700 Subject: [PATCH] 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 * Update src/OpenTelemetry.Collector.AspNetCore/RequestsCollectorOptions.cs --- samples/Exporters/TestHttpClient.cs | 2 +- .../Implementation/HttpInListener.cs | 9 +- .../RequestsCollector.cs | 29 ++---- .../RequestsCollectorOptions.cs | 28 ++++-- .../AssemblyInfo.cs | 8 ++ .../AzureSdkDiagnosticListener.cs | 11 +-- .../DependenciesCollector.cs | 46 +++------- .../DependenciesCollectorOptions.cs | 55 +++++++++-- .../HttpHandlerDiagnosticListener.cs | 7 +- .../Collector/DiagnosticSourceListener.cs | 6 +- .../Collector/DiagnosticSourceSubscriber.cs | 18 ++-- .../Collector/ListenerHandler.cs | 8 +- .../BasicTests.cs | 92 ++++++++++++++----- .../BasicTests.cs | 52 ++++++++++- .../HttpClientTests.cs | 2 +- test/TestApp.AspNetCore.2.0/Startup.cs | 5 +- 16 files changed, 240 insertions(+), 138 deletions(-) diff --git a/samples/Exporters/TestHttpClient.cs b/samples/Exporters/TestHttpClient.cs index bd1cb649240..622c2435561 100644 --- a/samples/Exporters/TestHttpClient.cs +++ b/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())) { diff --git a/src/OpenTelemetry.Collector.AspNetCore/Implementation/HttpInListener.cs b/src/OpenTelemetry.Collector.AspNetCore/Implementation/HttpInListener.cs index 873d3017440..be25ccccc38 100644 --- a/src/OpenTelemetry.Collector.AspNetCore/Implementation/HttpInListener.cs +++ b/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 + 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 private readonly PropertyFetcher beforeActionTemplateFetcher = new PropertyFetcher("Template"); private readonly bool hostingSupportsW3C = false; - public HttpInListener(string name, ITracer tracer, Func 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) { diff --git a/src/OpenTelemetry.Collector.AspNetCore/RequestsCollector.cs b/src/OpenTelemetry.Collector.AspNetCore/RequestsCollector.cs index 14bdd928a1e..082c9b351eb 100644 --- a/src/OpenTelemetry.Collector.AspNetCore/RequestsCollector.cs +++ b/src/OpenTelemetry.Collector.AspNetCore/RequestsCollector.cs @@ -29,45 +29,30 @@ namespace OpenTelemetry.Collector.AspNetCore /// public class RequestsCollector : IDisposable { - private readonly DiagnosticSourceSubscriber diagnosticSourceSubscriber; + private readonly DiagnosticSourceSubscriber diagnosticSourceSubscriber; /// /// Initializes a new instance of the class. /// /// Configuration options for dependencies collector. /// TracerFactory which creates the Tracer to record traced with. - /// Sampler to use to sample dependency calls. - public RequestsCollector(RequestsCollectorOptions options, ITracerFactory tracerFactory, ISampler sampler) + public RequestsCollector(RequestsCollectorOptions options, ITracerFactory tracerFactory) { const string name = "Microsoft.AspNetCore"; - this.diagnosticSourceSubscriber = new DiagnosticSourceSubscriber( - new Dictionary, ListenerHandler>>() + this.diagnosticSourceSubscriber = new DiagnosticSourceSubscriber( + new Dictionary>() { { - 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(); } diff --git a/src/OpenTelemetry.Collector.AspNetCore/RequestsCollectorOptions.cs b/src/OpenTelemetry.Collector.AspNetCore/RequestsCollectorOptions.cs index dad814dcb82..76871059b84 100644 --- a/src/OpenTelemetry.Collector.AspNetCore/RequestsCollectorOptions.cs +++ b/src/OpenTelemetry.Collector.AspNetCore/RequestsCollectorOptions.cs @@ -17,29 +17,39 @@ namespace OpenTelemetry.Collector.AspNetCore { using System; - using Microsoft.AspNetCore.Http; - using OpenTelemetry.Trace; /// /// Options for requests collector. /// public class RequestsCollectorOptions { - private static readonly Func DefaultSampler = (req) => { return null; }; + /// + /// Initializes a new instance of the class. + /// + public RequestsCollectorOptions() + { + this.EventFilter = DefaultFilter; + } /// /// Initializes a new instance of the class. /// - /// Custom sampling function, if any. - public RequestsCollectorOptions(Func sampler = null) + /// Custom filtering predicate for DiagnosticSource events, if any. + internal RequestsCollectorOptions(Func 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; } /// - /// 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. /// - public Func CustomSampler { get; private set; } + internal Func EventFilter { get; } + + private static bool DefaultFilter(string activityName, object arg1, object unused) + { + return true; + } } } diff --git a/src/OpenTelemetry.Collector.Dependencies/AssemblyInfo.cs b/src/OpenTelemetry.Collector.Dependencies/AssemblyInfo.cs index 23da982fdad..28d168e75bc 100644 --- a/src/OpenTelemetry.Collector.Dependencies/AssemblyInfo.cs +++ b/src/OpenTelemetry.Collector.Dependencies/AssemblyInfo.cs @@ -13,3 +13,11 @@ // See the License for the specific language governing permissions and // limitations under the License. // + +using System.Runtime.CompilerServices; + +#if SIGNED +[assembly: InternalsVisibleTo("OpenTelemetry.Collector.Dependencies.Tests, PublicKey=002400000480000094000000060200000024000052534131000400000100010051c1562a090fb0c9f391012a32198b5e5d9a60e9b80fa2d7b434c9e5ccb7259bd606e66f9660676afc6692b8cdc6793d190904551d2103b7b22fa636dcbb8208839785ba402ea08fc00c8f1500ccef28bbf599aa64ffb1e1d5dc1bf3420a3777badfe697856e9d52070a50c3ea5821c80bef17ca3acffa28f89dd413f096f898")] +#else +[assembly: InternalsVisibleTo("OpenTelemetry.Collector.Dependencies.Tests")] +#endif diff --git a/src/OpenTelemetry.Collector.Dependencies/AzureSdkDiagnosticListener.cs b/src/OpenTelemetry.Collector.Dependencies/AzureSdkDiagnosticListener.cs index febcac9c3c3..1f4f5f472ec 100644 --- a/src/OpenTelemetry.Collector.Dependencies/AzureSdkDiagnosticListener.cs +++ b/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 + 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 ?? Array.Empty(); diff --git a/src/OpenTelemetry.Collector.Dependencies/DependenciesCollector.cs b/src/OpenTelemetry.Collector.Dependencies/DependenciesCollector.cs index 2961f3667e1..b976a717ac3 100644 --- a/src/OpenTelemetry.Collector.Dependencies/DependenciesCollector.cs +++ b/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; /// /// Dependencies collector. /// public class DependenciesCollector : IDisposable { - private readonly DiagnosticSourceSubscriber diagnosticSourceSubscriber; + private readonly DiagnosticSourceSubscriber diagnosticSourceSubscriber; /// /// Initializes a new instance of the class. /// /// Configuration options for dependencies collector. /// TracerFactory to create a Tracer to record traced with. - /// Sampler to use to sample dependency calls. - public DependenciesCollector(DependenciesCollectorOptions options, ITracerFactory tracerFactory, ISampler sampler) + public DependenciesCollector(DependenciesCollectorOptions options, ITracerFactory tracerFactory) { - this.diagnosticSourceSubscriber = new DiagnosticSourceSubscriber( - new Dictionary, ListenerHandler>>() + this.diagnosticSourceSubscriber = new DiagnosticSourceSubscriber( + new Dictionary>() { { - "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(); } diff --git a/src/OpenTelemetry.Collector.Dependencies/DependenciesCollectorOptions.cs b/src/OpenTelemetry.Collector.Dependencies/DependenciesCollectorOptions.cs index 53fcbc806e1..31113cc0c93 100644 --- a/src/OpenTelemetry.Collector.Dependencies/DependenciesCollectorOptions.cs +++ b/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; /// /// Options for dependencies collector. /// public class DependenciesCollectorOptions { - private static readonly Func DefaultSampler = (req) => { return ((req.RequestUri != null) && req.RequestUri.ToString().Contains("zipkin.azurewebsites.net")) ? Samplers.NeverSample : null; }; + /// + /// Initializes a new instance of the class. + /// + public DependenciesCollectorOptions() + { + this.EventFilter = DefaultFilter; + } /// /// Initializes a new instance of the class. /// - /// Custom sampling function, if any. - public DependenciesCollectorOptions(Func sampler = null) + /// Custom filtering predicate for DiagnosticSource events, if any. + internal DependenciesCollectorOptions(Func eventFilter) { - this.CustomSampler = sampler ?? DefaultSampler; + // TODO This API is unusable and likely to change, let's not expose it for now. + + this.EventFilter = eventFilter; } /// - /// 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. /// - public Func CustomSampler { get; private set; } + internal Func 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; + } } } diff --git a/src/OpenTelemetry.Collector.Dependencies/Implementation/HttpHandlerDiagnosticListener.cs b/src/OpenTelemetry.Collector.Dependencies/Implementation/HttpHandlerDiagnosticListener.cs index cf741f55110..c4a0f84a3c5 100644 --- a/src/OpenTelemetry.Collector.Dependencies/Implementation/HttpHandlerDiagnosticListener.cs +++ b/src/OpenTelemetry.Collector.Dependencies/Implementation/HttpHandlerDiagnosticListener.cs @@ -26,7 +26,7 @@ namespace OpenTelemetry.Collector.Dependencies.Implementation using System.Threading.Tasks; using OpenTelemetry.Trace; - internal class HttpHandlerDiagnosticListener : ListenerHandler + internal class HttpHandlerDiagnosticListener : ListenerHandler { private readonly PropertyFetcher startRequestFetcher = new PropertyFetcher("Request"); private readonly PropertyFetcher stopResponseFetcher = new PropertyFetcher("Response"); @@ -34,8 +34,8 @@ internal class HttpHandlerDiagnosticListener : ListenerHandler 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(); diff --git a/src/OpenTelemetry/Collector/DiagnosticSourceListener.cs b/src/OpenTelemetry/Collector/DiagnosticSourceListener.cs index 98c1b4592e6..4f689dddfb3 100644 --- a/src/OpenTelemetry/Collector/DiagnosticSourceListener.cs +++ b/src/OpenTelemetry/Collector/DiagnosticSourceListener.cs @@ -20,11 +20,11 @@ namespace OpenTelemetry.Collector using System.Collections.Generic; using System.Diagnostics; - internal class DiagnosticSourceListener : IObserver>, IDisposable + internal class DiagnosticSourceListener : IObserver>, IDisposable { - private readonly ListenerHandler handler; + private readonly ListenerHandler handler; - public DiagnosticSourceListener(ListenerHandler handler) + public DiagnosticSourceListener(ListenerHandler handler) { this.handler = handler ?? throw new ArgumentNullException(nameof(handler)); } diff --git a/src/OpenTelemetry/Collector/DiagnosticSourceSubscriber.cs b/src/OpenTelemetry/Collector/DiagnosticSourceSubscriber.cs index 15ebee9348e..666b4eb6ec4 100644 --- a/src/OpenTelemetry/Collector/DiagnosticSourceSubscriber.cs +++ b/src/OpenTelemetry/Collector/DiagnosticSourceSubscriber.cs @@ -23,21 +23,21 @@ namespace OpenTelemetry.Collector using System.Threading; using OpenTelemetry.Trace; - public class DiagnosticSourceSubscriber : IDisposable, IObserver + public class DiagnosticSourceSubscriber : IDisposable, IObserver { - private readonly Dictionary, ListenerHandler>> handlers; + private readonly Dictionary> handlers; private readonly ITracerFactory tracerFactory; - private readonly Func sampler; - private ConcurrentDictionary> subscriptions; + private readonly Func filter; + private ConcurrentDictionary subscriptions; private long disposed; private IDisposable subscription; - public DiagnosticSourceSubscriber(Dictionary, ListenerHandler>> handlers, ITracerFactory tracerFactory, Func sampler) + public DiagnosticSourceSubscriber(Dictionary> handlers, ITracerFactory tracerFactory, Func filter) { - this.subscriptions = new ConcurrentDictionary>(); + this.subscriptions = new ConcurrentDictionary(); this.handlers = handlers ?? throw new ArgumentNullException(nameof(handlers)); this.tracerFactory = tracerFactory ?? throw new ArgumentNullException(nameof(tracerFactory)); - this.sampler = sampler ?? throw new ArgumentNullException(nameof(sampler)); + this.filter = filter; } public void Subscribe() @@ -56,8 +56,8 @@ public void OnNext(DiagnosticListener value) { this.subscriptions?.GetOrAdd(value.Name, name => { - var dl = new DiagnosticSourceListener(this.handlers[value.Name](this.tracerFactory, this.sampler)); - dl.Subscription = value.Subscribe(dl); + var dl = new DiagnosticSourceListener(this.handlers[value.Name](this.tracerFactory)); + dl.Subscription = this.filter == null ? value.Subscribe(dl) : value.Subscribe(dl, this.filter); return dl; }); } diff --git a/src/OpenTelemetry/Collector/ListenerHandler.cs b/src/OpenTelemetry/Collector/ListenerHandler.cs index 7986967c24a..05ee713bac0 100644 --- a/src/OpenTelemetry/Collector/ListenerHandler.cs +++ b/src/OpenTelemetry/Collector/ListenerHandler.cs @@ -16,21 +16,17 @@ namespace OpenTelemetry.Collector { - using System; using System.Diagnostics; using OpenTelemetry.Trace; - public abstract class ListenerHandler + public abstract class ListenerHandler { protected readonly ITracer Tracer; - protected readonly Func SamplerFactory; - - public ListenerHandler(string sourceName, ITracer tracer, Func samplerFactory) + public ListenerHandler(string sourceName, ITracer tracer) { this.SourceName = sourceName; this.Tracer = tracer; - this.SamplerFactory = samplerFactory; } public string SourceName { get; } diff --git a/test/OpenTelemetry.Collector.AspNetCore.Tests/BasicTests.cs b/test/OpenTelemetry.Collector.AspNetCore.Tests/BasicTests.cs index 4bfe07a802d..6741a9f154f 100644 --- a/test/OpenTelemetry.Collector.AspNetCore.Tests/BasicTests.cs +++ b/test/OpenTelemetry.Collector.AspNetCore.Tests/BasicTests.cs @@ -1,4 +1,4 @@ -// +// // Copyright 2018, OpenTelemetry Authors // // Licensed under the Apache License, Version 2.0 (the "License"); @@ -14,6 +14,7 @@ // limitations under the License. // +using System.Threading; using OpenTelemetry.Resources; namespace OpenTelemetry.Collector.AspNetCore.Tests @@ -67,18 +68,7 @@ void ConfigureTestServices(IServiceCollection services) => // Assert response.EnsureSuccessStatusCode(); // Status Code 200-299 - for (var i = 0; i < 10; i++) - { - if (spanProcessor.Invocations.Count == 2) - { - break; - } - - // We need to let End callback execute as it is executed AFTER response was returned. - // In unit tests environment there may be a lot of parallel unit tests executed, so - // giving some breezing room for the End callback to complete - await Task.Delay(TimeSpan.FromSeconds(1)); - } + WaitForProcessorInvocations(spanProcessor, 2); } @@ -101,7 +91,7 @@ public async Task SuccessfulTemplateControllerCallUsesParentContext() tf.Setup(m => m.Extract(It.IsAny(), It.IsAny>>())).Returns(new SpanContext( expectedTraceId, expectedSpanId, - ActivityTraceFlags.None)); + ActivityTraceFlags.Recorded)); var tracerFactory = new TracerFactory(spanProcessor.Object, null, tf.Object); @@ -121,29 +111,81 @@ public async Task SuccessfulTemplateControllerCallUsesParentContext() // Assert response.EnsureSuccessStatusCode(); // Status Code 200-299 - for (var i = 0; i < 10; i++) + WaitForProcessorInvocations(spanProcessor, 2); + } + + Assert.Equal(2, spanProcessor.Invocations.Count); // begin and end was called + var span = ((Span)spanProcessor.Invocations[1].Arguments[0]); + + Assert.Equal(SpanKind.Server, span.Kind); + Assert.Equal("api/Values/{id}", span.Name); + Assert.Equal("/api/values/2", span.Attributes.GetValue("http.path")); + + Assert.Equal(expectedTraceId, span.Context.TraceId); + Assert.Equal(expectedSpanId, span.ParentSpanId); + } + + [Fact] + public async Task FilterOutRequest() + { + var spanProcessor = new Mock(new NoopSpanExporter()); + var tracerFactory = new TracerFactory(spanProcessor.Object); + + void ConfigureTestServices(IServiceCollection services) + { + bool Filter(string eventName, object arg1, object _) { - if (spanProcessor.Invocations.Count == 2) + if (eventName == "Microsoft.AspNetCore.Hosting.HttpRequestIn" && + arg1 is HttpContext context && + context.Request.Path == "/api/values/2") { - break; + return false; } - // We need to let End callback execute as it is executed AFTER response was returned. - // In unit tests environment there may be a lot of parallel unit tests executed, so - // giving some breezing room for the End callback to complete - await Task.Delay(TimeSpan.FromSeconds(1)); + return true; } + + services.AddSingleton(_ => new RequestsCollectorOptions(Filter)); + services.AddSingleton(tracerFactory); } + // Arrange + using (var client = this.factory + .WithWebHostBuilder(builder => + builder.ConfigureTestServices(ConfigureTestServices)) + .CreateClient()) + { + + // Act + var response1 = await client.GetAsync("/api/values"); + var response2 = await client.GetAsync("/api/values/2"); + + // Assert + response1.EnsureSuccessStatusCode(); // Status Code 200-299 + response2.EnsureSuccessStatusCode(); // Status Code 200-299 + + WaitForProcessorInvocations(spanProcessor, 2); + } + + // we should only create one span and never call processor with another Assert.Equal(2, spanProcessor.Invocations.Count); // begin and end was called var span = ((Span)spanProcessor.Invocations[1].Arguments[0]); Assert.Equal(SpanKind.Server, span.Kind); - Assert.Equal("api/Values/{id}", span.Name); - Assert.Equal("/api/values/2", span.Attributes.GetValue("http.path")); + Assert.Equal("/api/values", span.Attributes.GetValue("http.path")); + } - Assert.Equal(expectedTraceId, span.Context.TraceId); - Assert.Equal(expectedSpanId, span.ParentSpanId); + private static void WaitForProcessorInvocations(Mock spanProcessor, int invocationCount) + { + // We need to let End callback execute as it is executed AFTER response was returned. + // In unit tests environment there may be a lot of parallel unit tests executed, so + // giving some breezing room for the End callback to complete + Assert.True(SpinWait.SpinUntil(() => + { + Thread.Sleep(10); + return spanProcessor.Invocations.Count >= 2; + }, + TimeSpan.FromSeconds(1))); } } } diff --git a/test/OpenTelemetry.Collector.Dependencies.Tests/BasicTests.cs b/test/OpenTelemetry.Collector.Dependencies.Tests/BasicTests.cs index 4913359204c..73c32ef7970 100644 --- a/test/OpenTelemetry.Collector.Dependencies.Tests/BasicTests.cs +++ b/test/OpenTelemetry.Collector.Dependencies.Tests/BasicTests.cs @@ -19,11 +19,11 @@ namespace OpenTelemetry.Collector.Dependencies.Tests using Moq; using OpenTelemetry.Trace; using OpenTelemetry.Trace.Export; - using OpenTelemetry.Trace.Sampler; using System; using System.Diagnostics; using System.Linq; using System.Net.Http; + using System.Threading; using System.Threading.Tasks; using Xunit; @@ -62,7 +62,7 @@ public async Task HttpDependenciesCollectorInjectsHeadersAsync() .Start(); parent.TraceStateString = "k1=v1,k2=v2"; - using (new DependenciesCollector(new DependenciesCollectorOptions(), tracerFactory, Samplers.AlwaysSample)) + using (new DependenciesCollector(new DependenciesCollectorOptions(), tracerFactory)) using (var c = new HttpClient()) { await c.SendAsync(request); @@ -99,7 +99,7 @@ public async Task HttpDependenciesCollectorBacksOffIfAlreadyInstrumented() request.Headers.Add("traceparent", "00-0123456789abcdef0123456789abcdef-0123456789abcdef-01"); - using (new DependenciesCollector(new DependenciesCollectorOptions(), tracerFactory, Samplers.AlwaysSample)) + using (new DependenciesCollector(new DependenciesCollectorOptions(), tracerFactory)) using (var c = new HttpClient()) { await c.SendAsync(request); @@ -108,6 +108,52 @@ public async Task HttpDependenciesCollectorBacksOffIfAlreadyInstrumented() Assert.Equal(0, spanProcessor.Invocations.Count); } + [Fact] + public async Task HttpDependenciesCollectorFiltersOutRequests() + { + var spanProcessor = new Mock(new NoopSpanExporter()); + var tracerFactory = new TracerFactory(spanProcessor.Object); + + var options = new DependenciesCollectorOptions((activityName, arg1, _) => !(activityName == "System.Net.Http.HttpRequestOut" && + arg1 is HttpRequestMessage request && + request.RequestUri.OriginalString.Contains(url))); + + using (new DependenciesCollector(options, tracerFactory)) + using (var c = new HttpClient()) + { + await c.GetAsync(url); + } + + Assert.Equal(0, spanProcessor.Invocations.Count); + } + + + [Fact] + public async Task HttpDependenciesCollectorFiltersOutRequestsToExporterEndpoints() + { + var spanProcessor = new Mock(new NoopSpanExporter()); + var tracerFactory = new TracerFactory(spanProcessor.Object); + + var options = new DependenciesCollectorOptions(); + + using (new DependenciesCollector(options, tracerFactory)) + using (var c = new HttpClient()) + using (var cts = new CancellationTokenSource(TimeSpan.FromMilliseconds(100))) + { + try + { + await c.PostAsync("https://dc.services.visualstudio.com/", new StringContent(""), cts.Token); + await c.PostAsync("https://localhost:9411/api/v2/spans", new StringContent(""), cts.Token); + } + catch + { + // ignore all, whatever response is, we don't want anything tracked + } + } + + Assert.Equal(0, spanProcessor.Invocations.Count); + } + public void Dispose() { serverLifeTime?.Dispose(); diff --git a/test/OpenTelemetry.Collector.Dependencies.Tests/HttpClientTests.cs b/test/OpenTelemetry.Collector.Dependencies.Tests/HttpClientTests.cs index 2e5ffffa255..675da1c1f54 100644 --- a/test/OpenTelemetry.Collector.Dependencies.Tests/HttpClientTests.cs +++ b/test/OpenTelemetry.Collector.Dependencies.Tests/HttpClientTests.cs @@ -100,7 +100,7 @@ public async Task HttpOutCallsAreCollectedSuccessfullyAsync(HttpOutTestCase tc) using (serverLifeTime) { - using (var dc = new DependenciesCollector(new DependenciesCollectorOptions(), tracerFactory, Samplers.AlwaysSample)) + using (var dc = new DependenciesCollector(new DependenciesCollectorOptions(), tracerFactory)) { try diff --git a/test/TestApp.AspNetCore.2.0/Startup.cs b/test/TestApp.AspNetCore.2.0/Startup.cs index befa4596d5f..ca599b3692d 100644 --- a/test/TestApp.AspNetCore.2.0/Startup.cs +++ b/test/TestApp.AspNetCore.2.0/Startup.cs @@ -26,6 +26,7 @@ using System.Net.Http; using OpenTelemetry.Exporter.Zipkin; using OpenTelemetry.Trace.Configuration; +using Microsoft.Extensions.DependencyInjection.Extensions; namespace TestApp.AspNetCore._2._0 { @@ -45,9 +46,9 @@ public void ConfigureServices(IServiceCollection services) services.AddSingleton(); services.AddSingleton(Samplers.AlwaysSample); - services.AddSingleton(new RequestsCollectorOptions()); + services.TryAddSingleton(new RequestsCollectorOptions()); services.AddSingleton(); - services.AddSingleton(new DependenciesCollectorOptions()); + services.TryAddSingleton(new DependenciesCollectorOptions()); services.AddSingleton(); services.AddSingleton(new CallbackMiddleware.CallbackMiddlewareImpl()); services.AddSingleton(new ZipkinTraceExporterOptions { ServiceName = "tracing-to-zipkin-service" });