[ASP.NET] Update enrich callbacks

src/OpenTelemetry.Instrumentation.AspNet/.publicApi/net462/PublicAPI.Unshipped.txt

@@ -1,7 +1,11 @@
 OpenTelemetry.Instrumentation.AspNet.AspNetInstrumentationOptions
 OpenTelemetry.Instrumentation.AspNet.AspNetInstrumentationOptions.AspNetInstrumentationOptions() -> void
-OpenTelemetry.Instrumentation.AspNet.AspNetInstrumentationOptions.Enrich.get -> System.Action<System.Diagnostics.Activity, string, object>
-OpenTelemetry.Instrumentation.AspNet.AspNetInstrumentationOptions.Enrich.set -> void
+OpenTelemetry.Instrumentation.AspNet.AspNetInstrumentationOptions.EnrichWithException.get -> System.Action<System.Diagnostics.Activity, System.Exception>
+OpenTelemetry.Instrumentation.AspNet.AspNetInstrumentationOptions.EnrichWithException.set -> void
+OpenTelemetry.Instrumentation.AspNet.AspNetInstrumentationOptions.EnrichWithHttpRequest.get -> System.Action<System.Diagnostics.Activity, System.Web.HttpRequest>
+OpenTelemetry.Instrumentation.AspNet.AspNetInstrumentationOptions.EnrichWithHttpRequest.set -> void
+OpenTelemetry.Instrumentation.AspNet.AspNetInstrumentationOptions.EnrichWithHttpResponse.get -> System.Action<System.Diagnostics.Activity, System.Web.HttpResponse>
+OpenTelemetry.Instrumentation.AspNet.AspNetInstrumentationOptions.EnrichWithHttpResponse.set -> void
 OpenTelemetry.Instrumentation.AspNet.AspNetInstrumentationOptions.Filter.get -> System.Func<System.Web.HttpContext, bool>
 OpenTelemetry.Instrumentation.AspNet.AspNetInstrumentationOptions.Filter.set -> void
 OpenTelemetry.Instrumentation.AspNet.AspNetInstrumentationOptions.RecordException.get -> bool

src/OpenTelemetry.Instrumentation.AspNet/AspNetInstrumentationOptions.cs

@@ -46,11 +46,27 @@ public class AspNetInstrumentationOptions
     /// </summary>
     /// <remarks>
     /// <para><see cref="Activity"/>: the activity being enriched.</para>
-    /// <para>string: the name of the event.</para>
-    /// <para>object: the raw object from which additional information can be extracted to enrich the activity.
-    /// The type of this object depends on the event, which is given by the above parameter.</para>
+    /// <para><see cref="HttpRequest"/>: the HttpRequest object from which additional information can be extracted to enrich the activity.</para>
     /// </remarks>
-    public Action<Activity, string, object> Enrich { get; set; }
+    public Action<Activity, HttpRequest> EnrichWithHttpRequest { get; set; }
+
+    /// <summary>
+    /// Gets or sets an action to enrich an Activity.
+    /// </summary>
+    /// <remarks>
+    /// <para><see cref="Activity"/>: the activity being enriched.</para>
+    /// <para><see cref="HttpResponse"/>: the HttpResponse object from which additional information can be extracted to enrich the activity.</para>
+    /// </remarks>
+    public Action<Activity, HttpResponse> EnrichWithHttpResponse { get; set; }
+
+    /// <summary>
+    /// Gets or sets an action to enrich an Activity.
+    /// </summary>
+    /// <remarks>
+    /// <para><see cref="Activity"/>: the activity being enriched.</para>
+    /// <para><see cref="Exception"/>: the Exception object from which additional information can be extracted to enrich the activity.</para>
+    /// </remarks>
+    public Action<Activity, Exception> EnrichWithException { get; set; }
 
     /// <summary>
     /// Gets or sets a value indicating whether the exception will be recorded as ActivityEvent or not.

src/OpenTelemetry.Instrumentation.AspNet/CHANGELOG.md

@@ -2,6 +2,16 @@
 
 ## Unreleased
 
+* Breaking change The Enrich callback option has been removed. For better
+  usability, it has been replaced by three separate options:
+  EnrichWithHttpRequest, EnrichWithHttpResponse and EnrichWithException.
+  Previously, the single Enrich callback required the consumer to detect which
+  event triggered the callback to be invoked (e.g., request start, response end,
+  or an exception) and then cast the object received to the appropriate type:
+  HttpRequest, HttpResponse, or Exception. The separate callbacks make it clear
+  what event triggers them and there is no longer the need to cast the argument
+  to the expected type.
+  ([#940](https://github.com/open-telemetry/opentelemetry-dotnet-contrib/pull/940))
 * Removes `AddAspNetInstrumentation` method with default configure parameter.
   ([#942](https://github.com/open-telemetry/opentelemetry-dotnet-contrib/pull/942))
 

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

@@ -111,7 +111,7 @@ private void OnStartActivity(Activity activity, HttpContext context)
 
             try
             {
-                this.options.Enrich?.Invoke(activity, "OnStartActivity", request);
+                this.options.EnrichWithHttpRequest?.Invoke(activity, request);
             }
             catch (Exception ex)
             {
@@ -163,7 +163,7 @@ private void OnStopActivity(Activity activity, HttpContext context)
 
             try
             {
-                this.options.Enrich?.Invoke(activity, "OnStopActivity", response);
+                this.options.EnrichWithHttpResponse?.Invoke(activity, response);
             }
             catch (Exception ex)
             {
@@ -185,7 +185,7 @@ private void OnException(Activity activity, HttpContext context, Exception excep
 
             try
             {
-                this.options.Enrich?.Invoke(activity, "OnException", exception);
+                this.options.EnrichWithException?.Invoke(activity, exception);
             }
             catch (Exception ex)
             {

src/OpenTelemetry.Instrumentation.AspNet/README.md

@@ -117,36 +117,33 @@ and the `Filter` option does the filtering *before* the Sampler is invoked.
 
 ### Enrich
 
-This option allows one to enrich the activity with additional information from
-the raw `HttpRequest`, `HttpResponse` objects. The `Enrich` action is called
+This instrumentation library provides `EnrichWithHttpRequest`,
+`EnrichWithHttpResponse` and `EnrichWithException` options that can be used to
+enrich the activity with additional information from the raw `HttpRequest`,
+`HttpResponse` and `Exception` objects respectively. These actions are called
 only when `activity.IsAllDataRequested` is `true`. It contains the activity
-itself (which can be enriched), the name of the event, and the actual raw
-object. For event name "OnStartActivity", the actual object will be
-`HttpRequest`. For event name "OnStopActivity", the actual object will be
-`HttpResponse`
+itself (which can be enriched) and the actual raw object.
 
-The following code snippet shows how to add additional tags using `Enrich`.
+The following code snippet shows how to enrich the activity using all 3
+different options.
 
 ```csharp
 this.tracerProvider = Sdk.CreateTracerProviderBuilder()
-    .AddAspNetInstrumentation((options) => options.Enrich
-        = (activity, eventName, rawObject) =>
-    {
-        if (eventName.Equals("OnStartActivity"))
+    .AddAspNetInstrumentation(o =>
         {
-            if (rawObject is HttpRequest httpRequest)
+            o.EnrichWithHttpRequest = (activity, httpRequest) =>
             {
                 activity.SetTag("physicalPath", httpRequest.PhysicalPath);
-            }
-        }
-        else if (eventName.Equals("OnStopActivity"))
-        {
-            if (rawObject is HttpResponse httpResponse)
+            };
+            o.EnrichWithHttpResponse = (activity, httpResponse) =>
             {
                 activity.SetTag("responseType", httpResponse.ContentType);
-            }
-        }
-    })
+            };
+            o.EnrichWithException = (activity, exception) =>
+            {
+                activity.SetTag("exceptionType", exception.GetType().ToString());
+            };
+        })
     .Build();
 ```
 

test/OpenTelemetry.Instrumentation.AspNet.Tests/HttpInListenerTests.cs

@@ -137,7 +137,14 @@ public void AspNetRequestsAreCollectedSuccessfully(
                            return httpContext.Request.Path != filter;
                        };
 
-                       options.Enrich = GetEnrichmentAction(setStatusToErrorInEnrich ? ActivityStatusCode.Error : default);
+                       // TODO: update to test activity enrichment for other enrich options.
+                       options.EnrichWithHttpResponse = (activity, _) =>
+                       {
+                           if (setStatusToErrorInEnrich)
+                           {
+                               activity.SetStatus(ActivityStatusCode.Error);
+                           }
+                       };
 
                        options.RecordException = recordException;
                    })

test/OpenTelemetry.Instrumentation.AspNet.Tests/HttpInMetricsListenerTests.cs

@@ -39,13 +39,10 @@ public void HttpDurationMetricIsEmitted()
         // as it is created using ActivitySource inside TelemetryHttpModule
         // TODO: This should not be needed once the dependency on activity is removed from metrics
         using var tracerProvider = Sdk.CreateTracerProviderBuilder()
-            .AddAspNetInstrumentation(opts => opts.Enrich
-                = (activity, eventName, rawObject) =>
+            .AddAspNetInstrumentation(opts => opts.EnrichWithHttpResponse
+                = (activity, _) =>
                 {
-                    if (eventName.Equals("OnStopActivity"))
-                    {
-                        duration = activity.Duration.TotalMilliseconds;
-                    }
+                    duration = activity.Duration.TotalMilliseconds;
                 })
             .Build();