From c13bcf23654d03518a26bc2cca8a1a964a1a6e03 Mon Sep 17 00:00:00 2001 From: Vishwesh Bankwar Date: Mon, 30 Jan 2023 21:31:34 -0800 Subject: [PATCH 1/3] Update enrich callbacks --- .../.publicApi/net462/PublicAPI.Unshipped.txt | 8 +++- .../AspNetInstrumentationOptions.cs | 24 ++++++++++-- .../CHANGELOG.md | 10 +++++ .../Implementation/HttpInListener.cs | 6 +-- .../README.md | 37 +++++++++---------- .../HttpInListenerTests.cs | 8 +++- .../HttpInMetricsListenerTests.cs | 9 ++--- 7 files changed, 66 insertions(+), 36 deletions(-) diff --git a/src/OpenTelemetry.Instrumentation.AspNet/.publicApi/net462/PublicAPI.Unshipped.txt b/src/OpenTelemetry.Instrumentation.AspNet/.publicApi/net462/PublicAPI.Unshipped.txt index a9ba7f916d..b6b7c1a5c7 100644 --- a/src/OpenTelemetry.Instrumentation.AspNet/.publicApi/net462/PublicAPI.Unshipped.txt +++ b/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 -OpenTelemetry.Instrumentation.AspNet.AspNetInstrumentationOptions.Enrich.set -> void +OpenTelemetry.Instrumentation.AspNet.AspNetInstrumentationOptions.EnrichWithException.get -> System.Action +OpenTelemetry.Instrumentation.AspNet.AspNetInstrumentationOptions.EnrichWithException.set -> void +OpenTelemetry.Instrumentation.AspNet.AspNetInstrumentationOptions.EnrichWithHttpRequest.get -> System.Action +OpenTelemetry.Instrumentation.AspNet.AspNetInstrumentationOptions.EnrichWithHttpRequest.set -> void +OpenTelemetry.Instrumentation.AspNet.AspNetInstrumentationOptions.EnrichWithHttpResponse.get -> System.Action +OpenTelemetry.Instrumentation.AspNet.AspNetInstrumentationOptions.EnrichWithHttpResponse.set -> void OpenTelemetry.Instrumentation.AspNet.AspNetInstrumentationOptions.Filter.get -> System.Func OpenTelemetry.Instrumentation.AspNet.AspNetInstrumentationOptions.Filter.set -> void OpenTelemetry.Instrumentation.AspNet.AspNetInstrumentationOptions.RecordException.get -> bool diff --git a/src/OpenTelemetry.Instrumentation.AspNet/AspNetInstrumentationOptions.cs b/src/OpenTelemetry.Instrumentation.AspNet/AspNetInstrumentationOptions.cs index 50ded07c2c..6e0b9b4aa9 100644 --- a/src/OpenTelemetry.Instrumentation.AspNet/AspNetInstrumentationOptions.cs +++ b/src/OpenTelemetry.Instrumentation.AspNet/AspNetInstrumentationOptions.cs @@ -46,11 +46,27 @@ public class AspNetInstrumentationOptions /// /// /// : the activity being enriched. - /// string: the name of the event. - /// 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. + /// : the HttpRequest object from which additional information can be extracted to enrich the activity. /// - public Action Enrich { get; set; } + public Action EnrichWithHttpRequest { get; set; } + + /// + /// Gets or sets an action to enrich an Activity. + /// + /// + /// : the activity being enriched. + /// : the HttpResponse object from which additional information can be extracted to enrich the activity. + /// + public Action EnrichWithHttpResponse { get; set; } + + /// + /// Gets or sets an action to enrich an Activity. + /// + /// + /// : the activity being enriched. + /// : the Exception object from which additional information can be extracted to enrich the activity. + /// + public Action EnrichWithException { get; set; } /// /// Gets or sets a value indicating whether the exception will be recorded as ActivityEvent or not. diff --git a/src/OpenTelemetry.Instrumentation.AspNet/CHANGELOG.md b/src/OpenTelemetry.Instrumentation.AspNet/CHANGELOG.md index 2e146bf12e..790468e092 100644 --- a/src/OpenTelemetry.Instrumentation.AspNet/CHANGELOG.md +++ b/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. + ## 1.0.0-rc9.7 Released 2022-Nov-28 diff --git a/src/OpenTelemetry.Instrumentation.AspNet/Implementation/HttpInListener.cs b/src/OpenTelemetry.Instrumentation.AspNet/Implementation/HttpInListener.cs index a8c1ddb9a4..288bb1e460 100644 --- a/src/OpenTelemetry.Instrumentation.AspNet/Implementation/HttpInListener.cs +++ b/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) { diff --git a/src/OpenTelemetry.Instrumentation.AspNet/README.md b/src/OpenTelemetry.Instrumentation.AspNet/README.md index aa73d15b14..af2cd7443a 100644 --- a/src/OpenTelemetry.Instrumentation.AspNet/README.md +++ b/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(); ``` diff --git a/test/OpenTelemetry.Instrumentation.AspNet.Tests/HttpInListenerTests.cs b/test/OpenTelemetry.Instrumentation.AspNet.Tests/HttpInListenerTests.cs index 70d477c627..10fea3fc33 100644 --- a/test/OpenTelemetry.Instrumentation.AspNet.Tests/HttpInListenerTests.cs +++ b/test/OpenTelemetry.Instrumentation.AspNet.Tests/HttpInListenerTests.cs @@ -137,7 +137,13 @@ public void AspNetRequestsAreCollectedSuccessfully( return httpContext.Request.Path != filter; }; - options.Enrich = GetEnrichmentAction(setStatusToErrorInEnrich ? ActivityStatusCode.Error : default); + options.EnrichWithHttpResponse = (activity, _) => + { + if (setStatusToErrorInEnrich) + { + activity.SetStatus(ActivityStatusCode.Error); + } + }; options.RecordException = recordException; }) diff --git a/test/OpenTelemetry.Instrumentation.AspNet.Tests/HttpInMetricsListenerTests.cs b/test/OpenTelemetry.Instrumentation.AspNet.Tests/HttpInMetricsListenerTests.cs index 8ba1903aa2..2304da51b1 100644 --- a/test/OpenTelemetry.Instrumentation.AspNet.Tests/HttpInMetricsListenerTests.cs +++ b/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(); From cab28dc324a65a21c0bd1672579a5513f6194f0c Mon Sep 17 00:00:00 2001 From: Vishwesh Bankwar Date: Mon, 30 Jan 2023 21:33:59 -0800 Subject: [PATCH 2/3] todo --- .../HttpInListenerTests.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/test/OpenTelemetry.Instrumentation.AspNet.Tests/HttpInListenerTests.cs b/test/OpenTelemetry.Instrumentation.AspNet.Tests/HttpInListenerTests.cs index 10fea3fc33..55b5cfc996 100644 --- a/test/OpenTelemetry.Instrumentation.AspNet.Tests/HttpInListenerTests.cs +++ b/test/OpenTelemetry.Instrumentation.AspNet.Tests/HttpInListenerTests.cs @@ -137,6 +137,7 @@ public void AspNetRequestsAreCollectedSuccessfully( return httpContext.Request.Path != filter; }; + // TODO: update to test activity enrichment for other enrich options. options.EnrichWithHttpResponse = (activity, _) => { if (setStatusToErrorInEnrich) From b6e74e9789844733b9364442d0589d31a820e9bb Mon Sep 17 00:00:00 2001 From: Vishwesh Bankwar Date: Mon, 30 Jan 2023 21:52:57 -0800 Subject: [PATCH 3/3] Update src/OpenTelemetry.Instrumentation.AspNet/CHANGELOG.md MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Piotr Kiełkowicz --- src/OpenTelemetry.Instrumentation.AspNet/CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/src/OpenTelemetry.Instrumentation.AspNet/CHANGELOG.md b/src/OpenTelemetry.Instrumentation.AspNet/CHANGELOG.md index 790468e092..3c393f7394 100644 --- a/src/OpenTelemetry.Instrumentation.AspNet/CHANGELOG.md +++ b/src/OpenTelemetry.Instrumentation.AspNet/CHANGELOG.md @@ -11,6 +11,7 @@ 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)) ## 1.0.0-rc9.7