[Instrumentation.AspNetCore] Fix span names when http.request.method is reporter as _OTHER (#5484)

Co-authored-by: Robert Pająk <[email protected]>
Co-authored-by: Vishwesh Bankwar <[email protected]>
Co-authored-by: Mikel Blanchard <[email protected]>

Author: 4 people

src/OpenTelemetry.Instrumentation.AspNetCore/CHANGELOG.md

@@ -13,6 +13,10 @@
   the original value `Get` will be stored in `http.request.method_original`.
   ([#5471](https://github.com/open-telemetry/opentelemetry-dotnet/pull/5471))
 
+* Fixed the name of spans that have `http.request.method` attribute set to `_OTHER`.
+  The span name will be set as `HTTP {http.route}` as per the [specification](https://github.com/open-telemetry/semantic-conventions/blob/v1.24.0/docs/http/http-spans.md#name).
+  ([#5484](https://github.com/open-telemetry/opentelemetry-dotnet/pull/5484))
+
 ## 1.7.1
 
 Released 2024-Feb-09

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

@@ -177,7 +177,7 @@ public void OnStartActivity(Activity activity, object payload)
             }
 
             var path = (request.PathBase.HasValue || request.Path.HasValue) ? (request.PathBase + request.Path).ToString() : "/";
-            activity.DisplayName = GetDisplayName(request.Method);
+            RequestMethodHelper.SetActivityDisplayName(activity, request.Method);
 
             // see the spec https://github.com/open-telemetry/semantic-conventions/blob/v1.23.0/docs/http/http-spans.md
 
@@ -241,7 +241,7 @@ public void OnStopActivity(Activity activity, object payload)
                     context.GetEndpoint() as RouteEndpoint)?.RoutePattern.RawText;
             if (!string.IsNullOrEmpty(routePattern))
             {
-                activity.DisplayName = GetDisplayName(context.Request.Method, routePattern);
+                RequestMethodHelper.SetActivityDisplayName(activity, context.Request.Method, routePattern);
                 activity.SetTag(SemanticConventions.AttributeHttpRoute, routePattern);
             }
 #endif
@@ -388,13 +388,4 @@ private static void AddGrpcAttributes(Activity activity, string grpcMethod, Http
             }
         }
     }
-
-    private static string GetDisplayName(string httpMethod, string httpRoute = null)
-    {
-        var normalizedMethod = RequestMethodHelper.GetNormalizedHttpMethod(httpMethod);
-
-        return string.IsNullOrEmpty(httpRoute)
-            ? normalizedMethod
-            : $"{normalizedMethod} {httpRoute}";
-    }
 }

src/OpenTelemetry.Instrumentation.Http/Implementation/HttpHandlerDiagnosticListener.cs

@@ -135,7 +135,7 @@ public void OnStartActivity(Activity activity, object payload)
                 return;
             }
 
-            RequestMethodHelper.SetHttpClientActivityDisplayName(activity, request.Method.Method);
+            RequestMethodHelper.SetActivityDisplayName(activity, request.Method.Method);
 
             if (!IsNet7OrGreater)
             {

src/OpenTelemetry.Instrumentation.Http/Implementation/HttpWebRequestActivitySource.netfx.cs

@@ -95,7 +95,7 @@ internal static HttpClientTraceInstrumentationOptions TracingOptions
     [MethodImpl(MethodImplOptions.AggressiveInlining)]
     private static void AddRequestTagsAndInstrumentRequest(HttpWebRequest request, Activity activity)
     {
-        RequestMethodHelper.SetHttpClientActivityDisplayName(activity, request.Method);
+        RequestMethodHelper.SetActivityDisplayName(activity, request.Method);
 
         if (activity.IsAllDataRequested)
         {

src/Shared/RequestMethodHelper.cs

@@ -1,6 +1,8 @@
 // Copyright The OpenTelemetry Authors
 // SPDX-License-Identifier: Apache-2.0
 
+#nullable enable
+
 #if NET8_0_OR_GREATER
 using System.Collections.Frozen;
 #endif
@@ -62,9 +64,11 @@ public static void SetHttpMethodTag(Activity activity, string originalHttpMethod
         }
     }
 
-    public static void SetHttpClientActivityDisplayName(Activity activity, string method)
+    public static void SetActivityDisplayName(Activity activity, string method, string? httpRoute = null)
     {
-        // https://github.com/open-telemetry/semantic-conventions/blob/v1.23.0/docs/http/http-spans.md#name
-        activity.DisplayName = KnownMethods.TryGetValue(method, out var httpMethod) ? httpMethod : "HTTP";
+        // https://github.com/open-telemetry/semantic-conventions/blob/v1.24.0/docs/http/http-spans.md#name
+
+        var namePrefix = KnownMethods.TryGetValue(method, out var httpMethod) ? httpMethod : "HTTP";
+        activity.DisplayName = string.IsNullOrEmpty(httpRoute) ? namePrefix : $"{namePrefix} {httpRoute}";
     }
 }

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

@@ -629,18 +629,18 @@ void ConfigureTestServices(IServiceCollection services)
     }
 
     [Theory]
-    [InlineData("CONNECT", "CONNECT", null)]
-    [InlineData("DELETE", "DELETE", null)]
-    [InlineData("GET", "GET", null)]
-    [InlineData("PUT", "PUT", null)]
-    [InlineData("HEAD", "HEAD", null)]
-    [InlineData("OPTIONS", "OPTIONS", null)]
-    [InlineData("PATCH", "PATCH", null)]
-    [InlineData("Get", "GET", "Get")]
-    [InlineData("POST", "POST", null)]
-    [InlineData("TRACE", "TRACE", null)]
-    [InlineData("CUSTOM", "_OTHER", "CUSTOM")]
-    public async Task HttpRequestMethodIsSetAsPerSpec(string originalMethod, string expectedMethod, string expectedOriginalMethod)
+    [InlineData("CONNECT", "CONNECT", null, "CONNECT")]
+    [InlineData("DELETE", "DELETE", null, "DELETE")]
+    [InlineData("GET", "GET", null, "GET")]
+    [InlineData("PUT", "PUT", null, "PUT")]
+    [InlineData("HEAD", "HEAD", null, "HEAD")]
+    [InlineData("OPTIONS", "OPTIONS", null, "OPTIONS")]
+    [InlineData("PATCH", "PATCH", null, "PATCH")]
+    [InlineData("Get", "GET", "Get", "GET")]
+    [InlineData("POST", "POST", null, "POST")]
+    [InlineData("TRACE", "TRACE", null, "TRACE")]
+    [InlineData("CUSTOM", "_OTHER", "CUSTOM", "HTTP")]
+    public async Task HttpRequestMethodAndActivityDisplayIsSetAsPerSpec(string originalMethod, string expectedMethod, string expectedOriginalMethod, string expectedDisplayName)
     {
         var exportedItems = new List<Activity>();
 
@@ -683,6 +683,7 @@ void ConfigureTestServices(IServiceCollection services)
 
         Assert.Equal(expectedMethod, activity.GetTagValue(SemanticConventions.AttributeHttpRequestMethod));
         Assert.Equal(expectedOriginalMethod, activity.GetTagValue(SemanticConventions.AttributeHttpRequestMethodOriginal));
+        Assert.Equal(expectedDisplayName, activity.DisplayName);
     }
 
     [Fact]

test/OpenTelemetry.Instrumentation.AspNetCore.Tests/RouteTests/RoutingTests.cs

@@ -14,8 +14,6 @@ namespace RouteTests;
 
 public class RoutingTests : IClassFixture<RoutingTestFixture>
 {
-    private const string OldHttpStatusCode = "http.status_code";
-    private const string OldHttpMethod = "http.method";
     private const string HttpStatusCode = "http.response.status_code";
     private const string HttpMethod = "http.request.method";
     private const string HttpRoute = "http.route";