detect MVC versioning in route

CHANGELOG.md

@@ -8,6 +8,7 @@
 
 ### Fixes
 
+- Detect MVC versioning in route ([#1731](https://github.com/getsentry/sentry-dotnet/pull/1731))
 - Fix error with `ConcurrentHashMap` on Android <= 9 ([#1761](https://github.com/getsentry/sentry-dotnet/pull/1761))
 - Minor improvements to `BackgroundWorker` ([#1773](https://github.com/getsentry/sentry-dotnet/pull/1773))
 - Make GzipRequestBodyHandler respect async ([#1776](https://github.com/getsentry/sentry-dotnet/pull/1776))

src/Sentry.AspNetCore/Extensions/HttpContextExtensions.cs

@@ -50,9 +50,16 @@ internal static class HttpContextExtensions
 
             // Skip route pattern if it resembles to a MVC route or null  e.g.
             // {controller=Home}/{action=Index}/{id?}
-            return RouteHasMvcParameters(routePattern)
-                ? builder.Append(ReplaceMvcParameters(routePattern, context)).ToString()
-                : builder.Append(routePattern).ToString();
+            if (RouteHasMvcParameters(routePattern))
+            {
+                builder.Append(ReplaceMvcParameters(routePattern, context));
+            }
+            else
+            {
+                builder.Append(routePattern);
+            }
+
+            return builder.ToString();
         }
 
         // Internal for testing.
@@ -63,64 +70,87 @@ internal static class HttpContextExtensions
             // despite the annotations claiming otherwise.
             var routeData = context.GetRouteData();
 
-            var controller = routeData?.Values["controller"]?.ToString();
-            var action = routeData?.Values["action"]?.ToString();
-            var area = routeData?.Values["area"]?.ToString();
+            // GetRouteData can return null on netstandard2
+            if (routeData == null)
+            {
+                return null;
+            }
 
-            if (!string.IsNullOrWhiteSpace(action))
+            var values = routeData.Values;
+
+            if (values["action"] is not string action)
             {
-                var builder = new StringBuilder();
-                if (context.Request.PathBase.HasValue)
-                {
-                    builder.Append(context.Request.PathBase.Value?.TrimStart('/'))
-                        .Append('.');
-                }
+                // If the handler doesn't use routing (i.e. it checks `context.Request.Path` directly),
+                // then there is no way for us to extract anything that resembles a route template.
+                return null;
+            }
 
-                if (!string.IsNullOrWhiteSpace(area))
-                {
-                    builder.Append(area)
-                        .Append('.');
-                }
+            var builder = new StringBuilder();
+            if (context.Request.PathBase.HasValue)
+            {
+                builder.Append(context.Request.PathBase.Value?.TrimStart('/'))
+                    .Append('.');
+            }
+
+            if (values["area"] is string area)
+            {
+                builder.Append(area)
+                    .Append('.');
+            }
 
+            if (values["controller"] is string controller)
+            {
                 builder.Append(controller)
-                    .Append('.')
-                    .Append(action);
-                return builder.ToString();
+                    .Append('.');
             }
 
-            // If the handler doesn't use routing (i.e. it checks `context.Request.Path` directly),
-            // then there is no way for us to extract anything that resembles a route template.
-            return null;
+            builder.Append(action);
+
+            return builder.ToString();
         }
 
         // Internal for testing.
-        internal static string ReplaceMvcParameters(string route, HttpContext? context)
+        internal static string ReplaceMvcParameters(string route, HttpContext context)
         {
-            // Return RouteData or Null, marking the HttpContext as nullable since the output doesn't
-            // shows the nullable output.
-            var routeData = context?.GetRouteData();
+            var routeData = context.GetRouteData();
 
-            if (routeData?.Values["controller"]?.ToString() is { } controller)
+            // GetRouteData can return null on netstandard2
+            if (routeData == null)
+            {
+                return route;
+            }
+
+            var values = routeData.Values;
+
+            if (values["controller"] is string controller)
             {
                 route = Regex.Replace(route, "{controller=[^}]+}", controller);
             }
 
-            if (routeData?.Values["action"]?.ToString() is { } action)
+            if (values["action"] is string action)
             {
                 route = Regex.Replace(route, "{action=[^}]+}", action);
             }
 
-            if (routeData?.Values["area"]?.ToString() is { } area)
+            if (values["area"] is string area)
             {
                 route = Regex.Replace(route, "{area=[^}]+}", area);
             }
 
+            if (values["version"] is string version)
+            {
+                route = Regex.Replace(route, "{version:[^}]+}", version);
+            }
+
             return route;
         }
 
         // Internal for testing.
         internal static bool RouteHasMvcParameters(string route)
-            => route.Contains("{controller=") || route.Contains("{action=") || route.Contains("{area=");
+            => route.Contains("{controller=") ||
+               route.Contains("{action=") ||
+               route.Contains("{version:") ||
+               route.Contains("{area=");
 
         public static string? TryGetTransactionName(this HttpContext context)
         {

src/Sentry.AspNetCore/ScopeExtensions.cs

@@ -65,25 +65,28 @@ public static void Populate(this Scope scope, HttpContext context, SentryAspNetC
         try
         {
             var routeData = context.GetRouteData();
-            var controller = routeData.Values["controller"]?.ToString();
-            var action = routeData.Values["action"]?.ToString();
-            var area = routeData.Values["area"]?.ToString();
+            var values = routeData.Values;
 
-            if (controller != null)
+            if (values["controller"] is string controller)
             {
                 scope.SetTag("route.controller", controller);
             }
 
-            if (action != null)
+            if (values["action"] is string action)
             {
                 scope.SetTag("route.action", action);
             }
 
-            if (area != null)
+            if (values["area"] is string area)
             {
                 scope.SetTag("route.area", area);
             }
 
+            if (values["version"] is string version)
+            {
+                scope.SetTag("route.version", version);
+            }
+
             // Transaction Name may only be available afterward the creation of the Transaction.
             // In this case, the event will update the transaction name if captured during the
             // pipeline execution, allowing it to match the correct transaction name as the current

src/Sentry.AspNetCore/Sentry.AspNetCore.csproj

@@ -10,7 +10,6 @@
     <ProjectReference Include="..\Sentry.Extensions.Logging\Sentry.Extensions.Logging.csproj" />
   </ItemGroup>
 
-
   <ItemGroup Condition="'$(TargetFramework)' == 'netstandard2.0'">
     <ProjectReference Include="..\Sentry.DiagnosticSource\Sentry.DiagnosticSource.csproj" />
 

test/Sentry.AspNetCore.Tests/Extensions/HttpContextExtensionsTests.cs

@@ -25,7 +25,8 @@ public static HttpContext GetMvcSut(
             string area = null,
             string controller = null,
             string action = null,
-            string pathBase = null)
+            string pathBase = null,
+            string version = null)
         {
             var httpContext = new DefaultHttpContext();
             if (pathBase is not null)
@@ -37,6 +38,7 @@ public static HttpContext GetMvcSut(
             AddRouteValuesIfNotNull(httpContext.Request.RouteValues, "controller", controller);
             AddRouteValuesIfNotNull(httpContext.Request.RouteValues, "action", action);
             AddRouteValuesIfNotNull(httpContext.Request.RouteValues, "area", area);
+            AddRouteValuesIfNotNull(httpContext.Request.RouteValues, "version", version);
             return httpContext;
         }
 
@@ -66,7 +68,6 @@ public void ReplaceMcvParameters_ParsedParameters(string routeInput, string asse
     {
         // Arrange
         var httpContext = Fixture.GetMvcSut(area, controller, action);
-
         // Act
         var filteredRoute = HttpContextExtensions.ReplaceMvcParameters(routeInput, httpContext);
 
@@ -81,6 +82,7 @@ public void ReplaceMcvParameters_ParsedParameters(string routeInput, string asse
     [InlineData("abc/{controller=}/")]
     [InlineData("{action=Index}/{id?}")]
     [InlineData("{area=Index}/{id?}")]
+    [InlineData("v{version:apiVersion}/Target")]
     public void RouteHasMvcParameters_RouteWithMvcParameters_True(string route)
     {
         // Assert
@@ -122,17 +124,18 @@ public void NewRouteFormat_MvcRouteWithPathBase_ParsedParameters(string routeInp
     }
 
     [Theory]
-    [InlineData("{area=MyArea}/{controller=Home}/{action=Index}/{id?}", "theArea/house/about/{id?}", "house", "about", "theArea")]
-    [InlineData("{area=MyArea}/{controller=Home}/{action=Index}/{id?}", "{area=MyArea}/house/about/{id?}", "house", "about", null)]
-    [InlineData("{area=}/{controller=}/{action=}/{id?}", "{area=}/{controller=}/{action=}/{id?}", "house", "about", "theArea")]
-    [InlineData("{controller=Home}/{action=Index}/{id?}", "house/about/{id?}", "house", "about", null)]
-    [InlineData("{controller=Home}/{action=Index}", "house/about", "house", "about", null)]
-    [InlineData("{controller=Home}/{id?}", "house/{id?}", "house", "about", null)]
-    [InlineData("{action=Index}/{id?}", "about/{id?}", null, "about", null)]
-    public void NewRouteFormat_MvcRouteWithoutPathBase_ParsedParameters(string routeInput, string expectedOutput, string controller, string action, string area)
+    [InlineData("{area=MyArea}/{controller=Home}/{action=Index}/{id?}", "theArea/house/about/{id?}", "house", "about", "theArea", null)]
+    [InlineData("{area=MyArea}/{controller=Home}/{action=Index}/{id?}", "{area=MyArea}/house/about/{id?}", "house", "about", null, null)]
+    [InlineData("{area=}/{controller=}/{action=}/{id?}", "{area=}/{controller=}/{action=}/{id?}", "house", "about", "theArea", null)]
+    [InlineData("{controller=Home}/{action=Index}/{id?}", "house/about/{id?}", "house", "about", null, null)]
+    [InlineData("{controller=Home}/{action=Index}", "house/about", "house", "about", null, null)]
+    [InlineData("{controller=Home}/{id?}", "house/{id?}", "house", "about", null, null)]
+    [InlineData("{action=Index}/{id?}", "about/{id?}", null, "about", null, null)]
+    [InlineData("v{version:apiVersion}/Target", "v1.1/Target", null, "about", null, "1.1")]
+    public void NewRouteFormat_MvcRouteWithoutPathBase_ParsedParameters(string routeInput, string expectedOutput, string controller, string action, string area, string version)
     {
         // Arrange
-        var httpContext = Fixture.GetMvcSut(area, controller, action);
+        var httpContext = Fixture.GetMvcSut(area, controller, action, null, version);
 
         // Act
         var filteredRoute = HttpContextExtensions.NewRouteFormat(routeInput, httpContext);

test/Sentry.AspNetCore.Tests/ScopeExtensionsTests.Populate_RouteData_SetToScope.verified.txt

@@ -0,0 +1,14 @@
+{
+  Request: {
+    Method: GET,
+    QueryString: 
+  },
+  User: {},
+  TransactionName: GET Ctrl.Actn,
+  Tags: {
+    route.action: Actn,
+    route.controller: Ctrl,
+    route.version: 1.1,
+    TraceIdentifier: 
+  }
+}

test/Sentry.AspNetCore.Tests/ScopeExtensionsTests.cs

@@ -5,7 +5,7 @@
 using Microsoft.Extensions.Primitives;
 
 namespace Sentry.AspNetCore.Tests;
-
+[UsesVerify]
 public class ScopeExtensionsTests
 {
     private readonly Scope _sut = new(new SentryOptions());
@@ -317,14 +317,23 @@ public void Populate_PayloadExtractors_DoesNotConsiderInvalidResponse(object exp
     }
 
     [Fact]
-    public void Populate_RouteData_SetToScope()
+    public Task Populate_RouteData_SetToScope()
     {
         // Arrange
         const string controller = "Ctrl";
         const string action = "Actn";
+        const string version = "1.1";
         var routeFeature = new RoutingFeature
         {
-            RouteData = new RouteData { Values = { { "controller", controller }, { "action", action }, } }
+            RouteData = new RouteData
+            {
+                Values =
+                {
+                    { "controller", controller },
+                    { "action", action },
+                    { "version", version },
+                }
+            }
         };
         var features = new FeatureCollection();
         features.Set<IRoutingFeature>(routeFeature);
@@ -334,8 +343,8 @@ public void Populate_RouteData_SetToScope()
         // Act
         _sut.Populate(_httpContext, SentryAspNetCoreOptions);
 
-        // Assert
-        Assert.Equal($"GET {controller}.{action}", _sut.TransactionName);
+        return Verify(_sut)
+            .IgnoreStandardSentryMembers();
     }
 
     public static IEnumerable<object[]> InvalidRequestBodies()

test/Sentry.AspNetCore.Tests/Sentry.AspNetCore.Tests.csproj

@@ -8,6 +8,11 @@
     <ProjectReference Include="..\..\src\Sentry.AspNetCore\Sentry.AspNetCore.csproj" />
     <ProjectReference Include="..\Sentry.Testing\Sentry.Testing.csproj" />
   </ItemGroup>
+  <ItemGroup Condition="'$(TargetFramework)' == 'net6.0'">
+    <PackageReference Include="Microsoft.AspNetCore.Mvc.Testing" Version="6.0.0" />
+    <PackageReference Include="Microsoft.AspNetCore.Mvc.Versioning" Version="5.0.0"/>
+    <PackageReference Include="Verify.AspNetCore" Version="1.5.0" />
+  </ItemGroup>
 
   <ItemGroup Condition="'$(TargetFramework)' == 'netcoreapp2.1' OR '$(TargetFramework)' == 'net461'">
     <PackageReference Include="Microsoft.AspNetCore.Mvc.Testing" Version="2.2.0" />

test/Sentry.AspNetCore.Tests/VersioningTests.Simple.verified.txt

@@ -0,0 +1,72 @@
+{
+  result: Hello world,
+  payloads: [
+    {
+      Source: {
+        Name: GET v1.1/Target,
+        Platform: csharp,
+        Operation: http.server,
+        Description: ,
+        Status: Ok,
+        IsSampled: true,
+        Request: {
+          Method: GET,
+          QueryString: 
+        },
+        User: {},
+        Environment: production,
+        Breadcrumbs: [
+          {
+            Message: Request starting HTTP/1.1 GET http://localhost:5000/v1.1/Target - -,
+            Data: {
+              eventId: 1
+            },
+            Category: Microsoft.AspNetCore.Hosting.Diagnostics
+          },
+          {
+            Message: Executing endpoint 'VersioningTests+TargetController.Method (Sentry.AspNetCore.Tests)',
+            Data: {
+              eventId: ExecutingEndpoint
+            },
+            Category: Microsoft.AspNetCore.Routing.EndpointMiddleware
+          },
+          {
+            Message: Route matched with {action = "Method", controller = "Target"}. Executing controller action with signature System.String Method() on controller VersioningTests+TargetController (Sentry.AspNetCore.Tests).,
+            Data: {
+              eventId: ControllerActionExecuting
+            },
+            Category: Microsoft.AspNetCore.Mvc.Infrastructure.ControllerActionInvoker
+          },
+          {
+            Message: Executing ObjectResult, writing value of type 'System.String'.,
+            Data: {
+              eventId: ObjectResultExecuting
+            },
+            Category: Microsoft.AspNetCore.Mvc.Infrastructure.ObjectResultExecutor
+          },
+          {
+            Data: {
+              eventId: ActionExecuted
+            },
+            Category: Microsoft.AspNetCore.Mvc.Infrastructure.ControllerActionInvoker
+          },
+          {
+            Message: Executed endpoint 'VersioningTests+TargetController.Method (Sentry.AspNetCore.Tests)',
+            Data: {
+              eventId: ExecutedEndpoint
+            },
+            Category: Microsoft.AspNetCore.Routing.EndpointMiddleware
+          }
+        ],
+        Tags: {
+          ActionId: Guid_1,
+          ActionName: VersioningTests+TargetController.Method (Sentry.AspNetCore.Tests),
+          route.action: Method,
+          route.controller: Target,
+          route.version: 1.1
+        },
+        IsFinished: true
+      }
+    }
+  ]
+}