[ASP.NET Core] Fix missing http.route for requests outside of MVC

src/OpenTelemetry.Instrumentation.AspNetCore/AspNetCoreInstrumentation.cs

@@ -27,7 +27,9 @@ internal sealed class AspNetCoreInstrumentation : IDisposable
             "Microsoft.AspNetCore.Hosting.HttpRequestIn",
             "Microsoft.AspNetCore.Hosting.HttpRequestIn.Start",
             "Microsoft.AspNetCore.Hosting.HttpRequestIn.Stop",
+#if !NET6_0_OR_GREATER
             "Microsoft.AspNetCore.Mvc.BeforeAction",
+#endif
             "Microsoft.AspNetCore.Diagnostics.UnhandledException",
             "Microsoft.AspNetCore.Hosting.UnhandledException",
         };

src/OpenTelemetry.Instrumentation.AspNetCore/CHANGELOG.md

@@ -2,6 +2,9 @@
 
 ## Unreleased
 
+* Fixed an issue of missing `http.route` tag outside of MVC requests.
+([#4104](https://github.com/open-telemetry/opentelemetry-dotnet/pull/4104))
+
 * Added direct reference to `System.Text.Encodings.Web` with minimum version of
 `4.7.2` due to [CVE-2021-26701](https://github.com/dotnet/runtime/issues/49377).
 This impacts target frameworks `netstandard2.0` and `netstandard2.1` which has a

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

@@ -21,7 +21,7 @@
 #endif
 using Microsoft.AspNetCore.Http;
 #if NET6_0_OR_GREATER
-using Microsoft.AspNetCore.Mvc.Diagnostics;
+using Microsoft.AspNetCore.Routing;
 #endif
 using OpenTelemetry.Context.Propagation;
 #if !NETSTANDARD2_0
@@ -87,12 +87,14 @@ public override void OnEventWritten(string name, object payload)
                     }
 
                     break;
+#if !NET6_0_OR_GREATER
                 case OnMvcBeforeActionEvent:
                     {
                         this.OnMvcBeforeAction(Activity.Current, payload);
                     }
 
                     break;
+#endif
                 case OnUnhandledHostingExceptionEvent:
                 case OnUnHandledDiagnosticsExceptionEvent:
                     {
@@ -242,6 +244,15 @@ public void OnStopActivity(Activity activity, object payload)
 
                 activity.SetTag(SemanticConventions.AttributeHttpStatusCode, TelemetryHelper.GetBoxedStatusCode(response.StatusCode));
 
+#if NET6_0_OR_GREATER
+                var route = (context.GetEndpoint() as RouteEndpoint)?.RoutePattern.RawText;
+                if (!string.IsNullOrEmpty(route))
+                {
+                    activity.DisplayName = route;
+                    activity.SetTag(SemanticConventions.AttributeHttpRoute, route);
+                }
+#endif
+
 #if !NETSTANDARD2_0
                 if (this.options.EnableGrpcAspNetCoreSupport && TryGetGrpcMethod(activity, out var grpcMethod))
                 {
@@ -294,6 +305,7 @@ public void OnStopActivity(Activity activity, object payload)
             }
         }
 
+#if !NET6_0_OR_GREATER
         public void OnMvcBeforeAction(Activity activity, object payload)
         {
             // We cannot rely on Activity.Current here
@@ -324,26 +336,19 @@ public void OnMvcBeforeAction(Activity activity, object payload)
 
             if (activity.IsAllDataRequested)
             {
-#if !NET6_0_OR_GREATER
                 _ = this.beforeActionActionDescriptorFetcher.TryFetch(payload, out var actionDescriptor);
                 _ = this.beforeActionAttributeRouteInfoFetcher.TryFetch(actionDescriptor, out var attributeRouteInfo);
                 _ = this.beforeActionTemplateFetcher.TryFetch(attributeRouteInfo, out var template);
-#else
-                var beforeActionEventData = payload as BeforeActionEventData;
-                var template = beforeActionEventData.ActionDescriptor?.AttributeRouteInfo?.Template;
-#endif
+
                 if (!string.IsNullOrEmpty(template))
                 {
                     // override the span name that was previously set to the path part of URL.
                     activity.DisplayName = template;
                     activity.SetTag(SemanticConventions.AttributeHttpRoute, template);
                 }
-
-                // TODO: Should we get values from RouteData?
-                // private readonly PropertyFetcher beforeActionRouteDataFetcher = new PropertyFetcher("routeData");
-                // var routeData = this.beforeActionRouteDataFetcher.Fetch(payload) as RouteData;
             }
         }
+#endif
 
         public void OnException(Activity activity, object payload)
         {

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

@@ -675,10 +675,10 @@ public async Task ActivitiesStartedInMiddlewareBySettingHostActivityToNullShould
             Assert.Equal(activityName, middlewareActivity.OperationName);
             Assert.Equal(activityName, middlewareActivity.DisplayName);
 
-            // tag http.route should not be added on activity started by asp.net core as it will not be found during OnEventWritten event
-            Assert.DoesNotContain(aspnetcoreframeworkactivity.TagObjects, t => t.Key == SemanticConventions.AttributeHttpRoute);
+            // tag http.route should be added on activity started by asp.net core as it is set during onStop event
+            Assert.Contains(aspnetcoreframeworkactivity.TagObjects, t => t.Key == SemanticConventions.AttributeHttpRoute);
             Assert.Equal("Microsoft.AspNetCore.Hosting.HttpRequestIn", aspnetcoreframeworkactivity.OperationName);
-            Assert.Equal("/api/values/2", aspnetcoreframeworkactivity.DisplayName);
+            Assert.Equal("api/Values/{id}", aspnetcoreframeworkactivity.DisplayName);
         }
 
 #if NET7_0_OR_GREATER
@@ -777,12 +777,6 @@ void ConfigureTestServices(IServiceCollection services)
                                             numberofSubscribedEvents++;
                                         }
 
-                                        break;
-                                    case HttpInListener.OnMvcBeforeActionEvent:
-                                        {
-                                            numberofSubscribedEvents++;
-                                        }
-
                                         break;
                                     default:
                                         {
@@ -812,7 +806,7 @@ void ConfigureTestServices(IServiceCollection services)
             }
 
             Assert.Equal(0, numberOfUnSubscribedEvents);
-            Assert.Equal(3, numberofSubscribedEvents);
+            Assert.Equal(2, numberofSubscribedEvents);
         }
 
         [Fact]
@@ -842,12 +836,6 @@ void ConfigureTestServices(IServiceCollection services)
                                             numberofSubscribedEvents++;
                                         }
 
-                                        break;
-                                    case HttpInListener.OnMvcBeforeActionEvent:
-                                        {
-                                            numberofSubscribedEvents++;
-                                        }
-
                                         break;
 
                                     // TODO: Add test case for validating name for both the types
@@ -896,7 +884,7 @@ void ConfigureTestServices(IServiceCollection services)
 
             Assert.Equal(1, numberOfExceptionCallbacks);
             Assert.Equal(0, numberOfUnSubscribedEvents);
-            Assert.Equal(4, numberofSubscribedEvents);
+            Assert.Equal(3, numberofSubscribedEvents);
         }
 
         [Fact]
@@ -991,7 +979,7 @@ static void ThrowException(IApplicationBuilder app)
         }
 
         [Fact]
-        public async Task RouteInformationIsNotAddedToRequestsOutsideOfMVC()
+        public async Task RouteInformationIsAddedToRequestsOutsideOfMVC()
         {
             var exportedItems = new List<Activity>();
 
@@ -1031,12 +1019,10 @@ public async Task RouteInformationIsNotAddedToRequestsOutsideOfMVC()
 
             Assert.NotNull(activity);
 
-            // After fix update to Contains http.route
-            Assert.DoesNotContain(activity.TagObjects, t => t.Key == SemanticConventions.AttributeHttpRoute);
+            Assert.Contains(activity.TagObjects, t => t.Key == SemanticConventions.AttributeHttpRoute);
             Assert.Equal("Microsoft.AspNetCore.Hosting.HttpRequestIn", activity.OperationName);
 
-            // After fix this should be /custom/{name:alpha}
-            Assert.Equal("/custom/abc", activity.DisplayName);
+            Assert.Equal("/custom/{name:alpha}", activity.DisplayName);
 
             await app.DisposeAsync().ConfigureAwait(false);
         }