[ASP.NET Core] Update enrich callbacks to use specific type. (#3749)

open-telemetry · Oct 21, 2022 · 0c31fb9 · 0c31fb9
1 parent f94ccb7
commit 0c31fb9
Show file tree

Hide file tree

Showing 10 changed files with 96 additions and 56 deletions.
diff --git a/src/OpenTelemetry.Instrumentation.AspNetCore/.publicApi/net6.0/PublicAPI.Unshipped.txt b/src/OpenTelemetry.Instrumentation.AspNetCore/.publicApi/net6.0/PublicAPI.Unshipped.txt
@@ -2,8 +2,12 @@ OpenTelemetry.Instrumentation.AspNetCore.AspNetCoreInstrumentationOptions
 OpenTelemetry.Instrumentation.AspNetCore.AspNetCoreInstrumentationOptions.AspNetCoreInstrumentationOptions() -> void
 OpenTelemetry.Instrumentation.AspNetCore.AspNetCoreInstrumentationOptions.EnableGrpcAspNetCoreSupport.get -> bool
 OpenTelemetry.Instrumentation.AspNetCore.AspNetCoreInstrumentationOptions.EnableGrpcAspNetCoreSupport.set -> void
-OpenTelemetry.Instrumentation.AspNetCore.AspNetCoreInstrumentationOptions.Enrich.get -> System.Action<System.Diagnostics.Activity, string, object>
-OpenTelemetry.Instrumentation.AspNetCore.AspNetCoreInstrumentationOptions.Enrich.set -> void
+OpenTelemetry.Instrumentation.AspNetCore.AspNetCoreInstrumentationOptions.EnrichWithException.get -> System.Action<System.Diagnostics.Activity, System.Exception>
+OpenTelemetry.Instrumentation.AspNetCore.AspNetCoreInstrumentationOptions.EnrichWithException.set -> void
+OpenTelemetry.Instrumentation.AspNetCore.AspNetCoreInstrumentationOptions.EnrichWithHttpRequest.get -> System.Action<System.Diagnostics.Activity, Microsoft.AspNetCore.Http.HttpRequest>
+OpenTelemetry.Instrumentation.AspNetCore.AspNetCoreInstrumentationOptions.EnrichWithHttpRequest.set -> void
+OpenTelemetry.Instrumentation.AspNetCore.AspNetCoreInstrumentationOptions.EnrichWithHttpResponse.get -> System.Action<System.Diagnostics.Activity, Microsoft.AspNetCore.Http.HttpResponse>
+OpenTelemetry.Instrumentation.AspNetCore.AspNetCoreInstrumentationOptions.EnrichWithHttpResponse.set -> void
 OpenTelemetry.Instrumentation.AspNetCore.AspNetCoreInstrumentationOptions.Filter.get -> System.Func<Microsoft.AspNetCore.Http.HttpContext, bool>
 OpenTelemetry.Instrumentation.AspNetCore.AspNetCoreInstrumentationOptions.Filter.set -> void
 OpenTelemetry.Instrumentation.AspNetCore.AspNetCoreInstrumentationOptions.RecordException.get -> bool
@@ -13,4 +17,4 @@ OpenTelemetry.Trace.TracerProviderBuilderExtensions
 static OpenTelemetry.Metrics.MeterProviderBuilderExtensions.AddAspNetCoreInstrumentation(this OpenTelemetry.Metrics.MeterProviderBuilder builder) -> OpenTelemetry.Metrics.MeterProviderBuilder
 static OpenTelemetry.Trace.TracerProviderBuilderExtensions.AddAspNetCoreInstrumentation(this OpenTelemetry.Trace.TracerProviderBuilder builder) -> OpenTelemetry.Trace.TracerProviderBuilder
 static OpenTelemetry.Trace.TracerProviderBuilderExtensions.AddAspNetCoreInstrumentation(this OpenTelemetry.Trace.TracerProviderBuilder builder, string name, System.Action<OpenTelemetry.Instrumentation.AspNetCore.AspNetCoreInstrumentationOptions> configureAspNetCoreInstrumentationOptions) -> OpenTelemetry.Trace.TracerProviderBuilder
-static OpenTelemetry.Trace.TracerProviderBuilderExtensions.AddAspNetCoreInstrumentation(this OpenTelemetry.Trace.TracerProviderBuilder builder, System.Action<OpenTelemetry.Instrumentation.AspNetCore.AspNetCoreInstrumentationOptions> configureAspNetCoreInstrumentationOptions) -> OpenTelemetry.Trace.TracerProviderBuilder
+static OpenTelemetry.Trace.TracerProviderBuilderExtensions.AddAspNetCoreInstrumentation(this OpenTelemetry.Trace.TracerProviderBuilder builder, System.Action<OpenTelemetry.Instrumentation.AspNetCore.AspNetCoreInstrumentationOptions> configureAspNetCoreInstrumentationOptions) -> OpenTelemetry.Trace.TracerProviderBuilder
diff --git a/src/OpenTelemetry.Instrumentation.AspNetCore/.publicApi/net7.0/PublicAPI.Unshipped.txt b/src/OpenTelemetry.Instrumentation.AspNetCore/.publicApi/net7.0/PublicAPI.Unshipped.txt
@@ -2,8 +2,12 @@ OpenTelemetry.Instrumentation.AspNetCore.AspNetCoreInstrumentationOptions
 OpenTelemetry.Instrumentation.AspNetCore.AspNetCoreInstrumentationOptions.AspNetCoreInstrumentationOptions() -> void
 OpenTelemetry.Instrumentation.AspNetCore.AspNetCoreInstrumentationOptions.EnableGrpcAspNetCoreSupport.get -> bool
 OpenTelemetry.Instrumentation.AspNetCore.AspNetCoreInstrumentationOptions.EnableGrpcAspNetCoreSupport.set -> void
-OpenTelemetry.Instrumentation.AspNetCore.AspNetCoreInstrumentationOptions.Enrich.get -> System.Action<System.Diagnostics.Activity, string, object>
-OpenTelemetry.Instrumentation.AspNetCore.AspNetCoreInstrumentationOptions.Enrich.set -> void
+OpenTelemetry.Instrumentation.AspNetCore.AspNetCoreInstrumentationOptions.EnrichWithException.get -> System.Action<System.Diagnostics.Activity, System.Exception>
+OpenTelemetry.Instrumentation.AspNetCore.AspNetCoreInstrumentationOptions.EnrichWithException.set -> void
+OpenTelemetry.Instrumentation.AspNetCore.AspNetCoreInstrumentationOptions.EnrichWithHttpRequest.get -> System.Action<System.Diagnostics.Activity, Microsoft.AspNetCore.Http.HttpRequest>
+OpenTelemetry.Instrumentation.AspNetCore.AspNetCoreInstrumentationOptions.EnrichWithHttpRequest.set -> void
+OpenTelemetry.Instrumentation.AspNetCore.AspNetCoreInstrumentationOptions.EnrichWithHttpResponse.get -> System.Action<System.Diagnostics.Activity, Microsoft.AspNetCore.Http.HttpResponse>
+OpenTelemetry.Instrumentation.AspNetCore.AspNetCoreInstrumentationOptions.EnrichWithHttpResponse.set -> void
 OpenTelemetry.Instrumentation.AspNetCore.AspNetCoreInstrumentationOptions.Filter.get -> System.Func<Microsoft.AspNetCore.Http.HttpContext, bool>
 OpenTelemetry.Instrumentation.AspNetCore.AspNetCoreInstrumentationOptions.Filter.set -> void
 OpenTelemetry.Instrumentation.AspNetCore.AspNetCoreInstrumentationOptions.RecordException.get -> bool
@@ -13,4 +17,4 @@ OpenTelemetry.Trace.TracerProviderBuilderExtensions
 static OpenTelemetry.Metrics.MeterProviderBuilderExtensions.AddAspNetCoreInstrumentation(this OpenTelemetry.Metrics.MeterProviderBuilder builder) -> OpenTelemetry.Metrics.MeterProviderBuilder
 static OpenTelemetry.Trace.TracerProviderBuilderExtensions.AddAspNetCoreInstrumentation(this OpenTelemetry.Trace.TracerProviderBuilder builder) -> OpenTelemetry.Trace.TracerProviderBuilder
 static OpenTelemetry.Trace.TracerProviderBuilderExtensions.AddAspNetCoreInstrumentation(this OpenTelemetry.Trace.TracerProviderBuilder builder, string name, System.Action<OpenTelemetry.Instrumentation.AspNetCore.AspNetCoreInstrumentationOptions> configureAspNetCoreInstrumentationOptions) -> OpenTelemetry.Trace.TracerProviderBuilder
-static OpenTelemetry.Trace.TracerProviderBuilderExtensions.AddAspNetCoreInstrumentation(this OpenTelemetry.Trace.TracerProviderBuilder builder, System.Action<OpenTelemetry.Instrumentation.AspNetCore.AspNetCoreInstrumentationOptions> configureAspNetCoreInstrumentationOptions) -> OpenTelemetry.Trace.TracerProviderBuilder
+static OpenTelemetry.Trace.TracerProviderBuilderExtensions.AddAspNetCoreInstrumentation(this OpenTelemetry.Trace.TracerProviderBuilder builder, System.Action<OpenTelemetry.Instrumentation.AspNetCore.AspNetCoreInstrumentationOptions> configureAspNetCoreInstrumentationOptions) -> OpenTelemetry.Trace.TracerProviderBuilder
diff --git a/...penTelemetry.Instrumentation.AspNetCore/.publicApi/netstandard2.0/PublicAPI.Unshipped.txt b/...penTelemetry.Instrumentation.AspNetCore/.publicApi/netstandard2.0/PublicAPI.Unshipped.txt
@@ -1,7 +1,11 @@
 OpenTelemetry.Instrumentation.AspNetCore.AspNetCoreInstrumentationOptions
 OpenTelemetry.Instrumentation.AspNetCore.AspNetCoreInstrumentationOptions.AspNetCoreInstrumentationOptions() -> void
-OpenTelemetry.Instrumentation.AspNetCore.AspNetCoreInstrumentationOptions.Enrich.get -> System.Action<System.Diagnostics.Activity, string, object>
-OpenTelemetry.Instrumentation.AspNetCore.AspNetCoreInstrumentationOptions.Enrich.set -> void
+OpenTelemetry.Instrumentation.AspNetCore.AspNetCoreInstrumentationOptions.EnrichWithException.get -> System.Action<System.Diagnostics.Activity, System.Exception>
+OpenTelemetry.Instrumentation.AspNetCore.AspNetCoreInstrumentationOptions.EnrichWithException.set -> void
+OpenTelemetry.Instrumentation.AspNetCore.AspNetCoreInstrumentationOptions.EnrichWithHttpRequest.get -> System.Action<System.Diagnostics.Activity, Microsoft.AspNetCore.Http.HttpRequest>
+OpenTelemetry.Instrumentation.AspNetCore.AspNetCoreInstrumentationOptions.EnrichWithHttpRequest.set -> void
+OpenTelemetry.Instrumentation.AspNetCore.AspNetCoreInstrumentationOptions.EnrichWithHttpResponse.get -> System.Action<System.Diagnostics.Activity, Microsoft.AspNetCore.Http.HttpResponse>
+OpenTelemetry.Instrumentation.AspNetCore.AspNetCoreInstrumentationOptions.EnrichWithHttpResponse.set -> void
 OpenTelemetry.Instrumentation.AspNetCore.AspNetCoreInstrumentationOptions.Filter.get -> System.Func<Microsoft.AspNetCore.Http.HttpContext, bool>
 OpenTelemetry.Instrumentation.AspNetCore.AspNetCoreInstrumentationOptions.Filter.set -> void
 OpenTelemetry.Instrumentation.AspNetCore.AspNetCoreInstrumentationOptions.RecordException.get -> bool

diff --git a/...penTelemetry.Instrumentation.AspNetCore/.publicApi/netstandard2.1/PublicAPI.Unshipped.txt b/...penTelemetry.Instrumentation.AspNetCore/.publicApi/netstandard2.1/PublicAPI.Unshipped.txt
@@ -2,8 +2,12 @@ OpenTelemetry.Instrumentation.AspNetCore.AspNetCoreInstrumentationOptions
 OpenTelemetry.Instrumentation.AspNetCore.AspNetCoreInstrumentationOptions.AspNetCoreInstrumentationOptions() -> void
 OpenTelemetry.Instrumentation.AspNetCore.AspNetCoreInstrumentationOptions.EnableGrpcAspNetCoreSupport.get -> bool
 OpenTelemetry.Instrumentation.AspNetCore.AspNetCoreInstrumentationOptions.EnableGrpcAspNetCoreSupport.set -> void
-OpenTelemetry.Instrumentation.AspNetCore.AspNetCoreInstrumentationOptions.Enrich.get -> System.Action<System.Diagnostics.Activity, string, object>
-OpenTelemetry.Instrumentation.AspNetCore.AspNetCoreInstrumentationOptions.Enrich.set -> void
+OpenTelemetry.Instrumentation.AspNetCore.AspNetCoreInstrumentationOptions.EnrichWithException.get -> System.Action<System.Diagnostics.Activity, System.Exception>
+OpenTelemetry.Instrumentation.AspNetCore.AspNetCoreInstrumentationOptions.EnrichWithException.set -> void
+OpenTelemetry.Instrumentation.AspNetCore.AspNetCoreInstrumentationOptions.EnrichWithHttpRequest.get -> System.Action<System.Diagnostics.Activity, Microsoft.AspNetCore.Http.HttpRequest>
+OpenTelemetry.Instrumentation.AspNetCore.AspNetCoreInstrumentationOptions.EnrichWithHttpRequest.set -> void
+OpenTelemetry.Instrumentation.AspNetCore.AspNetCoreInstrumentationOptions.EnrichWithHttpResponse.get -> System.Action<System.Diagnostics.Activity, Microsoft.AspNetCore.Http.HttpResponse>
+OpenTelemetry.Instrumentation.AspNetCore.AspNetCoreInstrumentationOptions.EnrichWithHttpResponse.set -> void
 OpenTelemetry.Instrumentation.AspNetCore.AspNetCoreInstrumentationOptions.Filter.get -> System.Func<Microsoft.AspNetCore.Http.HttpContext, bool>
 OpenTelemetry.Instrumentation.AspNetCore.AspNetCoreInstrumentationOptions.Filter.set -> void
 OpenTelemetry.Instrumentation.AspNetCore.AspNetCoreInstrumentationOptions.RecordException.get -> bool

diff --git a/src/OpenTelemetry.Instrumentation.AspNetCore/AspNetCoreInstrumentationOptions.cs b/src/OpenTelemetry.Instrumentation.AspNetCore/AspNetCoreInstrumentationOptions.cs
@@ -38,11 +38,27 @@ public class AspNetCoreInstrumentationOptions
         /// </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.

diff --git a/src/OpenTelemetry.Instrumentation.AspNetCore/CHANGELOG.md b/src/OpenTelemetry.Instrumentation.AspNetCore/CHANGELOG.md
@@ -2,6 +2,17 @@
 
 ## 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.
+  ([#3749](https://github.com/open-telemetry/opentelemetry-dotnet/pull/3749))
+
 * Added back `netstandard2.0` and `netstandard2.1` targets.
 ([#3755](https://github.com/open-telemetry/opentelemetry-dotnet/pull/3755))
 

diff --git a/src/OpenTelemetry.Instrumentation.AspNetCore/Implementation/HttpInListener.cs b/src/OpenTelemetry.Instrumentation.AspNetCore/Implementation/HttpInListener.cs
@@ -222,7 +222,7 @@ public void OnStartActivity(Activity activity, object payload)
 
                 try
                 {
-                    this.options.Enrich?.Invoke(activity, "OnStartActivity", request);
+                    this.options.EnrichWithHttpRequest?.Invoke(activity, request);
                 }
                 catch (Exception ex)
                 {
@@ -264,7 +264,7 @@ public void OnStopActivity(Activity activity, object payload)
 
                 try
                 {
-                    this.options.Enrich?.Invoke(activity, "OnStopActivity", response);
+                    this.options.EnrichWithHttpResponse?.Invoke(activity, response);
                 }
                 catch (Exception ex)
                 {
@@ -369,7 +369,7 @@ public void OnException(Activity activity, object payload)
 
                 try
                 {
-                    this.options.Enrich?.Invoke(activity, "OnException", exc);
+                    this.options.EnrichWithException?.Invoke(activity, exc);
                 }
                 catch (Exception ex)
                 {

diff --git a/src/OpenTelemetry.Instrumentation.AspNetCore/README.md b/src/OpenTelemetry.Instrumentation.AspNetCore/README.md
@@ -120,39 +120,36 @@ and the `Filter` option does the filtering *after* 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 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`
+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) 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
 services.AddOpenTelemetryTracing((builder) =>
 {
-    builder.AddAspNetCoreInstrumentation((options) => options.Enrich
-        = (activity, eventName, rawObject) =>
+    builder.AddAspNetCoreInstrumentation(o =>
     {
-        if (eventName.Equals("OnStartActivity"))
+        o.EnrichWithHttpRequest = (activity, httpRequest) =>
         {
-            if (rawObject is HttpRequest httpRequest)
-            {
-                activity.SetTag("requestProtocol", httpRequest.Protocol);
-            }
-        }
-        else if (eventName.Equals("OnStopActivity"))
+            activity.SetTag("requestProtocol", httpRequest.Protocol);
+        };
+        o.EnrichWithHttpResponse = (activity, httpResponse) =>
         {
-            if (rawObject is HttpResponse httpResponse)
-            {
-                activity.SetTag("responseLength", httpResponse.ContentLength);
-            }
-        }
+            activity.SetTag("responseLength", httpResponse.ContentLength);
+        };
+        o.EnrichWithException = (activity, exception) =>
+        {
+            activity.SetTag("exceptionType", exception.GetType().ToString());
+        };
     })
 });
-```
+
 
 [Processor](../../docs/trace/extending-the-sdk/README.md#processor),
 is the general extensibility point to add additional properties to any activity.

diff --git a/test/OpenTelemetry.Instrumentation.AspNetCore.Tests/BasicTests.cs b/test/OpenTelemetry.Instrumentation.AspNetCore.Tests/BasicTests.cs
@@ -105,7 +105,8 @@ void ConfigureTestServices(IServiceCollection services)
                     {
                         if (shouldEnrich)
                         {
-                            options.Enrich = ActivityEnrichment;
+                            options.EnrichWithHttpRequest = (activity, request) => { activity.SetTag("enrichedOnStart", "yes"); };
+                            options.EnrichWithHttpResponse = (activity, response) => { activity.SetTag("enrichedOnStop", "yes"); };
                         }
                     })
                     .AddInMemoryExporter(exportedItems)
@@ -132,7 +133,8 @@ void ConfigureTestServices(IServiceCollection services)
 
             if (shouldEnrich)
             {
-                Assert.NotEmpty(activity.Tags.Where(tag => tag.Key == "enriched" && tag.Value == "yes"));
+                Assert.NotEmpty(activity.Tags.Where(tag => tag.Key == "enrichedOnStart" && tag.Value == "yes"));
+                Assert.NotEmpty(activity.Tags.Where(tag => tag.Key == "enrichedOnStop" && tag.Value == "yes"));
             }
 
             ValidateAspNetCoreActivity(activity, "/api/values");
@@ -519,7 +521,8 @@ void ConfigureTestServices(IServiceCollection services)
         public async Task FilterAndEnrichAreOnlyCalledWhenSampled(SamplingDecision samplingDecision, bool shouldFilterBeCalled, bool shouldEnrichBeCalled)
         {
             bool filterCalled = false;
-            bool enrichCalled = false;
+            bool enrichWithHttpRequestCalled = false;
+            bool enrichWithHttpResponseCalled = false;
             void ConfigureTestServices(IServiceCollection services)
             {
                 this.tracerProvider = Sdk.CreateTracerProviderBuilder()
@@ -531,9 +534,13 @@ void ConfigureTestServices(IServiceCollection services)
                             filterCalled = true;
                             return true;
                         };
-                        options.Enrich = (activity, methodName, request) =>
+                        options.EnrichWithHttpRequest = (activity, request) =>
                         {
-                            enrichCalled = true;
+                            enrichWithHttpRequestCalled = true;
+                        };
+                        options.EnrichWithHttpResponse = (activity, request) =>
+                        {
+                            enrichWithHttpResponseCalled = true;
                         };
                     })
                     .Build();
@@ -550,7 +557,8 @@ void ConfigureTestServices(IServiceCollection services)
 
             // Assert
             Assert.Equal(shouldFilterBeCalled, filterCalled);
-            Assert.Equal(shouldEnrichBeCalled, enrichCalled);
+            Assert.Equal(shouldEnrichBeCalled, enrichWithHttpRequestCalled);
+            Assert.Equal(shouldEnrichBeCalled, enrichWithHttpResponseCalled);
         }
 
         [Fact]