[Instrumentation.AspNet] Spans - semantic convention v1.24.0

src/OpenTelemetry.Instrumentation.AspNet/CHANGELOG.md

@@ -8,6 +8,8 @@
 * **Breaking Change**: `server.address` and `server.port` no longer added
   for `http.server.request.duration` metric.
   ([#1606](https://github.com/open-telemetry/opentelemetry-dotnet/pull/1606))
+* **Breaking change** Spans attributes based on [HTTP semantic convention v1.24.0](https://github.com/open-telemetry/semantic-conventions/blob/v1.24.0/docs/http/http-spans.md).
+  ([#1607](https://github.com/open-telemetry/opentelemetry-dotnet-contrib/pull/1607))
 
 ## 1.7.0-beta.2
 

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

@@ -14,6 +14,7 @@ internal sealed class HttpInListener : IDisposable
 {
     private readonly HttpRequestRouteHelper routeHelper = new();
     private readonly AspNetTraceInstrumentationOptions options;
+    private readonly RequestDataHelper requestDataHelper = new();
 
     public HttpInListener(AspNetTraceInstrumentationOptions options)
     {
@@ -35,16 +36,6 @@ public void Dispose()
         TelemetryHttpModule.Options.OnExceptionCallback -= this.OnException;
     }
 
-    /// <summary>
-    /// Gets the OpenTelemetry standard uri tag value for a span based on its request <see cref="Uri"/>.
-    /// </summary>
-    /// <param name="uri"><see cref="Uri"/>.</param>
-    /// <returns>Span uri value.</returns>
-    private static string GetUriTagValueFromRequestUri(Uri uri)
-    {
-        return string.IsNullOrEmpty(uri.UserInfo) ? uri.ToString() : string.Concat(uri.Scheme, Uri.SchemeDelimiter, uri.Authority, uri.PathAndQuery, uri.Fragment);
-    }
-
     private void OnStartActivity(Activity activity, HttpContext context)
     {
         if (activity.IsAllDataRequested)
@@ -76,23 +67,41 @@ private void OnStartActivity(Activity activity, HttpContext context)
             var request = context.Request;
             var requestValues = request.Unvalidated;
 
-            // see the spec https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/trace/semantic_conventions/http.md
+            // see the spec https://github.com/open-telemetry/semantic-conventions/blob/v1.24.0/docs/http/http-spans.md
             var path = requestValues.Path;
             activity.DisplayName = path;
 
-            if (request.Url.Port == 80 || request.Url.Port == 443)
+            var url = request.Url;
+            activity.SetTag(SemanticConventions.AttributeServerAddress, url.Host);
+            activity.SetTag(SemanticConventions.AttributeServerPort, url.Port);
+            activity.SetTag(SemanticConventions.AttributeUrlScheme, url.Scheme);
+
+            var originalHttpMethod = request.HttpMethod;
+            this.requestDataHelper.SetHttpMethodTag(activity, originalHttpMethod);
+
+            var protocolVersion = RequestDataHelper.GetHttpProtocolVersion(request);
+            if (!string.IsNullOrEmpty(protocolVersion))
             {
-                activity.SetTag(SemanticConventions.AttributeHttpHost, request.Url.Host);
+                activity.SetTag(SemanticConventions.AttributeNetworkProtocolVersion, protocolVersion);
             }
-            else
+
+            activity.SetTag(SemanticConventions.AttributeUrlPath, path);
+
+            // TODO url.query should be sanitized
+            var query = url.Query;
+            if (!string.IsNullOrEmpty(query))
             {
-                activity.SetTag(SemanticConventions.AttributeHttpHost, request.Url.Host + ":" + request.Url.Port);
+                if (query.StartsWith("?", StringComparison.InvariantCulture))
+                {
+                    activity.SetTag(SemanticConventions.AttributeUrlQuery, query.Substring(1));
+                }
+                else
+                {
+                    activity.SetTag(SemanticConventions.AttributeUrlQuery, query);
+                }
             }
 
-            activity.SetTag(SemanticConventions.AttributeHttpMethod, request.HttpMethod);
-            activity.SetTag(SemanticConventions.AttributeHttpTarget, path);
-            activity.SetTag(SemanticConventions.AttributeHttpUserAgent, request.UserAgent);
-            activity.SetTag(SemanticConventions.AttributeHttpUrl, GetUriTagValueFromRequestUri(request.Url));
+            activity.SetTag(SemanticConventions.AttributeUserAgentOriginal, request.UserAgent);
 
             try
             {
@@ -111,7 +120,7 @@ private void OnStopActivity(Activity activity, HttpContext context)
         {
             var response = context.Response;
 
-            activity.SetTag(SemanticConventions.AttributeHttpStatusCode, response.StatusCode);
+            activity.SetTag(SemanticConventions.AttributeHttpResponseStatusCode, response.StatusCode);
 
             if (activity.Status == ActivityStatusCode.Unset)
             {
@@ -148,6 +157,7 @@ private void OnException(Activity activity, HttpContext context, Exception excep
             }
 
             activity.SetStatus(ActivityStatusCode.Error, exception.Message);
+            activity.SetTag(SemanticConventions.AttributeErrorType, exception.GetType().FullName);
 
             try
             {

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

@@ -12,7 +12,7 @@ namespace OpenTelemetry.Instrumentation.AspNet.Implementation;
 internal sealed class HttpInMetricsListener : IDisposable
 {
     private readonly HttpRequestRouteHelper routeHelper = new();
-    private readonly RequestMethodHelper requestMethodHelper = new();
+    private readonly RequestDataHelper requestDataHelper = new();
     private readonly Histogram<double> httpServerDuration;
     private readonly AspNetMetricsInstrumentationOptions options;
 
@@ -31,18 +31,6 @@ 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)
     {
         var request = context.Request;
@@ -59,10 +47,10 @@ private void OnStopActivity(Activity activity, HttpContext context)
             tags.Add(SemanticConventions.AttributeServerPort, url.Port);
         }
 
-        var normalizedMethod = this.requestMethodHelper.GetNormalizedHttpMethod(request.HttpMethod);
+        var normalizedMethod = this.requestDataHelper.GetNormalizedHttpMethod(request.HttpMethod);
         tags.Add(SemanticConventions.AttributeHttpRequestMethod, normalizedMethod);
 
-        var protocolVersion = GetHttpProtocolVersion(request);
+        var protocolVersion = RequestDataHelper.GetHttpProtocolVersion(request);
         if (!string.IsNullOrEmpty(protocolVersion))
         {
             tags.Add(SemanticConventions.AttributeNetworkProtocolVersion, protocolVersion);

src/OpenTelemetry.Instrumentation.AspNet/Implementation/RequestMethodHelper.cs → src/OpenTelemetry.Instrumentation.AspNet/Implementation/RequestDataHelper.cs

@@ -3,11 +3,14 @@
 
 using System;
 using System.Collections.Generic;
+using System.Diagnostics;
 using System.Linq;
+using System.Web;
+using OpenTelemetry.Trace;
 
 namespace OpenTelemetry.Instrumentation.AspNet.Implementation;
 
-internal sealed class RequestMethodHelper
+internal sealed class RequestDataHelper
 {
     private const string KnownHttpMethodsEnvironmentVariable = "OTEL_INSTRUMENTATION_HTTP_KNOWN_METHODS";
 
@@ -20,7 +23,7 @@ internal sealed class RequestMethodHelper
     // List of known HTTP methods as per spec.
     private readonly Dictionary<string, string> knownHttpMethods;
 
-    public RequestMethodHelper()
+    public RequestDataHelper()
     {
         var suppliedKnownMethods = Environment.GetEnvironmentVariable(KnownHttpMethodsEnvironmentVariable)
             ?.Split(SplitChars, StringSplitOptions.RemoveEmptyEntries);
@@ -40,10 +43,37 @@ public RequestMethodHelper()
             };
     }
 
+    public static string GetHttpProtocolVersion(HttpRequest request)
+    {
+        return GetHttpProtocolVersion(request.ServerVariables["SERVER_PROTOCOL"]);
+    }
+
+    public void SetHttpMethodTag(Activity activity, string originalHttpMethod)
+    {
+        var normalizedHttpMethod = this.GetNormalizedHttpMethod(originalHttpMethod);
+        activity.SetTag(SemanticConventions.AttributeHttpRequestMethod, normalizedHttpMethod);
+
+        if (originalHttpMethod != normalizedHttpMethod)
+        {
+            activity.SetTag(SemanticConventions.AttributeHttpRequestMethodOriginal, originalHttpMethod);
+        }
+    }
+
     public string GetNormalizedHttpMethod(string method)
     {
         return this.knownHttpMethods.TryGetValue(method, out var normalizedMethod)
             ? normalizedMethod
             : OtherHttpMethod;
     }
+
+    internal static string GetHttpProtocolVersion(string protocol)
+    {
+        return protocol switch
+        {
+            "HTTP/1.1" => "1.1",
+            "HTTP/2" => "2",
+            "HTTP/3" => "3",
+            _ => protocol,
+        };
+    }
 }

src/Shared/SemanticConventions.cs

@@ -95,17 +95,22 @@ internal static class SemanticConventions
     public const string AttributeExceptionType = "exception.type";
     public const string AttributeExceptionMessage = "exception.message";
     public const string AttributeExceptionStacktrace = "exception.stacktrace";
+    public const string AttributeErrorType = "error.type";
 
     // v1.21.0
     // https://github.com/open-telemetry/semantic-conventions/blob/v1.21.0/docs/http/http-metrics.md#http-server
     public const string AttributeHttpRequestMethod = "http.request.method"; // replaces: "http.method" (AttributeHttpMethod)
+    public const string AttributeHttpRequestMethodOriginal = "http.request.method_original";
     public const string AttributeHttpResponseStatusCode = "http.response.status_code"; // replaces: "http.status_code" (AttributeHttpStatusCode)
     public const string AttributeUrlScheme = "url.scheme"; // replaces: "http.scheme" (AttributeHttpScheme)
+    public const string AttributeUrlPath = "url.path"; // replaces: "http.target" (AttributeHttpTarget)
+    public const string AttributeUrlQuery = "url.query"; // replaces: "http.target" (AttributeHttpTarget)
 
     // v1.23.0
     // https://github.com/open-telemetry/semantic-conventions/blob/v1.23.0/docs/http/http-metrics.md#http-server
     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)
+    public const string AttributeUserAgentOriginal = "user_agent.original"; // replaces: http.user_agent (AttributeHttpUserAgent)
 #pragma warning restore CS1591 // Missing XML comment for publicly visible type or member
 }

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

@@ -20,31 +20,39 @@ namespace OpenTelemetry.Instrumentation.AspNet.Tests;
 public class HttpInListenerTests
 {
     [Theory]
-    [InlineData("http://localhost/", "http://localhost/", 0, null)]
-    [InlineData("http://localhost/", "http://localhost/", 0, null, true)]
-    [InlineData("https://localhost/", "https://localhost/", 0, null)]
-    [InlineData("https://localhost/", "https://user:pass@localhost/", 0, null)] // Test URL sanitization
-    [InlineData("http://localhost:443/", "http://localhost:443/", 0, null)] // Test http over 443
-    [InlineData("https://localhost:80/", "https://localhost:80/", 0, null)] // Test https over 80
-    [InlineData("https://localhost:80/Home/Index.htm?q1=v1&q2=v2#FragmentName", "https://localhost:80/Home/Index.htm?q1=v1&q2=v2#FragmentName", 0, null)] // Test complex URL
-    [InlineData("https://localhost:80/Home/Index.htm?q1=v1&q2=v2#FragmentName", "https://user:password@localhost:80/Home/Index.htm?q1=v1&q2=v2#FragmentName", 0, null)] // Test complex URL sanitization
-    [InlineData("http://localhost:80/Index", "http://localhost:80/Index", 1, "{controller}/{action}/{id}")]
-    [InlineData("https://localhost:443/about_attr_route/10", "https://localhost:443/about_attr_route/10", 2, "about_attr_route/{customerId}")]
-    [InlineData("http://localhost:1880/api/weatherforecast", "http://localhost:1880/api/weatherforecast", 3, "api/{controller}/{id}")]
-    [InlineData("https://localhost:1843/subroute/10", "https://localhost:1843/subroute/10", 4, "subroute/{customerId}")]
-    [InlineData("http://localhost/api/value", "http://localhost/api/value", 0, null, false, "/api/value")] // Request will be filtered
-    [InlineData("http://localhost/api/value", "http://localhost/api/value", 0, null, false, "{ThrowException}")] // Filter user code will throw an exception
-    [InlineData("http://localhost/", "http://localhost/", 0, null, false, null, true)] // Test RecordException option
+    [InlineData("http://localhost/", "http", "/", null, "localhost", 80, "GET", "GET", null, 0, null)]
+    [InlineData("http://localhost/?foo=bar&baz=test", "http", "/", "foo=bar&baz=test", "localhost", 80, "POST", "POST", null, 0, null, true)]
+    [InlineData("https://localhost/", "https", "/", null, "localhost", 443, "NonStandard", "_OTHER", "NonStandard", 0, null)]
+    [InlineData("https://user:pass@localhost/", "https", "/", null, "localhost", 443, "GeT", "GET", "GeT", 0, null)] // Test URL sanitization
+    [InlineData("http://localhost:443/", "http", "/", null, "localhost", 443, "GET", "GET", null, 0, null)] // Test http over 443
+    [InlineData("https://localhost:80/", "https", "/", null, "localhost", 80, "GET", "GET", null, 0, null)] // Test https over 80
+    [InlineData("https://localhost:80/Home/Index.htm?q1=v1&q2=v2#FragmentName", "https", "/Home/Index.htm", "q1=v1&q2=v2", "localhost", 80, "GET", "GET", null, 0, null)] // Test complex URL
+    [InlineData("https://user:password@localhost:80/Home/Index.htm?q1=v1&q2=v2#FragmentName", "https", "/Home/Index.htm", "q1=v1&q2=v2", "localhost", 80, "GET", "GET", null, 0, null)] // Test complex URL sanitization
+    [InlineData("http://localhost:80/Index", "http", "/Index", null, "localhost", 80, "GET", "GET", null, 1, "{controller}/{action}/{id}")]
+    [InlineData("https://localhost:443/about_attr_route/10", "https", "/about_attr_route/10", null, "localhost", 443, "GET", "GET", null, 2, "about_attr_route/{customerId}")]
+    [InlineData("http://localhost:1880/api/weatherforecast", "http", "/api/weatherforecast", null, "localhost", 1880, "GET", "GET", null, 3, "api/{controller}/{id}")]
+    [InlineData("https://localhost:1843/subroute/10", "https", "/subroute/10", null, "localhost", 1843, "GET", "GET", null, 4, "subroute/{customerId}")]
+    [InlineData("http://localhost/api/value", "http", "/api/value", null, "localhost", 80, "GET", "GET", null, 0, null, false, "/api/value")] // Request will be filtered
+    [InlineData("http://localhost/api/value", "http", "/api/value", null, "localhost", 80, "GET", "GET", null, 0, null, false, "{ThrowException}")] // Filter user code will throw an exception
+    [InlineData("http://localhost/", "http", "/", null, "localhost", 80, "GET", "GET", null, 0, null, false, null, true, "System.InvalidOperationException")] // Test RecordException option
     public void AspNetRequestsAreCollectedSuccessfully(
-        string expectedUrl,
         string url,
+        string expectedUrlScheme,
+        string expectedUrlPath,
+        string expectedUrlQuery,
+        string expectedHost,
+        int expectedPort,
+        string requestMethod,
+        string expectedRequestMethod,
+        string? expectedOriginalRequestMethod,
         int routeType,
         string routeTemplate,
         bool setStatusToErrorInEnrich = false,
         string? filter = null,
-        bool recordException = false)
+        bool recordException = false,
+        string? expectedErrorType = null)
     {
-        HttpContext.Current = RouteTestHelper.BuildHttpContext(url, routeType, routeTemplate);
+        HttpContext.Current = RouteTestHelper.BuildHttpContext(url, routeType, routeTemplate, requestMethod);
 
         typeof(HttpRequest).GetField("_wr", BindingFlags.Instance | BindingFlags.NonPublic).SetValue(HttpContext.Current.Request, new TestHttpWorkerRequest());
 
@@ -113,40 +121,20 @@ public void AspNetRequestsAreCollectedSuccessfully(
         Assert.Equal(ActivityKind.Server, span.Kind);
         Assert.True(span.Duration != TimeSpan.Zero);
 
-        Assert.Equal(200, span.GetTagValue(SemanticConventions.AttributeHttpStatusCode));
+        Assert.Equal(200, span.GetTagValue("http.response.status_code"));
 
-        var expectedUri = new Uri(expectedUrl);
-        var actualUrl = span.GetTagValue(SemanticConventions.AttributeHttpUrl);
+        Assert.Equal(expectedHost, span.GetTagValue("server.address"));
+        Assert.Equal(expectedPort, span.GetTagValue("server.port"));
 
-        Assert.Equal(expectedUri.ToString(), actualUrl);
+        Assert.Equal(expectedRequestMethod, span.GetTagValue("http.request.method"));
+        Assert.Equal(expectedOriginalRequestMethod, span.GetTagValue("http.request.method_original"));
+        Assert.Equal("FakeHTTP/123", span.GetTagValue("network.protocol.version"));
 
-        // Url strips 80 or 443 if the scheme matches.
-        if ((expectedUri.Port == 80 && expectedUri.Scheme == "http") || (expectedUri.Port == 443 && expectedUri.Scheme == "https"))
-        {
-            Assert.DoesNotContain($":{expectedUri.Port}", actualUrl as string);
-        }
-        else
-        {
-            Assert.Contains($":{expectedUri.Port}", actualUrl as string);
-        }
-
-        // Host includes port if it isn't 80 or 443.
-        if (expectedUri.Port is 80 or 443)
-        {
-            Assert.Equal(
-                expectedUri.Host,
-                span.GetTagValue(SemanticConventions.AttributeHttpHost) as string);
-        }
-        else
-        {
-            Assert.Equal(
-                $"{expectedUri.Host}:{expectedUri.Port}",
-                span.GetTagValue(SemanticConventions.AttributeHttpHost) as string);
-        }
-
-        Assert.Equal(HttpContext.Current.Request.HttpMethod, span.GetTagValue(SemanticConventions.AttributeHttpMethod) as string);
-        Assert.Equal(HttpContext.Current.Request.Path, span.GetTagValue(SemanticConventions.AttributeHttpTarget) as string);
-        Assert.Equal(HttpContext.Current.Request.UserAgent, span.GetTagValue(SemanticConventions.AttributeHttpUserAgent) as string);
+        Assert.Equal(expectedUrlPath, span.GetTagValue("url.path"));
+        Assert.Equal(expectedUrlQuery, span.GetTagValue("url.query"));
+        Assert.Equal(expectedUrlScheme, span.GetTagValue("url.scheme"));
+        Assert.Equal("Custom User Agent v1.2.3", span.GetTagValue("user_agent.original"));
+        Assert.Equal(expectedErrorType, span.GetTagValue("error.type"));
 
         if (recordException)
         {
@@ -271,7 +259,8 @@ void EnrichAction(Activity activity, string method, object obj)
 
                     break;
 
-                default:
+                case "OnException":
+                    Assert.True(obj is Exception);
                     break;
             }
         }

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

@@ -44,7 +44,7 @@ public void AspNetMetricTagsAreCollectedSuccessfully(
         bool enableServerAttributesForRequestDuration = true)
     {
         double duration = 0;
-        HttpContext.Current = RouteTestHelper.BuildHttpContext(url, routeType, routeTemplate);
+        HttpContext.Current = RouteTestHelper.BuildHttpContext(url, routeType, routeTemplate, "GET");
         HttpContext.Current.Response.StatusCode = expectedStatus;
 
         // This is to enable activity creation