[Instrumentation.AspNet] Update semantic conventions for metrics

src/OpenTelemetry.Instrumentation.AspNet/CHANGELOG.md

@@ -8,6 +8,23 @@
   * New overload of `AddAspNetInstrumentation` now accepts a configuration delegate.
   * The `Enrich` can be used to add additional metric attributes.
 
+* HTTP server metrics now follow stable
+  [semantic conventions](https://github.com/open-telemetry/semantic-conventions/blob/v1.23.0/docs/http/http-metrics.md#http-server).
+
+  The `http.request.duration` metric is replaced with `http.server.request.duration`.
+  Note that the unit changes from milliseconds to seconds.
+  ([#1429](https://github.com/open-telemetry/opentelemetry-dotnet-contrib/pull/1429)):
+
+  The following metric attributes has been added:
+
+  * `http.request.method` (previously `http.method`)
+  * `http.response.status_code` (previously `http.status_code`)
+  * `url.scheme` (previously `http.scheme`)
+  * `server.address`
+  * `server.port`
+  * `network.protocol.version` (`1.1`, `2`, `3`)
+  * `http.route`
+
 ## 1.6.0-beta.2
 
 Released 2023-Nov-06

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

@@ -17,7 +17,6 @@
 using System;
 using System.Diagnostics;
 using System.Web;
-using System.Web.Routing;
 using OpenTelemetry.Context.Propagation;
 using OpenTelemetry.Internal;
 using OpenTelemetry.Trace;
@@ -26,8 +25,7 @@ namespace OpenTelemetry.Instrumentation.AspNet.Implementation;
 
 internal sealed class HttpInListener : IDisposable
 {
-    private readonly PropertyFetcher<object> routeFetcher = new("Route");
-    private readonly PropertyFetcher<string> routeTemplateFetcher = new("RouteTemplate");
+    private readonly HttpRequestRouteHelper routeHelper = new();
     private readonly AspNetInstrumentationOptions options;
 
     public HttpInListener(AspNetInstrumentationOptions options)
@@ -133,26 +131,7 @@ private void OnStopActivity(Activity activity, HttpContext context)
                 activity.SetStatus(SpanHelper.ResolveActivityStatusForHttpStatusCode(activity.Kind, response.StatusCode));
             }
 
-            var routeData = context.Request.RequestContext.RouteData;
-
-            string? template = null;
-            if (routeData.Values.TryGetValue("MS_SubRoutes", out object msSubRoutes))
-            {
-                // WebAPI attribute routing flows here. Use reflection to not take a dependency on microsoft.aspnet.webapi.core\[version]\lib\[framework]\System.Web.Http.
-
-                if (msSubRoutes is Array attributeRouting && attributeRouting.Length == 1)
-                {
-                    var subRouteData = attributeRouting.GetValue(0);
-
-                    _ = this.routeFetcher.TryFetch(subRouteData, out var route);
-                    _ = this.routeTemplateFetcher.TryFetch(route, out template);
-                }
-            }
-            else if (routeData.Route is Route route)
-            {
-                // MVC + WebAPI traditional routing & MVC attribute routing flow here.
-                template = route.Url;
-            }
+            var template = this.routeHelper.GetRouteTemplate(context.Request);
 
             if (!string.IsNullOrEmpty(template))
             {

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

@@ -24,12 +24,16 @@ namespace OpenTelemetry.Instrumentation.AspNet.Implementation;
 
 internal sealed class HttpInMetricsListener : IDisposable
 {
+    private readonly HttpRequestRouteHelper routeHelper = new();
     private readonly Histogram<double> httpServerDuration;
     private readonly AspNetMetricsInstrumentationOptions options;
 
     public HttpInMetricsListener(Meter meter, AspNetMetricsInstrumentationOptions options)
     {
-        this.httpServerDuration = meter.CreateHistogram<double>("http.server.duration", "ms", "Measures the duration of inbound HTTP requests.");
+        this.httpServerDuration = meter.CreateHistogram<double>(
+            "http.server.request.duration",
+            unit: "s",
+            description: "Measures the duration of inbound HTTP requests.");
         TelemetryHttpModule.Options.OnRequestStoppedCallback += this.OnStopActivity;
         this.options = options;
     }
@@ -39,17 +43,43 @@ public void Dispose()
         TelemetryHttpModule.Options.OnRequestStoppedCallback -= this.OnStopActivity;
     }
 
+    private static string GetHttpProtocolVersion(HttpRequest request)
+    {
+        var protocol = request.ServerVariables["SERVER_PROTOCOL"];
+        return protocol switch
+        {
+            "HTTP/1.1" => "1.1",
+            "HTTP/2" => "2",
+            "HTTP/3" => "3",
+            _ => protocol,
+        };
+    }
+
     private void OnStopActivity(Activity activity, HttpContext context)
     {
-        // TODO: This is just a minimal set of attributes. See the spec for additional attributes:
-        // https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/metrics/semantic_conventions/http-metrics.md#http-server
+        var request = context.Request;
+        var url = request.Url;
         var tags = new TagList
         {
-            { SemanticConventions.AttributeHttpMethod, context.Request.HttpMethod },
-            { SemanticConventions.AttributeHttpScheme, context.Request.Url.Scheme },
-            { SemanticConventions.AttributeHttpStatusCode, context.Response.StatusCode },
+            { SemanticConventions.AttributeServerAddress, url.Host },
+            { SemanticConventions.AttributeServerPort, url.Port },
+            { SemanticConventions.AttributeHttpRequestMethod, request.HttpMethod },
+            { SemanticConventions.AttributeUrlScheme, url.Scheme },
+            { SemanticConventions.AttributeHttpResponseStatusCode, context.Response.StatusCode },
         };
 
+        var protocolVersion = GetHttpProtocolVersion(request);
+        if (!string.IsNullOrEmpty(protocolVersion))
+        {
+            tags.Add(SemanticConventions.AttributeNetworkProtocolVersion, protocolVersion);
+        }
+
+        var template = this.routeHelper.GetRouteTemplate(request);
+        if (!string.IsNullOrEmpty(template))
+        {
+            tags.Add(SemanticConventions.AttributeHttpRoute, template);
+        }
+
         if (this.options.Enrich is not null)
         {
             try
@@ -62,6 +92,6 @@ private void OnStopActivity(Activity activity, HttpContext context)
             }
         }
 
-        this.httpServerDuration.Record(activity.Duration.TotalMilliseconds, tags);
+        this.httpServerDuration.Record(activity.Duration.TotalSeconds, tags);
     }
 }

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

@@ -0,0 +1,61 @@
+// <copyright file="HttpRequestRouteHelper.cs" company="OpenTelemetry Authors">
+// Copyright The OpenTelemetry Authors
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+// </copyright>
+
+using System;
+using System.Web;
+using System.Web.Routing;
+
+namespace OpenTelemetry.Instrumentation.AspNet.Implementation;
+
+/// <summary>
+/// Helper class for processing http requests.
+/// </summary>
+internal class HttpRequestRouteHelper
+{
+    private readonly PropertyFetcher<object> routeFetcher = new("Route");
+    private readonly PropertyFetcher<string> routeTemplateFetcher = new("RouteTemplate");
+
+    /// <summary>
+    /// Extracts the route template from the <see cref="HttpRequest"/>.
+    /// </summary>
+    /// <param name="request">The <see cref="HttpRequest"/> being processed.</param>
+    /// <returns>The route template or <see langword="null"/>.</returns>
+    internal string? GetRouteTemplate(HttpRequest request)
+    {
+        var routeData = request.RequestContext.RouteData;
+
+        string? template = null;
+        if (routeData.Values.TryGetValue("MS_SubRoutes", out object msSubRoutes))
+        {
+            // WebAPI attribute routing flows here. Use reflection to not take a dependency on microsoft.aspnet.webapi.core\[version]\lib\[framework]\System.Web.Http.
+
+            if (msSubRoutes is Array attributeRouting && attributeRouting.Length == 1)
+            {
+                var subRouteData = attributeRouting.GetValue(0);
+
+                _ = this.routeFetcher.TryFetch(subRouteData, out var route);
+                _ = this.routeTemplateFetcher.TryFetch(route, out template);
+            }
+        }
+        else if (routeData.Route is Route route)
+        {
+            // MVC + WebAPI traditional routing & MVC attribute routing flow here.
+            template = route.Url;
+        }
+
+        return template;
+    }
+}

src/Shared/SemanticConventions.cs

@@ -114,5 +114,12 @@ internal static class SemanticConventions
     public const string AttributeHttpRequestMethod = "http.request.method"; // replaces: "http.method" (AttributeHttpMethod)
     public const string AttributeHttpResponseStatusCode = "http.response.status_code"; // replaces: "http.status_code" (AttributeHttpStatusCode)
     public const string AttributeUrlScheme = "url.scheme"; // replaces: "http.scheme" (AttributeHttpScheme)
+
+    // v1.23.0
+    // https://github.com/open-telemetry/semantic-conventions/blob/v1.23.0/docs/http/http-metrics.md#http-server
+    public const string AttributeNetworkProtocolName = "network.protocol.name"; // replaces: "net.transport" (AttributeNetTransport)
+    public const string AttributeNetworkProtocolVersion = "network.protocol.version"; // replaces: "http.flavor" (AttributeHttpFlavor)
+    public const string AttributeServerAddress = "server.address"; // replaces: "net.host.name" (AttributeNetHostName)
+    public const string AttributeServerPort = "server.port"; // replaces: "net.host.port" (AttributeNetHostPort)
 #pragma warning restore CS1591 // Missing XML comment for publicly visible type or member
 }

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

@@ -29,27 +29,33 @@ namespace OpenTelemetry.Instrumentation.AspNet.Tests;
 public class HttpInMetricsListenerTests
 {
     [Theory]
-    [InlineData("http://localhost/", 0, null, null, "http")]
-    [InlineData("https://localhost/", 0, null, null, "https")]
-    [InlineData("http://localhost/api/value", 0, null, null, "http")]
-    [InlineData("http://localhost/api/value", 1, "{controller}/{action}", null, "http")]
-    [InlineData("http://localhost/api/value", 2, "{controller}/{action}", null, "http")]
-    [InlineData("http://localhost/api/value", 3, "{controller}/{action}", null, "http")]
-    [InlineData("http://localhost/api/value", 4, "{controller}/{action}", null, "http")]
-    [InlineData("http://localhost:8080/api/value", 0, null, null, "http")]
-    [InlineData("http://localhost:8080/api/value", 1, "{controller}/{action}", null, "http")]
-    [InlineData("http://localhost:8080/api/value", 3, "{controller}/{action}", "enrich", "http")]
-    [InlineData("http://localhost:8080/api/value", 3, "{controller}/{action}", "throw", "http")]
-    [InlineData("http://localhost:8080/api/value", 3, "{controller}/{action}", null, "http")]
+    [InlineData("http://localhost/", 0, null, null, "http", "localhost", null, 80, 200)]
+    [InlineData("https://localhost/", 0, null, null, "https", "localhost", null, 443, 200)]
+    [InlineData("http://localhost/api/value", 0, null, null, "http", "localhost", null, 80, 200)]
+    [InlineData("http://localhost/api/value", 1, "{controller}/{action}", null, "http", "localhost", "{controller}/{action}", 80, 200)]
+    [InlineData("http://localhost/api/value", 2, "{controller}/{action}", null, "http", "localhost", "{controller}/{action}", 80, 201)]
+    [InlineData("http://localhost/api/value", 3, "{controller}/{action}", null, "http", "localhost", "{controller}/{action}", 80, 200)]
+    [InlineData("http://localhost/api/value", 4, "{controller}/{action}", null, "http", "localhost", "{controller}/{action}", 80, 200)]
+    [InlineData("http://localhost/api/value", 1, "{controller}/{action}", null, "http", "localhost", "{controller}/{action}", 80, 500)]
+    [InlineData("http://localhost:8080/api/value", 0, null, null, "http", "localhost", null, 8080, 200)]
+    [InlineData("http://localhost:8080/api/value", 1, "{controller}/{action}", null, "http", "localhost", "{controller}/{action}", 8080, 200)]
+    [InlineData("http://localhost:8080/api/value", 3, "{controller}/{action}", "enrich", "http", "localhost", "{controller}/{action}", 8080, 200)]
+    [InlineData("http://localhost:8080/api/value", 3, "{controller}/{action}", "throw", "http", "localhost", "{controller}/{action}", 8080, 200)]
+    [InlineData("http://localhost:8080/api/value", 3, "{controller}/{action}", null, "http", "localhost", "{controller}/{action}", 8080, 200)]
     public void AspNetMetricTagsAreCollectedSuccessfully(
         string url,
         int routeType,
         string routeTemplate,
         string enrichMode,
-        string expectedScheme)
+        string expectedScheme,
+        string expectedHost,
+        string expectedRoute,
+        int? expectedPort,
+        int expectedStatus)
     {
         double duration = 0;
         HttpContext.Current = RouteTestHelper.BuildHttpContext(url, routeType, routeTemplate);
+        HttpContext.Current.Response.StatusCode = expectedStatus;
 
         // This is to enable activity creation
         // as it is created using ActivitySource inside TelemetryHttpModule
@@ -60,7 +66,7 @@ public void AspNetMetricTagsAreCollectedSuccessfully(
                 {
                     if (eventName.Equals("OnStopActivity"))
                     {
-                        duration = activity.Duration.TotalMilliseconds;
+                        duration = activity.Duration.TotalSeconds;
                     }
                 })
             .Build();
@@ -107,12 +113,19 @@ public void AspNetMetricTagsAreCollectedSuccessfully(
         var sum = metricPoint.GetHistogramSum();
 
         Assert.Equal(MetricType.Histogram, exportedItems[0].MetricType);
-        Assert.Equal("http.server.duration", exportedItems[0].Name);
+        Assert.Equal("http.server.request.duration", exportedItems[0].Name);
+        Assert.Equal("s", exportedItems[0].Unit);
         Assert.Equal(1L, count);
         Assert.Equal(duration, sum);
         Assert.True(duration > 0, "Metric duration should be set.");
 
-        var expectedTagCount = 3;
+        var expectedTagCount = 5;
+
+        if (!string.IsNullOrEmpty(expectedRoute))
+        {
+            expectedTagCount++;
+        }
+
         if (enrichMode == "enrich")
         {
             expectedTagCount++;
@@ -130,9 +143,16 @@ public void AspNetMetricTagsAreCollectedSuccessfully(
             ExpectTag("true", "enriched");
         }
 
-        ExpectTag("GET", SemanticConventions.AttributeHttpMethod);
-        ExpectTag(200, SemanticConventions.AttributeHttpStatusCode);
-        ExpectTag(expectedScheme, SemanticConventions.AttributeHttpScheme);
+        // Do not use constants from SemanticConventions here in order to detect mistakes.
+        // https://github.com/open-telemetry/semantic-conventions/blob/v1.23.0/docs/http/http-metrics.md#http-server
+        // Unable to check for "network.protocol.version" because we can't set server variables due to the accessability
+        // of the ServerVariables property.
+        ExpectTag("GET", "http.request.method");
+        ExpectTag(expectedStatus, "http.response.status_code");
+        ExpectTag(expectedRoute, "http.route");
+        ExpectTag(expectedHost, "server.address");
+        ExpectTag(expectedPort, "server.port");
+        ExpectTag(expectedScheme, "url.scheme");
 
         void ExpectTag<T>(T? expected, string tagName)
         {