Add support for .NET8.0 HttpClient metrics

src/OpenTelemetry.Instrumentation.Http/CHANGELOG.md

@@ -34,6 +34,31 @@
   `http.client.request.duration` metrics on .NET Framework for `HttpWebRequest`.
   ([#4870](https://github.com/open-telemetry/opentelemetry-dotnet/pull/4870))
 
+* Following `HttpClient` metrics will now be enabled by default when targeting
+  `.NET8.0` framework or newer.
+
+  * **Meter** : `System.Net.Http`
+    * `http.client.request.duration`
+    * `http.client.active_requests`
+    * `http.client.open_connections`
+    * `http.client.connection.duration`
+    * `http.client.request.time_in_queue`
+
+  * **Meter** : `System.Net.NameResolution`
+    * `dns.lookups.duration`
+
+  **NOTES**:
+  * When targeting `.NET8.0` framework, `http.client.request.duration` metric
+    will only follow
+    [v1.22.0](https://github.com/open-telemetry/semantic-conventions/blob/v1.22.0/docs/http/http-metrics.md#metric-httpclientrequestduration)
+    semantic conventions specification. Ability to switch behavior to older
+    conventions using  `OTEL_SEMCONV_STABILITY_OPT_IN` environment variable is
+    not available.
+  * Users can opt-out of metrics that are not required using
+    [views](https://github.com/open-telemetry/opentelemetry-dotnet/tree/main/docs/metrics/customizing-the-sdk#drop-an-instrument).
+
+  ([#4931](https://github.com/open-telemetry/opentelemetry-dotnet/pull/4931))
+
 ## 1.5.1-beta.1
 
 Released 2023-Jul-20

src/OpenTelemetry.Instrumentation.Http/MeterProviderBuilderExtensions.cs

@@ -14,10 +14,13 @@
 // limitations under the License.
 // </copyright>
 
+#if !NET8_0_OR_GREATER
 using Microsoft.Extensions.DependencyInjection;
 using Microsoft.Extensions.Options;
 using OpenTelemetry.Instrumentation.Http;
 using OpenTelemetry.Instrumentation.Http.Implementation;
+#endif
+
 using OpenTelemetry.Internal;
 
 namespace OpenTelemetry.Metrics;
@@ -37,6 +40,11 @@ public static MeterProviderBuilder AddHttpClientInstrumentation(
     {
         Guard.ThrowIfNull(builder);
 
+#if NET8_0_OR_GREATER
+        return builder
+        .AddMeter("System.Net.Http")
+        .AddMeter("System.Net.NameResolution");
+#else
         // Note: Warm-up the status code mapping.
         _ = TelemetryHelper.BoxedStatusCodes;
 
@@ -45,12 +53,6 @@ public static MeterProviderBuilder AddHttpClientInstrumentation(
             services.RegisterOptionsFactory(configuration => new HttpClientMetricInstrumentationOptions(configuration));
         });
 
-        // TODO: Handle HttpClientMetricInstrumentationOptions
-        //   SetHttpFlavor - seems like this would be handled by views
-        //   Filter - makes sense for metric instrumentation
-        //   Enrich - do we want a similar kind of functionality for metrics?
-        //   RecordException - probably doesn't make sense for metric instrumentation
-
 #if NETFRAMEWORK
         builder.AddMeter(HttpWebRequestActivitySource.MeterName);
 
@@ -70,5 +72,6 @@ public static MeterProviderBuilder AddHttpClientInstrumentation(
             sp.GetRequiredService<IOptionsMonitor<HttpClientMetricInstrumentationOptions>>().Get(Options.DefaultName)));
 #endif
         return builder;
+#endif
     }
 }

test/OpenTelemetry.Instrumentation.Http.Tests/HttpClientTests.cs

@@ -18,8 +18,10 @@
 #if NETFRAMEWORK
 using System.Net.Http;
 #endif
+#if !NET8_0_OR_GREATER
 using System.Reflection;
 using System.Text.Json;
+#endif
 using Microsoft.Extensions.Configuration;
 using Microsoft.Extensions.DependencyInjection;
 using OpenTelemetry.Metrics;
@@ -33,6 +35,7 @@ public partial class HttpClientTests
 {
     public static readonly IEnumerable<object[]> TestData = HttpTestData.ReadTestCases();
 
+#if !NET8_0_OR_GREATER
     [Theory]
     [MemberData(nameof(TestData))]
     public async Task HttpOutCallsAreCollectedSuccessfullyTracesAndMetricsOldSemanticConventionsAsync(HttpTestData.HttpOutTestCase tc)
@@ -71,29 +74,30 @@ await HttpOutCallsAreCollectedSuccessfullyBodyAsync(
             enableMetrics: true,
             semanticConvention: HttpSemanticConvention.Dupe).ConfigureAwait(false);
     }
+#endif
 
     [Theory]
     [MemberData(nameof(TestData))]
-    public async Task HttpOutCallsAreCollectedSuccessfullyTracesOnlyAsync(HttpTestData.HttpOutTestCase tc)
+    public async Task HttpOutCallsAreCollectedSuccessfullyMetricsOnlyAsync(HttpTestData.HttpOutTestCase tc)
     {
         await HttpOutCallsAreCollectedSuccessfullyBodyAsync(
             this.host,
             this.port,
             tc,
-            enableTracing: true,
-            enableMetrics: false).ConfigureAwait(false);
+            enableTracing: false,
+            enableMetrics: true).ConfigureAwait(false);
     }
 
     [Theory]
     [MemberData(nameof(TestData))]
-    public async Task HttpOutCallsAreCollectedSuccessfullyMetricsOnlyAsync(HttpTestData.HttpOutTestCase tc)
+    public async Task HttpOutCallsAreCollectedSuccessfullyTracesOnlyAsync(HttpTestData.HttpOutTestCase tc)
     {
         await HttpOutCallsAreCollectedSuccessfullyBodyAsync(
             this.host,
             this.port,
             tc,
-            enableTracing: false,
-            enableMetrics: true).ConfigureAwait(false);
+            enableTracing: true,
+            enableMetrics: false).ConfigureAwait(false);
     }
 
     [Theory]
@@ -108,6 +112,7 @@ await HttpOutCallsAreCollectedSuccessfullyBodyAsync(
             enableMetrics: false).ConfigureAwait(false);
     }
 
+#if !NET8_0_OR_GREATER
     [Fact]
     public async Task DebugIndividualTestAsync()
     {
@@ -140,6 +145,7 @@ public async Task DebugIndividualTestAsync()
         var t = (Task)this.GetType().InvokeMember(nameof(this.HttpOutCallsAreCollectedSuccessfullyTracesAndMetricsOldSemanticConventionsAsync), BindingFlags.InvokeMethod, null, this, HttpTestData.GetArgumentsFromTestCaseObject(input).First());
         await t.ConfigureAwait(false);
     }
+#endif
 
     [Fact]
     public async Task CheckEnrichmentWhenSampling()
@@ -148,6 +154,65 @@ public async Task CheckEnrichmentWhenSampling()
         await CheckEnrichment(new AlwaysOnSampler(), true, this.url).ConfigureAwait(false);
     }
 
+#if NET8_0_OR_GREATER
+    [Theory]
+    [MemberData(nameof(TestData))]
+    public async Task ValidateNet8MetricsAsync(HttpTestData.HttpOutTestCase tc)
+    {
+        var metrics = new List<Metric>();
+        var meterProvider = Sdk.CreateMeterProviderBuilder()
+            .AddHttpClientInstrumentation()
+            .AddInMemoryExporter(metrics)
+            .Build();
+
+        var testUrl = HttpTestData.NormalizeValues(tc.Url, this.host, this.port);
+
+        try
+        {
+            using var c = new HttpClient();
+            using var request = new HttpRequestMessage
+            {
+                RequestUri = new Uri(testUrl),
+                Method = new HttpMethod(tc.Method),
+            };
+
+            request.Headers.Add("contextRequired", "false");
+            request.Headers.Add("responseCode", (tc.ResponseCode == 0 ? 200 : tc.ResponseCode).ToString());
+            await c.SendAsync(request).ConfigureAwait(false);
+        }
+        catch (Exception)
+        {
+            // test case can intentionally send request that will result in exception
+        }
+        finally
+        {
+            meterProvider.Dispose();
+        }
+
+        // dns.lookups.duration is a typo
+        // https://github.com/dotnet/runtime/issues/92917
+        var requestMetrics = metrics
+            .Where(metric =>
+            metric.Name == "http.client.request.duration" ||
+            metric.Name == "http.client.active_requests" ||
+            metric.Name == "http.client.request.time_in_queue" ||
+            metric.Name == "http.client.connection.duration" ||
+            metric.Name == "http.client.open_connections" ||
+            metric.Name == "dns.lookups.duration")
+            .ToArray();
+
+        if (tc.ResponseExpected)
+        {
+            Assert.Equal(6, requestMetrics.Count());
+        }
+        else
+        {
+            // http.client.connection.duration and http.client.open_connections will not be emitted.
+            Assert.Equal(4, requestMetrics.Count());
+        }
+    }
+#endif
+
     private static async Task HttpOutCallsAreCollectedSuccessfullyBodyAsync(
         string host,
         int port,
@@ -210,11 +275,6 @@ private static async Task HttpOutCallsAreCollectedSuccessfullyBodyAsync(
             {
                 RequestUri = new Uri(testUrl),
                 Method = new HttpMethod(tc.Method),
-#if NETFRAMEWORK
-                Version = new Version(1, 1),
-#else
-                Version = new Version(2, 0),
-#endif
             };
 
             if (tc.Headers != null)
@@ -351,6 +411,7 @@ private static async Task HttpOutCallsAreCollectedSuccessfullyBodyAsync(
                 Assert.Single(requestMetrics);
             }
 
+#if !NET8_0_OR_GREATER
             if (semanticConvention == null || semanticConvention.Value.HasFlag(HttpSemanticConvention.Old))
             {
                 var metric = requestMetrics.FirstOrDefault(m => m.Name == "http.client.duration");
@@ -419,7 +480,7 @@ private static async Task HttpOutCallsAreCollectedSuccessfullyBodyAsync(
                     expected: new List<double> { 0, 5, 10, 25, 50, 75, 100, 250, 500, 750, 1000, 2500, 5000, 7500, 10000, double.PositiveInfinity },
                     actual: histogramBounds);
             }
-
+#endif
             if (semanticConvention != null && semanticConvention.Value.HasFlag(HttpSemanticConvention.New))
             {
                 var metric = requestMetrics.FirstOrDefault(m => m.Name == "http.client.request.duration");

test/OpenTelemetry.Instrumentation.Http.Tests/HttpTestData.cs

@@ -49,11 +49,7 @@ public static string NormalizeValues(string value, string host, int port)
         return value
             .Replace("{host}", host)
             .Replace("{port}", port.ToString())
-#if NETFRAMEWORK
             .Replace("{flavor}", "1.1");
-#else
-            .Replace("{flavor}", "2.0");
-#endif
     }
 
     public class HttpOutTestCase