[AspNetCore Instrumentation] AspNetCoreMetricsInstrumentationOptions …

…follow-ups & nits (#3982)

* Updates to take care of nits.

* Public API fixes.

* CHANGELOG patch.

src/OpenTelemetry.Instrumentation.AspNetCore/.publicApi/net6.0/PublicAPI.Unshipped.txt

@@ -17,7 +17,7 @@ OpenTelemetry.Instrumentation.AspNetCore.AspNetCoreMetricsInstrumentationOptions
 OpenTelemetry.Instrumentation.AspNetCore.AspNetCoreMetricsInstrumentationOptions.AspNetCoreMetricsInstrumentationOptions() -> void
 OpenTelemetry.Instrumentation.AspNetCore.AspNetCoreMetricsInstrumentationOptions.Enrich.get -> OpenTelemetry.Instrumentation.AspNetCore.AspNetCoreMetricsInstrumentationOptions.AspNetCoreMetricEnrichmentFunc
 OpenTelemetry.Instrumentation.AspNetCore.AspNetCoreMetricsInstrumentationOptions.Enrich.set -> void
-OpenTelemetry.Instrumentation.AspNetCore.AspNetCoreMetricsInstrumentationOptions.Filter.get -> System.Func<Microsoft.AspNetCore.Http.HttpContext, bool>
+OpenTelemetry.Instrumentation.AspNetCore.AspNetCoreMetricsInstrumentationOptions.Filter.get -> System.Func<string, Microsoft.AspNetCore.Http.HttpContext, bool>
 OpenTelemetry.Instrumentation.AspNetCore.AspNetCoreMetricsInstrumentationOptions.Filter.set -> void
 OpenTelemetry.Metrics.MeterProviderBuilderExtensions
 OpenTelemetry.Trace.TracerProviderBuilderExtensions

src/OpenTelemetry.Instrumentation.AspNetCore/.publicApi/net7.0/PublicAPI.Unshipped.txt

@@ -17,7 +17,7 @@ OpenTelemetry.Instrumentation.AspNetCore.AspNetCoreMetricsInstrumentationOptions
 OpenTelemetry.Instrumentation.AspNetCore.AspNetCoreMetricsInstrumentationOptions.AspNetCoreMetricsInstrumentationOptions() -> void
 OpenTelemetry.Instrumentation.AspNetCore.AspNetCoreMetricsInstrumentationOptions.Enrich.get -> OpenTelemetry.Instrumentation.AspNetCore.AspNetCoreMetricsInstrumentationOptions.AspNetCoreMetricEnrichmentFunc
 OpenTelemetry.Instrumentation.AspNetCore.AspNetCoreMetricsInstrumentationOptions.Enrich.set -> void
-OpenTelemetry.Instrumentation.AspNetCore.AspNetCoreMetricsInstrumentationOptions.Filter.get -> System.Func<Microsoft.AspNetCore.Http.HttpContext, bool>
+OpenTelemetry.Instrumentation.AspNetCore.AspNetCoreMetricsInstrumentationOptions.Filter.get -> System.Func<string, Microsoft.AspNetCore.Http.HttpContext, bool>
 OpenTelemetry.Instrumentation.AspNetCore.AspNetCoreMetricsInstrumentationOptions.Filter.set -> void
 OpenTelemetry.Metrics.MeterProviderBuilderExtensions
 OpenTelemetry.Trace.TracerProviderBuilderExtensions

src/OpenTelemetry.Instrumentation.AspNetCore/.publicApi/netstandard2.0/PublicAPI.Unshipped.txt

@@ -15,7 +15,7 @@ OpenTelemetry.Instrumentation.AspNetCore.AspNetCoreMetricsInstrumentationOptions
 OpenTelemetry.Instrumentation.AspNetCore.AspNetCoreMetricsInstrumentationOptions.AspNetCoreMetricsInstrumentationOptions() -> void
 OpenTelemetry.Instrumentation.AspNetCore.AspNetCoreMetricsInstrumentationOptions.Enrich.get -> OpenTelemetry.Instrumentation.AspNetCore.AspNetCoreMetricsInstrumentationOptions.AspNetCoreMetricEnrichmentFunc
 OpenTelemetry.Instrumentation.AspNetCore.AspNetCoreMetricsInstrumentationOptions.Enrich.set -> void
-OpenTelemetry.Instrumentation.AspNetCore.AspNetCoreMetricsInstrumentationOptions.Filter.get -> System.Func<Microsoft.AspNetCore.Http.HttpContext, bool>
+OpenTelemetry.Instrumentation.AspNetCore.AspNetCoreMetricsInstrumentationOptions.Filter.get -> System.Func<string, Microsoft.AspNetCore.Http.HttpContext, bool>
 OpenTelemetry.Instrumentation.AspNetCore.AspNetCoreMetricsInstrumentationOptions.Filter.set -> void
 OpenTelemetry.Metrics.MeterProviderBuilderExtensions
 OpenTelemetry.Trace.TracerProviderBuilderExtensions

src/OpenTelemetry.Instrumentation.AspNetCore/.publicApi/netstandard2.1/PublicAPI.Unshipped.txt

@@ -17,7 +17,7 @@ OpenTelemetry.Instrumentation.AspNetCore.AspNetCoreMetricsInstrumentationOptions
 OpenTelemetry.Instrumentation.AspNetCore.AspNetCoreMetricsInstrumentationOptions.AspNetCoreMetricsInstrumentationOptions() -> void
 OpenTelemetry.Instrumentation.AspNetCore.AspNetCoreMetricsInstrumentationOptions.Enrich.get -> OpenTelemetry.Instrumentation.AspNetCore.AspNetCoreMetricsInstrumentationOptions.AspNetCoreMetricEnrichmentFunc
 OpenTelemetry.Instrumentation.AspNetCore.AspNetCoreMetricsInstrumentationOptions.Enrich.set -> void
-OpenTelemetry.Instrumentation.AspNetCore.AspNetCoreMetricsInstrumentationOptions.Filter.get -> System.Func<Microsoft.AspNetCore.Http.HttpContext, bool>
+OpenTelemetry.Instrumentation.AspNetCore.AspNetCoreMetricsInstrumentationOptions.Filter.get -> System.Func<string, Microsoft.AspNetCore.Http.HttpContext, bool>
 OpenTelemetry.Instrumentation.AspNetCore.AspNetCoreMetricsInstrumentationOptions.Filter.set -> void
 OpenTelemetry.Metrics.MeterProviderBuilderExtensions
 OpenTelemetry.Trace.TracerProviderBuilderExtensions

src/OpenTelemetry.Instrumentation.AspNetCore/AspNetCoreInstrumentationOptions.cs

@@ -25,11 +25,21 @@ namespace OpenTelemetry.Instrumentation.AspNetCore
     public class AspNetCoreInstrumentationOptions
     {
         /// <summary>
-        /// Gets or sets a Filter function that determines whether or not to collect telemetry about requests on a per request basis.
-        /// The Filter gets the HttpContext, and should return a boolean.
-        /// If Filter returns true, the request is collected.
-        /// If Filter returns false or throw exception, the request is filtered out.
+        /// Gets or sets a filter function that determines whether or not to
+        /// collect telemetry on a per request basis.
         /// </summary>
+        /// <remarks>
+        /// Notes:
+        /// <list type="bullet">
+        /// <item>The return value for the filter function is interpreted as:
+        /// <list type="bullet">
+        /// <item>If filter returns <see langword="true" />, the request is
+        /// collected.</item>
+        /// <item>If filter returns <see langword="false" /> or throws an
+        /// exception the request is NOT collected.</item>
+        /// </list></item>
+        /// </list>
+        /// </remarks>
         public Func<HttpContext, bool> Filter { get; set; }
 
         /// <summary>

src/OpenTelemetry.Instrumentation.AspNetCore/AspNetCoreMetricsInstrumentationOptions.cs

@@ -27,17 +27,30 @@ public class AspNetCoreMetricsInstrumentationOptions
         /// <summary>
         /// Delegate for enrichment of recorded metric with additional tags.
         /// </summary>
+        /// <param name="name">The name of the metric being enriched.</param>
         /// <param name="context"><see cref="HttpContext"/>: the HttpContext object. Both Request and Response are available.</param>
         /// <param name="tags"><see cref="TagList"/>: List of current tags. You can add additional tags to this list. </param>
-        public delegate void AspNetCoreMetricEnrichmentFunc(HttpContext context, ref TagList tags);
+        public delegate void AspNetCoreMetricEnrichmentFunc(string name, HttpContext context, ref TagList tags);
 
         /// <summary>
-        /// Gets or sets a Filter function that determines whether or not to collect telemetry about requests on a per request basis.
-        /// The Filter gets the HttpContext, and should return a boolean.
-        /// If Filter returns true, the request is collected.
-        /// If Filter returns false or throw exception, the request is filtered out.
+        /// Gets or sets a filter function that determines whether or not to
+        /// collect telemetry on a per request basis.
         /// </summary>
-        public Func<HttpContext, bool> Filter { get; set; }
+        /// <remarks>
+        /// Notes:
+        /// <list type="bullet">
+        /// <item>The first parameter is the name of the metric being
+        /// filtered.</item>
+        /// <item>The return value for the filter function is interpreted as:
+        /// <list type="bullet">
+        /// <item>If filter returns <see langword="true" />, the request is
+        /// collected.</item>
+        /// <item>If filter returns <see langword="false" /> or throws an
+        /// exception the request is NOT collected.</item>
+        /// </list></item>
+        /// </list>
+        /// </remarks>
+        public Func<string, HttpContext, bool> Filter { get; set; }
 
         /// <summary>
         /// Gets or sets an function to enrich a recorded metric with additional custom tags.

src/OpenTelemetry.Instrumentation.AspNetCore/CHANGELOG.md

@@ -18,10 +18,12 @@
   * `http.host` will no longer be populated on activity. `net.host.name` and
   `net.host.port` attributes will be populated instead.
   ([#3858](https://github.com/open-telemetry/opentelemetry-dotnet/pull/3858))
-  * Extension method `AddAspNetCoreInstrumentation` on `MeterProviderBuilder` now
+
+* Extension method `AddAspNetCoreInstrumentation` on `MeterProviderBuilder` now
   supports `AspNetCoreMetricsInstrumentationOptions`. This option class exposes
-   configuration properties for metric filtering and tag enrichment.
-  ([#3948](https://github.com/open-telemetry/opentelemetry-dotnet/pull/3948))
+  configuration properties for metric filtering and tag enrichment.
+  ([#3948](https://github.com/open-telemetry/opentelemetry-dotnet/pull/3948),
+  [#3982](https://github.com/open-telemetry/opentelemetry-dotnet/pull/3982))
 
 ## 1.0.0-rc9.9
 

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

@@ -42,7 +42,7 @@ public void NullPayload(string handlerName, string eventName)
             this.WriteEvent(1, handlerName, eventName);
         }
 
-        [Event(2, Message = "Request is filtered out.", Level = EventLevel.Verbose)]
+        [Event(2, Message = "Request is filtered out. EventName: '{0}'.", Level = EventLevel.Verbose)]
         public void RequestIsFilteredOut(string eventName)
         {
             this.WriteEvent(2, eventName);

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

@@ -26,6 +26,7 @@ namespace OpenTelemetry.Instrumentation.AspNetCore.Implementation
 {
     internal sealed class HttpInMetricsListener : ListenerHandler
     {
+        private const string HttpServerDurationMetricName = "http.server.duration";
         private const string OnStopEvent = "Microsoft.AspNetCore.Hosting.HttpRequestIn.Stop";
 
         private readonly Meter meter;
@@ -37,7 +38,7 @@ internal HttpInMetricsListener(string name, Meter meter, AspNetCoreMetricsInstru
         {
             this.meter = meter;
             this.options = options;
-            this.httpServerDuration = meter.CreateHistogram<double>("http.server.duration", "ms", "measures the duration of the inbound HTTP request");
+            this.httpServerDuration = meter.CreateHistogram<double>(HttpServerDurationMetricName, "ms", "measures the duration of the inbound HTTP request");
         }
 
         public override void OnEventWritten(string name, object payload)
@@ -53,9 +54,9 @@ public override void OnEventWritten(string name, object payload)
 
                 try
                 {
-                    if (this.options.Filter?.Invoke(context) == false)
+                    if (this.options.Filter?.Invoke(HttpServerDurationMetricName, context) == false)
                     {
-                        AspNetCoreInstrumentationEventSource.Log.RequestIsFilteredOut(Activity.Current.OperationName);
+                        AspNetCoreInstrumentationEventSource.Log.RequestIsFilteredOut(HttpServerDurationMetricName);
                         return;
                     }
                 }
@@ -100,7 +101,7 @@ public override void OnEventWritten(string name, object payload)
                 {
                     try
                     {
-                        this.options.Enrich(context, ref tags);
+                        this.options.Enrich(HttpServerDurationMetricName, context, ref tags);
                     }
                     catch (Exception ex)
                     {

src/OpenTelemetry.Instrumentation.Http/HttpClientInstrumentationOptions.cs

@@ -31,7 +31,7 @@ public class HttpClientInstrumentationOptions
     {
         /// <summary>
         /// Gets or sets a filter function that determines whether or not to
-        /// collect telemetry about requests on a per request basis.
+        /// collect telemetry on a per request basis.
         /// </summary>
         /// <remarks>
         /// <para><b>FilterHttpRequestMessage is only executed on .NET and .NET
@@ -83,7 +83,7 @@ public class HttpClientInstrumentationOptions
 
         /// <summary>
         /// Gets or sets a filter function that determines whether or not to
-        /// collect telemetry about requests on a per request basis.
+        /// collect telemetry on a per request basis.
         /// </summary>
         /// <remarks>
         /// <para><b>FilterHttpWebRequest is only executed on .NET Framework

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

@@ -98,7 +98,7 @@ public async Task MetricNotCollectedWhenFilterIsApplied()
             void ConfigureTestServices(IServiceCollection services)
             {
                 this.meterProvider = Sdk.CreateMeterProviderBuilder()
-                    .AddAspNetCoreInstrumentation(opt => opt.Filter = (ctx) => ctx.Request.Path != "/api/values/2")
+                    .AddAspNetCoreInstrumentation(opt => opt.Filter = (name, ctx) => ctx.Request.Path != "/api/values/2")
                     .AddInMemoryExporter(metricItems)
                     .Build();
             }
@@ -150,7 +150,7 @@ public async Task MetricEnrichedWithCustomTags()
             void ConfigureTestServices(IServiceCollection services)
             {
                 this.meterProvider = Sdk.CreateMeterProviderBuilder()
-                    .AddAspNetCoreInstrumentation(opt => opt.Enrich = (HttpContext _, ref TagList tags) =>
+                    .AddAspNetCoreInstrumentation(opt => opt.Enrich = (string _, HttpContext _, ref TagList tags) =>
                     {
                         foreach (var keyValuePair in tagsToAdd)
                         {