Make span.Name settable, update it with the route info for ASP.NET Core (#80)

* better name

* make it work

* revert wrong addition

* addressed code review

* fix build failure and added comment for the sampler

* added changelog entry

* added route into the attributs as well

Author: SergeyKanzhelev

CHANGELOG.md

@@ -7,14 +7,18 @@ the release.
 
 ## Unreleased
 
+- ASP.NET Core collector now uses `http.route` for the span name.
+
 ## 0.1.0-alpha-33381
 
-Released [12/18/2018](https://github.com/census-instrumentation/opencensus-csharp/releases/tag/0.1.0-alpha-33381).
+Released
+[12/18/2018](https://github.com/census-instrumentation/opencensus-csharp/releases/tag/0.1.0-alpha-33381).
 
 - Collectors for ASP.NET Core and .NET Core HttpClient.
 - Initial version of Ocagent exporter implemented.
 - Initial version of StackDriver exporter implemented.
-- Support double attributes according to the [spec change](https://github.com/census-instrumentation/opencensus-specs/issues/172).
+- Support double attributes according to the [spec
+  change](https://github.com/census-instrumentation/opencensus-specs/issues/172).
 - Initial implementation of Prometheus exporter.
 - Initial version of Application Insights exporter implemented.
 - Zipkin exporter implemented.

src/OpenCensus.Abstractions/Trace/ISampler.cs

@@ -35,7 +35,11 @@ public interface ISampler
         /// <param name="hasRemoteParent">Indicates whether it was a remote parent.</param>
         /// <param name="traceId">Trace ID of a span to be created.</param>
         /// <param name="spanId">Span ID of a span to be created.</param>
-        /// <param name="name">Name of a span to be created.</param>
+        /// <param name="name"> Name of a span to be created. Note, that the name of the span is settable.
+        /// So this name can be changed later and <see cref="ISampler"/> implementation should assume that.
+        /// Typical example of a name change is when <see cref="ISpan"/> representing incoming http request
+        /// has a name of url path and then being updated with route name when rouing complete.
+        /// </param>
         /// <param name="parentLinks">Links associated with the parent span.</param>
         /// <returns>True of span needs to be created. False otherwise.</returns>
         bool ShouldSample(ISpanContext parentContext, bool hasRemoteParent, ITraceId traceId, ISpanId spanId, string name, IList<ISpan> parentLinks);

src/OpenCensus.Abstractions/Trace/ISpan.cs

@@ -27,6 +27,11 @@ namespace OpenCensus.Trace
     /// </summary>
     public interface ISpan
     {
+        /// <summary>
+        /// Gets or sets the span name.
+        /// </summary>
+        string Name { get; set; }
+
         /// <summary>
         /// Gets the span context.
         /// </summary>

src/OpenCensus.Abstractions/Trace/SpanBase.cs

@@ -53,6 +53,8 @@ protected SpanBase(ISpanContext context, SpanOptions options = SpanOptions.None)
             this.Options = options;
         }
 
+        public abstract string Name { get; set; }
+
         /// <inheritdoc/>
         public virtual ISpanContext Context { get; }
 
@@ -65,11 +67,6 @@ protected SpanBase(ISpanContext context, SpanOptions options = SpanOptions.None)
         /// <inheritdoc/>
         public abstract SpanKind? Kind { get; set; }
 
-        /// <summary>
-        /// Gets the name of the span.
-        /// </summary>
-        public abstract string Name { get; }
-
         /// <inheritdoc/>
         public SpanBase Next { get; set; }
 

src/OpenCensus.Abstractions/Trace/SpanExtensions.cs

@@ -108,6 +108,23 @@ public static ISpan PutHttpHostAttribute(this ISpan span, string hostName, int p
             return span;
         }
 
+        /// <summary>
+        /// Helper method that populates span properties from route
+        /// to https://github.com/census-instrumentation/opencensus-specs/blob/4954074adf815f437534457331178194f6847ff9/trace/HTTP.md.
+        /// </summary>
+        /// <param name="span">Span to fill out.</param>
+        /// <param name="route">Route used to resolve url to controller.</param>
+        /// <returns>Span with populated route properties.</returns>
+        public static ISpan PutHttpRouteAttribute(this ISpan span, string route)
+        {
+            if (!string.IsNullOrEmpty(route))
+            {
+                span.PutAttribute(SpanAttributeConstants.HttpRouteKey, AttributeValue.StringAttributeValue(route));
+            }
+
+            return span;
+        }
+
         /// <summary>
         /// Helper method that populates span properties from host and port
         /// to https://github.com/census-instrumentation/opencensus-specs/blob/4954074adf815f437534457331178194f6847ff9/trace/HTTP.md.

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

@@ -16,7 +16,6 @@
 
 namespace OpenCensus.Collector.AspNetCore.Implementation
 {
-    using System;
     using System.Diagnostics;
     using System.Linq;
     using System.Text;
@@ -31,6 +30,9 @@ internal class HttpInListener : ListenerHandler
         private const string UnknownHostName = "UNKNOWN-HOST";
         private readonly PropertyFetcher startContextFetcher = new PropertyFetcher("HttpContext");
         private readonly PropertyFetcher stopContextFetcher = new PropertyFetcher("HttpContext");
+        private readonly PropertyFetcher beforeActionActionDescriptorFetcher = new PropertyFetcher("actionDescriptor");
+        private readonly PropertyFetcher beforeActionAttributeRouteInfoFetcher = new PropertyFetcher("AttributeRouteInfo");
+        private readonly PropertyFetcher beforeActionTemplateFetcher = new PropertyFetcher("Template");
         private readonly IPropagationComponent propagationComponent;
 
         public HttpInListener(ITracer tracer, ISampler sampler, IPropagationComponent propagationComponent)
@@ -84,15 +86,16 @@ public override void OnStopActivity(Activity activity, object payload)
 
             if (context == null)
             {
-                // Debug.WriteLine("context is null");
+                // TODO: Debug.WriteLine("context is null");
                 return;
             }
 
             var span = this.Tracer.CurrentSpan;
 
             if (span == null)
             {
-                // report lost span
+                // TODO: report lost span
+                return;
             }
 
             var response = context.Response;
@@ -101,6 +104,41 @@ public override void OnStopActivity(Activity activity, object payload)
             span.End();
         }
 
+        public override void OnCustom(string name, Activity activity, object payload)
+        {
+            if (name == "Microsoft.AspNetCore.Mvc.BeforeAction")
+            {
+                var span = this.Tracer.CurrentSpan;
+
+                if (span == null)
+                {
+                    // TODO: report lost span
+                    return;
+                }
+
+                // See https://github.com/aspnet/Mvc/blob/2414db256f32a047770326d14d8b0e2afd49ba49/src/Microsoft.AspNetCore.Mvc.Core/MvcCoreDiagnosticSourceExtensions.cs#L36-L44
+                // Reflection accessing: ActionDescriptor.AttributeRouteInfo.Template
+                // The reason to use reflection is to avoid a reference on MVC package.
+                // This package can be used with non-MVC apps and this logic simply wouldn't run.
+                // Taking reference on MVC will increase size of deployment for non-MVC apps.
+                var actionDescriptor = this.beforeActionActionDescriptorFetcher.Fetch(payload);
+                var attributeRouteInfo = this.beforeActionAttributeRouteInfoFetcher.Fetch(actionDescriptor);
+                var template = this.beforeActionTemplateFetcher.Fetch(attributeRouteInfo) as string;
+
+                if (!string.IsNullOrEmpty(template))
+                {
+                    // override the span name that was previously set to the path part of URL.
+                    span.Name = template;
+
+                    span.PutHttpRouteAttribute(template);
+                }
+
+                // TODO: Should we get values from RouteData?
+                // private readonly PropertyFetcher beforActionRouteDataFetcher = new PropertyFetcher("routeData");
+                // var routeData = this.beforActionRouteDataFetcher.Fetch(payload) as RouteData;
+            }
+        }
+
         private static string GetUri(HttpRequest request)
         {
             var builder = new StringBuilder();

src/OpenCensus.Collector.AspNetCore/RequestsCollector.cs

@@ -41,7 +41,9 @@ public RequestsCollector(RequestsCollectorOptions options, ITracer tracer, ISamp
         {
             this.diagnosticSourceSubscriber = new DiagnosticSourceSubscriber(
                 new Dictionary<string, Func<ITracer, ISampler, ListenerHandler>>()
-                { { "Microsoft.AspNetCore", (t, s) => new HttpInListener(t, s, propagationComponent) } },
+                {
+                    { "Microsoft.AspNetCore", (t, s) => new HttpInListener(t, s, propagationComponent) },
+                },
                 tracer,
                 sampler);
             this.diagnosticSourceSubscriber.Subscribe();

src/OpenCensus.Collector.Implementation.Common/DiagnosticSourceListener.cs

@@ -63,6 +63,10 @@ public void OnNext(KeyValuePair<string, object> value)
                 {
                     this.handler.OnException(Activity.Current, value.Value);
                 }
+                else
+                {
+                    this.handler.OnCustom(value.Key, Activity.Current, value.Value);
+                }
             }
             catch (Exception e)
             {

src/OpenCensus.Collector.Implementation.Common/ListenerHandler.cs

@@ -61,5 +61,10 @@ public virtual void OnException(Activity activity, object payload)
 
             // TODO: gather exception information
         }
+
+        public virtual void OnCustom(string name, Activity activity, object payload)
+        {
+            // if custom handler needs to react on other events - this method should be overridden
+        }
     }
 }

src/OpenCensus.Collector.Implementation.Common/PropertyFetcher.cs

@@ -33,7 +33,14 @@ public object Fetch(object obj)
         {
             if (this.innerFetcher == null)
             {
-                this.innerFetcher = PropertyFetch.FetcherForProperty(obj.GetType().GetTypeInfo().GetDeclaredProperty(this.propertyName));
+                var type = obj.GetType().GetTypeInfo();
+                var property = type.GetDeclaredProperty(this.propertyName);
+                if (property == null)
+                {
+                    property = type.GetProperty(this.propertyName);
+                }
+
+                this.innerFetcher = PropertyFetch.FetcherForProperty(property);
             }
 
             return this.innerFetcher?.Fetch(obj);