Enable OnException for AspNetCore instrumentation

src/OpenTelemetry.Instrumentation.AspNetCore/AspNetCoreInstrumentationOptions.cs

@@ -53,5 +53,13 @@ public class AspNetCoreInstrumentationOptions
         /// The type of this object depends on the event, which is given by the above parameter.</para>
         /// </remarks>
         public Action<Activity, string, object> Enrich { get; set; }
+
+        /// <summary>
+        /// Gets or sets a value indicating whether the exception will be recorded as ActivityEvent or not.
+        /// </summary>
+        /// <remarks>
+        /// https://github.com/open-telemetry/opentelemetry-specification/blob/master/specification/trace/semantic_conventions/exceptions.md.
+        /// </remarks>
+        public bool RecordException { get; set; }
     }
 }

src/OpenTelemetry.Instrumentation.AspNetCore/CHANGELOG.md

@@ -2,6 +2,10 @@
 
 ## Unreleased
 
+* Record `Exception` in AspNetCore instrumentation based on `RecordException` in
+  `AspNetCoreInstrumentationOptions`
+  ([#1408](https://github.com/open-telemetry/opentelemetry-dotnet/issues/1408))
+
 ## 0.7.0-beta.1
 
 Released 2020-Oct-16

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

@@ -34,6 +34,7 @@ internal class HttpInListener : ListenerHandler
         private static readonly Func<HttpRequest, string, IEnumerable<string>> HttpRequestHeaderValuesGetter = (request, name) => request.Headers[name];
         private readonly PropertyFetcher<HttpContext> startContextFetcher = new PropertyFetcher<HttpContext>("HttpContext");
         private readonly PropertyFetcher<HttpContext> stopContextFetcher = new PropertyFetcher<HttpContext>("HttpContext");
+        private readonly PropertyFetcher<Exception> stopExceptionFetcher = new PropertyFetcher<Exception>("Exception");
         private readonly PropertyFetcher<object> beforeActionActionDescriptorFetcher = new PropertyFetcher<object>("actionDescriptor");
         private readonly PropertyFetcher<object> beforeActionAttributeRouteInfoFetcher = new PropertyFetcher<object>("AttributeRouteInfo");
         private readonly PropertyFetcher<string> beforeActionTemplateFetcher = new PropertyFetcher<string>("Template");
@@ -225,6 +226,34 @@ public override void OnCustom(string name, Activity activity, object payload)
             }
         }
 
+        public override void OnException(Activity activity, object payload)
+        {
+            if (activity.IsAllDataRequested)
+            {
+                if (!this.stopExceptionFetcher.TryFetch(payload, out Exception exc) || exc == null)
+                {
+                    AspNetCoreInstrumentationEventSource.Log.NullPayload(nameof(HttpInListener), nameof(this.OnException));
+                    return;
+                }
+
+                try
+                {
+                    this.options.Enrich?.Invoke(activity, "OnException", exc);
+                }
+                catch (Exception ex)
+                {
+                    AspNetCoreInstrumentationEventSource.Log.EnrichmentException(ex);
+                }
+
+                if (this.options.RecordException)
+                {
+                    activity.RecordException(exc);
+                }
+
+                activity.SetStatus(Status.Error.WithDescription(exc.Message));
+            }
+        }
+
         private static string GetUri(HttpRequest request)
         {
             var builder = new StringBuilder();

src/OpenTelemetry.Instrumentation.Http/CHANGELOG.md

@@ -5,7 +5,7 @@
 * Instrumentation for `HttpWebRequest` no longer store raw objects like
   `HttpWebRequest` in Activity.CustomProperty. To enrich activity, use the
   Enrich action on the instrumentation.
-  ([#1261](https://github.com/open-telemetry/opentelemetry-dotnet/pull/1407))
+  ([#1407](https://github.com/open-telemetry/opentelemetry-dotnet/pull/1407))
 
 ## 0.7.0-beta.1
 

test/OpenTelemetry.Instrumentation.AspNetCore.Tests/IncomingRequestsCollectionsIsAccordingToTheSpecTests.cs

@@ -16,6 +16,7 @@
 
 using System;
 using System.Diagnostics;
+using System.Linq;
 using System.Threading.Tasks;
 using Microsoft.AspNetCore.Http;
 using Microsoft.AspNetCore.Http.Features;
@@ -46,11 +47,14 @@ public IncomingRequestsCollectionsIsAccordingToTheSpecTests(WebApplicationFactor
         [Theory]
         [InlineData("/api/values", "user-agent", 503, "503")]
         [InlineData("/api/values", null, 503, null)]
+        [InlineData("/api/exception", null, 503, null)]
+        [InlineData("/api/exception", null, 503, null, true)]
         public async Task SuccessfulTemplateControllerCallGeneratesASpan(
             string urlPath,
             string userAgent,
             int statusCode,
-            string reasonPhrase)
+            string reasonPhrase,
+            bool recordException = false)
         {
             var processor = new Mock<BaseProcessor<Activity>>();
 
@@ -60,7 +64,7 @@ public async Task SuccessfulTemplateControllerCallGeneratesASpan(
                     builder.ConfigureTestServices((IServiceCollection services) =>
                     {
                         services.AddSingleton<CallbackMiddleware.CallbackMiddlewareImpl>(new TestCallbackMiddlewareImpl(statusCode, reasonPhrase));
-                        services.AddOpenTelemetryTracing((builder) => builder.AddAspNetCoreInstrumentation()
+                        services.AddOpenTelemetryTracing((builder) => builder.AddAspNetCoreInstrumentation(options => options.RecordException = recordException)
                         .AddProcessor(processor.Object));
                     }))
                 .CreateClient())
@@ -106,7 +110,7 @@ public async Task SuccessfulTemplateControllerCallGeneratesASpan(
 
             if (statusCode == 503)
             {
-                Assert.Equal(Status.Error, activity.GetStatus());
+                Assert.Equal(Status.Error.StatusCode, activity.GetStatus().StatusCode);
             }
             else
             {
@@ -115,9 +119,25 @@ public async Task SuccessfulTemplateControllerCallGeneratesASpan(
 
             // Instrumentation is not expected to set status description
             // as the reason can be inferred from SemanticConventions.AttributeHttpStatusCode
-            Assert.True(string.IsNullOrEmpty(activity.GetStatus().Description));
+            if (!urlPath.EndsWith("exception"))
+            {
+                Assert.True(string.IsNullOrEmpty(activity.GetStatus().Description));
+            }
+            else
+            {
+                Assert.Equal("exception description", activity.GetStatus().Description);
+            }
+
+            if (recordException)
+            {
+                Assert.Single(activity.Events);
+                Assert.Equal("exception", activity.Events.First().Name);
+            }
 
             this.ValidateTagValue(activity, SemanticConventions.AttributeHttpUserAgent, userAgent);
+
+            activity.Dispose();
+            processor.Object.Dispose();
         }
 
         private void ValidateTagValue(Activity activity, string attribute, string expectedValue)
@@ -148,6 +168,12 @@ public override async Task<bool> ProcessAsync(HttpContext context)
                 context.Response.StatusCode = this.statusCode;
                 context.Response.HttpContext.Features.Get<IHttpResponseFeature>().ReasonPhrase = this.reasonPhrase;
                 await context.Response.WriteAsync("empty");
+
+                if (context.Request.Path.Value.EndsWith("exception"))
+                {
+                    throw new Exception("exception description");
+                }
+
                 return false;
             }
         }