Fix http.request.method and http.request.method_original

open-telemetry · Mar 11, 2024 · e31b687 · e31b687
1 parent 3d3ce2e
commit e31b687
Show file tree

Hide file tree

Showing 5 changed files with 38 additions and 21 deletions.
diff --git a/src/OpenTelemetry.Instrumentation.AspNet/Implementation/HttpInListener.cs b/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 RequestMethodHelper requestMethodHelper = new();
 
     public HttpInListener(AspNetTraceInstrumentationOptions options)
     {
@@ -84,7 +85,15 @@ private void OnStartActivity(Activity activity, HttpContext context)
             activity.SetTag(SemanticConventions.AttributeServerAddress, url.Host);
             activity.SetTag(SemanticConventions.AttributeServerPort, url.Port);
 
-            activity.SetTag(SemanticConventions.AttributeHttpMethod, request.HttpMethod);
+            var originalHttpMethod = request.HttpMethod;
+            var normalizedHttpMethod = this.requestMethodHelper.GetNormalizedHttpMethod(originalHttpMethod);
+            activity.SetTag(SemanticConventions.AttributeHttpRequestMethod, normalizedHttpMethod);
+
+            if (originalHttpMethod != normalizedHttpMethod)
+            {
+                activity.SetTag(SemanticConventions.AttributeHttpRequestMethodOriginal, originalHttpMethod);
+            }
+
             activity.SetTag(SemanticConventions.AttributeHttpTarget, path);
             activity.SetTag(SemanticConventions.AttributeHttpUserAgent, request.UserAgent);
             activity.SetTag(SemanticConventions.AttributeHttpUrl, GetUriTagValueFromRequestUri(request.Url));

diff --git a/src/Shared/SemanticConventions.cs b/src/Shared/SemanticConventions.cs
@@ -99,6 +99,7 @@ internal static class SemanticConventions
     // 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)
 

diff --git a/test/OpenTelemetry.Instrumentation.AspNet.Tests/HttpInListenerTests.cs b/test/OpenTelemetry.Instrumentation.AspNet.Tests/HttpInListenerTests.cs
@@ -20,33 +20,36 @@ namespace OpenTelemetry.Instrumentation.AspNet.Tests;
 public class HttpInListenerTests
 {
     [Theory]
-    [InlineData("http://localhost/", "http://localhost/", "localhost", 80, 0, null)]
-    [InlineData("http://localhost/", "http://localhost/", "localhost", 80, 0, null, true)]
-    [InlineData("https://localhost/", "https://localhost/", "localhost", 443, 0, null)]
-    [InlineData("https://localhost/", "https://user:pass@localhost/", "localhost", 443, 0, null)] // Test URL sanitization
-    [InlineData("http://localhost:443/", "http://localhost:443/", "localhost", 443, 0, null)] // Test http over 443
-    [InlineData("https://localhost:80/", "https://localhost:80/", "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", "localhost", 80, 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", "localhost", 80, 0, null)] // Test complex URL sanitization
-    [InlineData("http://localhost:80/Index", "http://localhost:80/Index", "localhost", 80, 1, "{controller}/{action}/{id}")]
-    [InlineData("https://localhost:443/about_attr_route/10", "https://localhost:443/about_attr_route/10", "localhost", 443, 2, "about_attr_route/{customerId}")]
-    [InlineData("http://localhost:1880/api/weatherforecast", "http://localhost:1880/api/weatherforecast", "localhost", 1880, 3, "api/{controller}/{id}")]
-    [InlineData("https://localhost:1843/subroute/10", "https://localhost:1843/subroute/10", "localhost", 1843, 4, "subroute/{customerId}")]
-    [InlineData("http://localhost/api/value", "http://localhost/api/value", "localhost", 80, 0, null, false, "/api/value")] // Request will be filtered
-    [InlineData("http://localhost/api/value", "http://localhost/api/value", "localhost", 80, 0, null, false, "{ThrowException}")] // Filter user code will throw an exception
-    [InlineData("http://localhost/", "http://localhost/", "localhost", 80, 0, null, false, null, true)] // Test RecordException option
+    [InlineData("http://localhost/", "http://localhost/", "localhost", 80, "GET", "GET", null, 0, null)]
+    [InlineData("http://localhost/", "http://localhost/", "localhost", 80, "POST", "POST", null, 0, null, true)]
+    [InlineData("https://localhost/", "https://localhost/", "localhost", 443, "NonStandard", "_OTHER", "NonStandard", 0, null)]
+    [InlineData("https://localhost/", "https://user:pass@localhost/", "localhost", 443, "GET", "GET", null, 0, null)] // Test URL sanitization
+    [InlineData("http://localhost:443/", "http://localhost:443/", "localhost", 443, "GET", "GET", null, 0, null)] // Test http over 443
+    [InlineData("https://localhost:80/", "https://localhost:80/", "localhost", 80, "GET", "GET", null, 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", "localhost", 80, "GET", "GET", null, 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", "localhost", 80, "GET", "GET", null, 0, null)] // Test complex URL sanitization
+    [InlineData("http://localhost:80/Index", "http://localhost:80/Index", "localhost", 80, "GET", "GET", null, 1, "{controller}/{action}/{id}")]
+    [InlineData("https://localhost:443/about_attr_route/10", "https://localhost:443/about_attr_route/10", "localhost", 443, "GET", "GET", null, 2, "about_attr_route/{customerId}")]
+    [InlineData("http://localhost:1880/api/weatherforecast", "http://localhost:1880/api/weatherforecast", "localhost", 1880, "GET", "GET", null, 3, "api/{controller}/{id}")]
+    [InlineData("https://localhost:1843/subroute/10", "https://localhost:1843/subroute/10", "localhost", 1843, "GET", "GET", null, 4, "subroute/{customerId}")]
+    [InlineData("http://localhost/api/value", "http://localhost/api/value", "localhost", 80, "GET", "GET", null, 0, null, false, "/api/value")] // Request will be filtered
+    [InlineData("http://localhost/api/value", "http://localhost/api/value", "localhost", 80, "GET", "GET", null, 0, null, false, "{ThrowException}")] // Filter user code will throw an exception
+    [InlineData("http://localhost/", "http://localhost/", "localhost", 80, "GET", "GET", null, 0, null, false, null, true)] // Test RecordException option
     public void AspNetRequestsAreCollectedSuccessfully(
         string expectedUrl,
         string url,
         string expectedHost,
         int expectedPort,
+        string requestMethod,
+        string expectedRequestMethod,
+        string? expectedOriginalRequestMethod,
         int routeType,
         string routeTemplate,
         bool setStatusToErrorInEnrich = false,
         string? filter = null,
         bool recordException = false)
     {
-        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());
 
@@ -135,7 +138,9 @@ public void AspNetRequestsAreCollectedSuccessfully(
         Assert.Equal(expectedHost, span.GetTagValue("server.address"));
         Assert.Equal(expectedPort, span.GetTagValue("server.port"));
 
-        Assert.Equal(HttpContext.Current.Request.HttpMethod, span.GetTagValue(SemanticConventions.AttributeHttpMethod) as string);
+        Assert.Equal(expectedRequestMethod, span.GetTagValue("http.request.method"));
+        Assert.Equal(expectedOriginalRequestMethod, span.GetTagValue("http.request.method_original"));
+
         Assert.Equal(HttpContext.Current.Request.Path, span.GetTagValue(SemanticConventions.AttributeHttpTarget) as string);
         Assert.Equal(HttpContext.Current.Request.UserAgent, span.GetTagValue(SemanticConventions.AttributeHttpUserAgent) as string);
 

diff --git a/test/OpenTelemetry.Instrumentation.AspNet.Tests/HttpInMetricsListenerTests.cs b/test/OpenTelemetry.Instrumentation.AspNet.Tests/HttpInMetricsListenerTests.cs
@@ -41,7 +41,7 @@ public void AspNetMetricTagsAreCollectedSuccessfully(
         int expectedStatus)
     {
         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

diff --git a/test/OpenTelemetry.Instrumentation.AspNet.Tests/RouteTestHelper.cs b/test/OpenTelemetry.Instrumentation.AspNet.Tests/RouteTestHelper.cs
@@ -10,7 +10,7 @@ namespace OpenTelemetry.Instrumentation.AspNet.Tests;
 
 internal static class RouteTestHelper
 {
-    public static HttpContext BuildHttpContext(string url, int routeType, string routeTemplate)
+    public static HttpContext BuildHttpContext(string url, int routeType, string routeTemplate, string requestMethod)
     {
         RouteData routeData;
         switch (routeType)
@@ -48,12 +48,14 @@ public static HttpContext BuildHttpContext(string url, int routeType, string rou
 
         var request = new HttpRequest(string.Empty, url, string.Empty)
         {
-            RequestContext = new RequestContext()
+            RequestContext = new RequestContext
             {
                 RouteData = routeData,
             },
         };
 
+        typeof(HttpRequest).GetField("_httpMethod", System.Reflection.BindingFlags.Instance | System.Reflection.BindingFlags.NonPublic).SetValue(request, requestMethod);
+
         return new HttpContext(request, new HttpResponse(new StringWriter()));
     }
 }