Move reusable Collector related classes into the OpenTelemetry SDK (#183

)

* Move reusable Collector related classes into OpenTelemetry

Four classes were essentially duplicated between the aspnetcore and dependencies collector projects. We can shared these classes between the two projects by making some generic.

* Innocuous whitespace change to trigger the build again. There is an unrelated non-deterministic test that is failing

* Increasing the allowable range from AssertApproxSameTimestamp from 10ms to 20ms. Tests keep failing with times like 12 and 14ms

* Removing the PropertyFetcher from the public interface

* Applying a bug fix in the PropertyFetcher. The same kind of fix was done in the OpenTracing ASP.NET Core instrumentation package

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

@@ -22,10 +22,9 @@ namespace OpenTelemetry.Collector.AspNetCore.Implementation
     using System.Text;
     using Microsoft.AspNetCore.Http;
     using Microsoft.AspNetCore.Http.Features;
-    using OpenTelemetry.Collector.AspNetCore.Common;
     using OpenTelemetry.Trace;
 
-    internal class HttpInListener : ListenerHandler
+    internal class HttpInListener : ListenerHandler<HttpRequest>
     {
         private static readonly string UnknownHostName = "UNKNOWN-HOST";
         private readonly PropertyFetcher startContextFetcher = new PropertyFetcher("HttpContext");
@@ -43,11 +42,12 @@ public HttpInListener(ITracer tracer, Func<HttpRequest, ISampler> samplerFactory
 
         public override void OnStartActivity(Activity activity, object payload)
         {
+            const string EventNameSuffix = ".OnStartActivity";
             var context = this.startContextFetcher.Fetch(payload) as HttpContext;
 
             if (context == null)
-            {
-                // Debug.WriteLine("context is null");
+            {
+                CollectorEventSource.Log.NullPayload(nameof(HttpInListener) + EventNameSuffix);
                 return;
             }
 
@@ -95,11 +95,12 @@ public override void OnStartActivity(Activity activity, object payload)
 
         public override void OnStopActivity(Activity activity, object payload)
         {
+            const string EventNameSuffix = ".OnStopActivity";
             var span = this.Tracer.CurrentSpan;
 
             if (span == null || span == BlankSpan.Instance)
             {
-                AspNetCoreCollectorEventSource.Log.NullOrBlankSpan("HttpInListener.OnStopActivity");
+                CollectorEventSource.Log.NullOrBlankSpan(nameof(HttpInListener) + EventNameSuffix);
                 return;
             }
 
@@ -111,7 +112,7 @@ public override void OnStopActivity(Activity activity, object payload)
 
             if (!(this.stopContextFetcher.Fetch(payload) is HttpContext context))
             {
-                AspNetCoreCollectorEventSource.Log.NullHttpContext("HttpInListener.OnStopActivity");
+                CollectorEventSource.Log.NullPayload(nameof(HttpInListener) + EventNameSuffix);
                 return;
             }
 
@@ -129,7 +130,7 @@ public override void OnCustom(string name, Activity activity, object payload)
 
                 if (span == null)
                 {
-                    AspNetCoreCollectorEventSource.Log.NullOrBlankSpan(name);
+                    CollectorEventSource.Log.NullOrBlankSpan(name);
                     return;
                 }
 

src/OpenTelemetry.Collector.Dependencies/Implementation/OpenTelemetry.Collector.Dependencies.Common/PropertyFetcher.cs → src/OpenTelemetry.Collector.AspNetCore/Implementation/PropertyFetcher.cs

@@ -14,9 +14,10 @@
 // limitations under the License.
 // </copyright>
 
-namespace OpenTelemetry.Collector.Dependencies.Common
+namespace OpenTelemetry.Collector.AspNetCore.Implementation
 {
     using System;
+    using System.Linq;
     using System.Reflection;
 
     internal class PropertyFetcher
@@ -34,7 +35,7 @@ public object Fetch(object obj)
             if (this.innerFetcher == null)
             {
                 var type = obj.GetType().GetTypeInfo();
-                var property = type.GetDeclaredProperty(this.propertyName);
+                var property = type.DeclaredProperties.FirstOrDefault(p => string.Equals(p.Name, this.propertyName, StringComparison.InvariantCultureIgnoreCase));
                 if (property == null)
                 {
                     property = type.GetProperty(this.propertyName);
@@ -91,4 +92,4 @@ public override object Fetch(object obj)
             }
         }
     }
-}
+}

src/OpenTelemetry.Collector.AspNetCore/OpenTelemetry.Collector.AspNetCore.csproj

@@ -6,7 +6,7 @@
   </PropertyGroup>
 
   <ItemGroup>
-    <ProjectReference Include="..\OpenTelemetry.Abstractions\OpenTelemetry.Abstractions.csproj" />
+    <ProjectReference Include="..\OpenTelemetry\OpenTelemetry.csproj" />
     <PackageReference Include="Microsoft.AspNetCore.Http.Abstractions" Version="2.0.0" />
     <PackageReference Include="Microsoft.AspNetCore.Http.Features" Version="2.0.0" />
   </ItemGroup>

src/OpenTelemetry.Collector.AspNetCore/RequestsCollector.cs

@@ -19,16 +19,15 @@ namespace OpenTelemetry.Collector.AspNetCore
     using System;
     using System.Collections.Generic;
     using Microsoft.AspNetCore.Http;
-    using OpenTelemetry.Collector.AspNetCore.Common;
     using OpenTelemetry.Collector.AspNetCore.Implementation;
     using OpenTelemetry.Trace;
 
     /// <summary>
-    /// Dependencies collector.
+    /// Requests collector.
     /// </summary>
     public class RequestsCollector : IDisposable
     {
-        private readonly DiagnosticSourceSubscriber diagnosticSourceSubscriber;
+        private readonly DiagnosticSourceSubscriber<HttpRequest> diagnosticSourceSubscriber;
 
         /// <summary>
         /// Initializes a new instance of the <see cref="RequestsCollector"/> class.
@@ -38,8 +37,8 @@ public class RequestsCollector : IDisposable
         /// <param name="sampler">Sampler to use to sample dependency calls.</param>
         public RequestsCollector(RequestsCollectorOptions options, ITracer tracer, ISampler sampler)
         {
-            this.diagnosticSourceSubscriber = new DiagnosticSourceSubscriber(
-                new Dictionary<string, Func<ITracer, Func<HttpRequest, ISampler>, ListenerHandler>>()
+            this.diagnosticSourceSubscriber = new DiagnosticSourceSubscriber<HttpRequest>(
+                new Dictionary<string, Func<ITracer, Func<HttpRequest, ISampler>, ListenerHandler<HttpRequest>>>()
                 {
                     { "Microsoft.AspNetCore", (t, s) => new HttpInListener(t, s) },
                 },
@@ -54,7 +53,7 @@ public RequestsCollector(RequestsCollectorOptions options, ITracer tracer, ISamp
                     catch (Exception e)
                     {
                         s = null;
-                        AspNetCoreCollectorEventSource.Log.ExceptionInCustomSampler(e);
+                        CollectorEventSource.Log.ExceptionInCustomSampler(e);
                     }
 
                     return s ?? sampler;
@@ -64,7 +63,7 @@ public RequestsCollector(RequestsCollectorOptions options, ITracer tracer, ISamp
 
         public void Dispose()
         {
-            this.diagnosticSourceSubscriber.Dispose();
+            this.diagnosticSourceSubscriber?.Dispose();
         }
     }
 }

src/OpenTelemetry.Collector.Dependencies/AzureSdkDiagnosticListener.cs

@@ -17,11 +17,11 @@
 namespace OpenTelemetry.Collector.Dependencies
 {
     using System;
-    using System.Diagnostics;
-    using OpenTelemetry.Collector.Dependencies.Common;
+    using System.Diagnostics;
+    using System.Net.Http;
     using OpenTelemetry.Trace;
 
-    internal class AzureSdkDiagnosticListener : ListenerHandler
+    internal class AzureSdkDiagnosticListener : ListenerHandler<HttpRequestMessage>
     {
         private readonly ITracer tracer;
 

src/OpenTelemetry.Collector.Dependencies/DependenciesCollector.cs

@@ -19,7 +19,6 @@ namespace OpenTelemetry.Collector.Dependencies
     using System;
     using System.Collections.Generic;
     using System.Net.Http;
-    using OpenTelemetry.Collector.Dependencies.Common;
     using OpenTelemetry.Collector.Dependencies.Implementation;
     using OpenTelemetry.Trace;
 
@@ -28,7 +27,7 @@ namespace OpenTelemetry.Collector.Dependencies
     /// </summary>
     public class DependenciesCollector : IDisposable
     {
-        private readonly DiagnosticSourceSubscriber diagnosticSourceSubscriber;
+        private readonly DiagnosticSourceSubscriber<HttpRequestMessage> diagnosticSourceSubscriber;
 
         /// <summary>
         /// Initializes a new instance of the <see cref="DependenciesCollector"/> class.
@@ -38,8 +37,8 @@ public class DependenciesCollector : IDisposable
         /// <param name="sampler">Sampler to use to sample dependnecy calls.</param>
         public DependenciesCollector(DependenciesCollectorOptions options, ITracer tracer, ISampler sampler)
         {
-            this.diagnosticSourceSubscriber = new DiagnosticSourceSubscriber(
-                new Dictionary<string, Func<ITracer, Func<HttpRequestMessage, ISampler>, ListenerHandler>>()
+            this.diagnosticSourceSubscriber = new DiagnosticSourceSubscriber<HttpRequestMessage>(
+                new Dictionary<string, Func<ITracer, Func<HttpRequestMessage, ISampler>, ListenerHandler<HttpRequestMessage>>>()
                 {
                     { "HttpHandlerDiagnosticListener", (t, s) => new HttpHandlerDiagnosticListener(t, s) },
                     { "Azure.Clients", (t, s) => new AzureSdkDiagnosticListener("Azure.Clients", t, sampler) },
@@ -56,7 +55,7 @@ public DependenciesCollector(DependenciesCollectorOptions options, ITracer trace
                     catch (Exception e)
                     {
                         s = null;
-                        DependenciesCollectorEventSource.Log.ExceptionInCustomSampler(e);
+                        CollectorEventSource.Log.ExceptionInCustomSampler(e);
                     }
 
                     return s ?? sampler;
@@ -66,7 +65,7 @@ public DependenciesCollector(DependenciesCollectorOptions options, ITracer trace
 
         public void Dispose()
         {
-            this.diagnosticSourceSubscriber.Dispose();
+            this.diagnosticSourceSubscriber?.Dispose();
         }
     }
 }